Commit Graph

1561 Commits

Author SHA1 Message Date
Patrick Buckley 891c8b1785 docs(mcp): operator guide for oauth_obo single-credential sign-in passthrough (slice 5)
Adds the oauth_obo section to docs/mcp-oauth.md:
- when to use it vs oauth_user (mode table row)
- deployment config ([oidc] capture_user_credential + obo_grant_profile,
  encryption-key requirement)
- per-IdP setup: Entra (delegated permissions + admin consent, plus the
  verified admin-consent-propagation AADSTS65001 gotcha) and Keycloak
  RFC 8693 (standard token exchange + audience client scopes)
- revocation & custody model: identity-unlink cuts a user off (credential
  + cache purge); flush-cache is an honest re-mint, not a revoke; per-server
  revocation is IdP-governed
- auth-type-transition + troubleshooting table rows for obo
- interim #682 note (Entra pre-authorized-clients removes the second
  consent for plain oauth_user, tenant-config only)

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley 202c39e634 feat(console): oauth_obo option in the admin MCP server form (slice 4 frontend)
Operators can now select sign-in passthrough (oauth_obo) in the console,
not just via the API:

- new 'Sign-in passthrough' auth-type radio with plain-language copy
  ('uses your org login - no separate connect')
- the shared OAuth fields block hides the oauth_user-only inputs
  (AS URL / registration / client id / secret) for obo and shows just
  the audience (marked required) plus scopes (hinted rfc8693-only), with
  an explanatory note
- client-side audience-required validation (inline error, not a 400)
- edit-populate + reset handle the new radio
- server list: obo servers get an honest 'flush cache (N)' action
  (drops minted tokens -> re-mint) instead of connect/bulk-revoke, with
  a confirm dialog that states it does NOT cut off access (that is
  IdP-governed / identity-unlink)

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley e5f8453e1a fix(mcp): complete oauth_obo revocation lifecycle + fix hot-path regression (follow-up review)
Addresses the high follow-up review of the first fix round:

Revocation lifecycle (the review's dominant theme):
- identity-unlink now purges the user's minted obo cache rows in addition
  to revoking the credential, and the response/audit report the actual
  effect (credential + N cache rows) instead of a blanket revoked=true;
  warmed-session residual (bounded by token TTL) documented
- bulk-revoke on obo is now an honest cache-FLUSH: distinct audit event
  (obo_cache_flushed) + response effect=cache_flush_remints, since the
  shared credential survives and the next dispatch re-mints (oauth_user
  keeps its durable revoke semantics)
- changing oauth_audience on a pool-backed row now purges cached tokens
  (audience is the token binding), like URL/name/auth_type changes
- flipping oauth_user->oauth_obo now clears the stale AS-consent scopes
  (else rfc8693 sends them -> invalid_scope loop); write path rejects
  oauth_scopes under the entra profile (it mints <audience>/.default)
- a cache row bearing a refresh token is never served as an obo token
  (guards the cross-node purge-vs-refresh race)

Self-inflicted regression:
- _clear_pending_consent_sync is now gated on an in-memory
  _pending_consent_written hint, so the common successful-dispatch path
  issues ZERO SQL (was an unconditional per-dispatch DELETE)

Observability + cleanups:
- restore the obo_mint_rejected log carrying the IdP error text (the
  shared-helper unification dropped it); event names passed as whole
  literals so alerting can grep them
- persist_rotation typed Callable[[str], Awaitable[None]] (was Any)
- _prime_one branches on _obo_server_names (no pre-lookup SQL for
  oauth_user)
- removed now-dead any_oauth_user_mcp_servers (3 impls + tests)

+13 regression tests. Full mcp/oidc/console suite 1888 green; mypy clean.

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley d02b9c0cf0 fix(mcp): oauth_obo credential lifecycle + pending-badge coverage (P1 per review)
- credential revocation (440): admin OIDC identity-unlink now deletes the
  captured IdP credential too (via delete_oidc_credential, previously
  zero callers), so a deprovisioned user stops minting — audited with
  obo_credential_revoked
- pending-consent badge gate (3539): new any_user_scoped_mcp_servers
  (oauth_user OR oauth_obo) replaces the oauth_user-only gate, so an
  obo-only install no longer short-circuits the badge to {pending: 0}
- pending-consent clear (5966): dispatch SUCCESS now clears the pending
  row (auth-blind _clear_pending_consent_sync) — the only clear path that
  covers obo, whose rows the token sweep (skips obo) and consent callback
  (obo never runs) would otherwise never clear
- test:50: strengthened the created-preservation assertion to plant a
  distinctly-past created via SQL so a reset is actually detectable

+4 tests (obo/user-scoped gate). NOTE: finding 1992 (orphan cache row on
concurrent delete-during-mint) accepted as bounded residual — the orphan
is a short-lived access-token cache row with NO refresh token, useless
without the deleted credential and self-expiring; a full fix needs FKs or
a delete-spanning lock. Tracked for follow-up.

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley 429a7fd13c fix(mcp): prime oauth_obo pools at session start (fixes inert feature)
The review's most severe finding: nothing warmed oauth_obo pools, so
their tools never entered any per-user catalog and the documented 'mint
on first dispatch' was unreachable (the model can't dispatch a tool it
can't see) — the whole feature was dead in chat.

prime_user_pools now iterates both pool-backed registries. _prime_one
fetches server_row first, then routes oauth_obo through
get_obo_access_token_classified (mints from the captured credential;
missing credential → skipped, the re-login rail handles it) and
oauth_user through its own path unchanged. _rebuild_user_tool_map is
already auth-type-blind, so a warmed obo entry surfaces its tools.

+2 regression tests (obo routed through mint + warmed; skipped cleanly
when the user has no credential).

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley d84393c25c fix(console): widen admin MCP surface for oauth_obo (P0/P1 per review)
- write-time validation (_enforce_oauth_obo_requirements): reject an
  oauth_obo row with no oauth_audience (400) or no encryption key (503,
  else it SystemExits the cluster at next boot) — at the save choke
  point, not per-dispatch (findings 10137/10163)
- update handler no longer nulls oauth_audience/oauth_scopes for
  oauth_obo (it needs them); clears only the oauth_user-only columns
  (10344)
- auth_type-transition purge now covers every pool-backed transition,
  including oauth_user->oauth_obo (was skipped: old per-server-AS refresh
  tokens leaked into the mint cache + left a live grant at the old AS
  unrevoked) and oauth_obo->static/none (10326)
- URL-change purge + https enforcement + client-secret clear now apply to
  oauth_obo, not just oauth_user (10339)
- bulk-revoke accepts oauth_obo — the documented remediation for the
  stale rows a flip leaves behind (10705)

+6 console tests (obo audience/key required, happy path, flip-purge, obo
bulk-revoke).

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley 17c44305d6 fix(mcp): harden oauth_obo mint engine per max review (P0 security core)
Addresses the review's B/D/F classes + single-sourcing:

- B (credential corruption): the rfc8693 refresh-leg rotation is now
  persisted the instant it is obtained, BEFORE the exchange leg, via a
  persist_rotation callback under the held credential lock. A rotated RT
  survives an exchange-leg failure (no more cascade lockout), and the
  exchange response's own audience-scoped RT is never written to the
  shared credential.
- D (wrong-audience bearer): the entra leg ALWAYS pins scope=<audience>/
  .default (scope is Entra's only audience carrier); per-server
  oauth_scopes no longer replaces it (that dropped the audience and
  leaked a Graph-audience token to the MCP server). oauth_scopes stays a
  rfc8693-only knob.
- F (state-machine divergence): extracted _handle_refresh_failure, called
  by BOTH oauth_user and oauth_obo — oauth_user behaviour byte-identical
  (1304 tests green). Fixes: obo cooldown now gated on needs-mint so a
  force_refresh 401-retry falls through (2063); credential decrypt errors
  classified not raised (2099); permanent-rejection arms the cooldown as
  a terminal backstop so it stops re-minting + re-auditing every dispatch
  (2156); malformed-200 resets the ambiguous streak (2196); misconfig
  arms the cooldown to dampen the log/SQL flood (2089); server_row
  threaded from the dispatch caller to drop a hot-path SQL round-trip (2069).
- messaging (5993): obo refresh_failed now points at re-login/admin, not a
  nonexistent per-server consent flow.
- single-source (580/9732/217/1830): USER_SCOPED_AUTH_TYPES +
  is_user_scoped_auth live in mcp_crypto (leaf), re-exported; OBO_GRANT_
  PROFILES derives from _OBO_MINT_LEGS and drives oidc validation (was
  dead-exported).

+5 obo regression tests (rotation-survives-exchange-fail, exchange-RT-
ignored, terminal cooldown, cooldown fall-through, decrypt classified).

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley e38e573f7c feat(mcp): route oauth_obo servers through the per-user pool
Gate sweep of the pool-backed class: oauth_obo joins oauth_user at
every pool-keying site, judged individually -

- _obo_server_names sibling registry (reconcile + boot); priming,
  keep-alive sweep, and consent-flow sites deliberately keep iterating
  _oauth_user_server_names only (obo has no per-server consent; its
  keep-alive lands with the credential lifecycle work)
- pool routing/status/static-health/tool-resolve gates use the shared
  is_user_scoped_auth predicate; status reports the real auth_type
- dispatch: _pool_token_lookup routes oauth_obo to the mint engine;
  'missing' detail becomes a re-login message (no per-server Connect
  URL is advertised - _build_consent_url already returns None)
- _db_servers_to_config skips obo rows from static auto-connect (would
  handshake-fail with empty headers and trip the breaker)
- web_search backend refusal covers both per-user auth types
- console: oauth_obo in _MCP_AUTH_TYPES, https enforcement extended;
  startup key requirement counts obo rows (encrypted mint cache)

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley fd60450700 test(mcp): pin oauth_obo mint engine wire shapes and custody semantics
14 cases with exact request-body assertions per the spike-verified
shapes: entra default-scope + per-server override, rfc8693 two-call
chain with subject-token threading and rotation write-back, cache-hit
zero-call fast path, permanent-rejection cache-drop-credential-kept
(with the token_revoked audit row), transient cooldown short-circuit,
and loud-but-retryable misconfiguration.

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley 012ad2a9b3 feat(mcp): oauth_obo mint engine - single-credential per-server token minting
get_obo_access_token_classified: sibling of the oauth_user classified
lookup sharing its result vocabulary, cache table, locks, and backoff,
but 'refresh' = mint from the user's captured credential via the
deployment grant leg ([oidc] obo_grant_profile):

- entra: one refresh-token redemption, scope=<audience>/.default
- rfc8693: refresh grant -> standard token exchange (audience=)

Both wire shapes are spike-verified (docs/design/obo-spike). Key
semantics: a missing cache row mints (no consent prerequisite); a
PERMANENT rejection drops only the per-server cache row - the shared
credential is never auto-deleted, so one mis-granted server cannot
lock a user out of the rest; rotation write-back persists the newest
credential BEFORE the cache write; mints single-flight cluster-wide on
a per-(user, issuer) advisory lock.

is_user_scoped_auth/USER_SCOPED_AUTH_TYPES define the pool-keyed auth
class once for the upcoming client-side gate sweep.

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley 0732f9a7d2 feat(mcp): capture the IdP refresh token at OIDC login (opt-in)
[oidc] capture_user_credential (default off; env
TURNSTONE_OIDC_CAPTURE_USER_CREDENTIAL) persists the user's IdP refresh
token - encrypted with the MCP token envelope - as the single
credential oauth_obo servers will redeem on demand.

- enabling the knob appends offline_access to the login scopes
  (idempotent when the operator already lists it)
- capture runs after user provisioning and is best-effort: a capture
  failure logs loudly but never blocks login; the mint path surfaces a
  missing credential on the reconnect rail
- startup hard-fails (SystemExit) when capture is enabled without a
  [security] token encryption key, same as the oauth_user enforcement

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley be22eb2fee feat(mcp): add oidc_user_credentials storage for single-credential minting
One captured IdP refresh token per (user, issuer), Fernet-encrypted with
the same envelope as mcp_user_tokens - the credential that
auth_type='oauth_obo' servers will redeem on demand for per-server
access tokens instead of holding per-(user, server) refresh tokens.

- migration 067 + mirrored create_all schema (parity-tested)
- storage protocol + both backends: upsert (replace-on-conflict),
  get, rotation write-back, delete, delete_user cascade
- MCPTokenStore encrypt/decrypt wrappers

Refs #551.
2026-07-12 19:03:35 -07:00
Patrick Buckley f4e54ce814 fix(install): gate get.docker.com by $ID instead of trapping all failures
Deciding the installer up front — get.docker.com for the IDs it recognizes,
Docker's repo directly for unrecognized derivatives — avoids treating a
transient get.docker.com failure (network, apt lock, EOL sleep) on a supported
distro as an "unsupported distro" and silently routing it into the repo path.

Recognized IDs now surface the real failure via die instead of masking it;
unrecognized derivatives (Nobara, Mint, …) skip the doomed call and its
"Unsupported distribution" output entirely rather than running it to fail.

Addresses review feedback on #829.
2026-07-11 18:41:05 -07:00
Patrick Buckley 4d423913b1 fix(install): install Docker on distros get.docker.com rejects
run.sh delegates Docker installation to get.docker.com, which detects the
distro from $ID alone and aborts with "Unsupported distribution '<id>'" on
any derivative it doesn't hardcode — Nobara (the reported case), Linux Mint,
Pop!_OS, AlmaLinux, Oracle Linux, and so on. run.sh's own detection already
resolves these via ID_LIKE/fallback, so the family is known; only the
delegated install fails.

When get.docker.com exits non-zero, fall back to adding Docker's official CE
repo for the upstream the family maps to and installing the same packages
(including the compose plugin the rest of run.sh depends on). Upstream is
chosen from PLATFORM_ID for the dnf family — Fedora is platform:fNN, Enterprise
Linux platform:elN, which ID_LIKE cannot distinguish (Nobara's is
"rhel centos fedora" yet it is pure Fedora) — and from UBUNTU_CODENAME for the
apt family, which is present only on Ubuntu lineage and is the exact codename
Docker's repo expects (Mint's VERSION_CODENAME is not).

Fixes #822.
2026-07-11 18:41:05 -07:00
Patrick Buckley a4876c00e2 fix(admin): add create-admin CLI; stop run.sh onboarding into a role-less user
The installer's "Finish setup" told users to run `turnstone-admin create-user`,
which creates a user with no role. Web login derives scopes solely from assigned
roles (empty perms -> read only), so that account logs in read-only and every
admin action fails with "Forbidden: token lacks 'approve' scope". Creating any
user also flips setup_required to false, so the browser first-run wizard -- the
only path that assigns the builtin-admin role -- never appears.

- run.sh: point "Finish setup" at the web setup wizard; use create-admin as the
  headless fallback instead of create-user
- admin.py: add `create-admin` -- creates a user + assigns builtin-admin, or
  promotes an existing role-less user (idempotent); guards on the seeded admin
  role and enforces the wizard's 8-char password floor for fresh accounts
- tests: cover fresh-grant (approve reaches the derived login scope), the
  promote/recovery path, idempotency, and both validation exits

Fixes #824
2026-07-11 18:15:48 -07:00
Patrick Buckley 6a94dc1d57 fix(judge): thread model-definition capabilities into judge completions
The intent judge and output-guard judge were the only create_completion
callers that never passed model-definition capabilities, so operator-declared
capabilities (effort passthrough, tool support, temperature, verbosity) were
silently ignored on judge calls. Every in-ChatSession lane threads them via
_resolve_capabilities; the judges live outside the session and never reached
it.

Add a shared _resolve_model_capabilities() helper mirroring
ChatSession._resolve_capabilities, and have both judges resolve
self._capabilities — from the judge alias's model definition, or the injected
session capabilities on the session-model fallback — and pass capabilities=
into create_completion. Replace each judge's context_window int arg with
session_capabilities: the fallback window now derives from the resolved caps
(identical to what the session passed before), while the alias path keeps
reading ModelConfig.context_window, a separate field the capability merge must
not touch.

Refresh the stale docs/judge.md note claiming sub-agents are exempt from intent
validation — task agents have been judge-gated since #773.

Refs #823
2026-07-11 17:53:24 -07:00
Patrick Buckley 8e2657248d docs(task-agent): record the decided durable-sub-turn id strategy at the mint site
If sub-turns ever persist: Turn-IR verbatim, re-mint at load (run_seq is
session-scoped), rebuild the wire map from the native lane's structural
1:1 pairing with the mirror; turns without native client tool blocks
need no entries. The map itself is never persisted — it is derivable,
and a second durable source of truth would have to be kept in lockstep
with the turns. Also documents why the mint must never be string-split
(not injective: parent and original may contain the delimiter).
2026-07-11 16:37:13 -07:00
Patrick Buckley 660aff6f1e fix(task-agent): guard the Google swap against partial lanes; fix a stale comment
The fidelity swap now requires the raw lane to be a faithful counterpart
of the mirror — same length, every id present — before replacing
tool_calls; a partially-corrupted lane (filtered non-dict elements)
would otherwise swap a shorter list over the mirror and orphan a
mirrored call whose tool result remains in history. The _run_agent
call-site comment now matches the builder's reasoning_text-only
blank-id rule.
2026-07-11 16:37:13 -07:00
Patrick Buckley dc52bc2b96 fix(task-agent): simplify the blank-id rule to reasoning_text-only and heal historical rows
The blank-id gate's strip-then-filter semantics left two residual
hazards (surviving Responses reasoning items whose pairing contract
needs their original sibling items; an asymmetric Messages-shaped lane
surviving when no client block was actually stripped). The rule is now
total and simpler: on a blank-id turn only the loose-text
reasoning_text synth block survives — it carries no id and is
shape-invalid on the Messages translator by design, and real-world
blank-id servers are Chat-Completions locals whose reasoning IS that
loose text. This also removes the builder's per-call provider import.

The Google fidelity swap now skips raw rows carrying a blank id
(historical captures that predate the gate would otherwise resurrect
the blank id on every replay — the sanitized mirror stays), guards
against non-dict lane elements, and legalizes via the new shared
lowering.legalize_tool_call_entry — the ONE per-entry legalizer the
sanitize pass also uses, so the two seats cannot drift on semantics or
the wire.tool_args_legalized breadcrumb.
2026-07-11 16:37:13 -07:00
Patrick Buckley 98cefc3660 fix(task-agent): move the blank-id gate into the shared native-lane builder
The blank-provider-id gate lived only at the _run_agent call site while
the main-loop stream accumulator has the identical back-fill-then-carry
seam — and it over-dropped, discarding the reasoning lane for exactly
the servers that emit blank ids. The gate now lives in
_finalize_provider_blocks as a had_blank_ids parameter both harnesses
thread: client tool blocks (which keep the blank id the mirror back-fill
never reached) are stripped, and when any were present the remaining
Messages-shaped blocks go with them (a surviving native lane REPLACES
the rebuilt content on the Anthropic translator, so a lane missing its
tool_use would orphan every mirrored call) — while shape-invalid
reasoning residuals (reasoning_text, Responses reasoning items) are
kept. This also closes the pre-existing main-loop case: a Gemini
openai-compat turn with a blank tool id no longer persists a raw
fidelity dict whose blank id the swap would resurrect on every replay.

The Google fidelity-swap legalization now reuses the canonical
lowering.legalized_arguments (made public) instead of a hand-rolled
narrower copy: dict-shaped arguments are serialized rather than
collapsed to {}, the standard wire.tool_args_legalized breadcrumb is
logged, and a degenerate non-dict function entry passes through
untouched instead of raising.
2026-07-11 16:37:13 -07:00
Patrick Buckley 646bceed52 fix(task-agent): review fixes for the native-lane carry
- Skip the native lane on a turn whose provider left a tool-call id
  blank: the uuid back-fill reaches only the tool_calls mirror, so a
  carried native tool_use block would replay the blank id and desync
  from the restored tool_result (Anthropic orphans the result; Google
  re-fills a fresh uuid). The rebuild path keeps every representation
  on the back-filled id — the pre-native behaviour, for exactly the
  degenerate case.
- Extract _reasoning_text as the ONE Chat-Completions reasoning
  extractor shared by the streaming and non-streaming paths: first
  non-empty STRING of reasoning/reasoning_content wins, so a server
  putting a structured object in reasoning can neither shadow valid
  text in reasoning_content nor leak a non-str into the session's
  reasoning accumulator.
- Legalize arguments when GoogleProvider's fidelity swap replaces the
  sanitized tool_calls mirror with the raw provider dicts — the swap
  could resurrect a malformed arguments string the upstream sanitize
  pass had fixed (pre-existing on the main loop; ids and
  thought_signature untouched).
- Drop the redundant emptiness guard on the agent seam's
  reasoning_parts (the shared finalize helper already guards) and
  document the wire_id_map lifetime invariant for future
  resumable/background agents.
2026-07-11 16:37:13 -07:00
Patrick Buckley d660819142 feat(task-agent): carry the provider-native reasoning lane in the sub-harness
A task agent's replayed turns now carry the native reasoning lane the
model produced (Anthropic thinking blocks + signatures, OpenAI Responses
reasoning items, Gemini thought_signature blocks, vLLM/llama.cpp parsed
reasoning text) instead of being rebuilt from content + tool_calls with
the reasoning dropped — restoring reasoning continuity across the
agent's own multi-turn tool loop on every provider lane.

The prerequisite is the id half: replace legalize_tool_call_ids with
restore_provider_tool_ids, a lowering pass that maps the session-minted
sub-tool ids back to the provider's own ids on the transient wire copy
(from the per-run mint map, never by string-splitting). The native
tool_use block is replayed verbatim — its id and signature untouched —
and the top-level mirror and tool_result agree with it on every request.
The minted id stays the sole internal key (registry, DOM, recall,
cancel ledger), #820 unchanged.

Chat-Completions lane: non-streaming create_completion now surfaces
reasoning/reasoning_content as CompletionResult.reasoning (the twin of
the streaming reasoning_delta extraction), and the agent seam runs the
Phase 5 vLLM reasoning-field replay against the agent's own provider
and alias. The native lane is finalized by a shared helper
(_finalize_provider_blocks) so the main loop and the sub-harness cannot
drift; replay honors the per-model replay_reasoning_to_model flag on
every lane, and llama.cpp stays capture-only, matching the main loop.
2026-07-11 16:37:13 -07:00
Patrick Buckley a5c3dc00fc fix(task-agent): address Copilot review on the id projection
- wire_safe_tool_call_id: SHA-256 not SHA-1 for the deterministic token —
  matches the codebase convention for fingerprints (attachments, auth,
  session) and drops the SHA-1 scanner flag. Non-crypto use, ids unchanged
  in shape (tid_ + 32 hex); no test pins the literal value.
- interactive.js: the two sub-agent child-id example comments now show the
  real minted shape (<parent>::r{run}s{step}::<id>), not a stale <seq> form.
2026-07-11 13:12:27 -07:00
Patrick Buckley 110d6b4fc0 fix(task-agent): mint session-unique sub-tool ids
Sub-agent tool ids were namespaced {parent}::{provider_id} — unique
across concurrent agents but not across turns within one agent. A local
provider reissuing "call_0" every response minted the same id twice, so
the live card's DOM row lookup collapsed distinct calls onto one row
while FIFO recall kept them apart: two views of one trajectory disagreed
on identical input (the bug-3 id-consistency defect). When the provider
also reuses the PARENT call id, sequential runs repeated the collision
one level up.

Mint {parent}::r{run}s{step}::{provider_id} at the single rewrite point:
a session-monotonic run tag (lock-allocated; runs start concurrently on
the 4-wide task pool) plus a per-run step tag make each id unique within
the session, and every consumer — nesting registry, error flags, DOM
data-call-id, recall projection, cancel ledger — keys on that one id.
The FIFO pairing helper stays as honest pairing for un-minted input
(unparented runs, direct construction), with its rationale rewritten.

The agent wire seam (_run_agent's _api_call) also runs the same two
validity passes the main loop already ran — sanitize_tool_call_arguments
(a documented vLLM deepseek_v4 renders malformed args and 400s; agents
hit the same backends) and legalize_tool_call_ids (projects the long,
::-containing ids to plain tokens, call/result pairing preserved). The
id projection is DEFENSIVE hardening, not a fix for an observed break:
the ids replay fine on the lenient anthropic-compatible deployment (the
prior ::-containing format ran reliably), it just keeps an agent's
self-built history valid on a hypothetically stricter backend. Applied
at the agent seam only — main-loop assistant turns carry a provider-
native block lane whose id must stay byte-identical to the mirrored
tool_calls, so the projection cannot run there without desyncing them.

Follow-ups: parent-level card aliasing under a reused parent id; the same
id hygiene for the main conversation loop / native lane.
2026-07-11 13:12:27 -07:00
Patrick Buckley 062a260c88 fix(console): scope response-control dirty flag to the identity that set it
A dirty flag set by touching a verbosity/reasoning-mode select survives a
model/provider/surface change, so the merge-side delete could destroy a key
hand-typed into the Advanced JSON for the renamed row. Honor the dirty
override only while the identity still matches the row that made it dirty.

Also document the captured-value fallback contract at both sites (the
baseline is deliberately not consulted: it arrives async or never on the
compat lane, capture has already lifted the value out of the row JSON, and
emission is gated server-side on the merged supports_* flag) and pin the
fallback plus the scoped dirty-delete in test_app_js.
2026-07-10 15:59:40 -07:00
Patrick Buckley 03861e0cf5 feat(console): verbosity and reasoning-mode controls in the model shelf
- capability-gated "Response controls" on the Models create/edit
  shelf: Output verbosity (low/medium/high) and Reasoning mode
  (Standard/Pro), shown only for Responses-surface models; the empty
  selection means provider default and omits the capability key
- values lift out of the capabilities JSON into the selects on edit
  and merge back on save with identity tracking, so changing the
  provider/model/surface resets them instead of carrying a value
  across models; the Advanced JSON textarea wins unless the select
  was touched last
- known GPT-5.6 models inherit support from the static table without
  persisting redundant support flags; OpenAI-compatible models pinned
  to the Responses surface opt in via the supports_verbosity /
  supports_pro_mode tiles
- invalidate in-flight capability lookups on any identity field
  change and on modal open so a stale response cannot clobber a fresh
  shelf; API-surface changes now run the full field-change path
- model list rows surface verbosity= / mode= override chips
2026-07-10 15:59:40 -07:00
Patrick Buckley b450b9ad20 fix(providers): align GPT-5.6 with the GA API surface
- every 5.6 tier accepts effort "max" and reasoning.mode
  "standard"/"pro" (GA docs: pro is a request mode on any GPT-5.6
  model) -- drop the Sol-only gating
- GPT-5.6 deprecates prompt_cache_retention; send
  prompt_cache_options={"ttl": "30m"} (its only supported lifetime)
  and keep the 24h retention policy for pre-5.6 models
- never inject commercial cache params into local lanes: dropped from
  the Chat Completions lane (which serves only openai-compatible and
  google) and gated off the compat-pinned Responses lane -- a gpt-5*
  served-model name is not an OpenAI account
- account cache writes: usage *_tokens_details.cache_write_tokens
  flows into cache_creation_tokens (5.6 bills writes at 1.25x the
  uncached input rate)
- drop non-string verbosity/reasoning_mode overrides with a warning
  instead of raising on unhashable capability-JSON values
- keep ModelCapabilities' public positional prefix stable by appending
  the verbosity/pro fields at the tail; pin it with a constructor test
- openai floor 2.44 -> 2.45, the first release with the typed
  prompt_cache_options kwarg
2026-07-10 15:59:40 -07:00
Patrick Buckley dc647f4d63 fix(bash): report exit code for killed shells; single import style in registry tests
Copilot: bash_output's schema promises the exit code once the shell has
exited, but the formatting attached it only to 'completed' — a killed
shell has one too (the negated signal number). Attach it to any exited
state.

Code-quality: the registry test file mixed a top-level from-import with
function-local 'import ... as bg_mod' for monkeypatching module
attributes; one from-style module alias at the top now serves all of
them.
2026-07-10 15:42:33 -07:00
Patrick Buckley ab7d56e0ba feat(bash): opt-in background shells with delta output reader and kill tool (#817)
Restore 'start a dev server, use it in a later call' as an explicit opt-in
after #816 made bash reap its whole process group on return. The surface
mirrors the dominant coding-agent convention: bash(run_in_background=true)
returns a bash_N handle immediately; bash_output(id, filter?) returns only
output produced since the previous read plus status and exit code;
kill_shell(id) terminates the shell's whole process group.

- Per-session BackgroundShellRegistry: capped rolling line buffer with
  drop-oldest gap accounting, exit-order record pruning, owner scoping for
  task_agents (shells reaped when the agent finishes), liveness-guarded
  group kills (a stale pgid is never signalled), budgeted teardown joins.
- Exit notices ride a shared external-event rail (sanitize, soft cap,
  channel 'any', idle wake) now common to watch fires; a new 'quiet'
  NudgeQueue channel lets a user cancel defer pending notices without
  letting them re-wake the stopped workstream, and failed wake delivery
  re-queues external notices seq- and predicate-intact without re-arming
  the wake gate.
- The bash_output filter runs in a killable subprocess: sre holds the GIL
  for an entire search, so no in-process timeout can bound a hostile
  pattern. Scrubbed child env, pinned UTF-8 pipes, honest timeout-vs-
  helper-failure error taxonomy, per-line match window with explicit
  clipping notes; a failed filter never consumes the delta.
- run_in_background rides the bash intent-judge projection; bash_output is
  exempt from the repeat warning but still recorded so interleaved polls
  keep breaking other tools' streaks; all bash boolean args share one
  lenient coercion dialect.
- Shells survive generation cancel and die with the workstream: every
  teardown path funnels through ChatSession.close(); CLI exit and the
  server lifespan now close every loaded session, signal-first and
  Ctrl-C-safe, so nothing detached outlives a graceful shutdown.
2026-07-10 15:42:33 -07:00
Patrick Buckley bec757a96b test(bash): use tmp_path fixture instead of tempfile.mktemp
CodeQL flagged tempfile.mktemp as an insecure temporary file and Copilot flagged the same call as race-prone (the path is not reserved). Use the pytest tmp_path fixture, which reserves a unique per-test directory and is cleaned up automatically.
2026-07-10 00:04:40 -07:00
Patrick Buckley c1cfb668b9 docs(changelog): sync 1.7.x release notes from stable/1.7; note bash fix
main was missing the 1.7.1 through 1.7.3 sections and the two-track preamble that shipped on stable/1.7; bring them in and add an Unreleased entry for the bash background-hang fix.
2026-07-10 00:04:40 -07:00
Patrick Buckley f1f488aa55 fix(bash): do not hang when a command backgrounds a long-lived process
A bash command that leaves a process running in the background (server &, a daemon) could wedge the whole workstream forever: the tool read stdout/stderr to EOF, which never arrives because the child inherits the pipe, and the timeout watchdog bailed the moment the tracked bash exited.

Wait on the tracked process bounded by the tool timeout (keyed on process exit, not pipe EOF) and terminate its whole session group on every exit path, reaping any backgrounded survivor, forcing the drain threads to EOF, and leaving nothing to leak. Decode with errors=replace so undecodable output is preserved instead of dropped, and pre-bind proc so a Popen failure surfaces the real error.

Behavior change: a process the command backgrounds no longer survives the call. First-class opt-in backgrounding is left as a separate change.
2026-07-10 00:04:40 -07:00
Patrick Buckley 9668862a7f fix(personas): engineer prompt wording from PR feedback
Name the task_agent tool literally so the model connects the guidance
to the tool the persona grants, and restore "asking for permission".
2026-07-09 19:19:02 -07:00
Patrick Buckley a0d7e2266e feat(personas): harden engineer base prompt with process discipline
engineer.md is the default BASE module for non-coordinator sessions.
Rework it from posture-level guidance to explicit process discipline:
phased work (understand, design, plan, edit, verify) with ceremony
scaled to the size of the change, red-green as the default for
testable work, minimal-diff scoping, a thrash-stop after repeated
failed attempts, and reporting only observed results. Exploration
delegates to task agents; push-back happens once, then defers with
the disagreement stated for the record.
2026-07-09 19:19:02 -07:00
Patrick Buckley 2ca4113ce5 docs(hypothesis): carry the factored Q_E reading into the glossary; primer wording
Review follow-ups: the s-shorthand convention and its glossary echo now
cover Q_E's own state argument (s -> w where Q_E reads it), and the Q_E
glossary row carries the factored (w, a) ~> (w', o) reading so the
symbol table no longer reintroduces the environment-reads-all-of-s
interpretation the outer-kernel note warns against. PRIMER: the
top-alone-widens bullet keeps owner language anchored to the
simple-case top; success is defined as an accepted end, consistent
with the declared-vs-actually-right distinction two sentences later.
2026-07-09 19:14:32 -07:00
Patrick Buckley 31301ba2a6 docs(hypothesis): harden the normal form; sync PRIMER
HYPOTHESIS.md:
- carry the initial law mu_0 in the tuple (and its displayed signature);
  split the rejection symbol into parse failure vs authorization
  refusal, with gamma(s, bot_Y) = bot_A as an axiom and a positional
  convention for the remaining bare bots
- factor the state s = (q, w) and retype Q_E to (w, a) ~> (w', o) so the
  latent world has a generator and the displayed T is its stated
  projection; quantify fail-closed over a rejection-invariant safe set K
- read H_ok as operational acceptance (H_acc) against analysis-only
  success G, with a convention for which claims read which side; score
  C6's ceiling against G and pin C5's slack to the correct-halting
  drift, resolving the tension with its own falsifier
- state ledger integrity relative to an attestation assumption (reported
  vs actual effects); split cancellation into safe vs unresolved and
  count unresolved as possibly-bad; add the realizability clause to
  C1/C3; admit multi-principal trust tops as deployment choices
- reversibility is declared in the tool contract the gate reads at
  authorization; the returned record's mark is confirmation, not source

PRIMER.md: mirror the same corrections in plain language -- ceiling not
cliff for the desk wall, contract-first reversibility, the multi-party
trust top (including the summary line), declared-vs-actual success on
dashboards, reported-vs-actual ledger honesty, cancel is not
automatically safe.
2026-07-09 19:14:32 -07:00
Patrick Buckley f5f721a979 fix(providers): include allowed reasoning modes in the unknown-mode warning
Mirror the verbosity warning so an operator typo in reasoning_mode logs the allowed values, not just the offending one.
2026-07-09 18:48:51 -07:00
Patrick Buckley 47f908c9e0 feat(providers): add OpenAI GPT-5.6 (Sol/Terra/Luna) support
Onboard the GPT-5.6 family (GA 2026-07-09) to the OpenAI Responses lane.

- Capability rows for gpt-5.6 (= Sol alias/catch-all), gpt-5.6-terra, and
  gpt-5.6-luna: 1.05M context, 128K output, tool_search/vision/pdf/reasoning
  replay, default effort medium, temperature only at effort=none.
- "max" reasoning effort, Sol-only; Terra/Luna cap at xhigh (the knob's "max"
  snaps to the xhigh ceiling). First commercial OpenAI use of "max" — the
  ordinal knob already ranked it, so no effort-ladder change was needed.
- Verbosity and pro mode as operator-declared capability fields
  (supports_verbosity/verbosity, supports_pro_mode/reasoning_mode), merged
  from the model-definition capabilities JSON and emitted on the Responses
  wire as text.verbosity and reasoning.mode. Both are gated by a supports
  flag plus an enum guard that drops unknown values with a warning. Pro mode
  is Sol-only. There is no gpt-5.6-pro model — "pro" is the reasoning.mode
  param, not a separate model id.
- Raise the openai floor to >=2.44 for the 5.6 Responses params.

Unit and wire-golden tests cover the rows, max->xhigh snapping, the two
levers, and the enum guards. Validated live against the OpenAI API: gpt-5.6
accepts the model id, effort "max", text.verbosity, and reasoning.mode="pro".
2026-07-09 18:48:51 -07:00
Patrick Buckley 2a32211e4a feat(webui): port SSE overflow-recovery companions to the coordinator pane
The #805 server-side fixes (emit-time batching, _ListenerQueue poison,
out-of-band closing) already cover every SSE stream, but the client-side
companions lived only in the interactive pane. Port them to coordinator.js
and extract the drift-prone pure core into a shared module (closes #806).

- shared_static/sse_overflow.js (new): storm-guard constants +
  overflowWindowTripped + degradedCooldownStep, imported by both panes so the
  trip threshold and cooldown ladder have one source of truth. interactive.js
  imports these instead of holding local copies; the two node runtime probes
  move to tests/test_sse_overflow_js.py.
- coordinator.js: handle the stream_overflow frame (storm guard -> degraded
  catch-up with a doubling cooldown; the reconnect replays from the ring, or
  falls to the replay_truncated -> /history floor); add the close-on-hide /
  replay-on-show visibilitychange handler plus a document.hidden guard at the
  connectSSE chokepoint; add drop-vs-render-wedge counters (onmessage now wraps
  the dispatch in try/catch -- the coordinator previously had no wedge guard,
  so a handler throw silently poisoned every later turn).
- After a stream gap the children/tasks sidebar re-syncs only when the ring
  replay cannot cover it: no resume cursor, a replay_truncated envelope, a gap
  beyond the cursor-trust window, or a live event id below the saved cursor (a
  process restart reset the counter, which the replay path reports as a false
  replay_ok). child_ws_*/task events are ordinary ring entries, so an ordinary
  short reconnect heals the sidebar through the live handlers with no REST
  rebuild -- a momentary blur/focus under close-on-hide rebuilds nothing.
- Close-session teardown detaches the visibility handler before the close POST
  so a hide/show mid-close can't resurrect a dying stream. A replay_truncated
  seen mid-stream is deferred (not dropped) and re-synced from /history on the
  next idle -- repairing both a ring-evicted gap and a turn stranded by
  close-on-hide (stream_end evicted while hidden), matching interactive.js's
  _pendingTruncatedResync.

The extraction stops at the pure core: interactive.js's stateful glue is
hard-pinned by its source-assertion suite, so its class-method shape stays put
and the coordinator reimplements the equivalent glue as closure functions.

Tests: new test_sse_overflow_js.py (module exports + the two runtime probes);
coordinator parity + lifecycle pins in test_app_js.py (replay-aware sidebar
refresh, restart detection, truncated-resync deferral, close-session
visibility detach); interactive's moved probes replaced by an extraction pin.
All JS-source suites green.
2026-07-08 16:58:23 -07:00
Patrick Buckley d115111756 docs(hypothesis): daemons + the outer loop; plain-language PRIMER
HYPOTHESIS.md:
- New appendix entry "Daemons (the recurrent harness)": a daemon as the
  regenerative process of concatenated runs — ready-set recurrence,
  renewal-reward lifting exactly at regeneration points, accumulation as
  what breaks regeneration (cross-cycle provenance meet, renewal events
  that reset accumulated risk), and authority under intermittence
  (owner contact as a renewal point for authority; TOCTOU at cycle
  scale).
- New body section "The loop": the task-dispatching outer loop as the
  harness construction applied one level out — the composition
  correspondence read at the top level, the daemon as its single-agent
  special case, the bare while-loop as the trivial-group harness one
  level up. Flagged as a sketch; outer fail-closed/reach-avoid
  treatment deferred to later rounds.
- Veto caveat threaded to match: judge-as-veto safety scoped to the
  authority lattice, and the nonblocking escape degrades to an
  always-enabled safe halt when the principal is unreachable.
- Consistency: Grounding's Asserted tier now covers "The loop";
  "always-enabled escalation" -> "escape" (the appendix's own term, now
  that the escape has an unattended form); brace the one unbraced \bot
  subscript (linter section-B HIT).

PRIMER.md: new plain-language companion — same object, no symbols, the
formal doc wins every disagreement. README's entry link now points at
the primer, which links onward to HYPOTHESIS.md.
2026-07-08 02:49:46 -07:00
Patrick Buckley 5dcf66c284 fix(webui): share renderer-output CSS so the console + coordinator highlight code
highlight.js, KaTeX and Mermaid all run on every surface via the shared
renderer (renderer.js), but their theme/wrapper CSS lived only in
ui/static/style.css. The console and coordinator load /static/style.css from
console/static/ — a different file on a different server — so hljs token spans
fell back to --fg (flat monospace for several releases), and the KaTeX/Mermaid
wrappers lacked their overflow containers, letting wide equations/diagrams
overflow the pane.

Move the hljs theme, .katex-display/.katex-error and the .mermaid-* wrappers
into shared_static/chat.css, which every surface loads via /shared/chat.css.
Restate the mermaid width-clamp for the preview pane (.preview-markdown) too,
since its content isn't a .msg.assistant message.

Drop the redundant background on .msg.assistant pre code.hljs so the <pre>
carries the code surface on every surface — otherwise the console/coordinator
(where the pre is --panel, not --code-bg) showed a darker box inside a lighter
padding band.
2026-07-08 01:37:46 -07:00
Patrick Buckley c328bebecd feat(schedules): add persona and project settings to scheduled tasks
A scheduled task could pin the model and skill of the workstream each
firing creates; it can now also pin its persona and project, so a
schedule can run under, e.g., the researcher persona attached to a
specific project's memory bucket.

The two values live on scheduled_tasks (migration 066, Text NOT NULL
default '') and are passed verbatim to create_workstream at dispatch,
where the node resolves the persona for the workstream kind and gates
the project attach. Empty means "kind-default persona / no project",
resolved late at each firing (mirrors how empty model/skill already
behave) -- existing schedules keep byte-identical dispatch behaviour,
so there is no backfill.

Also fixes a latent bug this feature depends on: admin_create_schedule
read created_by from request.state.user_id, which AuthMiddleware never
sets, so every scheduled task stored created_by=''. It now reads
auth_result.user_id like every other console endpoint. This is now
load-bearing -- the scheduler dispatches under created_by and the node
gates the project attach against it. admin_update_schedule adopts the
editing admin as owner when a project is assigned to a pre-fix orphaned
('') schedule, and re-validates persona/project only when they change
so a since-disabled persona or lost membership does not block unrelated
edits (the node re-checks at dispatch either way).

Wired through: schema + migration (up/down + parity tested), both
storage backends, API schemas, SDK create_workstream and console
create_schedule/update_schedule, scheduler dispatch, and the admin
schedule shelf (persona + project pickers, current value preserved so
an edit cannot silently clear a filtered-out selection).
2026-07-08 00:59:14 -07:00
Patrick Buckley d5ddc95e9f fix(web_fetch): inherit model settings for the extraction completion
The URL-extraction call hard-coded max_tokens=8192 and rode the "low"
reasoning default, which broke local-inference models whose registry entry
advertises a tighter output limit or a different reasoning config. Inherit
the session/registry max_tokens and reasoning_effort instead (temperature
already was) — the same knobs the main turn uses.

max_tokens is capped to context_window // 4, the ~25% output slice Phase 2
already reserves, matching the main turn's response reserve
(_remaining_token_budget), so a large operator budget can't push
prompt + output past a small context window on strict runtimes.
2026-07-08 00:30:44 -07:00
Patrick Buckley 026c646116 test(sse): normalize session_ui_base imports to a single style
github-code-quality flagged 8 spots where tests imported
turnstone.core.session_ui_base both as `from ... import` and `import ... as
suib` (the alias was only there to monkeypatch the module-level batch
constants). Drop the alias and patch via string target
(`monkeypatch.setattr("turnstone.core.session_ui_base._TOKEN_BATCH_WINDOW_SECS", ...)`),
which resolves to the same module global — behavior-identical. The one test
that READS the constant imports the symbol directly. Test-only, no
production change.
2026-07-07 23:32:20 -07:00
Patrick Buckley dfe09d029b fix(sse): guard connectSSE against opening into a hidden tab; fix stale closing comment
PR #805 review (Copilot + the round-3 finding it corroborates):

- connectSSE opened a new EventSource even when the tab was already hidden
  (e.g. a first load in a background tab), where the close-on-hide handler
  never fires because there is no open stream to close — so a throttled
  hidden tab could still become the slow consumer this PR prevents. Add the
  document.hidden guard at the single connect chokepoint, after the wsId
  assignment + visibilitychange-handler install (so the show edge reconnects)
  and before new EventSource (so nothing opens). The timer callbacks keep
  their own pre-checks (the recover beat's also gates failCount); this closes
  the fresh-connect path they never covered.

- Fix the stale _ListenerQueue.closing docstring: it claimed the drain loop
  checks closing BEFORE poisoned, but the round-2 fix moved that check INSIDE
  the poison branch (poisoned+closing -> clean close; a healthy closing queue
  drains its tail to the ws_closed sentinel). Wording now matches the code.
2026-07-07 23:32:20 -07:00
Patrick Buckley 5083f67e96 fix(sse): batch fast-stream tokens and recover overflowed listeners
A long live session driven by a fast local model (500-2000 tok/s) showed
corrupted / missing spans of assistant text while the backend stayed
healthy. Root cause: on_content_token/on_reasoning_token enqueued one SSE
event per model delta, so the per-listener queue (cap 500) overflowed
against any slow consumer; put_nowait on a full queue silently dropped the
newest event. Once saturated, drops scatter (the consumer keeps freeing
single slots), so the client's lastEventId sails past the holes and
reconnect-replay (eid > last_event_id) can never heal them. A dropped
fence-closer reshapes all downstream markdown -> reads as heavy corruption.

Fix B (primary) - emit-time micro-batching:
Coalesce content/reasoning fragments over a ~25 ms window (or 4 KB) into
one _enqueue, cutting the wire event rate ~10-20x at local-inference
speeds. A batch is assembled before it gets an _event_id, so it is one
ordinary ring entry no cursor can fall inside (unlike the forbidden
in-ring coalesce). Two conditions are load-bearing and pinned:
  1. The inflight-buffer append and the enqueue are one _ws_lock section,
     so a snapshot's snap_seq stays a true high-water mark for its text.
     Splitting them lets a straddling snapshot double-render (the client
     content path is a blind +=, no dedup).
  2. Every non-token emit flushes the pending batch first, enforced at the
     single _enqueue choke point, so stream_end/tool_*/state_change can't
     overtake trailing content and repaint it into a new bubble.

Fix A (recovery net) - poison-at-first-overflow:
_ListenerQueue latches `poisoned` atomically at the FIRST rejected put and
refuses every later put, freezing its contents as a contiguous prefix; the
drain loop closes the stream after an id-less stream_overflow frame and the
native EventSource reconnect replays the whole gap from the ring buffer.
Poisoning at the first full (not after N) is required: any deliver-while-
dropping window advances lastEventId past interior holes that reconnect
can't replay. A ws teardown that races the overflow sets an out-of-band
`closing` flag (mark_closing), checked inside the drain loop's poison
branch: a poisoned+closing queue returns clean (no false overflow frame),
while a healthy closing queue still drains its full tail FIFO to the in-band
ws_closed sentinel -- so a slow-but-unpoisoned client never loses the turn's
final content batch + stream_end at teardown.

Client (interactive.js):
- Reconnect storm guard: after 3 overflow closes in 60 s the pane drops to
  a degraded catch-up (stop live streaming, "connection is slow" state,
  reconnect after a doubling 15->120 s cooldown that resyncs from the ring
  or the uncapped /history floor). The cooldown ladder is keyed off a
  last-trip timestamp, not the overflow-window array (which the trip
  clears), so the escalation survives its own backoff.
- Close-on-hide / replay-on-show: a visibilitychange handler closes the
  EventSource on tab-hide (a throttled hidden tab is the likeliest slow
  consumer) and reconnects with the saved Last-Event-ID on show. The
  factory recovery beat defers when hidden, and giveUp() detaches the
  handler, so a dead or backgrounded controller can't reopen a stream.
- Drop-vs-render-wedge counters distinguish this bug (server overflow
  closes) from the handler-wedge class (render/finalize throws) in the
  field. No global gap-detector: live ids are not strictly monotonic
  across concurrent tool+content emit, so a naive id!=last+1 check would
  false-positive; recovery is server-signalled instead.

Corrects the stale _resolve_event_buffer_max comment that justified the
50k ring on a "PR-G closes connections on hide" mitigation that never
existed (the close-on-hide handler above is the real one).

Negative-tested (revert the guarantee, confirm the pin fails, restore):
per-token inflight append -> snapshot straddle double-render; removed
choke-point flush -> stream_end split; no poison latch -> silent drops;
top-of-loop closing check -> healthy-close tail loss; missing mark_closing
wiring / drain closing check -> clean close mis-reported as overflow;
_noteStreamOverflow cooldown reset -> ladder never escalates; removed
hidden-tab recovery guard / giveUp handler removal -> hidden-tab reconnect.
2026-07-07 23:32:20 -07:00
Patrick Buckley e5e48a788a fix(renderer): drop the indent an indented fence close drags into code content
Copilot review on PR #804:
- An indented closing fence line ("  ```") left its leading spaces as a
  trailing whitespace-only line inside the rendered code block: the content
  capture runs up to the backtick run and the close-line indent precedes it, so
  it was captured as content. Strip a trailing newline PLUS any trailing indent
  (/\n[ \t]*$/ instead of /\n$/); a column-0 close is unaffected. Red-green
  pinned (content is exactly "  x = 1", no trailing whitespace line).
- Correct a stale test docstring claiming the fence open anchor allows "up to 3
  spaces" of indent — it allows arbitrary indent (the 4-space case is pinned
  separately).
2026-07-07 23:03:47 -07:00
Patrick Buckley a164d61552 fix(renderer): contain markdown sentinel-forgery and recursive-frame content loss
The markdown renderer protects structural blocks with in-band NUL-framed
sentinels (chr(0)+tag+index+chr(0)). escapeHtml preserves U+0000, so
model/tool text could forge sentinels, and recursively-rendered <details>
bodies re-rendered against fresh block arrays and lost their content. This
lands the ordered containment fixes from the render-containment brief.

Fixes (each pinned in tests/test_renderer_js.py; all NUL-sensitive cases also
confirmed in real headless Chrome, which drops a U+0000 token the node harness
preserves):

- B1/B2/B3 — forged sentinels: strip U+0000 at the TOP-LEVEL render entry only
  (_fnDepth === 0). renderer.js is the sole NUL producer and every restore
  regex is NUL-framed, so removing NUL closes every forgery path (block
  duplication/relocation, out-of-range "undefined", cross-container injection)
  while generated sentinels in recursive frames survive. Only NUL is stripped,
  so a code fence still shows pasted control bytes (ESC/FF/VT/DEL) verbatim.
- B4 — blockquote-in-fence (the common one): the code-fence pass now runs
  before the line-based blockquote pass. Its open matches at line start after
  optional indent and an optional list marker (`- `, `1. `), and re-emits that
  indent+marker before the sentinel so the fence keeps its document position
  (a nested-list item stays nested; a fence continuing a footnote definition
  keeps the indent its continuation scan needs). A blockquoted fence (`> ```)
  is not matched (`>` is neither indent nor a list marker), so the blockquote
  pass extracts that `> ` run and its recursion renders the fence. A `> ` line
  inside a plain fence stays literal.
- B5 — <details> open anchored to line start (^[ \t]*), so a `<details>`
  mentioned mid-line inside inline code no longer starts a block.
- NEW-1 — recursive-frame content loss: <details> extraction runs AFTER fence
  protection and restores a fenced body from a saved raw-source array
  (codeBlockRaw) back to raw markdown before the recursive render, so
  code-in-details renders in-frame instead of restoring to "undefined". Running
  after fence also means a </details> shown as example code inside a fence
  can't close the block early, and a <details> shown inside a fence stays
  literal — no offset-based fence-awareness needed. Inline-code/math in footnote
  definitions render via the restore round-trip the undefined-guard enables
  (documented at the append site).
- NEW-3 — code blocks gained the <p>SENTINEL</p> unwrap variant DT/BQ/MB/TB
  already had, removing a stray empty <p> before a standalone <pre>. The CB
  unwrap is whitespace-tolerant so an indented own-line fence (whose indent the
  fence pass re-emits) also doesn't leave a stray <p>.
- Defense-in-depth: every restore callback returns the matched sentinel
  (inert; the browser drops the NUL) instead of the array's `undefined`.

Non-obvious decisions:
- Control chars are authored as literal \xNN hex escapes (byte-verified: only
  \uXXXX decodes to raw bytes in this toolchain; \xNN matches the file's
  existing \x00 sentinel convention).
- Open anchors allow arbitrary leading indent (the fence open also allows a
  list marker), not CommonMark's ^ {0,3}: the renderer has no indented-code
  fallback, so preserving the prior behaviour of matching indented/list-nested
  fences beats CommonMark strictness, while still excluding `> ``` and mid-line
  forms.
- codeBlockRaw (the <details> raw-fence array) and the restore callbacks are
  factored through a _restorer(arr) helper; codeBlockRaw is only populated when
  the text contains a <details> tag (its sole reader).
- The entry strip is depth-0-only on purpose: an unconditional strip would
  shred the generated sentinels recursive frames carry, foreclosing NEW-1.

Negative-tested (reverted the production line, confirmed the pin fails):
- fence anchor: unanchored swallows a blockquoted fence.
- NEW-1 codeBlockRaw restore: without it, code inside <details> is lost.

Deferred (called out per the brief):
- B6/NEW-4 bidi controls (U+202A–202E, U+2066–2069, U+200E/F) still pass
  through unescaped; they are not C0 so the entry strip misses them. Left to a
  follow-up — stripping risks corrupting legitimate RTL text and <bdi>
  isolation is involved for a string renderer.
2026-07-07 23:03:47 -07:00
Patrick Buckley 2c5adb7aca fix(web): sanitize the latin1_safe_filename fallback too
Review follow-up: the helper returned `fallback` verbatim when the name
sanitized to empty, so a future caller passing an unsafe fallback (non-latin-1,
control chars, quote, backslash) could reintroduce the header crash/corruption
the helper exists to prevent. Not reachable today — all call sites pass safe
ASCII literals — but the helper is a shared safety primitive whose contract is
wire-safe output.

Run the fallback through the same cleaning, backed by a safe constant if even
that is empty, so the return is always wire-safe and never filename="". Adds a
test.
2026-07-07 22:47:15 -07:00
Patrick Buckley fd3aed1eca fix(web): make Content-Disposition filenames safe on the wire
Attachment `/content`, preview, and workstream-export downloads built the
Content-Disposition `filename="..."` value straight from a user-supplied
name, stripping only quotes and CR/LF. Three input classes still broke the
header:

- Non-latin-1 names (CJK, em dash): Starlette encodes header values as
  latin-1 and raised, 500-ing the serving route. (The original get_content
  bug.)
- ASCII control bytes (NUL, form-feed, VT, DEL): latin-1-encodable, so they
  passed Starlette, but the HTTP server layer rejects control characters in a
  header value and 500s one layer later.
- Backslash: the RFC 6266 quoted-pair escape. A trailing backslash escaped
  the closing quote and corrupted the download filename (not a 500, but wrong
  output; Windows-origin uploads carry it legitimately).

Extract one `latin1_safe_filename()` helper in web_helpers that drops every
non-printable character plus the double-quote and backslash quoted-string
metacharacters, folds any surviving non-latin-1 codepoint to '?', and falls
back to a non-empty name so the header never emits an empty filename. Route
get_content, preview_response_headers, and the export handler through it,
replacing three near-duplicate inline strips.

Adds unit tests for the helper (non-latin-1 fold, control-char and backslash
stripping, per-site fallback) and an endpoint regression test.
2026-07-07 22:47:15 -07:00