Coordinator-kind workstreams get the same MCP surface as interactive
sessions — tools, resources, and prompts (read_resource/use_prompt go
dual-kind) — gated per-persona exactly like interactive, with no
separate feature flag.
The console hosts its manager with node parity end to end: boot calls
create_mcp_client inline (same catalog resolution: DB rows, then
mcp.config_path, then this host's config.toml), the admin reload
fan-out lazily constructs and reconciles it under a lock (the node's
unlocked equivalent is #873), per-server refresh/reconnect and the
admin MCP status view cover it under the collector's console
pseudo-node id, and shutdown follows LIFO teardown. Sessions read the
live manager through a per-construction getter — the console
counterpart of the node factory's mcp_ref[0] read; client presence is
the session-level contract, and the kind-aware tool assembly runs the
same listener/prime/rebind skeleton as interactive. bind_acting_user
re-scopes listeners and per-user pools, which is security-critical for
multi-sender coordinators.
The wire-safety status projections move verbatim to core/mcp_utils so
both hosts present one schema (node endpoint bodies byte-identical);
the console's per-server action classification is a pinned COPY of the
node endpoints', with a parity test driving both sides across the
outcome matrix that fails if either drifts.
The shared MCP error card (consent / re-consent / forbidden / operator)
moves to mcp_error.js + mcp_error.css, linked by all three card hosts
and pinned by className→rule and host→link parity tests; the module
joins the whole-file sink-scan and var-ratchet lists. Reload reporting
is honest about the console entry: excluded from the unreached-node
warning's list and denominator, and the toast claims "+ console" only
for a real reconcile, with an explicit note on failure.
The pending-consent badge (#874's console half) ships too: the console
defines the same onConsentDetected seam the node dashboard exposes —
lighting up the shared pane host's existing bridge for hosted
interactive panes — and the coordinator pane threads its card's
detections through the single MCP-error helper. The badge rides the
Admin > MCP Servers rail row, hydrates at boot from the Phase 9
pending-consent endpoint the console already serves, re-syncs to DB
truth when the operator views the MCP panel, and the rail-less
standalone page carries a status-bar chip instead. A coordinator that
hits a consent wall unattended now has a persistent, glanceable signal.
Pre-existing bugs fixed along the way: create_mcp_client returned None
on pool-only installs, leaving any host managerless after restart until
the next admin MCP write; admin_import_mcp_config never scheduled the
reload fan-out (stale catalogs after import); the admin settings UI
rendered the coordinator settings section unordered and unlabeled.
Follow-ups: #873 (node reload double-construct race); #874 narrows to
the admin-MCP-view per-server indicator.
Apply review round-2 finding: the wrap-both-lanes-through-_apply_cwd_notes
pattern was hand-copied at three sites (construction, MCP list_changed,
MCP disconnect), leaving the notes invariant convention-enforced. Route
all five interactive build sites through one _set_interactive_tools(
mcp_tools) helper — merge_mcp_tools with [] is a fresh copy of the
builtin base, so the no-MCP sites pass [] and the invariant becomes
structural. Coordinator branch keeps its direct build (no cwd-dependent
tools) and gains the explicit _task_tools annotation mypy now needs.
- docs/tools.md: sync the tool-JSON metadata-keys table to _META_KEYS —
it had drifted to 3 of 8 keys (coordinator, interactive, kind_variants
were already missing; cwd_note/workspace_note are new).
- tests: cover the third note-rebuild trigger (_drop_mcp_surface) with a
count==1 assertion on both lanes, and pin the deliberately uniform
workspace_note wording across the fs tools so a one-file reword cannot
drift the copies apart.
The process cwd was nowhere in the model's context: shells start in the
inherited process cwd (spawn_group_leader passes no cwd), relative file
paths resolve against it, but nothing told the model where it was
standing — in stock Docker every shell ran in /data while user files sat
in the /workspace mount, and the model's only recourse was to probe with
pwd (#857, #833).
Lower both facts into the tool schemas, where they gate intrinsically on
tool availability (a persona without fs tools carries no note, and
coordinator envelopes are untouched):
- tools/*.json: cwd_note/workspace_note metadata templates on bash,
read_file, write_file, edit_file, search, diff_file; bash also states
the fresh-shell-per-call semantics (cd does not persist) and drops a
stale reference to the removed man tool.
- tools.apply_cwd_context(): renders the notes into descriptions;
deep-copies noted tools (the fs dicts are shared across
TOOLS/INTERACTIVE_TOOLS/TASK_AGENT_TOOLS and aliased through
merge_mcp_tools), passes note-less tools through by reference.
- ChatSession._apply_cwd_notes(): wraps every fresh interactive build of
_tools AND _task_tools (construction, MCP catalog change, MCP
disconnect) — assignment-time, so the wire tools block stays
byte-stable for provider prompt caches. os.getcwd() is OSError-guarded
(MCP rebuilds run on a background thread; eval tears down its
workdir); the workspace hint drops when the dir is missing or equals
the cwd. Task-agent sub-agents carry their own notes via _task_tools,
independent of parent persona visibility.
- config.get_workspace_dir(): [tools] workspace_dir with
TURNSTONE_WORKSPACE env fallback (searxng pattern), informational
only — no chdir, no path confinement (per-workstream working-dir
grants are a separate planned feature).
- Dockerfile: ENV TURNSTONE_WORKSPACE=/workspace so stock deployments
surface the mount with zero operator config.
- docs/docker.md: document the /data working directory, the
working_dir: /workspace compose override as the operator-level fix,
and the SQLite-fallback-DB-in-cwd caveat.
Closes#857
The launcher project picker restored `previous` unconditionally after a
choices rebuild: a since-deleted project landed the select on a blank
selectedIndex=-1 instead of the "No project" placeholder (submit was
safe — getOptionValue returned "" — but the select looked broken).
Route it through _restorePick like the other three pickers; the
"+ New project…" sentinel stays excluded (it is a command, not a state,
and it IS in choices so validity alone would not exclude it).
- ui: _paintFromCache returns its async-repaint promise;
_paintProjectPicker routes through it (fork/hint stay bespoke) and the
dashboard chains an Options-chip recompute on EVERY paint — an async
repaint can drop a server-removed pick (or revert persona to its kind
default) without firing 'change', and the chip must always name what
submit will send
- ui/console: the false "never worse than the pre-cache behavior" claim
replaced with the accepted-tradeoff ruling for module-load failure
(no per-picker retry — cache-busted re-imports split-brain the cache;
no inline-fetch fallback — that resurrects the deleted dual path)
- console: _paintHomeFromCache collapses the four verbatim
_refreshAndPopulate* wrapper bodies; _restorePick collapses the four
preserve-pick blocks (persona keeps its kind-default revert, now
pinned by a test)
- models/skills: drop the consumer-less loaded/error readers from the
modules + bridges (same omitted-not-exposed doctrine as onChange;
projects/personas keep theirs as pre-existing public surface)
- tests: boot-order guard pins ALL FOUR data-layer module tags before
shell.js (the boot anchor) in both index.html; wrapper/project-picker
guards redirected to the chokepoints; chip-recompute chains asserted
- list_cache: null-prototype _byKey — a row keyed "__proto__" swapped the
map's prototype via the inherited setter, and getByKey of inherited
members ("toString", "constructor") resolved them as rows; + guard test
- list_cache: document why _pending clears BEFORE the trailing refresh
(a .finally clear would coalesce a late force onto a stale fetch —
declines the reviewer's .finally suggestion with the ruling in-code)
- list_cache: extra() accessor doc reflects the conditional reset;
resetExtraOnError @param notes it is moot without extraDefaults
(declines per-module knobs in personas/skills, which have no extra)
- ui: extract _paintFromCache — sync-mirrors-freshOnOpen /
async-always-fresh:false now encoded once for the model/skill/persona
wrappers and asserted at the chokepoint
- ui: replaceChildren() for the model/judge/skill picker clears
(consistency with the persona/project populates)
- console: reword the skills fail-open comment to unambiguous past tense;
drop the orphaned _resolveModelLabel docstring
- tests: fork-gate asserts require each paint to open its own
`if (!_forkFromWsId)` block (the rfind+50 window false-passed a closed
gate; the model first-gate check was vacuous; the persona gate was
unasserted); drop one redundant `0 <=` (kept where it guards find()==-1)
An unprimed convergence re-review found a real login-recovery seam gap plus
cleanups (round 1's fix round manufactured one of them); fix-sanity vetted the plan.
- console onLoginSuccess recovery seam [0]+[2]: it re-warmed only skills+models
after an in-place login; projects+personas (same pre-auth-401 gap) stayed empty
(rail group-by-project flat, saved-coordinator raw slugs). Now force-refreshes
ALL FOUR caches on login — force so a still-in-flight failing pre-auth fetch
yields a trailing AUTHENTICATED refetch rather than coalescing onto the 401
(skills/personas have no *_changed event to recover). Threads an optional
callOpts through the four cache modules + console wrappers (backward-compatible;
every non-console caller passes nothing).
- fork skill paint [4]: the round-1 wrapper extraction left the modal skill paint
unconditional on a fork (wasted GET /v1/api/skills + hidden-select rebuild);
fork-gate it like model/persona/project.
- persona wrapper [5]: extract _paintPersonaSelect so all four composer pickers
share the sync-then-refresh wrapper instead of persona being inline-duplicated.
- dead machinery [6]: remove the zero-subscriber onModelsChange/onSkillsChange and
the models fpExtra fingerprint fold (and the now-orphaned core fpExtra branch).
The console repaints models via its direct models_changed handler, not a
subscription; the fold only fed the subscriber-only fingerprint.
- O(1) modelLabel [7]: index the models cache by alias (keyField) so modelLabel is
a getByKey, not a per-paint scan.
Declines documented in-code: forks-inherit-model [1] (deliberate) and the
fail-open cache [3] (intended, same policy as projects/personas). Deferral comment
at the ui onLoginSuccess twin (recovers on dashboard re-focus; follow-up).
Tests: rewrote the 7 guards the code changes moved (persona relocation, callOpts
threading, force, fpExtra removal) preserving their ordering intent, and added
fork-skill-gate, persona-wrapper, all-four-force, keyField, and
onModelsChange-removed coverage. 106 pass; ruff + mypy green.
A max-effort review of the composer-cache branch found 3 correctness + 2 cleanup
issues; fix-sanity refined the plan before implementing.
- Modal select stickiness [0]: the reused new-ws <dialog> kept the last open's
model/judge/skill pick and silently applied it to the next chat (sharp for a
fork — model/judge were sent unguarded). The composer selects now render fresh
each open (a fresh open has no `previous` selection to preserve) across ALL
five selects, while a within-open async repaint still preserves a mid-window
pick. The modal now shows the resolved default ("Default — gpt-5"). A fork
INHERITS its source's model + judge (hidden + submit-gated on !_forkFromWsId,
matching skill/persona/project).
- models default-alias reset [1]: the shared core's extra-reset-on-failure is
now opt-in (resetExtraOnError). projects keeps it (require_project gates the
picker, must fail open); models opts out, so a transient failure keeps the
last-known resolved-default annotation instead of blanking it.
- models_changed coalescing race [2]: an opt-in trailing refresh in the core —
a force caller (models_changed) awaits a refetch chained after the in-flight
one and converges to the latest state instead of a response predating the
change; startup/open callers stay coalesced.
- cleanups: the 4x paint-then-refresh block collapses into _paintModelSelects /
_paintSkillSelect [6]; the "alias (model)" label centralizes into models.js
modelLabel (registered on the window bridge) [7], deleting both local copies.
Tests: rewrote the 5 guards that pinned pre-fix literals + added fresh-matrix,
fork-inherit, bridge-registration, both-error-branch reset, and trailing-refresh
guards. 106 pass; ruff + mypy green.
The model and skill composer pickers had no client cache: the new-ws modal, the
dashboard quick-create, and the console launcher each re-fetched /v1/api/models
and /v1/api/skills inline on every open, flashing an empty dropdown for the
round-trip even though the data was usually already in memory. Add shared caches
(models.js, skills.js) the composers read SYNCHRONOUSLY, then refresh-and-repaint
— the pattern the project/persona pickers already use.
The coalescing / fail-open refresh / change-detection / window-bridge machinery
was ~70% duplicated between projects.js and personas.js. Extract it once into
list_cache.js (makeListCache) and retrofit projects.js + personas.js onto it,
preserving their full public surface byte-for-byte (rail.js + project_creator.js
import them by name; the classic bundles read the window bridges). The
require_project advisory rides projects.js as fail-open `extra` state; personas
keep their kind-filtered choices and name->label map.
models.js carries BOTH server schemas (the node sends default_alias, the console
sends coordinator_default_alias; both send judge_default_alias) so each app reads
its own, and folds them into the fingerprint so a role-alias change still fires
onChange. skills.js returns raw rows (the ui pickers add a " [MCP]" suffix the
console omits). Selection is preserved across the sync->async repaint on every
select, including model + judge independently.
Also: the dashboard model/skill fetch-once guard is dropped (refresh-on-open now,
matching project/persona); the console re-warms models on login too (the boot
pass runs pre-auth, so the dropdown used to stay empty until a reload); and
models_changed repaints via the single refresh wrapper (no double path).
The @claude mention responder (claude.yml) and the automatic PR review
(claude-code-review.yml) have been unreliable and are a frequent source
of CI breakage. Drop both; core CI (ci.yml, docker-publish, publish,
understone-example, vendor-js) is untouched and nothing else in the
tree references them.
Review of #868 flagged the sync-paint + refresh + required/optional hint block as copy-pasted between showNewWsModal and _loadDashboardOptionsLists, already diverging structurally, so a future tweak could drift and silently re-introduce the FOUC on the missed surface. Collapse both into a shared _paintProjectPicker(sel, hint, {fork}) -- the modal passes the fork flag, the dashboard never forks. Guards re-pointed at the helper + a new one pins its sync-before-async pattern.
The new-workstream modal, the dashboard composer, and the console launcher
painted their project and persona <select>s only inside the async
refresh().then(...) callback, so each open flashed an empty/stale dropdown for a
network round-trip even though the client caches are already warmed at startup.
Paint synchronously from the warm cache first, then refresh-and-repaint (still
catches items created elsewhere). On a cold cache the sync paint is a no-op the
async fills, so it is never worse than before.
Both project paints reuse the same _populateProjectSelect + reconcile, so the
require_project strict-picker invariant (never auto-select a real project into a
possibly-shared one) is unchanged; persona reuses _populatePersonaSelect, which
preserves a mid-window pick and only applies the kind default when nothing valid
is selected.
Also folds in two deferred require_project polish items in the same code: the
dashboard Project label now shows the "required"/"optional" hint (parity with the
modal), and _reconcileRequiredProjectSelection reuses the projectChoices() list
its caller already built instead of recomputing it.
Models/skills selectors are a separate follow-up (no client cache today).
Add an opt-in, default-off `server.require_project` setting. When an admin
enables it, creating an interactive chat is refused unless it is filed under a
project. The feature is inert and byte-identical when off, and can only ever
fail toward "off" (a missing config store or unset key reads as disabled).
- settings_registry: server.require_project (bool, default False, live read).
- auth: require_project_enabled + require_project_denies_create predicates
(service scope / coordinator token_source exempt; NOT admin.coordinator),
plus REQUIRE_PROJECT_ERROR / REQUIRE_PROJECT_CODE.
- node create gate via a declarative cfg.create_gate_require_project (wired True
on the interactive mount only; coordinator spawns stay ungated).
- fork/resume: a fork's project is structurally its source's. Any explicit
project_id is discarded, so a fork can never be re-filed under an unrelated
project (which would move its copied history across a tenancy boundary).
Inaccessible / projectless / nonexistent sources are uniform on body and
status, so there is no cross-tenant oracle.
- console cluster-create proxy surfaces only the coded require_project 400 and
masks every other node outcome (401/429/3xx/5xx, un-coded 400) to a sanitized
502, guarding both body reads.
- list_projects advisory field + projects.js requireProject() (fail-open).
- fresh-create project picker requires an explicit project choice under the flag
(no silent auto-select); forks hide the picker (inheritance is server-enforced)
and get an accurate refusal message.
- tests: predicate matrix, resume-inheritance oracle discriminators, console
masking, and end-to-end node-gate mount wiring.
The retry (_run) closure emitted the raw str(exc) to ui.on_error, so a
credential-bearing base-URL in a backend ConnectError
(https://user:pass@host) crossed into the dashboard SSE — the
confidentiality floor _record_fatal_error enforces, bypassed here.
Sanitize the display inline with the same sanitize_error_text redactor.
This is separable from the reused-session stale-flag hazard that keeps
_run off ensure_error_recorded: that hazard is about recording /
idempotency (deferred to #865); this is only the display string. The
double state emit and the pre-try no-persist remain in #865.
Adds a focused test that a retry-error's on_error is redacted.
Flagged by review on #866.
The initial-message worker (_run_initial) collapsed both cancel and
backend-error exits into one `except (Exception, GenerationCancelled)`
arm that always stamped state=idle, clobbering the state=error that
session.send's _record_fatal_error had persisted+emitted. A spawned
child's first-turn backend failure (unreachable model server, exhausted
quota, auth error) therefore read as an empty, successful turn — the
coordinator's wait/inspect surface reads last_error only for
state=='error' — and the real error surfaced only after a manual nudge
re-ran the turn synchronously.
Split the arm: cancel -> idle, exception -> error. The failed child now
settles at state=error and the first wait_for_workstream returns the
enriched backend error inline. Also fixes the same latent bug for
scheduled tasks, which dispatch through the same endpoint and closure.
A failed first turn is deliberately terminal for automated wakes: it
settles to a non-ready error terminal, not the idle ready-set that
timer/watch wakes recur to, so explicit user/coordinator action
reactivates it rather than a silent auto-retry (a self-healing
wake-from-error would be a separate wake-gate change).
The exception arm routes through a new ChatSession.ensure_error_recorded:
a no-op when send already recorded the error in-line (the common
backend-boundary path — no duplicate state emit), and the recorder when a
pre-try exception (model-registry refresh, user-turn append,
system-message recompose) bypassed send's own handler, so state=error
always carries a meaningful last_error. Its idempotency guard
(_has_persisted_error) is session-lifetime, so ensure_error_recorded is
scoped to _run_initial's FRESH first-turn session only; the docstring
spells out why a session-reuse caller (retry, /send, coord send, wake)
must not route through it until the per-turn error-recorded signal of
#865 lands.
The other half of making an errored workstream cheap for a model to
handle is a stable identifier: the enriched backend error now leads with
the model ALIAS the coordinator references everywhere (list_nodes, spawn)
and annotates the backend id for the operator —
"model=DeepSeek-V4-Flash (id=deepseek-v4-flash)" — so a model routing
around a failed model correlates it against those surfaces without a
lookup, instead of burning reasoning tokens reconciling the alias against
a backend id it never sees anywhere else. Collapses to one token when the
alias and id coincide.
Tests (TestInitialWorkerFailureState) assert the coordinator-visible
manager state and the persisted last_error across the matrix — common-
backend and pre-try errors both settle error with a readable last_error;
cancel-to-idle settles idle with no error recorded. De-forks the
create-app fixture and uses the shared monotonic wait_until helper.
The completion-notification honesty surface and the error-recording
hygiene of the other send-worker closures (retry / main send / coord send
/ wake) are deferred to #865.
The console OpenAPI spec had drifted from build_console_spec(): the committed
file was last generated at 1.7.0rc1 and was missing the persona and project_id
workstream-creation fields (Personas and Projects, both 1.7) plus the version
bump to 1.8.0a2. Regenerate via sdk/typescript/scripts/generate-types.py to
resync. Spec-only; no console API behavior change (openapi-server.json was
already current).
updateCompactionProgress coerced evt.retry_in with Number() and rendered it
unguarded, while the sibling part/total path two lines below is finiteness-
validated — a malformed backoff would render "retrying in NaNs". Validate
retry_in the same way (finite, non-negative), and keep the error text
regardless: the error is the load-bearing half of the note, so an unparseable
duration drops to "retrying (error)…" rather than suppressing the whole arm.
Addresses PR review feedback on the compaction reducer.
The settleSendResponse extraction left the two panes' call sites diverging on
the null-guard: interactive passed bare `data`, the coordinator passed
`data || {}` — reintroducing the copy-paste variation the shared helper existed
to erase. If a /send 2xx body were ever non-object JSON, the unknown/"ok"
fall-through would deref `data.attached_ids` and paint an already-delivered
message as a connection error; the endpoint always returns an object, so this
is a latent divergence, not a live bug.
Normalize the body once at the helper entry (`data = data || {}`) so both call
sites pass bare `data` and stay byte-identical, and every internal deref plus
any future caller is covered by the single chokepoint. The node settle-harness
gains a null-body case — red without the fix, since the call-arg evaluation
throws before the stub runs.
- INTERJECTION_CAP_CHARS joins PENDING_SENDS_MAX in workstream.py: the
2000-char interjection cap was triplicated (queue_message's truncation,
the defer-fidelity refusal, the test fake) and already drifting in
measurement — the defer check deliberately measures RAW text (raw >=
cleaned since parse_priority only strips, so it can only over-refuse
into a full-fidelity fresh spawn, never admit a truncation), now
stated in a comment. The four unrelated 2000s (notify tool, recall
preview, summary formatting, agent step cap) stay deliberately
unlinked — they are different contracts.
- The changelog's ~110-line compaction bullet is split into six per-seam
bullets matching house style, and the Breaking (1.8) compaction-event
notice moved under "### Changed" where integrators scanning bullet
heads will actually see it (cross-referenced both ways with the
pre-1.8 embedder compat bullet).
- SpawnMetricsHook takes (ui) only: the request parameter was threaded
through the whole dispatch-attempt path solely to be ignored by both
installed impls; the stale "coord wires None" claims in the rewritten
comment blocks are corrected too.
- The attachments tests' Mock-hardening block lives once in
_harden_ws_mock() — deliberately excluding _worker_running, which each
fixture chooses per scenario (one relies on the truthy auto-Mock).
- Two hand-rolled poll loops become wait_until (file convention,
diagnostic timeout) and the orphaned time import goes with them.
Three point-guards from the ceiling round (no primitive took a hit;
correctness yield halved at identical review sensitivity):
- The drain's clean-exit wake moved OUT of the function-level try: it
runs after the drain has already retired its slot, so a raise out of
the wake (the dispatcher re-raises Thread.start failures) could reach
the last-resort handler and clear a slot this thread no longer owned —
nulling a successor drain's live registration and letting two drains
service one list. The wake now runs post-try under its own guard
(mirroring _retry_pending_wake), only on the clean-exit path, and the
last-resort slot-clear is identity-guarded like every sibling exit
seam. The except arm needed a function-local threading import: the
module-top import is TYPE_CHECKING-only, so the guard would have
NameErrored inside the handler with strict mypy fully green.
- The shared settle helper promotes a non-deferred chip that binds onto
an already-idle pane: its only sweep fired mid-POST (unbound then) and
no message_dispatched ever comes for non-deferred sends, so the chip
stayed a permanently retractable "queued" bubble for a delivered
message. Keyed on post-bind chip state (also catching a raced folded
settle bind just reconciled) and skipping dismiss-in-flight chips —
the sweep's own aria-busy discipline. Pinned behaviorally: the helper
now executes under node (a 4-row missed-edge matrix), possible since
the consumer-less window bridge is gone.
- _claim_generation's on_generation_claimed emission is call-guarded:
it sits on send()'s pre-turn path, before the user turn is appended
and before the fatal handler's coverage, so a raising override
degrades to a lost latch-break instead of silently dropping every
user message on that session.
Cleanups: /command's transport catch and status-less non-2xx bodies are
loud now (threading {ok, status} through the parse — deliberately no
throw-on-!ok pre-gate, since the busy and error arms ride 409/503);
PENDING_SENDS_MAX lives in workstream.py and ChatSession._QUEUE_MAX
aliases it (one backpressure bound, structurally incapable of
diverging); the send handler's not-ok arm uses _queue_full_response();
the dead window.createQueueController bridge is deleted and the file
header's consumer map corrected.
Eight rounds of findings against the defer-and-drain seam shared one
generator: N sites each hand-copying M obligations (spawn discipline,
the order-barrier pair, backpressure, best-effort emission, the client
settle matrix), with every review finding an empty (site x obligation)
cell. This round makes each obligation a single primitive:
- The order barrier is Workstream.send_barrier_active() — one
definition of the two-term pair (pending entries OR drain alive),
consulted by the /send route, the coordinator adapter, and the
queued-nudge wake gate, which previously carried only the list term
and let a synthetic wake jump an acknowledged send during the
claimed-entry window. _PendingSend moved to workstream.py beside the
invariant that justifies the drain-alive term; the pending fields got
precise types and worker_kind became a Literal, so a typo'd
"command" comparison is now a type error instead of a silently
never-firing defer guard.
- _defer_send probes the barrier before constructing anything, bounds
acceptance at 10 pending (the interjection queue's own backpressure
contract — unbounded acceptance pinned message + attachment bytes
per entry for a whole command window and then ran one unattended
turn each), and spawns the drain with rollback: a Thread.start
failure pops the just-accepted entry and answers the retryable
queue_full instead of 500ing after registration (a phantom the
client could neither see nor retract, dispatched later as duplicate
turns). start() deliberately stays inside the lock, unlike
session_worker's outside-lock discipline: this slot is
is_alive()-gated, false for a constructed-but-unstarted thread, so
an outside-lock start would open a double-drain window.
- A /command whose worker never spawned answers 503
{"status": "error"} (spec + docs + a pane error arm) instead of the
generic 200 ok that told SDK callers their /clear ran.
- The compaction lifecycle emitter is raise-proof at its single
dispatch tail: a raising duck-typed hook degrades to a lost render,
never a lost end event — previously a raising on_error or a raising
failed-end emit left every pane a frozen progress bar, and a raising
SUCCESS end after the committed swap fabricated a failed end.
- The client settle matrix lives once: composer_queue's
settleSendResponse owns every /send response arm for both panes
(the near-verbatim twins were already drifting), parsePriority is
shared, and the busy stamp is centralized in setBusy(b, source) with
"server" as the fail-safe default. Deferred sends release the
composer (no worker exists for them; retracting the chip no longer
strands the pane in Stop mode), queue_full on an idle-looking pane
removes the optimistic bubble and restores busy (the refusal can now
fire with no worker and no drain to ever emit a state event), and
the pre-bind settle buffer is TTL-based — a burst of deferred
dispatches parked this tab's own raced settle first, where the old
size cap evicted exactly it.
- The command backstop / console proxy timeout inequality is enforced
by a test importing both named constants (both proxy_client
constructions, startup and the mTLS re-create); the compaction card
wears blue (magenta is reserved for the MCP surface); the redundant
TerminalUI.on_compaction override is gone (the inherited protocol
default is the policy site).
If thread creation raised (thread exhaustion, MemoryError), the
dispatcher had already claimed the worker slot under ws._lock — but the
flag's only clearer is _runner's finally, on a thread that never
started. The workstream then looked idle forever (no state change ever
fired) while every subsequent dispatch took the reuse path into a queue
no worker would drain, until an operator force-cancel.
Roll the claim back under the lock (identity-guarded, like _runner's
own clear, so a concurrent force-cancel's successor is never clobbered)
and re-raise. Re-raise rather than return False: callers' crash paths —
the deferred-send drain's per-iteration handler with its backoff — are
shaped for exceptions, and a False would masquerade as queue-full
backpressure and mislabel the wake gate's refusal log. worker_kind is
left stale, as documented (every reader conjoins _worker_running).
Affected every dispatch path: sends, wakes, retries, the deferred-send
drain, and workstream init.
Completes the defer-and-drain seam against the matrix rows round 6 never
enumerated (the defer contract itself took no hits):
- Crash row: a claimed entry survives a dispatch crash — the
per-iteration handler re-inserts it at head (claim-flagged so a
claim-section failure can't duplicate it), backs off ~1s, retries.
The last-resort handler spawns no successor (Thread.start fails under
the exact exhaustion that reaches it): the route's ensure-drain stays
the single spawn site, so single-flight is structural and a dead
drain revives on the next defer.
- Order row: the pending list is the order authority. The /send route
pre-checks pending/drain-alive under the same lock acquisition that
appends (one _defer_send helper serves the barrier and command-window
triggers); the coordinator adapter refuses via its return value; the
queued-nudge wake gate yields to pending sends and is re-armed by the
drain's clean exit — which covers lists emptied by pure retraction —
as well as every deferred turn's exit; retry-after-rewind is a
documented accepted overtake; init/create is fresh-ws-by-construction.
- Client settle row: queued responses carry "deferred": true
(SendResponse + regenerated openapi-server.json, status enumeration
completed); bind(el, msgId, {deferred, attachedCount}) replaces the
_deferredAttachments expando; the idle sweep skips deferred and
unbound chips; the shared dispatch attempt emits pane-tier
message_dispatched (folded: true for interjection fold-ins — the chip
clears only its deferred flag and keeps a live x while DELETE still
genuinely retracts); settles that beat bind() park in a bounded
buffer; an idle-thinking pane retro-converts its optimistic bubble
into a real queued chip instead of presenting a parked message as
sent. Rejection polling waits on the slot flags — one dispatch
attempt per slot-state change, not 4 Hz.
- SessionUI.on_compaction's protocol stub became a real default body
(the classic on_info rendering): explicit subclasses inherit protocol
members as real methods, which defeated _compaction_event's getattr
fallback for exactly the pre-1.8 embedders it serves.
- _coerce_event_id() rejects bools (isinstance(True, int) is True) at
all three duck-typed event-id coercions: the compaction marker stamp,
on_system_turn's persisted return, and _ui_event_id.
- Quick-command backstop 60s -> 25s, under the console proxy's 30s so
the degraded "running" answer can traverse a proxied pane (which now
surfaces it); /resume docs drop the fictional history SSE event
(clear_ui + REST re-fetch is the contract); /send response docs match
the wire.
Replace park-and-abandon /send semantics with defer-and-drain: a send
landing in a command window is answered {status: queued, msg_id}
immediately and dispatched full-fidelity by a per-workstream drain
thread when the window closes. Parking encoded client disconnect as
message retraction — true only for the composer's ✕-abort; every
bounded caller (coordinator client and console proxy at timeout=30,
SDKs, stock proxies) timed out and lost its message for the whole
window, and the compensating client machinery was racy (one-shot
sendAbortMs sample) and over-broad (_sendAbort fired on the
interjection path, dispatching dismissed messages while showing a
connection error). Dismissal is now uniformly bind() → DELETE, with a
fall-through that retracts pending entries; retracting an
attachment-bearing deferred send surfaces the discarded-attachments
consequence. The drain claims entries under ws._lock immediately
before dispatch (DELETE can never remove an in-flight message),
refuses the truncating interjection fallback for oversized or
attachment entries atomically inside the enqueue callback, and never
gives up while the workstream lives; durability is documented as
node-local at-most-once. sendAbortMs, _sendAbort, the 600s bound and
the park loop are deleted; route and drain share one dispatch
implementation (spawn metrics included).
Also: the initial-send completion notify is un-gated from slot
ownership (_fire_notify_targets has exactly one call site — successor
turns never notify, so the round-5 guard prevented a duplicate that
cannot exist while converting force-cancel into permanent notification
loss for scheduled workstreams); /compact on an ERROR workstream
restores the badge instead of stamping idle over it; duck-typed
SessionUIs without on_compaction get the classic on_info lines back
via a shared renderer (superseded OK ends included — a committed swap
must never be silent; pre-1.8 SSE clients are deliberately not
dual-emitted, documented as a 1.8 breaking change); failed-end notice
suppression is computed once by the emitter as a notice bool on the
end event (SDK py+ts), replacing the hand-synced cli/JS policy while
the panes keep their pane-local card-ownership clause.
Compaction becomes visible: a first-class 'compaction' SSE lifecycle
(start/progress/end, compaction_id-correlated, superseded-flagged ends)
replaces the loose info lines; both web panes render a progress-bar card
that settles into a persistent result card, re-rendered after reload via
the /history projection of the compaction marker row. Slash commands echo
as command chips instead of fake user turns.
The enabling rework: /command dispatches onto the workstream worker slot
(the old inline path blocked the node's event loop for whole compactions
and let /clear interleave with live turns). Busy refusals answer 409;
quick commands are awaited loop-natively with a 60s backstop; /compact is
fire-and-forget. Sends during a command window park in the /send route
and dispatch full-fidelity afterwards — the interjection queue (length
cap, cross-user guard, identity-swap hazards) is unreachable there — with
a compaction-aware client abort bound shared by both panes. compact_now()
carries send()'s full generation discipline; Stop aborts the in-flight
summary HTTP stream via a generation-scoped cancel ref; force-abandoned
compactions retire at their next checkpoint and their stragglers are
fenced off every surface (panes, pill latch, CLI). Every session retry
backoff is cancel-aware via one shared helper. Docs, OpenAPI spec, and
both SDKs updated.
Verified: 9457-test non-live suite, JS pin suites, headless-Chrome
reducer harness; five unprimed multi-agent review rounds (correctness
trend 15/6/6/4/4) with plan-level design passes on every fix round.
Use neutral "set" wording for operator skill markers so re-selecting the
current skill does not falsely claim a change. Update the regression
expectation for the persisted marker.
Operator-driven /skill changes were only shown in the UI, leaving no trajectory marker for the model. Persist a system turn for named skill changes and clears, with regression coverage for both paths.
Copilot review feedback (all three valid):
- _record_refresh_success docstring still claimed the push-driven
single-kind refresh calls it — round 8 deliberately stopped that (a
single kind can't declare a server-scoped 'ok'). Docstring now states
the full-pass-only contract and points at the push path's _record
closure for why.
- The admin refresh pill's skipped-tint logic (admin.js) and its
.mcp-refresh-pill-skip CSS were dead code: /v1/api/_internal/mcp-status
strips last_refresh_at/last_refresh_outcome via the read-scope
projection, so admin.js never sets newestRefreshAt and the pill block
never runs. Backed both out; the whole pill fix (whitelist the fields
with a read-scope-coarsened outcome, THEN the color logic + CSS) now
lives in #843. The CHANGELOG's false 'the admin console's refresh pill
paints…' claim is dropped — the /mcp refresh CLI and the 202-skipped
endpoint (which read last_refresh_outcome directly, not via the strip)
still work and remain documented.
The 5 github-code-quality 'statement has no effect' comments are the
known PR #840 false-positive class (the scanner reads 'await <name>' as
a valueless expression); each flagged await is load-bearing (drains a
parked runner so the next assertion is non-vacuous, delivers a
cancellation, or awaits a _noop to fabricate a done owner_task) — no
code change.
Refs #839, #843
- A single-kind push SUCCESS no longer clears the server error pill or
stamps 'ok': _last_error / _last_refresh are server-scoped but a push
refreshes only ONE kind, so a tools-failing server must not go green
because its prompts push succeeded (a wrong-healthy window, bounded by
the health tick — but a real 200-OK lie). Only a full pass declares
'ok'; the failure's armed health-tick retry runs it. This reverts the
over-reach of round 7's push-success outcome write (a self-inflicted
regression) — net simpler.
- The (server, kind) debounce uses a None sentinel, not a 0.0 default:
time.monotonic() counts from boot, so on a node whose process started
< _NOTIFICATION_DEBOUNCE (5s) after boot, the 0.0 compare would debounce
the VERY FIRST push — dropped with no recovery on the pool path. Absent
stamp = never refreshed = always admit.
- _record_refresh_skipped completes the outcome-helper set: the three
inline 'skipped' stamps now share one config-gated helper (with
_record_refresh_success / _record_refresh_failure), and the
reconnect-success branch routes through _record_refresh_success — no
more hand-copied gates to drift.
- The per-message refreshers dict + on_debounce_drop closure are built
ONCE per handler (both static and pool), not on every server->client
message before the isinstance/debounce/coalesce early-returns.
Accepted (documented): an operator /mcp refresh that finds the connect
lock busy skips + arms the retry rather than waiting (waiting
re-introduces the refresh-budget exhaustion busy-skip exists to prevent).
4 findings refuted. Suite 9408 green.
Refs #839
All three round-7 findings shared one root cause: last_refresh_outcome
(the single source of truth for the CLI / endpoint / admin pill) was
written inconsistently — ungated writes scattered across _refresh_server
and _refresh_all, never written by the push path, never popped on
removal. Consolidate every static outcome write through two config-gated
helpers so the invariant holds: _last_refresh[name] exists IFF the
server is configured and has a real outcome.
- _record_refresh_failure now stamps the (config-gated) error:<Class>
outcome; _record_refresh_success is its twin (gated ok stamp + pill
clear). The ungated writes inside _refresh_server (both the internal
error write and the success write) and _refresh_all's except are
removed — routed through the helpers. A failure observed for a
just-removed server no longer leaves a permanent stale error: row.
- The push-driven refresh path (_run_static_notification_refresh._record)
now records the outcome on BOTH success and failure, not just the
error pill — a green 'ok' outcome no longer persists under a red error
row after a push fails, and a successful push clears a prior error.
- remove_server_sync pops _last_refresh (via _clear_static_push_state
markers=True); a session drop KEEPS it (the outcome persists across a
reconnect — only removal clears it). The removed-mid-pass branch drops
any stale row too, so last_refresh_outcome doesn't report a departed
server's prior 'ok'.
_reap_bounded's pending-task concern was reviewed and REFUTED (a
pending child on external cancel during shutdown is correctly left to
loop teardown). Declined the per-notification refreshers-dict
allocation cleanup: trivial (a 3-entry dict on a rare debounced path),
and the late binding is deliberate for test overrides + mypy attribute
checks.
Tests: push-refresh success/failure write the outcome, removal pops it,
session drop keeps it, failure for a removed server leaves no stale
row; the 3 TestLastRefreshTracking tests updated to the split contract
(_refresh_server propagates, the caller records). Suite 9407 green.
Refs #839
Three residual gaps in the round-5 skip-outcome threading, all in
_refresh_all's other reconnect branches plus the endpoint ordering:
- The disconnected-server reconnect DEFERRAL (_ensure_static_connected
returns None: a sibling call in flight on the old stack, lock not
held) returned None without stamping 'skipped', so the endpoint and
pill read the STALE prior 'ok' and reported a never-run refresh as
current. Now stamps 'skipped' like every other skip branch.
- A server removed from config between the top-of-loop session check
and the cfg lookup fell through to with results[name]
UNSET, omitting it from the returned dict — an operator refreshing
that one server saw a bare 'refresh complete' with no line. Now
reports None so it renders.
- internal_mcp_refresh_one checked 'skipped' BEFORE the error pill, so
a skip on a server carrying a live error returned a benign 202
instead of 500 — a status-code-keyed caller would treat an erroring
server as healthy-but-busy. Error is now checked first.
- _reap_bounded swallowed an external CancelledError (shutdown / an
operator cancel of the refresh runner) — it now re-raises after a
best-effort exception retrieval, honouring the cancel. Dropped the
unneeded asyncio.shield in the process.
Tests: deferral stamps skipped, removed-mid-pass reported not omitted,
endpoint error-beats-skip → 500, reap re-raises external cancel. Suite
9403 green.
Refs #839
The 'skipped'/None refresh sentinel added in round 4 was only half
threaded: consumers still misreported it. Unify all operator surfaces
on ONE source of truth — the per-server last_refresh_outcome ('ok' /
'skipped' / 'error:<Class>') — exposed via a new last_refresh_outcome()
accessor:
- _refresh_all returns None (not ([], [])) for a FAILURE too, so a
failed refresh is never rendered as 'no changes' (the pre-#839 lie
the sentinel exists to close); None is disambiguated skipped-vs-failed
by the outcome. ([], []) now strictly means 'ran, no changes'.
- /mcp refresh renders skip ('skipped — retry scheduled') and failure
('refresh failed (error:X)') distinctly from 'no changes'.
- The node-internal refresh endpoint returns 202 'skipped' instead of a
misleading 200 'ok' for a refresh that never ran (the busy-lock skip);
it reads the outcome from the manager accessor because the public
status projection deliberately whitelists last_refresh_outcome out.
- admin.js paints 'skipped' with a neutral info pill
(.mcp-refresh-pill-skip), not the error-red any-non-'ok' used to get.
- _admit_list_changed rolls back BOTH the coalesce marker and the
debounce stamp when scheduling raises, so a same-kind push in the
window afterward isn't debounced against a refresh that never spawned
(the pool path has no on_debounce_drop recovery).
Tests: endpoint 202-skip, CLI skip/failure render, _refresh_all
failure→None + outcome, spawn-failure stamp+marker rollback. Suite
9399 green.
NOTE filed #843: the admin refresh pill's data (last_refresh_at/outcome)
is stripped by BOTH status projections and never reaches admin.js — a
pre-existing latent bug (the pill has never rendered); the admin.js
color fix here is correct-when-reachable. Out of #839 scope (the read
projection strips it for a privacy reason that needs its own coarsening
decision).
Refs #839
reconcile_sync no longer abandons a DB-driven removal that timed out:
both the removal loop and the config-update loop keep the name in
_db_managed (and skip the follow-on add) when remove_server_sync
returns its mutated-nothing False, so the next pass retries instead of
the deleted/reconfigured server serving stale tools until restart.
remove_server_sync is now cancel-safe end to end: it FORCE-drops the
session before queueing (parked push runners bail at their session
gate instead of serializing ≤30s list calls ahead of the removal —
the noisy #839 server was exactly the one whose runners could starve
its own removal), and wraps the post-lock cleanup in try/finally so a
caller-timeout cancel landing mid-teardown still completes the state
pop, catalog rebuild, and lock retirement rather than stranding a
config-gone ghost catalog. Config survives a park-cancel, so the
health loop recovers it.
_refresh_all reports None (not a fake ([], [])) for a busy-skip or
supersede, stamps a 'skipped' status row, and /mcp refresh renders it
distinctly — the operator is no longer told a never-refreshed server
is current. A same-kind push lost to the debounce window (the prior
runner already finished; the server won't re-announce) arms the
health-tick retry, closing the one staleness hole the per-kind
debounce still had; a push covered by a queued runner does not arm
(no lost change). Static resource/prompt catalogs are capped at
connect discovery and every refresh. _list_resource_pair's reap is
bounded so a future SDK cancel-regression can't wedge the lock.
Cleanups: _arm_refresh_retry (retry-arm gate, ×3), _spawn_full_refresh
(discard+spawn, ×3), _popen_mcp_server (live-server spawn, ×2), the
tautological stamp-arithmetic TestNotificationDebounce deleted. Suite
9395 green.
Refs #839
- Static resource/prompt catalogs are now size-capped at connect
discovery AND on every refresh (mirrors the pool twins and the static
tools path): a misbehaving server's push ran uncapped through the new
spawned refresh path and could balloon the shared node's merged
catalogs on every notification.
- remove_server_sync mutates NOTHING outside the per-name lock: the
up-front config pop meant a removal cancelled while parked (behind
the push-refresh runners that now share this lock) left a
half-removed server — config gone, session and published catalogs
alive, no driver able to reconnect or cleanly re-remove. A timed-out
removal is now honestly retryable.
- _refresh_all's DISCONNECTED branch busy-skips too (parking inside
_ensure_static_connected burned the pass's 30s budget on one
mid-reconnect server), and a busy-skip on either branch ARMS the
health-tick retry — an operator-requested refresh can no longer be
silently dropped with output indistinguishable from 'no changes'.
- reconnect_sync drops the session before queueing on the lock (FORCE
semantics already rebuilt live sessions): parked push runners bail
at their session gate instead of serializing up to one 30s list call
per kind ahead of the operator's recovery action. Residual: one
mid-list holder can still precede the 45s attempt; a timed-out
reconnect is honest and retryable.
- _refresh_server's supersede check gains the session arm: a spawned
retry/post-reconnect pass racing an eviction skipped instead of
manufacturing a false 'not connected' error pill (and a re-arm loop)
for a self-healing condition.
- The list_changed protocol twins are UNIFIED (Closes#842): the
admission half (_admit_list_changed) and the runner half
(_run_list_changed_refresh) each exist once as plain parametrized
methods — values and small closures, no factory layer (mcp v2 drops
the factory pattern; the two thin message_handler closures remain
only as SDK-v1 bindings). The one true asymmetry — coalesce-marker
ownership on the superseded path — is a documented boolean: pool
markers are only ever cleared by their runner; static markers are
cleared by remove_server_sync, so a present marker belongs to the
re-added generation. Both runners keep their names and signatures;
the notification suites pass unchanged.
- Cleanups: per-kind staleness rechecks stripped from the static
refreshers (unreachable under the lock discipline — the MUST-hold-
lock contract is documented instead); _run_hl (5th run-on-loop copy)
replaced at 44 call sites; _poll_until centralizes the live-test
wait loops; docs no longer describe the periodic refresh tier
removed in eb2a119d.
Refs #839
- _refresh_server never parks on a held connect lock: the holder is
itself a catalog publisher whose publish supersedes the pass, and
parking burned refresh_sync's whole 30s budget on ONE busy server (a
reconnect attempt holds the lock up to 45s), failing the operator
pass for every healthy server queued behind it. Busy → skip (None),
no publish, no status writes; the identity/state recheck stays as
belt-and-braces for the one-tick check→acquire race.
- _list_resource_pair: the ONE copy of the paired resources/templates
list protocol (both twins). Fail-fast — a fast real error (auth /
method rejection) surfaces as ITSELF instead of being masked behind
a hung sibling's eventual 30s TimeoutError — with the survivor
CANCELLED and REAPED inside the timeout scope, never left detached
on the shared session.
- Health-tick refresh retry: there is NO periodic refresh pass
(removed in eb2a119d; the docs still claimed the 4h tier — fixed),
so a push refresh that failed while the transport stayed up had no
automatic recovery and the shared catalog stayed stale for every
user until an operator intervened. Failures and busy-skips arm
_static_refresh_retry via the shared recorder; the health tick
drains it with one bounded, lock-serialized full pass per tick;
success, session drops, removal, and the post-reconnect spawns
clear it. This also un-latches the error pill: the retry's
completion clears it within a tick.
- _record_refresh_failure: the bearer-redaction policy (type +
message, never exc_info) lives exactly once; all three
refresh-failure sites route through it.
- Static runner discards its coalesce marker only AFTER the
lock-identity check: on the superseded path a marker present in the
set belongs to the re-added generation's parked runner, and
discarding it would mint duplicates past the one-parked-runner
bound (the pool runner deliberately differs — nothing else clears
pool markers, so its marker is its own to release).
- _clear_static_push_state: the ONE (server, kind) keyspace walk for
stamps + retry flag (+ markers on removal).
- Tests: busy-skip, superseded-no-status, fail-fast + reap (<5s
bound), retry arm/drain/re-arm/clear quartet, logged-wrapper
contract updated to the shared recorder's arg shape; vacuous
stamp-math test deleted (behavioral per-kind coverage retained);
_free_port/_wait_tcp_ready/_wait_session_live hoisted to conftest
for both live tests.
Refs #839
- _refresh_server: post-acquire lock-identity + state-existence recheck;
a pass superseded by remove (or remove + re-add) returns None and
writes NO status — it must not run its list calls as a second,
unserialized publisher against the re-add's discovery wiring,
resurrect status rows for a removed server, or stamp a false "ok"
over a generation it never refreshed. _refresh_all treats None as a
deliberate skip (no breaker success record).
- Debounce stamps are per (server, kind) on BOTH paths: refreshes are
kind-scoped, so a server-scoped stamp dropped a different-kind
notification inside the window outright — a tools push swallowed the
prompts push 100ms behind it, and nothing observed the prompt change
until the server pushed that kind again. Teardown pops loop the
kinds; remove_server_sync also discards the server's coalesce
markers so a parked old-generation runner's marker cannot coalesce
away a re-added server's first push.
- Resource refreshers (static + pool) gather with
return_exceptions=True: fail-fast gather left the surviving list
call running detached — outside the timeout scope and the lock
serialization — as an unbounded in-flight request on the shared
session.
- Spawned post-reconnect refreshes route through _refresh_server_logged:
the re-raise escaped into _spawn_background's done-callback, whose
exc_info log serializes the chained httpx.Request carrying the
configured bearer for auth_type=static servers; _refresh_all's
except drops exc_info for the same reason. Failure diagnostics widen
to "Type: message" in logs and the error pill — the message text is
header-free; only the serialized chain leaks.
- Accepted + documented: connect-lock contention on dispatch
reconnects is bounded to one in-flight list call (parked runners
bail instantly post-eviction); the error pill persists until the
next COMPLETED refresh (a notification's arrival proves nothing
about whether the failure resolved).
- Tests: per-kind debounce independence, superseded-pass writes
nothing, gather-sibling completion, logged-wrapper swallow with the
exc_info channel asserted SILENT, remove clears markers;
_run_on_loop/_drain_background hoisted to conftest (4 drifted
copies); proc.kill() portability in the live push test.
Runner-twin dedup (static/pool protocol duplication) deferred to #842.
Refs #839
The static-path notification handler awaited its catalog refresh inline
in the SDK's receive loop, but the refresh issues a request on the same
session — a request whose response only that (now parked) loop could
route. The refresh never completed, and every user's calls on the
shared per-node session stalled behind it, unbounded, until the health
loop's ping timeout tore the transport down — which was also the only
way a pushed catalog change ever landed. Port of the pool-path protocol
(#836) onto the static primitives:
- Refreshes are debounce-gated, coalesced per (server, kind), and
spawned as tracked tasks; the runner serializes on the per-name
connect lock so a refresh, a connect's discovery wiring, and the
manual/periodic _refresh_server pass can never publish out of order
(the remove -> re-add race is closed by lock identity, the static
twin of the pool's entry-identity check).
- The coalesce marker is cleared at lock-acquire so a change the
in-flight list missed spawns exactly one successor; the finally
discard is gated on non-acquisition so it never clobbers that
successor's marker.
- The debounce stamp survives a failed refresh (throttle over lost
window) and every teardown/eviction path now pops it via the paired
_drop_static_session_and_stamp, so a reconnected transport's first
notification refreshes immediately.
- All three static list calls are bounded by _CONNECT_TIMEOUT and
discard their result if the state entry was replaced mid-flight;
the resource pair rides one gather (mirrors the pool sibling).
- Failure logging is (Exception, BaseExceptionGroup) type-name-only:
an escaping group reaches _spawn_background's exc_info log, which
serializes the chained httpx request carrying the configured bearer
for auth_type=static servers; the recorded operator error string is
type-name-only for the same reason. Non-list-changed notifications
no longer clear the server's error pill (that pop was accidental —
only a completed refresh proves anything).
Includes a live end-to-end repro (FastMCP subprocess pushing
tools/list_changed through a real receive loop): pre-fix the triggering
call itself deadlocks (verified against main), post-fix it completes
with the catalog landing on the original session, no teardown.
Closes#839
Bot PR reviews (Copilot, code-quality) fired pull_request_review and
pull_request_review_comment runs that always gate out but pile up as
awaiting-approval clutter. @claude stays invocable via issue and PR
conversation comments, the only path actually used.
- Orphan argument deltas count as delivered output for the
finish_reason_optional shim, exactly as they count as a streamed
signal for the terminal harvest: a lax Responses server that never
announces items AND never sends a terminal event still delivered its
tool call — with the tolerance declared that is a completion, not an
IncompleteStreamError. (Review caught the shim/harvest inconsistency
the round-9 fix introduced.)
- Test style: single import style for the model_turn module, assert on
a local instead of a call expression, drop a pass-through lambda.
Correctness:
- Responses: orphan argument deltas (streamed without any
output_item.added) now count as a streamed tool-call signal, so the
terminal harvest stands down instead of re-emitting the same call
onto the same slot — the reproduced collision concatenated the
arguments JSON into an unparseable double copy.
Cleanup / documentation:
- finish_shim_due in _protocol is THE gate for the lax-server finish
shim — one predicate (and one definition of 'delivered output') for
all three adapter families, so the same capability flag cannot
acquire per-family completion semantics.
- The Responses error/response.failed branches share one failure tail
(only code/message extraction differs) — the same server failure can
never become retryable through one event type and fatal through the
other, pre- or post-terminal.
- _format_refusal pins the refusal rendering the streamed event and
the terminal harvest both use.
- The capability-table floor comment and CHANGELOG Removed entry now
state the real rationale: OpenAI has RETIRED the pruned ids from the
API — the rows described unreachable contracts, not unpopular ones.
- CHANGELOG names the stream-entitlement break class (verified-org
streaming, pre-stream_options gateway api-versions) with its
serving-side remediation; deliberately no non-streaming fallback.
- docs/architecture.md retry section describes the collapsed
transport: the two stacked retry ladders, IncompleteStreamError /
ResponsesStreamFailedError retryability, finish_reason_optional
remediation; stale non-streaming mentions updated (+ puml).
- Anthropic whole-block emission carries its residual hybrid-gateway
bet as an explicit comment.
Held on standing rulings: post-finish usage forfeiture (keep result +
warn, rounds 4/8), session merge_usage twin and StreamAbortRef twin
(#832), stream_options wire delta (round 2, caveat now names Azure).
Correctness:
- Responses: output that exists ONLY in the terminal payload (buffering
gateways that never fire output_text.delta / output_item.added) now
reaches CompletionResult.content and tool_calls — the retired
non-streaming _parse_response read this same payload, so the drain
must too instead of returning a clean-looking empty success (blank
compaction summary, silently-skipped tool call). Gated on nothing of
that kind having streamed; refusal parts render as the streaming
branch does.
- Anthropic: content pre-populated inside content_block_start (whole-
block lax-gateway emission — the real API sends start blocks empty)
is emitted for text, thinking, and tool_use input, type-guarded like
_reasoning_text so duck-typed blocks can't leak non-strings.
- Responses: a response.completed payload that OMITS status maps to
"stop" via the event type, matching the payload-less branch — the
empty-string status read as 'length' and fired truncation policies
on complete output.
- model_turn: the drain-retry loop re-checks cancel_ref.aborted after
the backoff sleep — an abort landing mid-sleep now kills the
abandoned worker with the original failure instead of issuing one
more full request behind the deadline's back.
- drain_stream: the post-finish transport-blip tolerance logs a
warning naming whether usage was captured — the kept result may
report usage=None (chat-lane usage trails the finish reason) and
that spend was vanishing from usage accounting with no signal.
Cleanup:
- ChatSession's inline tool-call fold adopts accumulate_tool_call_delta
(drop-in — same ToolCallDelta semantics), so THE merge rule now has
one implementation across the chat loop, drain_stream, and the
Google capture; the helper's mirror-mandate docstring is retired.
- The task-agent _api_call contract comment reconciles the two retry
layers (sub-harness owns request-level policy; model_turn owns
drain-time re-issue) instead of claiming model_turn is policy-free.
- Anthropic's three terminal-emission sites share one
_attach_terminal_blocks helper — replay fidelity can't depend on
which terminal path a stream took.
- Responses create_streaming resolves capabilities once.
Held on standing rulings: o-series capability-row removal (4th report;
deliberate break, release-noted), StreamAbortRef/_CancelRef unification
(#832; docstring mirror-mandate).
Correctness:
- ToolCallSlotter: a slot whose id is KNOWN never splits on an id-less
delta — on an id-disciplined server new calls arrive with ids, so an
id-less fragment (the call's FIRST name announcement included) is
always a continuation. Round-6 regression: {id} → {name} → {args}
emission split into an unnamed id-bearing call plus a nameless twin.
Also: a name arriving for a slot with no name yet never splits
(args-first emission), and a bare same-name delta after complete
arguments merges as a redundant footer instead of minting a phantom
zero-argument call that would re-run a side-effecting tool.
- finish_reason_optional is honored on every drained lane, not just
Chat Completions: Anthropic shims a missing message_delta
stop_reason + message_stop pair, Responses a missing terminal event
(both with collected blocks riding the shimmed finish) — the
documented capabilities-JSON remediation now works on the
anthropic-compatible/responses-compat gateways it was written for,
matching the retired non-streaming paths' tolerance.
- Responses: an in-band error/response.failed frame arriving AFTER the
terminal event is teardown noise — log and end the stream instead of
raising away a generation already in hand (the in-band twin of
drain_stream's post-finish transport-blip tolerance).
- model_turn drain retries pace like the SDK request retry they
replace: 0.5s base, doubling, ±50% jitter — instant re-issues
re-hit the still-active rate limit/overload and synchronize into
fleet-scale retry bursts.
- Responses slot bookkeeping survives lax servers: slots minted by a
counter (len(dict) collided calls after a duplicate/empty item-id
overwrite), orphan argument deltas route to the most recently
announced call instead of hardwired slot 0.
Cleanup:
- on_tool_call_delta now receives the normalized ToolCallDelta plus the
raw SDK delta — Google's capture accumulates the exact bytes the
mirror sees (the byte-identical extraction no longer exists twice).
- _ArgsScanner feeds only fully id-less slots (its verdict is never
consulted for id'd slots — dominant-case hot path).
- Anthropic retryable set hoisted to a class constant (per-access
frozenset allocation, same pattern already fixed on Responses).
- GoogleProvider class docstring names the hook-based capture instead
of the deleted _extract_tool_calls override.
Correctness:
- The chat-lane finish shim is now armed only by an operator-declared
finish_reason_optional capability (model-definition capabilities JSON).
Default lanes treat a clean finish-less end as died-mid-generation
(retryable) — SSE cannot distinguish lax-server completion from a
worker dying behind a clean-closing proxy, and the default must catch
truncation rather than bless it. When armed, reasoning-only output
counts as a completed generation (parity with the retired
non-streaming path's finish_reason-or-stop default).
- ToolCallSlotter v3: id-less call-boundary decisions now consult
argument JSON completeness (incremental scanner) and name identity
instead of a boolean has-args gate. Fixes both residual id-less
ambiguities: two zero-argument whole-delta parallel calls no longer
fuse (silently dropping an action), and redundant per-fragment name
headers no longer split one call into malformed half-JSON calls.
- model_turn re-issues transient mid-stream deaths (provider's
retryable_error_names, raised while draining) up to twice — the new
home of the SDK request-level retry the non-streaming transport gave
every single-shot lane (judge, title, perception, compaction).
Request-time failures keep the SDK's own policy; an aborted
cancel_ref suppresses re-issue (StreamAbortRef gains .aborted).
Cleanup:
- One slotter drives both the normalized mirror and Google's raw
fidelity capture via an on_tool_call_delta hook — raw/mirror slot
parity is structural now, not a maintained invariant.
- accumulate_tool_call_delta in _protocol.py is THE tool-call merge
rule; drain_stream and the Google capture use it (session's copy is
#832's tracked adoption).
- Responses terminal rebuild only runs when the terminal payload can
disagree with the .done-collected items (truncation or count
mismatch); on rebuild, annotations are replaced, not re-extended.
- Responses retryable set precomputed at class creation.
Held on standing rulings: o-series capability-row removal (deliberate,
release-noted with remediation), StreamAbortRef/_CancelRef unification
(#832; docstrings mandate mirroring until then).
The OpenAI commercial capability table floor is now gpt-5.4: o1,
o1-mini, o3, o3-mini, o3-pro, o4-mini, gpt-5, gpt-5-mini, gpt-5-nano,
gpt-5-pro, gpt-5.1, gpt-5.1-codex-max, gpt-5.2, gpt-5.2-pro, and
gpt-5.3 are effectively unused in the field. The gpt-5-search-api row
(different product surface) and the audio/STT/TTS rows stay.
A legacy id now resolves to OPENAI_DEFAULT (temperature sent, no
declared effort vocabulary, 200K window) — which those models may
reject; the remediation is the model definition's capabilities JSON or
a current model, release-noted under Unreleased → Removed.
This also retires the transport-collapse review's thrice-reported
"stream-rejecting o1-era models are stranded" finding by removing its
subject: no row in the table describes a non-streaming model anymore.
Tests migrate to 5.4-era equivalents that pin the same behaviors:
always-reasoning temperature suppression and off-list effort snap
(gpt-5.4-pro for gpt-5-pro/o3), explicit-none forwarding (gpt-5.4 for
gpt-5.1), empty-effort-vocabulary knob drop (gpt-5-search-api for
o1-mini), and the longest-prefix shadow hazard (gpt-5.4-pro vs gpt-5.4
for codex-max vs gpt-5.1).
Correctness:
- ToolCallSlotter's reannounce split is gated to ID-LESS deltas: id
equality proves the same call, so compat servers that repeat the
id+name header on every argument fragment merge back into one call
with valid JSON (round 4's ungated heuristic split them into
duplicate half-JSON calls — execution-confirmed by the review). The
residual id-less repeat-name-per-fragment shape is documented as
inherently ambiguous; ids are the only disambiguator.
- drain_stream keeps a completed result when the transport blips AFTER
the finish reason (trailing usage chunk / citation footer window):
the generation is in hand, so forfeit the trailing metadata instead
of discarding a fully-delivered verdict or re-paying a compaction.
- The chat iterator shims finish_reason="stop" when a stream ends
CLEANLY after delivering content or tool calls — the deleted
non-streaming `or "stop"` default for lax finish-reason-less servers,
now safe to restore because abrupt deaths surface as
httpx.TransportError (round 4) rather than clean exhaustion. This
supersedes the round-3 keep-the-gate ruling: the httpx catch changed
the calculus, and the Anthropic/Responses lanes already got their
marker-based shims. Empty/reasoning-only streams still fail the
complete-or-error gate. Two streaming tests gained the shim chunk.
Dispositions held: o1-era stream-rejecting models (third re-report)
stay a release-note remediation per the earlier ruling.
Cleanup: the two Responses terminal branches collapse into one path
(status derived from the event type when the payload is missing —
also fixes the end-of-stream debug log reporting finish_reason=None
for completed lax streams); the annotations walk is one shared helper
(the two copies had already diverged on None-content guarding);
_raise_responses_failure is annotated NoReturn; scripts/livepass.py
drops the phantom supports_streaming key; test_model_registry's
capture helpers ride scripted_chat_client; _openai_stream_chunk points
at its fake_chat_stream shape-twin for future consolidation.
Correctness:
- drain_stream chains raw httpx.TransportError from stream iteration
into retryable IncompleteStreamError (original type+message preserved
via __cause__): streaming moved the body read out of the SDK's
APIConnectionError-wrapped request, so mid-body connection drops and
read timeouts — retried transparently on 1.7 — were escaping every
single-shot retry loop as instantly-fatal raw httpx names.
- The index remap is extracted as ToolCallSlotter and GoogleProvider's
raw tap slots THROUGH IT over the same delta sequence as the base
iterator: round 3's mirror-side de-fusion had left the tap keying by
wire index, so a degenerate stream produced 2 mirror calls vs 1 fused
raw dict — _prepare_messages' length gate then silently dropped the
thought_signature lane (400 on signature-strict Gemini models).
- The slotter also splits ID-LESS degenerate parallel calls: a delta
announcing a name for a slot that already accumulated arguments is a
second whole call, not a fragment (fragmented single calls pinned
unaffected).
- A payload-less Responses terminal event keeps the provider_blocks
already collected from output_item.done events (they came from the
stream, not the missing payload); only usage is genuinely lost.
- The truncation-rebuild path walks the terminal output's message
annotations, so truncated web-search turns keep their Sources footer
(the in-flight item never received output_item.done).
Cleanup: one _raise_responses_failure ladder serves both in-band
failure shapes (error events + response.failed); IncompleteStreamError
joins the public providers export (docstrings tell callers to catch
it); the de-fusion tests ride the file's existing _openai_stream_chunk
helpers instead of a third hand-rolled SSE fake; the dead if-response
guard in the terminal branch is gone.
Deferred with note: classifying IncompleteStreamError once at the
retry-predicate consultation site instead of per-provider strings is
#832 territory (the predicate lives in ChatSession); the six-lane
parametrized test guards the listing until then.
Correctness:
- Responses _iter_stream handles the SDK's in-band `error` SSE event
(ResponseErrorEvent is YIELDED, not raised, and no response.failed
need follow): the real API code/message now surfaces — code-gated for
retryability like response.failed — instead of the stream exhausting
finish-less and hiding the cause behind a retried
IncompleteStreamError.
- Anthropic message_stop supplies a missing stop_reason: it is a genuine
terminal marker, so a compat /v1/messages shim whose message_delta
omits stop_reason completes (blocks intact) rather than failing a
generation that arrived — tolerance the retired non-streaming default
provided, restored without weakening the died-mid-response gate.
- A Responses terminal event without its response payload still emits
the finish reason its type implies (lax compat servers), losing only
usage/blocks rather than the whole result.
Dispositions held (documented, not re-coded): the complete-or-error
gate stays for finish-less Chat Completions streams — indistinguishable
in-band from a died generation, and silent partial-storage is the worse
failure; CHANGELOG now names the shape and each provider's accepted
terminal markers. supports_streaming deletion and the stream_options
wire delta were ruled earlier and keep their release-note remediations.
Cleanup: index-degenerate de-fusion MOVED from drain_stream into the
chat adapter's iterator (mirroring the Anthropic iterator's index
assignment) so the interactive loop is fixed too and the drain returns
to a plain mirror of the main-loop accumulator; a parametrized test
locks "IncompleteStreamError is retryable" across all six provider
lanes instead of trusting per-adapter memory; scripted_anthropic_client
joins scripted_chat_client (shared _ScriptedClient class, no function
attrs) and the two remaining hand-rolled anthropic closures convert.
Correctness (3 confirmed + 2 plausible, all fixed):
- drain_stream now raises typed, retryable IncompleteStreamError when a
stream exhausts without any finish reason — every adapter emits one on
a healthy stream, so its absence means the generation died
mid-response behind a cleanly-closing proxy. This restores the
retired transport's complete-or-error contract (a half-generated
compaction summary was previously returned as finish=stop and stored,
silently replacing real history) and DELETES round 1's suffix-info
fold: with no finish-less success path there is nothing to classify,
so a trailing status ping can never be stored as content either.
- Index-degenerate parallel tool calls get distinct slots: a delta whose
id differs from its slot's opens a new call (id-less fragments still
follow their index's current call), so historical compat servers that
emit every parallel call at index 0 no longer fuse distinct calls
into concatenated garbage arguments. Result order stays index-sorted
(stable) like the retired array parse.
- response.failed retryability is code-gated: only transient codes
(server_error, rate_limit_exceeded) raise the retryable typed error;
deterministic rejections (invalid prompt, image fetch, policy) raise
plain RuntimeError and stop retry loops on attempt zero instead of
running the full backoff ladder against a doomed request.
- Terminal Responses events rebuild provider_blocks from
response.output when present: the item being generated at
max_output_tokens truncation never receives output_item.done, and
storing a reasoning item without its required following item made the
next turn's replay a 400.
- merge_usage's base case uses dataclasses.replace so a future UsageInfo
field can't be silently zeroed on drained lanes.
Cleanup: run_abortable_with_deadline bundles the three-point abort
wiring (ref + cancel_ref + on_abandon) so it cannot be half-wired —
both judges converted; scripted_chat_client hoists the 14 chat-lane
fake_create closures (call scripts + .calls recording replace per-test
counter cells); fake_chat_stream gains reasoning=, collapsing the
reasoning-capture suite's hand-rolled chunk shape; FakeAnthropicBlock
hoists the duplicated _Block test class; the class and judge PlantUML
diagrams drop the retired create_completion flow.
Also converts test_model_registry's agent-model fakes, which returned
legacy response objects that iterated as EMPTY streams — they only
passed through the old drain's silent finish=stop default, exactly the
hazard the new gate exists to catch.
Correctness (4 confirmed + 1 plausible fixed, 2 accepted+documented):
- Anthropic _iter_anthropic_stream handles citations_delta: text-block
citations now ride the raw block into provider_blocks, as replay
requires (the retired non-streaming lane preserved them via
model_dump; the streaming lane dropped them — a pre-existing main-loop
gap the collapse would have extended to single-shot lanes).
- Anthropic text blocks separate with "\n" at each subsequent block
start, restoring the retired lane's "\n".join rendering on drained
lanes AND un-fusing streamed web-search responses in the chat loop.
- response.failed raises typed ResponsesStreamFailedError, listed in the
provider's retryable_error_names — retry loops treat an in-band
failure like the wire errors it stands in for instead of
hard-stopping on a bare RuntimeError (judges keep their heuristic
fallback after retries).
- drain_stream folds a finish-less stream's terminal citations footer
(suffix rule: pre-finish info invalidated by any later payload), so
lax compat servers that never send finish_reason keep their Sources.
- usage max-merge extracted as merge_usage() in _protocol.py — the one
definition drain uses now and the session's inline consumer adopts on
#832.
Accepted + release-noted instead of coded around: strict pre-2024
compat servers that 400 on stream_options (such a server already cannot
serve the chat loop; CHANGELOG caveat extended), and repeated-index
parallel tool-call merging on legacy compat servers (identical to the
main loop's accumulator semantics; a shared guard belongs in the #832
unification).
Cleanup: run_with_deadline grows on_abandon (best-effort, cannot mask
the deadline error) and both judges drop the copy-pasted abort
choreography; StreamAbortRef documents the _CancelRef adoption plan;
test_model_turn's fake replays through the shared as_stream adapter;
docs/architecture.md drops the retired Protocol row.
Tests: refusal handler pinned (was advertised, untested); typed-failed
retryability; citations capture; text-block separator (plus the mixed
text+search expectation updated for the separator chunk); finish-less
citation fold; on_abandon firing matrix; StreamAbortRef arrival race.
Every single-shot lane (model_turn: judges, titles, compaction, web-fetch
extraction, perception, eval, optimizer) now samples through the provider's
streaming entry and accumulates via a shared drain_stream(), deleting
create_completion from the Protocol and all three adapters (xai/google
inherit). Request shaping can no longer drift between the two consumption
styles, and callers keep the exact CompletionResult contract.
The drain mirrors the main loop's proven chunk semantics: per-field
max-merge for usage (Anthropic splits prompt/completion across
message_start/message_delta), tool-call assembly by delta index,
provider_blocks from the terminal emission, trailing citation info folded
back into content (byte-matching the old format_citations append),
mid-stream status pings dropped.
Also in this change:
- model_turn grows cancel_ref; both judges wire their run_with_deadline
abandon paths to a new StreamAbortRef (deadline.py) that closes the SDK
stream — a timed-out judge call now aborts its HTTP read instead of
pinning a daemon thread until the next upstream chunk. The append hook
covers the arrival race, mirroring ChatSession._CancelRef.
- Responses streaming gains the response.incomplete terminal handler
(truncated runs were mislabeled finish=stop and lost final usage AND
collected provider_blocks) and a refusal handler ([Refused: …] content,
matching the retired non-streaming rendering). Both also fix the main
chat loop, which shared the gaps.
- supports_streaming capability flag deleted (zero readers) along with
its admin capability tile; o1-era models that reject streaming need a
model alias pointing at a current model (release-noted).
- Helpers that existed only for the deleted transport go with it:
Responses._parse_response, chat/google._extract_tool_calls.
Known behavioral deltas (release-noted): OpenAI-compatible servers that
ignore stream_options.include_usage stop producing usage rows on these
lanes; multiple Anthropic text blocks concatenate without the old "\n"
joint (matching the main loop); model_turn lanes no longer risk client
read-timeouts on long generations — the reason the Anthropic adapter
already drained a stream internally.
Tests: new test_drain_stream.py pins the accumulator rules; shared fakes
(as_stream, fake_chat_stream, fake_anthropic_stream) migrate 11 suites to
the streaming transport, with the task-agent and adapter suites now
exercising the real _iter_stream + drain path end to end.
Close the round-8 review findings:
- The refresh runner's finally-discard releases the coalesce marker
ONLY when the lock was never acquired (cancelled while parked).
After the at-acquire discard, a marker present at exit belongs to
the successor spawned during the in-flight list call — discarding
it unconditionally let the handler mint one extra runner per
debounce window while the lock was congested, reopening the
unbounded runner FIFO the marker exists to bound.
- The observe-before-lookup preamble lives once in
_pool_lookup_checked (snapshot taken synchronously before the
lookup await, render paired with the convergence drop) instead of
verbatim in all three dispatchers — the ordering contract is now
structural rather than comment discipline.
- drop_session is paired with its debounce-stamp pop in
_drop_session_and_stamp, shared by the eviction, teardown, and
owner-death paths; the shutdown sweep clears the pool notification
stamp dict and the coalesce marker set alongside the other pool
state.
- _mcp_tools_change_seq is initialized unconditionally for every
session kind, so the attribute's existence no longer encodes
whether an MCP client was wired at construction.
Close the round-7 review findings:
- The dead-grant observation is now snapshotted BEFORE the classified
lookup's first await, by the callers (the three dispatchers via
_pool_lookup_failure, _prime_one, and the obo credential gate), and
_schedule_dead_grant_drop requires it as a parameter: snapshotting
after the lookup returned could capture a session the
consent-completion prime connected mid-lookup — its awaits can park
on executor hops — and the drop then evicted the just-restored
catalog it exists to spare, with no remaining re-prime path.
- Spawned list_changed refreshes coalesce on a per-(key, kind) marker:
set at spawn, cleared the moment the runner acquires open_lock
(before its list call, so a change the in-flight list missed spawns
exactly one successor). Admission was one per 5s debounce window
while each runner can hold the lock up to the 30s refresh timeout,
so a notifying-but-slow server accreted lock waiters without bound —
FIFO dispatch waits past the 120s budget, idle eviction starved by
the contested lock, and background tasks growing for as long as the
server kept notifying. The runner also returns quietly for an
evicted session instead of failing through the log. The residual
duty-cycle case (a wedged-but-notifying server defers idle eviction
of its own entry until the first dispatch, recovery, or silence) is
documented at the runner.
- Every teardown path now pops the notification debounce stamp:
_teardown_pool_entry and _on_pool_owner_death left it in place, so
the keep-stamp design's documented reconnect backstop did not exist
on the idle-collapse and connect-failure paths — a change announced
in a failed window could be debounced against a pre-collapse stamp
after reconnect and never land. The idle-close path's own pop is
now owned by _teardown_pool_entry.
- Cleanups: the notification table maps type to kind label only, with
the kind-to-refresher map bound at dispatch time (mypy-checked
attribute references, instance overrides keep working) instead of
getattr on a name string; _schedule_dead_grant_drop skips when there
is provably nothing to converge (no entry, or a session-less
catalog-less stub), sparing a tracked no-op task per unconsented
server per prime at scale; the fire-and-forget prime idiom's three
hand-synced copies collapse into try_prime_user_pools (session
construction, acting-user change, OIDC capture); the stale
lock-contract docstrings on the resources/prompts refreshers now
state the held-lock requirement; has_live_session_listener is the
sole listener-liveness predicate (the private alias is gone); the
construction-scoped tools-seq read is a constructor local instead of
a persistent ChatSession attribute.
Close the round-6 review findings, all in the round-5 surface:
- Dead-grant drops snapshot the entry's session when the failed lookup
is observed and skip only when the session CHANGED since: a warm
transport that predates the revocation is evicted with the catalog
(failed lookups short-circuit dispatch before any 401 could evict it,
so nothing else converges a warm entry until the idle TTL), while a
session a re-consent prime created after the observation still parks
the drop. The obo credential gate inherits the same semantics for
warm obo entries.
- The spawned notification refresh serializes on open_lock with a
same-entry recheck: unserialized it raced the connect wiring block
(older discovery snapshot republished over the refresh's newer
catalog, permanently hiding the change behind the consumed debounce
stamp) and sibling same-key refreshes (the slower list call
publishing the older catalog last).
- The refresh failure path keeps the debounce stamp instead of popping
it: pop-on-failure re-armed the handler on every notification, so a
fast-failing server spawned refresh tasks unthrottled at its
notification rate. Changes announced in a failed window converge on
the next list_changed or reconnect (teardown pops the stamp).
- The refresh runner catches BaseExceptionGroup alongside Exception: a
wedged anyio transport surfaces session-op failures as groups, which
escaped to the background-task failure log whose exc_info serializes
the chained httpx request carrying the user's bearer.
- Cleanups: the three list_changed handler branches collapse into one
table-driven path; _reprime_active_users reuses _live_listener_uids;
the obo gate's synthesized kind="missing" verdict is contract-pinned
to get_obo_access_token_classified's missing-credential return.
The headline finding is pre-existing and structural, surfaced by this
branch's timeout: the SDK awaits notification handlers INLINE in its
receive loop, so a handler that awaits a request on the same session
can never receive its response — push-driven catalog refreshes have
never completed against a healthy server, and with the new timeout
they also stalled every in-flight call on the session for its
duration. Refreshes are now spawned as tracked background tasks, and
a FAILED refresh returns the debounce stamp so the server's next
list_changed retries instead of being dropped inside the window.
Also from the round:
- The obo credential-presence gate skipped exactly the per-server
lookup whose kind='missing' would have dropped retained catalogs, so
unlinked users' ghosts survived every new-session prime. The gate
now schedules the same dead-grant drop for catalog-bearing obo
entries before skipping the servers.
- Dead-grant drops re-validate under open_lock via skip_if_connected:
a drop parked behind a re-consent prime's connect must not clear the
freshly restored catalog (a live session proves a connect succeeded
after the failed lookup that scheduled the drop). The explicit
revocation path still clears warm entries unconditionally.
- The constructor's convergence re-check moved to the end of tool
setup, where every _on_mcp_tools_changed dependency exists — the
while-loop re-read could still be clobbered by the tool-search
construction reading mixed state, and a mid-construction callback
crash (pre-existing, swallowed by the fan-out) loses its update.
- _drop_catalog_locked's docstring told the truth about its wait bound
(a same-key dispatch holds open_lock across its entire SDK call).
- The OIDC capture-site liveness gate call moved inside its try —
nothing on that best-effort path may fail a login.
- One _pool_lookup_failure helper pairs render+drop for all three
dispatchers; fake pool-tool seeds deduped to one module helper; the
sleep-based test syncs replaced with a deterministic
_background_tasks drain.
Round 4 confirmed six correctness bugs, all inside round 3's
catalog_gen machinery (a ChatSession.__init__ crash from the mirror-
race re-run, an orphaned-lock race created by the ensure-before-lookup
reorder, no generation memory across entry re-creation, gen reset on
re-ensure, a raw internal error surfacing to the session layer). Four
rounds of evidence: hardening this event-driven subsystem with new
concurrency machinery breeds interaction bugs about as fast as it
closes cosmetic races. Decision: remove the protocol, keep the core.
Stripped: PoolEntryState.catalog_gen, the expected_gen threading
through dispatch/prime/connect, the dispatcher ensure-before-lookup
reorders, _PoolGrantRevokedError, and the refresh gen-guards (the
entry-identity check stays — it protects against entry replacement
with no protocol). The publisher-suspended-across-a-drop races those
closed are now ACCEPTED RESIDUALS, documented at
_evict_session_drop_catalog: the ghost self-heals at next use via the
dead-grant drop (dispatch AND priming), and a reconnected stale bearer
dies at access-token expiry — the same bound every warm session
already rides at revocation time.
Kept from round 3 (stable, orthogonal): staged discovery publication,
prime-side dead-grant drops, the obo re-login prime, the single
_pool_lookup_verdict classification, drop_session() pairing, and the
tracked revocation drop task.
Fixed from round 4's orthogonal findings:
- ChatSession construction converges its tool lists with a bounded
re-read loop instead of calling _on_mcp_tools_changed, which
dereferences tool-search state initialized later in construction.
- _refresh_pool_server_tools gets the asyncio.timeout its resource and
prompt siblings already had — a wedged server no longer hangs the
notification-handler task.
- The OIDC capture-site prime is gated on the user having a live
session listener (new public has_live_session_listener): routine SSO
re-logins with nothing open no longer fan out mints and connects.
- _pool_lookup_verdict returns a Literal so a typo'd verdict
comparison fails mypy instead of silently never matching.
- The triplicated double-401 comment blocks shrink to two-liners
pointing at the single rationale in _evict_session's docstring.
Round 3 identified the class behind the remaining bugs: catalog
PUBLISHERS never re-validate revocation state, so anything that read a
token or suspended before a drop could republish (resurrect) a revoked
catalog that retention then keeps forever. One primitive closes the
class:
- PoolEntryState.catalog_gen, bumped by _evict_session_drop_catalog.
The three list_changed refreshes snapshot it before their awaits and
discard results if it moved; dispatch and priming snapshot it before
their token reads, and _connect_one_pool refuses to connect (raising
_PoolGrantRevokedError, a non-breaker failure) when the generation
moved past the caller's snapshot — the bearer in hand predates a
disconnect.
- _connect_one_pool stages all three discovery results locally and
publishes them together in the final wiring block: a mid-discovery
failure now leaves the retained catalog exactly as it was instead of
a torn half-update diverging from the per-user maps.
- Priming converges dead grants too: _prime_one schedules the same
catalog drop the dispatchers use, so a NEW session's prime clears
ghosts left by a disconnect made on another node.
- obo re-login is the obo restore moment: a successful credential
capture at the OIDC callback now schedules prime_user_pools, so a
previously dropped obo catalog returns to LIVE sessions (obo has no
consent flow to heal through).
- ChatSession construction re-runs its tool rebuild when the change
marker advanced during its authoritative read — the mirror race
where a fresher listener update was clobbered by the constructor's
staler snapshot.
- evict_user_session's drop task is now tracked (_spawn_background) so
shutdown cancels it instead of abandoning a parked task.
Dedup/altitude from the round: _schedule_dead_grant_drop is the single
drop block (was three byte-identical copies); _pool_lookup_verdict is
the single lookup classification — rendering and _lookup_grant_dead
both derive from it, with literal code strings kept so the consent-url
sibling audit still sees the sites (expected count 7 -> 5 after the
collapse); PoolEntryState.drop_session() pairs session/bound_token
clearing structurally (owner-death was missing the bearer clear).
Five confirmed correctness findings, all in the round-1 fix code:
- The dead-grant catalog drop at the token-lookup error sites is now
SCHEDULED instead of awaited: the drop waits on open_lock, which a
same-key dispatch holds across its entire SDK call, so awaiting let
a token-side error stall past the sync timeout and charge the
breaker it is documented to bypass.
- The double-401 drop is removed entirely: a second 401 after a
SUCCESSFUL forced refresh proves the grant is alive at the AS — it
is the resource server rejecting a fresh bearer (JWKS lag, audience
misconfig, clock skew), and dropping the catalog made RS recovery
unhealable for live sessions. A genuinely revoked grant converges
via the token-lookup drop (its row is gone by then).
- The drop decision has one source of truth (_lookup_grant_dead),
gated on the token store + storage actually being wired: the obo
lookup returns kind='missing' for boot-window infrastructure
absences too, which must not clear catalogs. The empty-token
fallback now classifies with its consent_required siblings.
- The LRU pass re-checks the LIVE warm count per iteration again —
the one-shot over-count never saw concurrent warm-set changes
(revocation evictions, owner deaths, connects) and closed healthy
transports below the cap.
- _on_pool_owner_death clears bound_token: the third session-drop
site the bearer-clearing sweep missed, and the one that cools an
entry indefinitely.
Also from the round: reconcile stores both pool-name registries as
adjacent assignments and _retain_cooled documents the residual
single-bytecode flip-tear window (restored by the same reconcile's
re-prime); catalog-less drops skip the zero-delta rebuild+notify
fan-out; session construction does one authoritative post-registration
read instead of read-twice; evict_user_session schedules the locked
drop directly.
Fix round for the review of the #836 catalog-retention change
(14 findings: 9 correctness, 5 cleanup):
- Cooled retention now requires the server to still exist in the pool
registries (_retain_cooled — ONE policy shared by the TTL skip and
the close path): an admin delete/disable/rename/auth-flip drops the
ghost catalog within one eviction tick. Pre-#836 the idle TTL
bounded such ghosts to ~10 minutes; retention made them immortal,
including a disabled server that stayed dispatchable and duplicate
tool names after a flip to static.
- A dispatch that learns the grant is durably GONE (token row missing
or refresh permanently rejected — the mcp_consent_required class)
drops that (user, server) catalog, so a disconnect made on another
node converges here at first touch instead of re-offering revoked
tools behind a consent card. Re-consent restores the tools through
the existing consent-completion single-server prime.
- Revocation drops serialize against an in-flight connect via the
entry's open_lock (_drop_catalog_locked): an unserialized drop was
republished (resurrected) by the connect's completing discovery,
with nothing left to ever clear it.
- The LRU pass counts closes incrementally and the TTL pass checks a
once-per-tick listener snapshot instead of scanning the listener
registry per entry under its lock.
- bound_token (a plaintext bearer) is cleared whenever the session is
dropped — it is dead on a session-less entry, and cooling otherwise
retained it for the life of the user's sessions.
- Per-user status falls back to the cooled catalog for its counts and
reports the idle pool separately (user_pools_idle): cooled is the
steady state now, and the warm-only view said '0 tools' for a
catalog the same user's chat was actively offered.
- Session construction re-reads the merged tool lists after listener
registration, closing the read-then-register window that missed a
concurrent drop's only notification.
- Dedup: one retention policy, one warm predicate, one rebuild+notify
sequence (was three copies), and the drop-catalog path now layers
on _evict_session instead of copying its prologue.
Known limits, deliberately deferred: shared-workstream participants
who are not the acting user still lose their catalogs at TTL (not a
regression — the next send re-primes), and the pre-existing
orphaned-lock race on full-drop is unchanged.
Idle-TTL eviction tore down a per-user pool entry, rebuilt the user's
tool/resource/prompt catalogs (now empty), and notified listeners — so
every live ChatSession for that user silently lost the server's tools
after 10 idle minutes, with no way back: prime_user_pools only runs at
session construction, acting-user change, and reconcile, and the
emptied catalog closes the session-side is_mcp_tool gate, so even a
history-motivated call can't reach the lazy-reconnect dispatch path.
The dispatch-failure paths (_evict_session on 401/403/transport)
cleared catalogs the same way, so a transport blip during a tool call
caused the same permanent loss with no TTL involved — and made the
breaker's half-open recovery and the consent/step-up cards unreachable.
Both now follow _on_pool_owner_death's evict-session-keep-entry shape:
- _evict_session drops only the session. The catalog stays; the next
dispatch connect-or-reuses and re-runs discovery, so drift
self-corrects and the refresh notification fans out then.
- TTL eviction COOLS entries of users with a live session (a
registered user-scoped tool listener): transport closed, entry and
catalog retained, no fan-out. Users without one keep the full drop,
so departed users' entries don't outlive their sessions.
- The LRU cap now bounds WARM entries — the connection resources it
exists to limit. Over the cap, live-listener users' entries are
cooled rather than dropped; cooled catalog-only entries are bounded
by live users x pool servers and reaped one tick after the user's
last listener goes away.
- Explicit disconnect keeps its semantics: evict_user_session routes
to the new _evict_session_drop_catalog (clear + rebuild + notify) —
the user asked for the tools to leave. Clearing the catalog also
marks the entry droppable, so it can't linger cooled.
Never-discovered stubs (no catalog) are always dropped, already-cooled
entries are skipped by later ticks, and a cooled entry keeps its
open_lock object for in-flight dispatchers.
Applies to oauth_user and oauth_obo alike: the pool and its eviction
are auth-type-agnostic, and for obo priming is the only path tools
enter a catalog at all.
Fixes#836
- The main streaming loop now applies the in-code model-definition rung
(caps.default_reasoning_effort) exactly like model_turn does, so the
same alias samples identically between chat and every auxiliary lane
(resolve_lane's stated contract). This also unblocks operator
temperature on gpt-5.x aliases whose declared default is "none" — the
main loop previously sent neither knob while aux lanes sent both.
- coordinator.reasoning_effort default "medium" -> "" (the missed unset
sentinel): coordinators inherit like every other lane; the role rung
fires only when the operator stored a value.
- admin webux: _onSettingChange no longer hides the save button for a
blanked nullable number input, so the blank-means-inherit save path is
actually reachable from the field it decorates.
- /model switch on STORE-LESS sessions (the CLI) keeps the user's
explicit --temperature//reason knobs when the target alias declares no
override — the current knobs are the only authority there (mirrors
the max_tokens fallback). Store-backed sessions still re-resolve.
- ModelLane docstring no longer documents the removed caller-default
effort rung; CLI status line shows any resolved effort ("medium" is no
longer a hidden code default); dead `u = usage` alias dropped; three
test docstrings re-pointed from the deleted
ChatSession._maybe_synth_reasoning_block to
model_turn.synth_reasoning_block.
Follow-up ruling on the round-2 batch: default_reasoning_effort is
removed entirely. On local lanes effort_passthrough forwards the value
VERBATIM with the template as the sole authority on validity, and we
explicitly do not define effort vocabularies (or floors) for local
models — so a code-chosen "low" is an unvetted token, and on
manual-thinking boxes it flips enable_thinking on for lanes the
operator never configured, diverging from the main loop's unset. The
effort scheme is now exactly the temperature scheme: explicit relay >
alias > stored config > model definition > omit.
Utility/guard consequences handled the honest way instead:
- title gen: _TITLE_MAX_TOKENS 2048 -> 8192 (the budget must fit a full
thinking pass at the MODEL'S OWN default now that code never bounds
it) and the prompt enforces a hard 3-word maximum so the visible
answer is trivially cheap regardless of what thinking spent.
- output guard: keeps its 512 cap; an unbounded thinking model that
overruns it parses to a labelled llm_error verdict (heuristic tier
stands) and the documented remediation is an effort value on the
guard's model alias.
Round-2 review fixes. The round-1 de-pinning collided with
ConfigStore.get's default-on-miss semantics: the registry defaults
(temperature 1.0, effort "medium") were manufactured onto every
store-backed lane's wire, making the documented "unset -> omit"
terminal unreachable. Unset is now representable end to end, and one
scheme governs every lane: per-model alias value > operator-stored
global setting > in-code model definition (effort only: caps
declaration) > field omitted, inference engine's default rules.
- settings_registry: model.temperature default None, model.reasoning_effort
default "" — the registered defaults ARE the unset sentinels, so the
admin UI and the wire agree. Admin webux renders nullable floats blank
("(inherit model default)") and maps blank-save to reset; the "" effort
choice reads "(inherit)".
- model_turn: resolve_temperature_setting/resolve_effort_setting are the
ONE pair of operator-rung resolvers, shared by resolve_lane, both
session factories, and the /model switch (the 4th-copy mirror is gone;
the switch no longer leaks the previous model's override on store-less
sessions). The caps rung moved out of the lane into model_turn's
effective computation, below a new request-shaped default_reasoning_effort
parameter (utility + output guard pass "low": budget coherence with
their small token caps, not sampling policy — any operator or
model-definition value beats it). The hidden "medium" terminal is gone.
- providers: Protocol + all adapters take reasoning_effort: str | None =
None (the Protocol-signature "medium" was the same manufactured pin one
layer down); ModelCapabilities.default_reasoning_effort defaults "" —
commercial rows all declare theirs explicitly, so only local lanes and
Anthropic change, both to match their real serving defaults (Anthropic
manual-thinking models no longer get implicit thinking-on-medium).
reasoning_template_kwargs distinguishes unset (inject nothing; template
default rules) from the explicit "none" off-switch. apply_temperature
skips temperature unless reasoning is EXPLICITLY off on none-declaring
models (unset leaves the server default in charge, possibly reasoning-on).
- session: ctor takes temperature: float | None / reasoning_effort:
str | None = None; _save_config/resume round-trip unset as "" (the
str(None) era guarded); _run_agent relays session temperature AND
effort on the same-alias fall-through only (a task alias's configured
knobs stay reachable in both directions).
- optimizer: the five meta lanes are decoupled from --temperature/
--reasoning-effort (test-model knobs, per their documented meaning);
registry-less meta lanes omit both fields.
- cli: --temperature/--reasoning-effort default unset and fall through
the model config instead of pinning 0.5/"medium" for every CLI session.
- cleanup from the review's below-cap findings: dead resolve_server_type
deleted (tests re-pointed at _server_type_of), stale ChatSession
comments in _openai_responses fixed, _store_get_or_none extracted,
eval system-turn conversion hoisted out of the per-turn loop, dead
_provider_extra_params patch removed, test_perception uses the shared
mock_completion_result, effort_ladder uses apply_capability_overrides
instead of a SimpleNamespace fake config.
Wire goldens regenerated: the only drift is the manufactured "medium"
effort vanishing from unset-effort requests (Responses reasoning.effort,
Chat/Google reasoning_effort, Anthropic output_config.effort) — pure
removals, no additions. Ladder tests now fake ConfigStore with the REAL
get() semantics (registry default on miss) so a forgiving fake can't
mask this class of bug again.
Patrick's rulings applied from the round-1 high review:
- reasoning_effort loses every code pin, same as temperature: ModelLane
resolves the ladder (ModelConfig.reasoning_effort → global
model.reasoning_effort setting → the lane capabilities'
default_reasoning_effort), model_turn takes str | None, and the pins
in both judges, all five optimizer lanes, _utility_completion's
signature default, and model_turn's own "medium" default are gone.
Explicit relays of user/operator knobs (session effort on the agent
seam and web-fetch, harness knobs in eval) stay relays. Effort's
terminal is the caps default, not wire omission — it gates thinking
modes, so unset ≠ the explicit "none" value.
- model.temperature setting default 0.5 → 1.0 (safer for modern models;
several providers no longer accept temperature at all — those drop it
via capabilities regardless). No judge-specific knob: a judge alias
with a per-model override is the remediation path.
- The agent seam keeps the alias ladder (configured → inherited global
→ none), per ruling; the ModelLane docstring no longer documents the
removed session-relay convention.
- Optimizer lanes get real operator knobs: the existing --temperature /
--reasoning-effort CLI flags now relay into all five internal LLM
steps (previously they reached only the eval sessions, leaving the
deleted pins with no replacement mechanism).
Round-1 cleanups: create_streaming widened to float | None (the
Protocol's two entry points agree; all callers pass explicitly);
model_turn's provider invocation is a direct keyword call again (strict
mypy re-checks it); perception threads the caller's already-resolved
capabilities (one config generation across gate and wire); redundant
extra_params pre-resolution dropped at utility/agent/eval; the synth
source-tag joins the one-fetch-per-call cfg chain; effort_ladder
delegates its capability merge to resolve_capabilities; stale
_maybe_synth_reasoning_block pointers fixed in the providers package.
Wire goldens regenerated: the only drift is the hidden
"temperature": 0.5 pin vanishing from unset-temperature requests.
The second xhigh review caught the fix-round design error one layer
down: omitting the temperature kwarg did not yield the server default —
every adapter's create_completion signature defaulted it to 0.5 and
apply_temperature wrote it to the wire, so the deleted lane pins had
silently become a hidden universal 0.5 pin.
The house rule is now implemented end to end:
- Protocol + adapters take temperature: float | None = None, and None
is OMITTED from the wire (apply_temperature None-gate; Anthropic's
builder keeps its API-required thinking=1.0 forcing but never writes
an unresolved value; Responses/xAI builders widened).
- resolve_lane climbs the documented ladder: ModelConfig.temperature →
ConfigStore global model.temperature (new config_store param,
threaded from ChatSession into both judges and perception) → None.
- perception.describe/describe_cached take alias/registry/config_store
so operator settings on the perception alias actually reach the wire
(previously structurally unreachable — no remediation path for a
degraded memoized description).
- The agent seam stops relaying the SESSION model's temperature: the
task/agent alias's own ladder governs, per the inherit-from-the-model
contract.
Generation-coherence and audit fixes from the same review:
- ChatSession._resolve_capabilities fetches its config UNCAUGHT again —
a registry failure on the session's own alias raises loudly instead
of silently caching degraded static-table caps for the session
lifetime (the never-crash fetch is a judge-constructor property).
- Judge constructors pass cfg=model_cfg (zero independent get_config
fetches; pinned by test); the per-evaluation lane's constructor-
frozen capabilities are documented as deliberate (window-coupled,
refreshed on judge swap).
- OutputGuardJudge splits _lane_alias from _judge_model_alias so the
audit label keeps its pre-#827 fallback semantics ("" → raw model id)
while lane resolution inherits the session alias.
- model_turn fetches the alias config ONCE per call and threads it into
both live flags (cfg sentinel standardized across the resolvers:
... = fetch for me, None = fetched-and-missed — also removes
resolve_lane's latent double-fetch on a miss).
- cap_tool_calls shared by the eval and optimizer loops; hand-built
ModelLane sites converted to resolve_lane; hand-rolled test result
namespaces consolidated onto mock_completion_result; stale synth-test
module docstring re-pointed.
create_completion now has exactly one caller: model_turn. The π-side
lanes migrate off hand-built OpenAI dicts:
- _utility_completion (title gen, compaction, web-fetch extraction)
takes list[Turn] and runs the session's primary lane through
model_turn; its three call sites build Turn.system/Turn.user.
- perception.describe builds a by-reference trajectory (AttachmentRef +
the prebuilt parts via resolve_attachments, reintroduced on
model_turn with its first caller and pinned by tests) — Turn IR never
carries inline media bytes, matching the main loop's wire path. Its
temperature=0.2 pin is gone (house rule).
- eval HeadlessSession's loop lowers system prompts through the
turns_from_dicts bridge and appends result.turn; the parallel-call
cap now also drops the native lane on a capped turn (a capped mirror
with a full native lane would replay orphan tool blocks).
- optimizer: all five sites (diversifier, observer, analyst loop,
tool optimizer, prompt optimizer) build Turn IR through per-function
lanes; every temperature pin (0.8/0.3/0.3/0.3/0.6) removed per house
rule — sampling behavior belongs in the model's configuration.
Test mocks move to the shared full-shape helper where the model_turn
re-ingest now runs; perception/attachment tests assert the
by-reference placeholder + resolver contract instead of inline parts.
Behavior fixes, per review + house rules:
- Judges no longer pin temperature=0.0 — the lane inherits the model's
configured temperature (ModelConfig.temperature via resolve_lane), and
model_turn omits the kwarg entirely when nothing resolves. House rule:
code never pins a temperature; modern models often misbehave below
1.0, so the model's configuration is the source of truth. This also
dissolves the extra_body-overrides-judge-pins collision: operator pins
reaching the judge lane is the doctrine working.
- Session-fallback judges inherit the session's registry alias
(session_model_alias threaded from ChatSession), so the registry-
resolved extra_params / replay flag / vLLM attach apply on the default
judge.model-unset configuration instead of only on explicit aliases.
- Blank-id native lanes are repaired, not dropped: model_turn backfills
the manufactured mirror ids into blank-id native client tool blocks
pairwise (the #825 1:1 ordering invariant), so thought_signature
survives Google's blank-id compat responses and thinking blocks keep
their continuity on blank-id locals. Only blank ids are ever written —
a provider-assigned id (possibly signature-covered) is never touched —
and any pairing mismatch falls back to the #825-converged total drop.
- model_turn(mint=...) without wire_id_map now raises: minted ids are
unrestorable without the recovery map, and the two parameters were
independently optional by accident.
- Lane resolution reads ONE defensively-fetched ModelConfig
(_get_config_or_none): a registry hot-reload mid-resolution can't mix
config generations, and an alias that raced away degrades each facet
to its miss behavior instead of aborting a judge constructor into the
silent session-model downgrade.
Extraction hygiene, per review:
- Dead session wrappers deleted (_resolve_server_type,
_maybe_synth_reasoning_block, _get_server_compat) and their tests
re-pointed at the module functions; the stranded reasoning-types
comment and two stale doc pointers cleaned up.
- Speculative extra_headers / resolve_attachments pass-throughs dropped
from model_turn until a caller lands (phase 2/4 reintroduces them
with their lane).
- _server_type_of(cfg) is the one reader of server_compat.server_type;
the vLLM-attach gate and resolve_server_type both use it, retiring
the change-both-readers discipline comment.
- dataclasses import hoisted; module docstring restated as the durable
contract (grep callers for coverage) instead of a rotting snapshot.
- mock_completion_result shared in tests/_session_helpers.py — one
definition of "every field the re-ingest reads".
The intent judge's evidence loop and the output-guard's single shot now
build list[Turn] and call model_turn — the hand-built OpenAI-dict
message construction is gone, and with it the judges' private
interlingua. The assistant turns they append carry the provider-native
lane, so the loop keeps reasoning continuity across its own turns.
That is what unblocks Gemini: thought_signature rides provider_blocks
and is reconstructed by the Google adapter's fidelity swap, so the
provider_name == "google" tool-skip is deleted — the Gemini judge runs
the same evidence-tool loop as every other provider instead of
degrading to a single-shot, tool-blind verdict.
judge.py's _resolve_model_capabilities mirror (#826) is deleted; both
judges resolve capabilities through the shared lane resolver, and each
evaluation builds a ModelLane (fresh client, constructor caps,
registry-resolved extra_params + live flags). The shared resolver
inherits the mirror's defensive non-dict capabilities check — without
it a malformed registry row would silently downgrade a judge to the
session model instead of just skipping the overrides.
Judge calls now resolve extra_params and replay_reasoning_to_model
from the registry like every other lane (previously: never sent, and
the protocol's back-compat default respectively).
Test mocks grow the CompletionResult fields the model_turn re-ingest
reads (provider_blocks, reasoning); alias-registry mocks wire
get_config, which the unified resolver uses.
Lower-and-sample is now one surface: core/model_turn.py owns the
Turn-IR lowering seam (dicts_from_turns -> sanitize_tool_call_arguments
-> restore_provider_tool_ids -> Phase 5 vLLM attach), the provider call,
and the re-ingest to an assistant Turn carrying the native lane.
ModelLane binds a resolved lane (provider, client, model, capabilities,
extra_params) and carries the registry so live operator toggles
(replay-reasoning, vLLM attach) keep re-resolving per call.
The task-agent seam is the first client: _run_agent builds a ModelLane
and calls model_turn with a mint closure; the inline mint/back-fill/
finalize block collapses to appending result.turn. Session capability/
extra-params/replay/finalize helpers become delegates to the module
functions, so lane resolution has exactly one logic path.
model_turn is policy-free by contract: retry, deadlines, tool
execution, and usage recording stay with each caller.
Two agent-path tests move their replay-flag pin to the module seam
(one had gone vacuous against the session wrapper); _record_aux_usage
now takes UsageInfo rather than a CompletionResult.
The auto-notify added in the prior commit awaited _notify_nodes_mcp_reload
inline in create/update/delete, coupling each admin write's latency — and
success — to cluster reachability: on a large cluster with slow/unreachable
nodes the write could hang up to ceil(nodes/fan_out_limit)*30s behind the
fan-out, and a post-commit fan-out error would 500 a write that already landed.
Schedule the fan-out as a BackgroundTask that runs AFTER the 200 instead — the
"trigger, not drain" contract already used by _cascade_cancel_to_children — so
the write's response is never blocked on, nor failed by, the fan-out. The
pre-existing registry-install path is converted the same way for consistency.
There is no periodic node->DB reconcile, so a node that misses the reload serves
a stale MCP catalog until the next POST /reload. The background _run therefore
logs any unreached node (or a systemic fan-out fault) at WARNING — visible at
the default INFO level — rather than swallowing it; the per-node status view
also surfaces the divergence. A non-2xx reply from a node's reload/action
endpoint now counts as a failure (raise_for_status) rather than a reached node,
so neither the WARNING nor the operator /reload results miss a 5xx node.
Revert the getattr None-guard on _notify_nodes_mcp_reload: it turned the
operator-triggered POST /reload into a silent success ({} with 200) when the
fan-out infra was absent — a fail-loudly violation — and diverged from the
unguarded sibling _notify_nodes_mcp_action. The helper is drain-style again,
awaited only by /reload (which must surface fan-out failures); writes go through
the best-effort scheduler.
Tests: assert the reload is NOT scheduled on a delete/update 404 or a create
secret-store 503; that an unreached-node, raising, or non-2xx fan-out is logged
at WARNING / recorded as an error; and that operator POST /reload fails loudly
(500) without fan-out infra.
oauth_user/oauth_obo servers hold no cluster-level session — they connect per-user on demand — so the admin status pill rendered 'connecting'/'idle', which reads as broken, when zero warm users is the normal resting state. Render 'per-user' for pool-backed servers instead.
admin_create/update/delete_mcp_server wrote to the DB but never told nodes to reconcile — only the registry-install path and the explicit /reload did — so a programmatic create/edit/delete was inert on nodes until a manual reload (and the mid-session re-prime self-heal never fired). Call _notify_nodes_mcp_reload after each write, mirroring registry-install; also make that helper best-effort (skip when the cluster fan-out infra is absent) so a write can't 500 on it.
Microsoft Entra's discovery document advertises userinfo_endpoint on graph.microsoft.com — a host distinct from the login.microsoftonline.com issuer — so discover_oidc's cross-host guard rejected it and disabled OIDC unless the operator set trusted_endpoint_hosts. Add login.microsoftonline.com to the built-in KNOWN_TRUSTED_OAUTH_ENDPOINT_HOSTS allow-list (mirroring the Google entry) so Azure AD OIDC works with no extra configuration. Surfaced by the live obo integration test.
- detect an in-place oauth_user<->oauth_obo flip by diffing the pool servers' (name -> auth_type) view instead of names only, so a migrated server re-primes active sessions (a name-only diff saw the same name on both sides and missed it);
- guard prime_user_pools per-user so one scheduling failure can't propagate out of reconcile_sync (500 the reload) or skip the remaining users;
- log what was SCHEDULED (prime is fire-and-forget and no-ops for credential-less users / a down loop), not 're-primed', and take an int changed-count instead of a set whose name falsely implied per-server scoping.
Startup key-enforcement counts ALL user-scoped auth types (oauth_user and oauth_obo, per is_user_scoped_auth), and the entra mint leg always carries scope=<audience>/.default (per-server oauth_scopes is ignored on that leg). The docstrings named only oauth_user / left the scope behavior ambiguous. Comment-only; no behavior change.
prime_user_pools runs once at ChatSession start, so an oauth_user/oauth_obo server registered while a session is already open never reached it — and for oauth_obo (no consent flow) priming is the ONLY path tools take into the catalog, so a mid-session registration stayed invisible until the session restarted. reconcile_sync now diffs the pool-server name set and re-primes every active session's user when a new server appears; idempotent (skips already-warm pools) and a no-op for users without a captured credential.
Round-12 review follow-up (no correctness findings). obo_grant_profile is
a static config field that OIDC runtime rediscovery never changes, so
recomputing profile/mint after maybe_rediscover_oidc was dead work that
implied the grant profile could change across a heal (it cannot). Re-read
only the discovery-derived state (enabled / token_endpoint).
The remaining review findings are accepted by design: the credential-
rotation CAS's sub-millisecond read->write window (self-heals on next
login; a full fix needs SELECT FOR UPDATE or a version column) and the
per-server delete loop on identity deletion (the per-server try/except
buys partial-failure resilience a single bulk delete would not).
Round-11 review follow-up — no correctness findings; efficiency/DRY cleanups.
- Session-start priming already confirms the captured credential exists
once for all of a user's obo servers, but each per-server
get_obo_access_token_classified re-read it pre-lock (N+1 reads). The
priming path now passes credential_present=True so the per-server
existence read is skipped; other callers keep their own read.
- _clear_pending_consent_best_effort (the sweep clear path) now routes
through _mark_pending_consent_cleared instead of inlining the
prune-then-stamp step, matching the helper's documented contract so the
two DB-confirmed clear sites can't drift.
The per-dispatch pending-consent clear's DELETE volume and the removed
interactive.js no-consent-URL fallback are left as-is: the former is the
deliberate, TTL-bounded cost of cross-node badge self-heal, and the
latter is unreachable for oauth_user (which always carries a consent_url)
and intended for oauth_obo (which has no per-server consent flow).
Round-10 review follow-up.
- A server scoped under obo_grant_profile=rfc8693 that survives a switch
to the entra profile mints <audience>/.default (the entra leg cannot
honor per-server oauth_scopes), but the cache row recorded the
configured narrow scope — so _is_fresh_obo_cache_row kept serving the
broad .default bearer believing it was narrow, and a scope change that
can't apply under entra looked like it had. The freshness gate and the
cache row now record the EFFECTIVE scope the leg actually mints ('' for
entra, the configured scope for rfc8693); the raw scope is still passed
to the mint so the entra leg's "oauth_scopes ignored" warning still
surfaces the misconfigured leftover.
Cleanup: the R9-5 single-per-mint client made every token-POST caller pass
a non-None client, so the transient-client fallback in _hardened_token_post
was dead and two doc/comment blocks described the opposite of the real
behavior. Removed the dead branch, tightened the http_client typing across
the mint chain, and corrected the docs.
Round-9 review follow-up.
- Runtime OIDC rediscovery was triggered only from the obo mint path,
which needs an already-signed-in user — so a single-node install (or
one where every node booted during a transient IdP outage) kept OIDC
LOGIN dark until an operator restart. The authorize and callback
handlers now trigger maybe_rediscover_oidc before their enabled gate,
so login self-heals too.
- A transient storage error on the obo mint-cache write (delete+create)
raised out of get_obo_access_token_classified, discarding a valid
just-minted token and breaking the classified-result contract. The
cache write is now best-effort — the working bearer is returned and the
next dispatch re-mints. Likewise the runtime rediscovery's discover_oidc
call is wrapped in except Exception (like the boot path) so an
unexpected discovery error can't escape the mint's contract.
- Login-time credential capture could race an in-flight mint on a
strict-rotation IdP: the mint's rotation write-back would clobber the
fresh login refresh token with a stale rotated one. The rotation
write-back is now a value compare-and-swap against the token the mint
read, so a credential a concurrent login just refreshed is not
overwritten.
Cleanup: the rfc8693 mint now opens one transient httpx client for the
whole mint so the token-exchange leg reuses the refresh leg's connection
instead of a second TLS handshake.
Round-8 review follow-up — two correctness follow-ons from the round-7
rediscovery/console-gate fixes, plus two cleanups.
- The console obo write gate ran the OIDC-deployment checks on EVERY
update, so once OIDC was operator-disabled any edit of an existing
oauth_obo server — including the natural remedy of setting
enabled=false — was rejected 400, leaving DELETE as the only way out.
The deployment-level checks (encryption key, OIDC enabled/configured,
capture opt-in, valid grant profile) now run only when a write is a NEW
obo enablement (create or flip INTO obo); a same-type edit keeps only
the per-server validity checks (audience required, entra-scope reject),
so an operator can always disable or edit an existing obo server.
- Probing rediscovery with enabled forced True carried the retryable boot
flag into discover_oidc, whose config-error branches returned enabled=
False without clearing it, so a config-invalid IdP (an endpoint failing
SSRF/same-origin validation) re-probed every 60s forever. The config-
error branches now latch discovery_retryable=False (terminal), and
maybe_rediscover installs that terminal config so the node stops
probing; the transient fetch/degraded branches keep retrying.
Cleanups: fold the obo missing-expires_in fallback into
_expires_at_from_response via a default_ttl_seconds param (one owner of
the stored-expiry format), and drop the redundant audience-change
inequality already guaranteed by the no-op normalization (matching the
sibling scopes_changing).
Round-7 review follow-up.
- The runtime OIDC re-discovery feature was dead code: discover_oidc
PRESERVES the input config's `enabled` flag on success (only
load_oidc_config ever sets it True), and maybe_rediscover_oidc always
probed from the disabled boot config, so a successful rediscovery still
returned enabled=False and the config swap was unreachable — the whole
boot-outage auto-heal never worked. It now probes with enabled forced on
so the flag is a reliable success signal. The unit test that "covered"
this was mocking discover_oidc to return enabled=True, masking the bug;
it now drives the real discover_oidc through a mocked HTTP discovery GET.
- The console never runs runtime rediscovery, so a transient discovery
failure at console boot made every oauth_obo server un-editable and
un-disable-able. The write gate now accepts a discovery_retryable config
(OIDC configured, discovery transiently down) and rejects only a
genuinely absent OIDC.
- The first rediscovery probe was suppressed for ~60s after host boot
because the "last probe" timestamp defaulted to 0.0; it now uses a None
sentinel for "never probed".
- Two behavior-preservation fixes for the pre-existing oauth_user path:
the shared hardened token-POST no longer escalates oauth_user oversized
error bodies (that status-based classification is opt-in for the obo
legs only), and the token_revoked audit fires unconditionally for
oauth_user again (a refresh failure means a real grant died) while
staying delete-gated for obo to avoid revocation rows for tokens that
never existed.
Cleanups: drop a throwaway set allocation in the pool-emptiness check,
compute the create handler's cleaned OAuth text once, remove a dead
no-op pop with a false comment, and simplify the cleared-map prune to two
non-overlapping passes.
Round-6 review follow-up — no CONFIRMED correctness bugs; one plausible
edge case and four DRY/drift cleanups.
- The shared hardened token-POST raised its 64KB body-size guard with the
default TRANSIENT class before the non-200 was classified, so a permanent
dead-grant whose error body exceeded the cap looped "please retry"
forever and never escalated. An over-sized client-error response is now
classified AMBIGUOUS by status (without reading the over-sized body), so
it still escalates to the honest re-login/admin remedy after the streak.
- The admin update handler re-derived the is_flip predicate inline in the
three token-purge guards (and computed target_auth / auth_type_now as two
names for the same effective auth type). Both now reuse the single
is_flip / target_auth derivations, so the purge guards and the column
scrub can't desync on what counts as a flip.
- The oauth_obo server-name scan was hand-rolled in two places (the
connections-list filter and the identity-delete cache purge) with
divergent null handling. Extracted obo_server_names(storage) so a change
to how sign-in-passthrough is recognised can't leave one path silently
missing servers.
- Inlined the two single-use _*_detail wrappers into direct
_pool_error_detail calls, keeping named wrappers only for the
multi-caller situations.
Round-5 review follow-up — three CONFIRMED (one security) plus two
correctness issues, all traceable to earlier fixes in this branch.
SECURITY: the round-2 redesign gated the "scrub OAuth columns this
auth_type doesn't use" on is_flip, replacing the old unconditional
scrub. A same-type static/none/obo edit could then inject an
oauth_authorization_server_url that survived a later flip to oauth_user
(which uses that column) and redirected every consenting user's OAuth
traffic to an attacker AS. The scrub is now applied on EVERY write, and a
flip into oauth_user recomputes the oauth_user-only columns from the
request so a stale value can't carry in — the persisted OAuth columns
are once again a pure function of the target auth_type.
- The oauth_obo write gate now also requires capture_user_credential to
be enabled: without it, login persists no credential and every dispatch
returns "missing" with a remedy that can never succeed — the permanent
misconfig the gate exists to reject.
- A permanent obo mint failure arms the cooldown (its shared credential
survives the per-server revoke), but the in-cooldown short-circuit
reported it as a retryable transient for the whole window, flapping
against the honest re-login/admin affordance. The backoff state now
records whether the arming failure was permanent, and the short-circuit
surfaces the matching classification.
- The ambiguous-escalation revoke cleared the cooldown without re-arming;
for obo (surviving credential) that let the next dispatch immediately
re-mint against the still-failing IdP. It now re-arms the same terminal
backstop the permanent branch has.
- The force-refresh reuse gate keyed on the cache row's 1-second `created`
time, which couldn't tell a concurrent peer's fresh mint from the
caller's own just-rejected token minted in the same second — so a retry
could re-serve the rejected bearer. It now decides by token identity
(the under-lock row differs from the pre-lock one), preserving the
single-flight reuse while never re-serving a rejected token.
Also: guard _pool_error_detail's str.format so placeholder-free copy
can't raise inside the error renderer, and note why the connections-list
classifies obo rows by authoritative auth_type on that cold path.
Round-4 review follow-up — no correctness findings; these are the four
cleanups it surfaced.
- The obo mint path decrypted the captured IdP refresh token twice per
mint: once pre-lock only to test presence, then again under the lock.
The pre-lock presence check now uses the raw existence read (no
decrypt), mirroring the priming path; the single authoritative decrypt
happens under the lock. Removes N throwaway decrypts per user at
session-start priming across N obo servers.
- The "arm the per-(user,server) cooldown" idiom was written inline at
four failure sites. Extracted _arm_cooldown (returns the backoff state
so the streak-mutating callers reuse it), so a change to how backoff
works is one edit.
- The oauth_user|obo pool-membership union was rebuilt inline at three
iteration sites. Added a _pool_server_names property, the set-level
counterpart to _is_pool_server, so a future third pool-backed auth type
is registered in one place.
- The four per-situation remediation-copy helpers each repeated the
oauth_user-vs-obo branch. Consolidated the copy into one
(auth_model, situation) table behind _pool_error_detail — the single
place the auth-model decision is made — so a dispatch site can't pair a
situation with the wrong auth model's copy (the wrong-remediation bug
class this review caught repeatedly). The named helpers remain as thin,
tested wrappers.
Round-3 review follow-up.
- The startup guard that refuses to boot without a token-encryption key
when capture_user_credential is enabled was gated on oidc_config.enabled.
Enabled reflects whether OIDC *discovery* succeeded, which is transient:
a node that boots while the IdP is unreachable comes up enabled=False,
so the guard was silently skipped exactly when it was needed, and runtime
rediscovery would later re-enable OIDC with the first login persisting a
refresh token and no key. Gate on the operator's capture opt-in alone
(a static config value), independent of discovery state.
- An obo mint response omitting the RFC 8693-optional expires_in cached
expires_at=NULL, which the freshness gate reads as never-expiring — fine
for opaque oauth_user tokens, wrong for a short-lived minted token, which
would then be served indefinitely and defeat audience/scope narrowing
that relies on TTL turnover. Fall back to a bounded default expiry.
- The empty-token fallback in the shared pool-lookup error mapping now uses
the auth-model-aware consent detail like its sibling missing branch, so an
obo row never shows per-server-consent copy with a null consent URL.
- Documented the _build_consent_url invariant at the chat error-card render
gate: oauth_user rows always carry a consent URL, so gating the Connect
button on its presence never hides a needed button for them; the button's
absence for sign-in passthrough is intended (the detail text is the
affordance).
Round-2 review follow-up. The headline is a redesign of the OAuth
column carry-over so scopes/audience can no longer leak or vanish across
an auth-type flip:
- oauth_audience and oauth_scopes keep their meaning only WITHIN an auth
type (a resource indicator vs. an IdP app id; AS-consent scopes vs. an
rfc8693 exchange scope). On any oauth_user<->oauth_obo flip they are
now recomputed from the request (present -> value, absent -> NULL) and
never carried from the old row. A shared _oauth_columns_to_clear policy
drives both the create and update handlers. No-op normalization of a
re-sent equal value applies only to same-type edits.
- The console form clears both semantic fields when the auth type
changes and always submits the visible values; the previous
"omit unchanged scopes" logic collided with the backend's flip
handling and could silently drop or carry scopes.
Write-time validation now rejects oauth_obo rows that can never mint —
OIDC disabled/unconfigured, or an invalid obo_grant_profile — instead of
letting them surface per-dispatch as a retryable transient that never
heals.
Honest failure affordances for sign-in passthrough (no per-server
consent flow exists):
- the token_revoked audit fires only when a row was actually deleted, so
a permanent mint rejection against a surviving credential no longer
appends a bogus revocation on every post-cooldown dispatch/prime;
- the 403 insufficient-scope detail and the chat error card's action
button are now auth-model-aware — obo errors point at the
administrator rather than a dead-end re-consent, and the Connect button
renders only when a real consent URL is present;
- the read-side freshness gate now enforces scopes as well as audience,
so an rfc8693 scope narrowing takes effect on the next dispatch even if
the best-effort admin cache purge failed.
Cleanups: the five decrypt-failure result constructions collapse into
_decrypt_failure_result; the cleared-pairs TTL bookkeeping into
_mark_pending_consent_cleared; drop the dead USER_SCOPED_AUTH_TYPES
re-export from mcp_oauth; correct the now-bidirectional oidc<->mcp_oauth
lazy-import note. Docs updated for the flip semantics and the OIDC
prerequisite.
Mint engine: guard the credential-rotation persist so a storage blip
cannot escape the classified-result contract mid-mint (and cannot brick
the user's other obo servers on strict-rotation IdPs); stop borrowing
the login flow's httpx client across event loops — mints use a transient
per-request client (obo_http_client remains as a test seam); retry OIDC
discovery at runtime (cooldown-gated, single-flight) so a node that
booted during an IdP outage can mint again without a restart; key the
under-lock force-refresh reuse gate on created, which delete+create
makes the mint time (obo rows never set last_refreshed, so the copied
oauth_user gate never fired and serialized waiters each re-redeemed).
Cross-node consent badges: the cleared-pairs set becomes a TTL map with
bounded growth, so a badge written by another node after this node's
last clear self-heals within one TTL window instead of surviving until
a restart.
Admin lifecycle: purge the mint cache when oauth_scopes changes on an
obo row (an rfc8693 privilege reduction now applies immediately, like
audience changes); normalize no-op scope/audience re-sends out of
updates — the admin form re-submits pre-filled fields on every save,
which both re-triggered purges and made entra-profile rows with legacy
scopes un-editable; make flip-into-obo scope handling grant-profile
aware (entra clears the carry-over, rfc8693 honors the request); clear
obo-era audience/scopes when flipping back to oauth_user (the IdP-side
app identifier is not a resource indicator); mirror the same column
policy in the create handler.
Revocation honesty: hide obo mint-cache rows from the user connections
list and refuse the per-server disconnect with 409 — deleting the row
returned 204, audited token_revoked, and then session-start priming
silently re-minted from the surviving captured credential.
Console form: keep the audience-from-URL autofill off for sign-in
passthrough (the audience there is an IdP application identifier, and
the prefilled URL passed every validation layer then failed every
mint); clear the autofill artifact when switching modes; omit unchanged
scopes from submissions.
Dispatchers: route tool/resource/prompt through one shared lookup-error
mapping and an auth-model-aware 401-exhausted detail (obo users are no
longer pointed at a consent flow that does not exist). The consent-url
audit count drops 13 → 7: the three per-dispatcher mapping copies
collapsed into _pool_lookup_error.
Priming: skip all obo servers for users with no captured credential via
one existence SELECT (previously three reads per server per session).
Also: USER_SCOPED_AUTH_TYPES now lives in storage._protocol so the
backend SQL predicates share the application layer's set; docs describe
the actual purge-on-transition behavior (the orphan-and-reactivate
claims were wrong); the entra e2e setup script no longer aborts
silently under set -e with suppressed stderr.
Frontend↔backend interaction bugs the backend-only rounds couldn't see:
- flip oauth_user->oauth_obo: the admin form re-submits the pre-filled
oauth_user scopes, so the flip-clear (gated on 'oauth_scopes' not in
body) was skipped -> rfc8693 mints broke permanently. Clear now
compares to the existing value, robust to the re-send.
- entra edit-lockout: update validated the MERGED scopes, so a
pre-existing scoped obo row under the entra profile became un-editable
(every PUT 400'd). Reject only when the request actually SETS scopes.
- flush-cache button never rendered: consented_users_count is now
populated for oauth_obo rows too, not just oauth_user.
Mint engine + priming:
- audience guard: a cached token minted for a since-narrowed audience is
no longer served (extracted _is_fresh_obo_cache_row, used pre/post-lock,
checks refresh-less + audience-match + fresh). _persist_obo_cache_row
now delete+creates so the row's audience column tracks the mint (a
plain update kept the stale audience -> re-mint loop).
- obo session priming passes revoke_ambiguous_escalation=False (new param
threaded through get_obo_...), so an IdP wobble during a bulk prime
can't escalate-revoke obo cache rows cluster-wide.
Cross-node + lifecycle:
- pending-consent success-clear now clears once-per-failure-cycle via a
_pending_consent_cleared set (was gated on 'we wrote it' -> never fired
cross-node/after-restart -> stale badge). Still no per-call SQL.
- identity-unlink cache purge: per-server try/except so one failure
doesn't leave other servers' bearers un-purged.
- entra ignored-scopes: warn once per audience (was per-mint flood ->
downgraded to debug -> no signal on a profile switch).
- entra_setup.sh writes single-quoted .env values (secret may contain $).
+6 regression tests. 1892 mcp/oidc/console tests green; mypy clean.
Refs #551.
Manual (non-CI) harnesses that exercise the real oauth_obo mint path
against a live IdP, kept for future validation of the feature:
- entra_e2e.py: real Entra tenant, one interactive sign-in, drives
get_obo_access_token_classified -> _obo_mint_entra (E1-E7)
- keycloak_e2e.py + .sh: ephemeral Keycloak, fully headless, drives the
rfc8693 leg (refresh grant -> token exchange)
- entra_spike.py: raw-OAuth wire probe (pre-implementation reference)
- entra_setup.sh: creates the Entra spike app registrations
- .env.example template; real creds stay in a gitignored .env
Both legs pass E1-E7 (mint + aud, cache hit, single-credential->multi-
audience, rotation write-back, force_refresh, unconsented->credential
survives, flush->re-mint). Not wired into CI.
Refs #551.
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.
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.
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.
- 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.
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.
- 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.
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.
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.
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.
[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.
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.
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.
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.
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
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
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).
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.
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.
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.
- 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.
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.
- 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.
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.
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.
- 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
- 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
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.
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.
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.
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.
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.
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.
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.
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.
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".
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.
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.
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.
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).
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.
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.
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.
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.
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).
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.
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.
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.
unsplit() closed each doomed (ephemeral) pane via close() — which already
renders/persists/notifies — then repeated that trio, firing intermediate
persist/notify passes mid-operation. A 2-cell split fully collapses inside
close(), so bail there; only a 3+-cell split (or an empty doom list) still
needs the trailing exit + refresh. The all-conversation path is unchanged.
Also reword the cell-chip CSS comment so it names the reversible hide vs
destructive close glyphs, now that an ephemeral pane can show the close glyph
in split mode.
The preview pane opens beside the conversation as a split cell. Dismissing
that cell — the per-cell chip, or Unsplit from the other pane — ran
closeCell(), which hides the pane but keeps it in _panes/_order, leaving an
orphan tab with no meaningful reopen (the reopen affordance is the transcript
chip, not the tab bar).
Add an `ephemeral` flag on ShellPane. For an ephemeral pane the cell chip and
Unsplit route to close() — destroying the pane and its tab — and the chip's
glyph/label read as a destructive close rather than a reversible hide. Unsplit
still spares the focused survivor even when it is ephemeral ("keep the focused
pane"). The preview pane sets the flag; conversational panes do not, so an
all-conversation split is unchanged (Unsplit reduces to the prior
_exitLayout(_activeId)).
The wake gate documented exactly one info line per call past its
gates, but a send() refusal (the authoritative under-lock _closed
re-check catching a teardown the gate's lockless peek missed) emitted
nothing — a dropped wake should stay traceable to its trigger, so the
refusal now logs nudge_wake.refused.
The already-dispatched branch's comment claimed a held reminder can
coexist with the terminal mark via a redelivery whose commit raised —
impossible with the current control flow (_redeliver_pending clears
the hold before committing). Reworded to what the clear actually is:
the last line of defense against any coexisting hold leaking forever
once this branch deactivates the row, since inactive rows never
re-list. Test comment updated to match.
A session whose _nudge_queue answers has_pending truthily while its
deliver_wake_nudge_from_queue consumes nothing turns the worker-exit
backstop into an infinite respawn loop: the gate passes, the wake
worker no-ops, the exit backstop re-runs the gate, forever.
Mock-backed test sessions riding real Workstreams are exactly that
shape, and one worker on such a pairing is enough to ignite a
wake-thread storm that trips the leaked-thread guard in every
subsequent test. The wake contract requires real drain semantics —
the spawned worker must CONSUME what the gate saw — so the gate now
refuses on type, not just presence.
str | None under-specified the field: the implementation and the TS SDK
union both constrain it to queue_full / refused_closed, and the Literal
projects a proper enum into the generated OpenAPI spec so clients
reject unexpected values. Specs regenerated.
Wake path:
- Denial metacog nudge moves to the tool channel so it drains with the
denied tool batch instead of the next user-message seam.
- wake_workstream_if_pending: shared wake gate for watch fires on
already-idle workstreams (no IDLE transition for the watcher to
observe), wired as wake_fn at every set_watch_runner site via the
shared _watch_fire_wake_fn helper (closes over the Workstream OBJECT
— after eviction+restore an id-keyed manager lookup would miss).
- session_worker exit backstop re-runs the wake gate the moment worker
ownership clears: IDLE fans out on the worker thread, so
transition-time wakes always landed on the reuse path and no-op'd
(the coordinator idle_children strand).
- deliver_wake_nudge_from_queue contains GenerationCancelled — it is
the wake worker's run() closure and only Exception is caught
downstream.
Watch delivery:
- Terminal fires that cannot reach their workstream are HELD and
redelivered on min(interval, 60s) without re-running the command,
bounded by MAX_DELIVERY_ATTEMPTS per cycle and the watch's own
max_polls across cycles; the poll charge commits durably at hold
time so restarts stay budget-bounded.
- Restore admission control: per-ws dedup + MAX_CONCURRENT_RESTORES
cap, presence-only re-check under the lock, detection-only stall
alerts (reclaiming a wedged admission would trade capped degradation
for total poll-pool collapse).
- Permanent-vs-transient restore taxonomy: corrupt persona stamp and
genuinely-missing history (confirmed by a raising storage probe —
the resume loader swallows read blips into []) deactivate the watch
immediately; everything else holds and retries.
- Cancel-race defense: delivery paths re-check is_watch_active before
stashing/dispatching, cancel paths write the row BEFORE
forget_terminal_dispatched, the HTTP cancel endpoint clears runner
state, and a per-tick sweep bounds the residual stash-after-clear
interleaving to one check_interval.
- Abandon/exhaustion commits are write-then-clear so storage that can
read but not write retries the row write instead of re-running the
command every cycle; the fresh-fire unrestorable path stashes before
its deactivation write for the same reason.
Registry follows identity:
- The dispatch registry is keyed by _ws_id at registration time; every
rebind now moves it: non-fork resume() and /new go through
_follow_watch_registration (new key live before the old is removed,
never stealing a registration another live session holds), removals
are owner-checked so tearing down a watch-restore shell or a
resumed-away session cannot unregister a live pane, the restore
shell yields to a registration that appears mid-restore, CLI
--resume registers after the successful resume, and both the open
path and the detail-GET lazy rehydrate wire the registration.
Teardown gating and backpressure honesty:
- cleanup_session_ui marks ws._closed FIRST under ws._lock — every
teardown path (close, close_idle, evict, delete, discard) funnels
through it — and session_worker.send re-checks under the same lock,
so a wake can never spawn a worker on a torn-down workstream.
- Create responses carry initial_message_status when the initial
message could not be delivered (queue_full / refused_closed) instead
of reading as success; staged attachments survive for the retry;
/send surfaces a closed workstream as 404 rather than queue_full.
Docs/spec: OpenAPI artifacts regenerated; api-reference documents the
new create-response field; TS SDK type extended.
Tests: ~30 new pins (cancel races, budget durability across restarts,
owner-checked registry moves, teardown gating, stall alerts,
backpressure surfaces, wait_until final re-check); wide subsystem
sweep green (2353 passed).
Review feedback (PR #800): the URL lane hard-capped fetched bodies at
10 MB before kind resolution, making the 32 MiB pdf cap unreachable for
URL targets while path targets honored it. The flat pre-check is gone;
the guarded fetch's max_bytes now tracks max(PREVIEW_SIZE_CAPS.values())
- mirroring the path lane's stat pre-check - and the per-kind caps after
resolution stay authoritative.
Also drops a redundant function-local asyncio import in test_console.py.
fetch_with_ssrf_guard now streams the response under a max_bytes budget
(default 32 MiB, counted on decoded bytes so gzip cannot expand past it)
instead of buffering blind - an unbounded body previously filled memory
before any caller-side size cap could run. Redirect-hop bodies are no
longer read at all, and the realized response drops stale wire-framing
headers (content-encoding/content-length/transfer-encoding) that no
longer describe the decoded content it carries.
Preview blob ids are salted out of the model-visible attachment
namespace (sha256("preview:" + body)): uploads use bare sha256(body)
and save_attachment freezes kind at first insert, so a byte-identical
preview/upload pair would otherwise share a row - whichever landed
second inherited the other's kind, silently hiding an upload from model
context or materializing preview bytes into a tool turn.
turnstone's primary audience self-hosts it beside other lab services —
a web_fetch or open_preview aimed at Grafana, Home Assistant, or a dev
node on the local network is the operator using their own network, not
an attack. The hard SSRF refusal made those targets unreachable.
New runtime setting tools.allow_private_network (settings registry,
default off, rendered in console Settings → Tools; hot — read per tool
call, no restart). When enabled, a call NAMING a private address
becomes approvable: the approval prompt tags it "(private network)" so
the operator approves it as what it is, and the human gate stays.
The redirect side-door stays closed either way: a PUBLIC target that
302s into private address space is refused regardless of the opt-in —
that address never appeared on the approval card, so it is never
fetched. Only a chain whose approved origin was itself private skips
hop screening (its redirects are the operator's own network).
Refusals now teach the knob (mirrors the oidc opt-in hint): the error
names tools.allow_private_network and where to enable it. Surfaces
without a ConfigStore (bare CLI, eval) stay strict — there is no admin
surface to have opted in on.
Four follow-ups to the preview pane:
- Probe-mode preflight: the pane preflights src-loaded kinds with
GET ?probe=1 (204, real hardening headers, no body) instead of HEAD —
the console reverse proxy forwards HEAD as a full GET, so the old
preflight dragged the whole blob across the node→console hop twice.
Ownership gate + renderable-type check still run on probes.
- Legacy-charset text: table/text/markdown now transcode to UTF-8 at
store time (declared charset → UTF-8 → cp1252-replace ladder), same
model the web kind already used. The ladder applies only when the
text kind was DECLARED (MIME/extension/override); the bare no-hint
fallback stays strict UTF-8 and NUL bytes still hard-reject, so
binary rejection is unchanged.
- Remote assets default OFF: previewed pages are now served under
"sandbox; default-src 'none'; style-src 'unsafe-inline'; img-src
data:; font-src data:" — they render with inline styling but cannot
contact their origin site (no viewer IP/traffic disclosure). A
per-pane "Load remote images & styles" checkbox (web previews only,
sticky, not persisted) reloads with ?assets=1 for the permissive
bare-sandbox mode.
- Markdown vendor parity: preview markdown now runs renderer.js's
postRenderMarkdown (hljs token coloring + lazy mermaid diagrams)
like the conversation pane, with preview-scoped code-block/KaTeX
chrome (the conversation theme is .msg.assistant-scoped).
Tests: probe/assets HTTP + policy coverage, charset ladder units +
stored-bytes round-trip, JS static guards for the probe form, the
default-off toggle, and the post-pass; headless-chrome harness grew to
41 assertions (probe-not-HEAD, toggle visibility/default, fenced-code
render). Full suite green.
Tool results only ever rendered as plain text in the transcript. This
adds the model-driven rich-preview lane every comparable surface has,
in turnstone's developer-tool idiom: a preview pane that opens BESIDE
the conversation, keyboard-operable, sandboxed, never replacing the
transcript that spawned it.
Backend
- New built-in open_preview(target, kind?, title?): resolves an http(s)
URL, a file path, or attachment:<id> to bytes; classifies into
web/pdf/image/table/text/markdown (magic bytes > MIME hint >
extension > UTF-8 fallback, legacy-charset pages transcoded); caps
size per kind; persists content-addressed with kind="preview" —
refcounted and GC'd with the workstream, skipped by trajectory
reconstruction so preview bytes can never materialize onto the wire.
URL targets gate like web_fetch (network egress); paths/attachments
run unprompted like read_file.
- New core.web.fetch_with_ssrf_guard: manual redirect walk that
SSRF-screens every hop BEFORE requesting it (follow_redirects=True
checked nothing between hops); adopted by both open_preview and
web_fetch. URL userinfo is stripped before the descriptor or the
stored bytes see it; <base href> is injected doctype-safely so
relative assets resolve without quirks mode.
- The preview descriptor rides the tool turn's meta side channel with
ONE shape on every boundary: the live tool_result SSE event, the
conversations.meta column, and the /history projection. Cancelled
batches commit an already-announced preview (blob + meta) instead of
stranding the open pane on a permanent 404.
- New GET {ws}/attachments/{id}/preview (read scope, same ownership
gate as /content) serves the STORED type with per-MIME hardening:
bare CSP sandbox for text/html (renderable, scriptless, opaque
origin), no CSP for application/pdf (Chromium's viewer refuses
sandboxed contexts), full default-src 'none' otherwise; filenames
fold to latin-1-safe ASCII. The console /node proxy now forwards
CSP/nosniff/disposition/cache-control instead of dropping them.
- History loads exclude preview blobs from the bulk content fetch at
the query (they were read and discarded on every load).
Frontend
- New "preview" pane type registered in the shared shell (server +
console): openPaneBeside placement, per-kind renderers — fully
sandboxed iframe for pages, browser PDF viewer, sortable tables
(CSV/TSV/JSON, ragged-file safe, 5k-row cap), rendered markdown,
text — plus back/forward history with arrow keys, reload persistence
via pane meta, and backoff auto-retry (0.9s..7.2s) bridging the gap
between the live descriptor and the batch fold that commits its blob.
- Tool results carrying a descriptor render a credential-redacted
preview chip (the reopen + replay affordance); live results auto-open
the pane only while the originating pane holds focus.
Docs: docs/tools.md + prompts/tools.md. Tests: policy unit tests, tool
prepare/exec (mocked fetch), serving route + proxy header pass-through,
storage exclusion on both backends, cancel-path commit, JS static
guards; a headless-Chrome harness drives the real module graph (32 DOM
assertions).
The discovered-endpoint wrapper converted every OAuthSSRFError to a
bare OIDCError, so a private-resolving endpoint or trusted host got the
non-public message without the allow_private_network remediation even
though the same knob fixes it. Hoist the hint into a module constant
and append it in both wrappers.
Also name "unspecified" in the refused-even-with-opt-in message so
0.0.0.0/:: rejections read unambiguously.
The SSRF guard on OIDC endpoint URLs hard-refused any hostname
resolving to a non-public address, which made it impossible to use a
self-hosted IdP (Keycloak, Authentik, Dex) on an internal network —
even though the login-flow issuer is operator-configured, i.e. trusted
input.
Add [oidc] allow_private_network in config.toml (or
TURNSTONE_OIDC_ALLOW_PRIVATE_NETWORK), default off. When set, the
issuer and its discovered endpoints may resolve to private-range,
unique-local, CGNAT, and loopback addresses. Link-local, multicast,
unspecified, and reserved ranges stay refused regardless — cloud
metadata services live on link-local and no legitimate IdP does. The
HTTPS requirement and same-origin endpoint checks are unchanged.
The private-address refusal now raises OAuthSSRFPrivateAddressError,
and the OIDC wrapper appends the remediation hint to the error message
so the failure is self-service. mcp_oauth call sites — where endpoint
URLs come from untrusted remote-server metadata — do not get the knob
and keep the strict public-address rule.
RFC 3986 schemes are case-insensitive, so POSTGRESQL+PSYCOPG2:// or
HTTPS://user:pass@host in tool output leaked the password past the
case-sensitive scheme alternation. Compile with IGNORECASE on both
sides of the FE/backend mirror; the structural userinfo requirement
is unchanged. Uppercase-scheme cases added to both test suites.
Connection-string redaction (the output_guard pattern and its frontend
mirror) only enumerated bare dialects plus +psycopg, so SQLAlchemy
dialect+driver URLs — postgresql+psycopg2://, postgresql+asyncpg://,
mysql+pymysql:// — leaked the password through every redaction surface.
The scheme now takes an optional +suffix instead of enumerating drivers.
redactCredentials() also gains a single early-exit prefilter scan ahead
of its sixteen replace passes, for plain-log tool output on card render.
The prefilter is documented and pinned as a superset of the pattern
set's required substrings, so a miss is provably a no-op: new smoke
cases assert bare sk-/AKIA/Bearer credentials with no '=', quote or '@'
anywhere in the text still redact, alongside the fast-path no-op and
the driver-scheme URLs on both sides of the mirror.
- Proven: name supervisory control's controllability and nonblocking
conditions as the ancestors of gate-early-on-irreversibles and the
always-enabled escalation required behind a learned veto; cite TCSEC
covert-channel analysis (NCSC-TG-030) for the verdict channel.
- Asserted: add the narrow-only rule's influence-side twin (verdict
payloads to the plant selected, never generated).
- New caveat paragraph: a denial is free only in the authority lattice;
in the dynamics it is an input (selection + targeted-liveness
channels), so a learned veto needs a nonblocking escape it cannot
disable, verdict payloads are selected rather than generated with the
symbols/tokens/language thresholds bounding the alphabet, and the
strongest form dissolves the verdict into scheduling over
deterministic checks.
Review: the wrapped ValueError is echoed in 503 bodies, and arbitrary
SDK exception text can embed filesystem paths. Echo the exception type
only; log the full exception with traceback at the raise site.
SDK client construction can fail on environment problems the config
never sees (e.g. httpx resolving a certifi CA path deleted by a venv
rebuild). Those escaped as bare exceptions and turned every workstream
open/create into an opaque 500; re-type them as ValueError in
ModelRegistry.get_client so routes answer 503 with the message and the
alias.
to_tsvector was computed inline over full row content, so one row
whose tsvector exceeds PostgreSQL's 1MB limit aborted every
search_history scan. Cap the FTS input at 250K chars (worst-case
tsvector expansion stays under the limit; giant rows remain findable
by their head). The ILIKE fallback also never ran on postgres: the
failed statement leaves the autobegun transaction aborted, so roll it
back before falling back.
Address review on #794. The prior gate deepcopied every agent tool, then compared, then discarded the copy on a no-op render; and its comment framed the equality case as 'no personas' when it also covers an idempotent re-render of unchanged aliases/personas. Compute the target model/persona descriptions from the current (read-only) schema first and only deepcopy when one differs — so a no-op render is genuinely allocation-free, not just fork-free. Behaviour is unchanged: identity preserved when nothing differs, stale text still cleared on reload.
_render_agent_tool_descriptions rebuilt self._tools and reassigned it on every session init, deep-copying task_agent even with no model aliases and no personas to inject — the single-model CLI case the docstring says is skipped. This regressed after the persona-discoverability change removed the early 'if self._registry is None: return' guard, breaking the session._tools is INTERACTIVE_TOOLS invariant (test_session_without_mcp).
Gate the reassignment on whether a description actually changed: keep the original tool object when the render is a no-op, fork self._tools only when something was injected. Restores the shared-constant invariant, makes repeated renders idempotent, and preserves clear-stale-on-reload (an emptied registry still takes the changed path).
Add .github/FUNDING.yml to enable the native GitHub Sponsor button, plus a Sponsor badge and a Support section in the README. Primary CTA is GitHub Sponsors (eous); PayPal (paypal.me/eousphoros) is offered as a one-off fallback.
Review feedback on #792: the "not found or disabled" branch
interpolated the raw input unquoted, so the whitespace-only and
trailing-space inputs the forgiving lookup explicitly handles rendered
invisibly in CLI output and logs. Use {name!r} like the other two
resolver errors already do.
Coordinators and interactive agents had no way to enumerate valid
persona names: task_agent / spawn_workstream / spawn_batch described
`persona=` but nothing listed what it accepts, and resolution was an
exact case-sensitive slug match - users reaching for the display name
or a case variant got an unexplained failure.
- Inject the live persona catalog (enabled, interactive-kind; children
and sub-agents are always interactive) into the `persona` parameter
description of task_agent / spawn_workstream / spawn_batch, riding
the same render path as the model-alias injection. Rebuilt from the
pristine TOOLS base every render, so repeated renders are idempotent
and archived personas drop out instead of lingering. Entries carry
name + default marker + <=96-char description; names-only past 25
personas. spawn_batch's persona property is nested per-child under
children.items.properties (located via _persona_property, null-safe
against name-colliding MCP tools). Storage-less sessions keep the
base text: the render runs at session construction, so it gates on
is_storage_initialized() rather than get_storage(), which would
auto-init SQLite as a side effect.
- resolve_persona_for_kind - the ONE shared rule behind the HTTP
create handler, CLI --persona, the coordinator spawn precheck, and
task_agent prep - is now forgiving: exact slug, then the lowercased
input (created names are regex-enforced lowercase slugs), then a
case-insensitive display-name match accepted only when unique among
the kind's enabled personas. Duplicates refuse loudly naming the
candidate slugs; a same-label persona of another kind neither blocks
nor wins (the label the caller saw came from a kind-filtered
surface); whitespace-only input never matches blank display names
(display_name defaults to ""). Every failure now enumerates the
kind's valid names, so a stale injected list or a typo self-corrects
on the next attempt.
- The canonical slug is stamped everywhere: task_agent prep rewrites
its arg from the resolved snapshot, _validate_child_persona returns
(canonical, error) and both spawn call sites adopt it - approval
chrome, the wire, and workstream_config never carry a forgiven
variant.
- Create-persona shelf: label hint under Name explaining agents and
the CLI launch the persona by this name (case-insensitive) and the
display name is only a list label. docs/personas.md gains a "How
agents discover personas" section and drops the stale claim that
task_agent has no persona parameter.
Tests: resolver unit suite (case/display/ambiguity/cross-kind/
whitespace/disabled/storage-failure) + guards for injection content
and ordering, idempotent re-render, archive drop, the 25-persona
prose cutoff, coordinator-kind exclusion, and canonical stamping
through spawn_workstream / spawn_batch / task_agent.
- _coordinator_tenant_check and _coord_attachment_owner resolved storage from
the global registry (get_workstream_row / for_request without a storage arg),
which can evaluate the project-tenancy decision against a different or
auto-initialised backend and fail OPEN on a missing project row. Use
request.app.state.auth_storage explicitly, matching cluster_ws_detail and
_resolve_coordinator_or_404; fail closed (404) when it is unset.
- reject_unassignable_scopes now derives its allowed-scope error message from
ASSIGNABLE_SCOPES so validation and the message can't drift.
Workstreams attached to a private project were visible -- including their
conversation content -- to holders of admin.cluster.inspect / admin.coordinator
(both default builtin-admin permissions), defeating the project's confidentiality
boundary. Enforce that a private project's resources are visible only to people
IN the project (owner, workstream creator, or an explicit member), even for admins.
Surfaces closed:
- WorkstreamProjectVisibility bypass narrowed to service scope only (node->console
machine plumbing, re-filtered per-user at the console edge). No human principal
bypasses; admin.cluster.inspect gates the inspect surfaces, not tenancy. This
flows to /dashboard, session listings, the attachment row-gate, cluster_workstreams,
cluster_node_detail, and cluster_snapshot/SSE.
- cluster_ws_detail 404-masks a workstream in a private project the caller can't
see; cluster_ws_live_bulk routes such ids to the denied list (no private-project
oracle).
- Coordinator operator verbs (history/export/detail/send/approve/set_title/open/
children/tasks/attachments) now enforce project tenancy: _coordinator_tenant_check
on coord_endpoint_config, the gate in _resolve_coordinator_or_404 (children/tasks),
the tenant_check now run in make_open_handler before rehydrate, and a
project-visibility check in _coord_attachment_owner. admin.coordinator gates the
surface cluster-wide, but a non-member is 404-masked. The tenant-check mirrors the
manager-first + coordinator-kind ladder so kind-isolation is preserved.
- service scope is no longer user-assignable: admin_create_token and both
turnstone-admin CLI mint paths reject it via reject_unassignable_scopes, so an
admin.users holder cannot self-mint a service token and restore the bypass. Service
scope is minted only by ServiceTokenManager / the JWT secret.
- The events/global node proxy (service-elevated cross-tenant firehose) is gated on
admin.cluster.inspect so a plain authenticated user cannot reach it through the
console proxy.
Updates the OpenAPI description, the row-gate/tenancy-filter docstrings, and adds
tests for every surface (visibility predicate + cluster detail/bulk + coordinator
history/export/children/open/attachments + events/global proxy + scope-mint
rejection); inverts the tests that pinned the old admin-bypass contract.
* fix(mcp): route static transport lifecycles through per-server owner tasks
A crash-looping MCP server drove the mcp-loop thread to a sustained,
climbing 100%+ CPU spin. Root cause: anyio cancel scopes are
host-task-bound, and the static path entered the SDK's transport /
ClientSession task-group scopes from short-lived connect tasks (every
health tick is a new task since #768). Once such a scope was cancelled
after its host task had finished - by anyio's task_done when a
transport child died with the server, or by ClientSession.__aexit__
during a cross-task teardown - CancelScope._deliver_cancellation could
never make progress (task.cancel() on a done task is a no-op) and
re-armed itself via call_soon every loop iteration, forever: ~900k
callbacks/s per zombie scope, one more per flap cycle (verified against
anyio 4.14.1; no upstream fix exists as of that release).
Fix: each static server's transport + session cms are now entered,
parked, and exited by ONE long-lived owner task
(_static_transport_owner), so scopes always have a live host and always
exit in the task that entered them. Teardown follows a one-cancel close
protocol (signal the close event before the first await, graceful
grace, then at most ONE cancel - never a second, which would abandon a
scope exit mid-flight). Connect timeouts now cancel only the waiting
caller; connect failures are delivered through a readiness future;
unrequested owner death (server died under a live session) evicts the
session immediately via a done-callback instead of waiting for the next
liveness ping. A rate-limited, mcp-loop-scoped gc-walk backstop
(_maybe_disarm_orphaned_scopes) disarms any zombie minted by paths not
yet migrated (the oauth_user pool keeps the old cross-task-close shape;
follow-up).
Also fixed: BaseExceptionGroup (BaseException-derived, as raised by
anyio task groups wrapping a stray CancelledError, e.g. an
accept-then-RST server) escaped `except Exception` in _connect_all and
killed it before the health/sweep loops were created - silently
disabling all autonomous recovery. Handled there and in the
health/sweep/eviction loops and the reconnect/refresh callers.
Verified: a live SIGKILL-flap repro went from 130%+ CPU (climbing, one
armed scope per cycle) to 0.3% flat with zero armed scopes; the RST
repro now leaves both background loops alive (previously both silently
dead). New tests: owner-lifecycle + close-protocol units (incl. an
exactly-one-cancel pin), a _connect_all BaseExceptionGroup regression,
a discriminating disarm-sweep test, and a ~10s live SIGKILL-flap smoke
test (real FastMCP subprocess, skips on environment gaps) asserting
zero armed scopes, exactly one live owner, and a post-recovery tool
call. Full 8470-test suite green; ruff+mypy clean.
* fix(mcp): route pool transport lifecycles through per-entry owner tasks
Completes the owner-task migration started for the static path: the
oauth_user pool path had the same latent anyio cancel-scope exposure
(host-task-bound scopes entered by short-lived connect tasks; a scope
cancelled after its host finished re-delivers cancellation via
call_soon forever - the 100%-CPU zombie), previously covered only by
the disarm backstop.
Each (user, server) pool entry's transport + ClientSession cms are now
entered, parked, and exited by ONE long-lived owner task
(_pool_transport_owner). The caller keeps building client_kwargs (the
per-user bearer and, when an auth-capture carrier is active, the
httpx_client_factory response hook) so 401/WWW-Authenticate capture
semantics are unchanged. Teardown is the shared one-cancel close
protocol (_teardown_pool_entry: signal before first await, graceful
grace, at most ONE cancel), used by the connect stale-guard, idle/LRU
eviction, and shutdown (parallel signal-then-reap). Unrequested owner
death evicts the session but keeps the entry and its discovered
catalog, matching the existing evict-session-keep-entry semantics the
auth_401 retry relies on.
Discovery still runs in the connecting caller while the transport is
hosted by the owner, so a transport collapse mid-discovery (e.g. the
SDK tearing its task group down on an upstream 401) cancels the OWNER,
not the caller - a bare await on the response stream would hang until
the 30s phase timeout. _await_pool_discovery races each discovery
await against owner completion and converts owner death into a prompt
ConnectionError (the owner is never cancelled there; teardown owns its
lifecycle). Carrier-first failure classification preserves auth_401
semantics for captured 401s.
With no cross-task stack closes left, _safe_close_stack and
_safe_teardown_on_connect_failure are deleted (zero callers).
Tests: new tests/test_mcp_pool_owner.py pins the pool close protocol
(graceful event-before-await close, exactly-one-cancel escalation,
owner-death eviction retaining entry+catalog, caller-cancel-mid-connect
cm-exit guarantee, factory-present-iff-capture, and the
owner-death-during-discovery fast-fail). 1010 mcp tests and the full
8471-test suite green, including the historical cross-task-anyio
sentinel test_integration_pool_reuse_401_refresh_and_retry_succeeds;
ruff+mypy clean; zero destroyed-task warnings.
* fix(mcp): harden disarm-sweep loop guard and owner BaseException arm
Review follow-ups on the owner-task migration:
- _maybe_disarm_orphaned_scopes now enforces its mcp-loop requirement
instead of trusting callers: it returns without walking (and without
advancing the rate-limit clock) unless the currently running loop IS
self._loop. A suppressed close can fire before start() or after
shutdown(), where the walk would be wasted at best and a cross-thread
reach at worst.
- The transport owner's BaseException arm now re-raises non-Exception,
non-group escapees (KeyboardInterrupt, SystemExit) after delivering
them to the readiness future - failure delivery is the arm's job;
swallowing an interpreter-level exit was not.
* fix(mcp): extend owner-death discovery fast-fail to the static path
The static connect path had the same exposure the pool's discovery race
closed: discovery runs in the connecting caller while the transport is
hosted by the owner task, so a transport collapse mid-discovery cancels
the OWNER and the caller's bare await on the response stream hung until
the caller-side attempt timeout (~45s) instead of failing promptly.
_await_pool_discovery is renamed to _await_owner_discovery (it is now
path-neutral) and wired into _connect_one_locked's four discovery
awaits. The helper also converts a discovery future that completes
CANCELLED without the race's own reap (an SDK-internal cancellation
shape) into the same ConnectionError, instead of leaking a bare
CancelledError the caller would misread as its own cancellation.
The pool transport owner's BaseException arm gains the same refinement
the static owner received in review: interpreter-level exits
(KeyboardInterrupt, SystemExit) re-raise after delivery to the
readiness future instead of being swallowed.
Tests: static owner-death-during-discovery fast-fail (<1s vs the ~45s
hang), and a direct pin on the cancelled-discovery-future conversion.
* fix(mcp): replace owner BaseException arm with targeted catch + finally delivery
The owner's failure arm now catches only (BaseExceptionGroup, Exception);
waiter delivery for everything else moves to a finally that resolves the
readiness future with a clean transport-failure ConnectionError before
the task unwinds. Interpreter exits and BaseException-derived library
control-flow escapes propagate from the owner exactly once, uncaught -
and the waiter can never be left hanging on an unresolved future (the
initial _connect_all connect has no outer bound). For SystemExit /
KeyboardInterrupt asyncio additionally stops the loop right after, so
the delivery is load-bearing for the non-exit BaseException shapes and
free for the exits.
Pinned by a test driving a BaseException-derived escape through the
owner: the waiter resolves promptly with ConnectionError while the
escape propagates unswallowed.
* fix(mcp): mirror targeted-catch + finally delivery in the pool owner
Same shape the static owner received in review: the failure arm catches
only (BaseExceptionGroup, Exception), and waiter delivery for anything
else moves to a finally that resolves the readiness future with a clean
ConnectionError before the task unwinds - interpreter exits and
BaseException-derived library escapes propagate exactly once, uncaught,
and the waiter can never be left hanging.
* test(mcp): narrow the escape test's waiter catch to explicit types
* test(mcp): narrow discovery-race waiter catches to explicit types
* refactor(mcp): make reap/synchronization awaits explicit to analyzers
Full-absorb reaps (cancel-then-drain of a future whose outcome is
deliberately consumed) become `await asyncio.gather(x,
return_exceptions=True)` - one line, self-describing, and in the
owner-died discovery reap it is also a small semantic improvement: a
caller cancellation arriving during the reap now propagates instead of
being masked by the ConnectionError. Bare synchronization awaits and
selective suppress blocks in tests keep their raise-through semantics
via throwaway assignment. Applied uniformly across the owner-task
test files, including sites introduced by the static-path PR.
The owner's failure arm now catches only (BaseExceptionGroup, Exception);
waiter delivery for everything else moves to a finally that resolves the
readiness future with a clean transport-failure ConnectionError before
the task unwinds. Interpreter exits and BaseException-derived library
control-flow escapes propagate from the owner exactly once, uncaught -
and the waiter can never be left hanging on an unresolved future (the
initial _connect_all connect has no outer bound). For SystemExit /
KeyboardInterrupt asyncio additionally stops the loop right after, so
the delivery is load-bearing for the non-exit BaseException shapes and
free for the exits.
Pinned by a test driving a BaseException-derived escape through the
owner: the waiter resolves promptly with ConnectionError while the
escape propagates unswallowed.
Review follow-ups on the owner-task migration:
- _maybe_disarm_orphaned_scopes now enforces its mcp-loop requirement
instead of trusting callers: it returns without walking (and without
advancing the rate-limit clock) unless the currently running loop IS
self._loop. A suppressed close can fire before start() or after
shutdown(), where the walk would be wasted at best and a cross-thread
reach at worst.
- The transport owner's BaseException arm now re-raises non-Exception,
non-group escapees (KeyboardInterrupt, SystemExit) after delivering
them to the readiness future - failure delivery is the arm's job;
swallowing an interpreter-level exit was not.
A crash-looping MCP server drove the mcp-loop thread to a sustained,
climbing 100%+ CPU spin. Root cause: anyio cancel scopes are
host-task-bound, and the static path entered the SDK's transport /
ClientSession task-group scopes from short-lived connect tasks (every
health tick is a new task since #768). Once such a scope was cancelled
after its host task had finished - by anyio's task_done when a
transport child died with the server, or by ClientSession.__aexit__
during a cross-task teardown - CancelScope._deliver_cancellation could
never make progress (task.cancel() on a done task is a no-op) and
re-armed itself via call_soon every loop iteration, forever: ~900k
callbacks/s per zombie scope, one more per flap cycle (verified against
anyio 4.14.1; no upstream fix exists as of that release).
Fix: each static server's transport + session cms are now entered,
parked, and exited by ONE long-lived owner task
(_static_transport_owner), so scopes always have a live host and always
exit in the task that entered them. Teardown follows a one-cancel close
protocol (signal the close event before the first await, graceful
grace, then at most ONE cancel - never a second, which would abandon a
scope exit mid-flight). Connect timeouts now cancel only the waiting
caller; connect failures are delivered through a readiness future;
unrequested owner death (server died under a live session) evicts the
session immediately via a done-callback instead of waiting for the next
liveness ping. A rate-limited, mcp-loop-scoped gc-walk backstop
(_maybe_disarm_orphaned_scopes) disarms any zombie minted by paths not
yet migrated (the oauth_user pool keeps the old cross-task-close shape;
follow-up).
Also fixed: BaseExceptionGroup (BaseException-derived, as raised by
anyio task groups wrapping a stray CancelledError, e.g. an
accept-then-RST server) escaped `except Exception` in _connect_all and
killed it before the health/sweep loops were created - silently
disabling all autonomous recovery. Handled there and in the
health/sweep/eviction loops and the reconnect/refresh callers.
Verified: a live SIGKILL-flap repro went from 130%+ CPU (climbing, one
armed scope per cycle) to 0.3% flat with zero armed scopes; the RST
repro now leaves both background loops alive (previously both silently
dead). New tests: owner-lifecycle + close-protocol units (incl. an
exactly-one-cancel pin), a _connect_all BaseExceptionGroup regression,
a discriminating disarm-sweep test, and a ~10s live SIGKILL-flap smoke
test (real FastMCP subprocess, skips on environment gaps) asserting
zero armed scopes, exactly one live owner, and a post-recovery tool
call. Full 8470-test suite green; ruff+mypy clean.
The bounded key prefix pattern (api_key=/secret_key= etc.) avoids
monkey/turkey false positives, but compound keys like secret_access_key
and aws_secret_access_key only matched on the access_key= suffix, leaking
the secret_ / aws_secret_ prefix. Added these as explicit alternations.
Also added bearer_token and secret_token to the token prefix list.
Bare alternatives (|token, |key) for standalone token= and key= assignments
were re-added. A negative lookbehind (?<![a-zA-Z0-9_]) prevents matching
word-suffixed identifiers like monkey=, turkey=, mytoken=, over_tokenized=.
Also applied the same protection to _RE_QUERY_CRED which had a bare |token
alternative without boundary protection.
- Replace unbounded [a-zA-Z0-9_]*key= prefix with specific credential
key suffix alternation to avoid false matches on monkey=, turkey=, etc.
Same for *token= (access_token=/auth_token= but not over_tokenized=).
- Hoist redactCredentials() out of per-line diff render loop in
buildConvCmd - 1 call on full text instead of N calls per line,
eliminating ~2800 regex passes worst-case.
- Remove bare 'key' from _RE_QUERY_CRED alternation (too aggressive).
- Add x-api-key / x_api_key to JSON secret key lists in both Python
and JS.
- Add re.IGNORECASE to configurable-mode credential_bearer pattern.
- Fix annotation dedup guard in _check_credentials (was checking flag
name against annotation prose list, always-true dead code).
Address review findings on the client-side credential redactor and mirror
each fix into the backend output guard so both surfaces censor identically:
- Detect and redact single-quoted JSON secrets such as
{'Authorization': 'Bearer ...'} (Python dict reprs / JS object literals),
which the double-quote-only pattern silently bypassed on both sides.
- Cover mongodb+srv://, rediss:// and amqps:// connection strings.
- Match the Bearer auth scheme case-insensitively (RFC 7235).
- Redact prefixed key/token assignments (api_key=, secret_key=,
access_token=) as a whole rather than chewing the tail into a garbled
"api_[REDACTED:api_key]", while still covering bare key=/token=. A word
boundary was rejected because it would drop coverage for <prefix>_key=.
- Remove the redundant |Authorization alternative (covered by /i) and swap
the manual value-slicing helper for a capture-group substitution.
The backend edits touch both detection sites and both redaction pipelines,
so single-quoted secrets are flagged (and therefore sanitized), not merely
rewritten. Adds JS runtime-smoke and backend unit coverage for every case.
New shared ES6+ module (redact_credentials.js) provides comprehensive
visual credential censorship matching the backend output guard patterns:
- PEM private key blocks, connection strings, Bearer tokens
- OpenAI / GitHub / AWS / Google API key formats
- Query-string and JSON-style credential values
- JSON secret keys (api_key, password, token, authorization, etc.)
- ENV secret lines (SECRET_KEY=, DATABASE_URL=, etc.)
Integrated into both frontend surfaces:
- interactive.js: replaces legacy minimal _redactApiKeys function
- conversation.js::buildConvResult (shared substrate, used by coordinator)
- coordinator.js::renderToolOutput fallback paths
Backend parity: added 'authorization' to the JSON secret regex in
output_guard.py so the output guard flags and redacts Authorization
headers in JSON tool output.
Tests: ported the runtime smoke test from the removed _redactApiKeys
to import the new module directly; added redact_credentials.js to the
var-free and const-reassign guard bundles.
`tool_args_preview` feeds `stream.tool_args_malformed` (WARNING) and
`wire.tool_args_legalized` (DEBUG), and tool arguments are model/user
controlled — they can carry secrets (a token in a bash command, a password in a
connection string) or raw CR/LF that break log lines. Route the preview through
`output_guard.redact_credentials` over the full value first (before the 120-char
cap, so a secret straddling the cut isn't half-shown past the pattern's reach),
then collapse every control char to a space, mirroring `audit._scrub_string`.
Addresses the PR review comments.
A tool call whose `arguments` is not a JSON-object string (an unterminated
string from a non-`length` truncation, or an empty `""` from a no-arg call)
was committed verbatim and replayed on every subsequent send. Strict renderers
that re-parse arguments at render time (vLLM's `deepseek_v4`
`_postprocess_messages` runs `json.loads` on them) reject the whole request
with HTTP 400, wedging the conversation. The only prior guard dropped partial
tool calls on `finish_reason == "length"`; a `stop`/`tool_calls` finish reason
carrying invalid JSON slipped through, and its synthetic "retry" result kept it
from being an orphan, so the orphan-repair pass never touched it.
Add `sanitize_tool_call_arguments`, a wire-neutral legalize pass in lowering
(fold, legalize, repair), normalizing any non-JSON-object `arguments` to `{}`
on the transient wire copy only. The canonical trajectory keeps the raw model
output, so a wedged session self-recovers on its next send. A
`wire_valid_arguments` predicate is shared with a non-destructive
`stream.tool_args_malformed` warning at the stream accumulator, which surfaces
the model-quality problem at production time.
Convert `lowering.py` to structlog so the new pass emits structured events.
The pane/workstream accelerators only worked on macOS. They were bound to
Ctrl, which on Windows/Linux IS the browser's own accelerator: Ctrl+T,
Ctrl+W and Ctrl+1-9 were swallowed by the browser (new tab / close tab /
switch tab) and never reached the page. macOS browsers own Cmd instead, so
Ctrl was free there and everything appeared to work.
On top of that the shortcuts were declared in three places that had drifted
apart — the "?" overlay, each app.js keydown handler, and the tab-menu
badges in shell.js. The console fell to convTabMenu's node-proxy fallback
lane, which dropped every shortcut badge (and Fork), so its tab menu showed
no accelerators and Ctrl+W there just closed the browser tab.
Choose the modifier per platform (Ctrl on macOS, Alt on Windows/Linux) and
make shell.js the single source of truth for the per-pane accelerators: a
stable accel registry drives both the platform-aware badge and one shared
keydown handler that invokes the ACTIVE pane's own menu item, so a badge
can't advertise a chord the handler ignores and each surface contributes
only what it supports (the console omits Fork; it has no fork surface yet).
Each surface's app.js keeps only its global accels (new / switch /
dashboard); the console regains switch + dashboard to match. Mod+W now
uniformly means Close pane (drop the tab, session keeps running), matching
its badge and the universal Ctrl+W convention — previously the standalone's
Ctrl+W stopped the session. The previously-dead "Refresh title / Ctrl+Shift+R"
is wired, and Ctrl+T / Ctrl+D yield to text editing while a field is focused
(macOS transpose / delete-forward).
* Fix send button stuck disabled by pruning orphaned approval cycles
When a DOM wipe (clear_ui / replay_truncated / replaceChildren)
detaches approval card elements while an approve_request event is
processed between the wipe and refetch-restore, the matching
approval_resolved may never arrive. The orphaned cycle entry in
approvalCycles keeps pendingApproval=true and the send button
disabled forever.
The fix adds a pruning pass at the top of _syncApprovalState():
cycles whose blockEls are all .isConnected === false are deleted
from the Map. This runs on every register/resolve/rebuild so
orphans are cleaned up promptly.
Also fixes an ordering bug in showInlineToolBlock discovered
during review: the block element was appended to the DOM after
_registerApprovalCycle, so the new isConnected prune would kill
the just-registered cycle before it took effect.
* Fix comment inaccuracy in showInlineToolBlock append-before-register guard
The comment said 'blockEls.every(el => el.isConnected)' but the
actual prune check is '!blockEls.some(el => el.isConnected)' -
no block elements are connected, not every element.
The wire-payload golden matrix had no anthropic-compatible coverage —
both AnthropicProvider rows are the native lane (compat=False), so the
distinct compat wire shape (reasoning control in
extra_body.chat_template_kwargs, never the native thinking param) was
unfrozen. Add the compat lane across all eight representative fixtures
with a manual-mode capability (the lane has no static table, so caps
ride in as a model definition would supply them) and reasoning_effort=
high: every golden now pins {enable_thinking: true, reasoning_effort:
high} in chat_template_kwargs, asserts the native thinking param is
absent, and preserves temperature (no forced 1.0). _capture gains an
optional caps override to support the no-static-table lane.
Copilot review caught the native Anthropic adaptive ladders (sonnet-5,
fable-5, opus-4.8/4.7/4.6) labeling the none position 'None — sends
adaptive' — raw mode vocabulary the plain-language rule exists to
prevent. But treating the 'adaptive' token identically to the local
lane's 'on' would still misframe it: on these models thinking is always
on and an effort level rides output_config on every graded position
(verified on the wire), so 'thinking stays on' is true everywhere and
not what distinguishes none. The none position uniquely means no effort
is pinned — the model self-regulates it — so it now reads 'None — model
sets effort'. The local adaptive lane's bare toggle keeps 'thinking
stays on' (no effort lever exists there).
Aliased knob positions were labeled after the lowest sibling sharing
their wire token — a toggle-only model rendered 'Max (= minimal)',
implying a minimal-effort downgrade the wire doesn't contain, and with
declared values 'High (= minimal)' while the wire carries high. Each
position now states its delivered level: exact matches stay plain
('Max'), snapped positions say 'Low — sends high', the adaptive none
position warns 'thinking stays on', budget detail stays in the
tooltip. Effort-param placeholder corrected to the real graded keys
(reasoning_effort / reasoning).
Local lanes dropped the knob's graded value unless the operator declared
reasoning_effort_values (and, on the template channel, an effort key) —
picking Max sent a bare thinking toggle and the effort select
degenerated into seven positions that all meant 'on'. The user's
setting now always rides:
- openai-compatible: the flat reasoning_effort param carries the knob
verbatim (effort_passthrough on the lane default); declared values
still snap ordinally, and a declared effort_param still claims the
template channel and suppresses the flat param.
- anthropic-compatible: the graded value rides chat_template_kwargs
alongside the toggle whenever reasoning control is engaged — under
the operator's effort_param, else the conventional fallback key
(reasoning_effort); templates that don't reference the kwarg ignore
it. thinking_mode=none still injects nothing.
- Commercial lanes untouched: empty declared values still mean 'no
effort control' (o1-mini) and the ordinal snap is unchanged.
Golden writer now pins ensure_ascii=False: the baselines' literal em
dashes came from a hand edit (03f82521) the default-escaping writer
could never reproduce — regens no longer churn unrelated lines.
Local-lane model ids are operator-chosen strings (vLLM
--served-model-name), so a prefix collision with a cloud model id
inherited that model's sampling and effort contract: a box named
o3-distill silently lost temperature support, and one named
gpt-5.5-my-finetune was sent gpt-5.5's snapped reasoning_effort values
it never declared. Both surfaces of the lane now return plain defaults
(OPENAI_COMPAT_DEFAULT in _openai_common): the chat class directly, and
the responses pin via a compat-mode OpenAIResponsesProvider mirroring
AnthropicProvider(compat=True). Everything beyond the defaults is
declared by the operator on the model definition, matching the
anthropic-compatible lane and lookup_model_capabilities' documented
'no static table for local models' contract. The commercial openai
lane (Responses-only) is untouched.
Pre-split tests that reached commercial rows through the chat-class
OpenAIProvider alias now source them from lookup_openai_capabilities;
their subject (registry rows + shared gating helpers) is unchanged.
The except path for a malformed capabilities column appended the row
without the key, so clients had to null-check a field the happy path
guarantees. Initialize each entry with an empty ladder and let the try
block overwrite it — the response schema is stable per row.
Three server.test.ts / server-attachments.test.ts cases still asserted
the pre-verb-lift send shape (POST /v1/api/send with ws_id in the body).
The server route has been POST /v1/api/workstreams/{ws_id}/send (ws_id
in the path, {message} body) since that lift, and the SDK send() was
updated with it — only these expectations rotted. They fail on main
too; unrelated to approvals, folded in here to leave the TS SDK suite
green. Test-only, no SDK source change.
ApproveRequestEvent.cycle_id and ApprovalResolvedEvent.cycle_id /
call_ids were typed required, but they are 1.7 additions: a pre-1.7
server omits them on the wire, so a current SDK talking to an older
node sees undefined. The UI and channel adapters already keep the
legacy no-selector fallback for exactly that case. Mark them optional
so consumer code can't assume a string that may be absent — matching
the Python SDK, whose dataclasses default all three.
The interactive composer had two uncoordinated writers of
sendBtn.disabled: _syncApprovalState wrote `pendingApproval || busy`
directly, while Composer.setBusy wrote it too via _reconcileDisabled.
In queueWhileBusy mode the composer's write re-enables send, so a
state_change to "attention" (exactly the pending-approval state)
firing after an approve_request card rendered raced the approval
disable back off. The `|| busy` term also defeated the "Queue
message…" affordance whenever _syncApprovalState ran mid-turn.
Opt the composer into externalDisable (it keeps rotating the
Send/Queue label + placeholder + stop button, but no longer writes
the flag) and route every axis — live approval cycle, cross-user send
gate, busy — through one App reconciler, _reconcileSendDisabled. Busy
is intentionally not a disable axis here: queueWhileBusy keeps Send
clickable as "Queue" while the agent runs.
Pre-existing on main (the old scattered direct writes had the same
collision); surfaced by the concurrent-cycle review.
The checked-in openapi-server.json / openapi-console.json still
described the removed singular pending_approval_detail field; re-run
generate-types.py so the reference specs carry the
pending_approval_details list + cycle_id. Retire two docstring
references to the deleted singular serializer and one stale comment.
New regression matrix for the release blockers: cross-approval
independence, lost-wakeup at gate entry, FIFO selector-less
resolution, resolve-all sweep, double-resolution no-op, cards/legacy
view tracking, and the generation-exactness set — stale delivery
rejection, Smart-Approvals origin check, purge keep_origin, the
purge-to-register window eviction, late cross-generation "superseded"
stamping, concurrent smart+human gates, and the pre-delivered-verdict
fast path. Plus sub-agent judge wiring (agent_gate off the main
slot, close() firing all generations) and endpoint tests for cycle
pinning and the Approve+Always race guard.
Gate threads run under one shared mock-patch harness — mock.patch
start/stop of the same target from concurrent threads corrupts the
patcher's restore stack — with a sweep-until-dead teardown so the
conftest leak guard can't trip. Existing suites migrate off the
singleton fields to cycle assertions and the
pending_approval_details wire shape.
interactive.js tracks live cycles in a Map: per-cycle action buttons
and feedback, per-cycle optimistic clears and resolved-status pills,
keyboard routed to the oldest (or the focused) cycle, announce-shell
dedupe, and a composer that stays disabled while ANY cycle is live.
Verdict glow is scored per batch — a sibling's verdict neither
recolors the oldest card nor leaves its own card stale.
coordinator.js renders one approval block per pending_approval_details
entry, posts child approve/deny with the block's cycle_id, clears
exactly the resolved cycle on approval_resolved (legacy events without
one clear all), and replays every entry from snapshots.
Track posted approval prompts by (ws_id, cycle_id) — one message per
concurrent cycle — so parallel task agents' prompts resolve
independently. Buttons carry the cycle in their value (Slack) /
footer (Discord); intent verdicts route onto the owning cycle's
message by call_id membership.
The exact-key-then-legacy-fallback lookup (an empty cycle_id from a
pre-multi-cycle server resolves the workstream's single tracked
entry) and the all-cycles sweep are centralised as shared _routing
helpers so the fallback semantics can't drift between adapters.
POST /approve accepts cycle_id / call_id selectors and 409s on stale
selectors with the current cycle's ids so clients re-render instead of
silently resolving an unrelated batch. Selector-less bodies pin the
resolve to the cycle the lookup returned (not "whichever is oldest by
the time the resolve runs"), and Approve+Always names apply only after
the pinned cycle actually resolved — the auto-approve whitelist can no
longer describe a different batch than the one that resolved.
approve_request carries cycle_id; approval_resolved carries cycle_id +
call_ids; SSE reconnect replays every live cycle's card. The console
collector, coordinator UI fan-out, and both SDKs (Python + TS) thread
the cycle correlation through.
BREAKING (1.7): the singular pending_approval_detail field is removed
from dashboard rows, workstream detail, and node snapshots — replaced
by the pending_approval_details list (one entry per live cycle, each
carrying its cycle_id). stable/1.6 keeps the old shape.
task_agent tool calls reached the approval gate judge-blind: no
heuristic verdict on the card, no LLM verdict, no audit row, and Smart
Approvals could never clear them. Run _evaluate_intent on the
sub-agent gate with the sub-agent's own trajectory as judge context —
its task prompt is the delegation contract the operator approved, so
"does this call serve the task" is the right local alignment question.
Sub-agent spawns are agent_gate generations: they never touch the main
loop's supersede slot (parallel siblings would make each other's
verdicts look stale), staleness is enforced per-cycle by the UI's
generation checks instead. Every generation registers in
_judge_cancel_events — kept exact by the judge's new done_callback —
so close() aborts all in-flight daemons; judge.cancel_on_approval
fires per-gate exactly like the main loop. CLI and eval UIs accept
the judge_event delivery kwarg.
The approval pipeline was a per-UI singleton (one card, one Event, one
result slot) multiplexed by N concurrent gates. With parallel task
agents that meant one click could resolve every parked batch with the
same verdict, a sibling's gate entry could eat a just-fired resolution
(3600s "stuck dialog" hang), and Approve+Always could whitelist a
different batch than the one on screen.
Replace the singleton with a registry of ApprovalCycle objects keyed
by cycle_id: per-cycle events/results/decisions/verdict parking,
oldest-first selector-less resolution, guarded double-resolution, a
resolve_all_approvals sweep for cancel/close/worker-recovery, and a
maintained oldest-cycle view in the legacy _pending_approval slot for
boolean-ish consumers.
Verdict bookkeeping is generation-exact end to end: the entry purge
spares verdicts the entering batch's own judge spawn already delivered
(a fast judge no longer stalls the Smart-Approvals wait to its full
budget), registration evicts stale-generation arrivals that land in
the purge-to-register window, recent decisions are generation-tagged
so a stale generation's late verdict stamps "superseded" instead of
stealing a reused call_id's decision, and Smart-Approvals
qualification identity-checks the delivering generation.
Capability-registry corrections verified against the official OpenAI
reasoning guide, the Azure reasoning-models matrix (2026-06 revision),
and the Anthropic models-overview/effort/migration pages (2026-07):
OpenAI (vocabulary confirmed none/minimal/low/medium/high/xhigh — no
"max" level exists; knob max rides the xhigh ceiling via the ordinal
snap):
- o1/o3/o3-mini/o3-pro/o4-mini declare low/medium/high (every o-series
model except o1-mini) — without declared values the session knob was
silently dropped for these models. o1-mini stays effort-free.
- gpt-5.5 default corrected none -> medium (5.5 reasons by default,
unlike 5.1-5.4).
- gpt-5.1-codex-max gets an explicit row: it prefix-matched the
gpt-5.1 row (no xhigh), capping the knob's xhigh at high on the one
model xhigh was introduced for.
Anthropic (effort-page matrix):
- claude-sonnet-5 row added — it previously fell through to
_ANTHROPIC_DEFAULT (manual budgets, 200k ctx, no effort), all wrong:
adaptive-by-default thinking (manual budgets are a 400), sampling
params rejected, 1M ctx / 128k out, effort low..max incl. xhigh.
- claude-sonnet-4-6 gains its documented "max" effort level (knob
xhigh now rides max, not high) and the stale 64k max_output becomes
the documented 128k.
- fable-5 / opus-4-8 / opus-4-7 / opus-4-6 / opus-4-5 rows verified
correct as declared.
Knob semantics completed: resolve_reasoning_effort now forwards the
knob's "none" position verbatim when the model DECLARES an explicit
none level (gpt-5.1+, grok-4.3) — omitting the param there leaves a
reasoning-on server default (gpt-5.5: medium) in charge of a knob
that promises off. Models without a declared none still omit, and
none is never a snap target. Parity harness swaps its synthetic
openai shape for the real gpt-5.5 registry row.
The knob domain grew xhigh/max after the snapping fallbacks were
written, which silently inverted their semantics: off-list meant
"unrecognized string" then, but now usually means "above the model's
ceiling", where falling back to the default tier is directionally
wrong (grok-4.3 at knob max got low; values low/medium/high at knob
xhigh got medium; Anthropic manual mode gave xhigh/max a 4096 budget
while high got 16384).
One rule everywhere now, via snap_reasoning_effort in _protocol:
exact match wins; otherwise the smallest declared level ranking at or
above the knob; above the ceiling, the ceiling. "none" is never a
snap target, and default_reasoning_effort only catches values the
ordinal snap cannot rank.
- resolve_reasoning_effort (flat chat / responses / validated
effort_param lanes) snaps ordinally: xhigh over (low, medium, high)
now sends high; xhigh over DeepSeek-style (high, max) sends max —
matching DeepSeek's official xhigh-to-max aliasing, so a declared
values list now reproduces that contract instead of defeating it.
- _map_reasoning_to_effort (native output_config) rounds up too:
knob xhigh on Opus 4.6 (low, medium, high, max) rides max instead
of silently dropping output_config.
- EFFORT_BUDGET_MAP is monotone across the whole knob domain:
minimal/low 1024 (API floor), medium 4096, high 16384, xhigh 32768,
max 65536. Unknown strings still fall to the 4096 default.
Google defaults are unaffected (ceiling and default coincide at
high); wire goldens unchanged. Parity harness caught the budget
clamp interacting with its own max_tokens during development —
capture budget raised above the largest manual budget.
Proves the effort-ladder projection against the real request path
instead of against the mapping helpers it shares with it. For 22
(provider lane x capability shape) points — both Anthropic lanes,
openai-compatible on both API surfaces, openai, google (default and
template-override hybrid), xai (default and inert-override), and the
DeepSeek/qwen template contracts — every knob position is driven
through the actual provider create_streaming against a recording fake
client, and two invariants are asserted per shape:
1. each ladder token decodes to an expected effort wire subset
(toggle / template effort / flat param / thinking budget /
output_config) that must equal the captured kwargs exactly;
2. two knob positions carry equal tokens iff they produce identical
effort-relevant wire payloads — the grouping promise the UI
annotations lean on.
The RecordingClient SDK-seam stub moves from the wire-payload golden
harness into tests/_wire_capture.py so both suites capture at the same
seam. Verified the harness catches the bug class it was built for:
re-adding xai to _CHAT_LANES fails xai-template-override-inert.
Second external audit round on the ladder. Verified and fixed:
- xai was in _CHAT_LANES on the false premise that XAIProvider
subclasses the chat provider. It subclasses OpenAIResponsesProvider,
whose surface ignores extra_body entirely, so a thinking_mode /
effort_param override never changes an xai request — but the ladder
claimed a template toggle ("on+low") that does not exist on the
wire. xai now projects through the flat channel only, like openai.
- The flat-param suppression rule (a declared effort_param claims the
template channel) was encoded independently in
apply_temperature_and_effort and the ladder. Extracted into
flat_effort_suppressed() in _protocol so the request path and the
projection cannot drift.
- admin_effort_ladder logs the swallowed resolver exception before
returning 400 (a genuine bug would otherwise hide as a silent 400).
- list_available_models reads server_compat with .get() instead of
destructively popping it out of the parsed capabilities dict.
- models_changed SSE now re-annotates the skill launch-config effort
select after invalidating the models cache instead of leaving a
stale ladder until the next keystroke.
- Capabilities JSON textarea placeholder hints the two effort fields
that have no structured control (reasoning_effort_values,
default_reasoning_effort).
The Gemini effort fix (ee9e9c1f) adds a flat reasoning_effort to every
Google chat-completions request at the session default knob — the
golden fixtures now carry it. Only the eight google__* goldens change
(one added key each); other providers' goldens are untouched — the
UPDATE_WIRE_GOLDENS pass also wanted to rewrite twelve passing goldens
with escape-format-only churn (raw em-dash vs \u2014), reverted to
keep the diff semantic.
The one that mattered: /v1/api/models passed the capabilities column —
a JSON STRING (sa.Text) — straight into effort_ladder_for_model, whose
field filter calls .items() on it; the per-row guard swallowed the
AttributeError, so effort_ladder was silently absent from every row and
the sklc annotation could never fire. The endpoint now parses the JSON
and splits the namespaced server_compat exactly like the model_registry
loader, and a regression test seeds a string-capabilities row.
Projection fidelity: effort_ladder_for_model threads api_surface (the
responses surface ignores extra_body — flat-param-only ladder, matching
create_provider's request-time divergence); google/xai route through
the chat-lane projection they actually inherit (_finalize_extra_body +
flat param); the native-Anthropic branch reflects that output_config
gates on supports_effort alone, independent of thinking_mode; budget
clamping to per-request max_tokens is documented as out of scope.
Hardening and symmetry: admin endpoint 400s (not 500s) on non-dict JSON
bodies and gained HTTP tests; the admin edit-load strips effort_param
from the raw JSON only for the lanes whose save path re-adds it, so
non-compat rows can't silently lose a stored key; the empty-model early
return bumps the ladder sequence so stale in-flight responses can't
re-annotate; alias labels only reference positions the target select
actually offers (the skill shelf omits none/minimal); the sklc models
cache no longer pins a rejected promise and is invalidated on the
models_changed event; debounces unified at 500ms;
merge_reasoning_template_kwargs now always returns a fresh dict for
non-empty input; the shared budget constants are public.
Seven knob positions render as seven behaviors in the UI, but the real
ladder depends on the lane and the model: qwen3.6 has two (off/on),
DeepSeek-V4 three, Claude 4.6 five. Operators had no way to see which
positions alias — the confusion class behind silently-equal effort
levels.
providers/effort_ladder.py projects the knob domain through the same
mapping functions the providers use at request time (resolve_reasoning_
effort, reasoning_template_kwargs, the manual budget map — hoisted to a
shared constant so the projection can't drift), yielding
{value, effective} rows where equal tokens promise identical requests.
/v1/api/models rows now carry the ladder (guarded per row), and
POST /v1/api/admin/models/effort-ladder computes it for the admin
modal's unsaved edits.
The admin per-model effort select and the skill launch-config effort
select annotate aliased positions ("Max (= high)", "None (model
default)") with a sends-tooltip; annotations refresh as thinking-mode /
effort-param / capabilities fields change. The ladder describes what
Turnstone sends — server-side templates may alias further (DeepSeek-V4
folds low/medium into its default high tier).
_GOOGLE_DEFAULT declared no reasoning_effort_values, so
resolve_reasoning_effort returned None and the session effort knob was
silently dropped for every Gemini model — the same bug class this
branch fixed on the local lanes. Gemini's OpenAI-compat surface
documents a flat reasoning_effort (2.5: thinking_budget mapping; 3.x:
thinking_level), so declaring values lights up the inherited
chat-completions path.
Values are the safe cross-model set (minimal/low/medium/high): "none"
is excluded because 2.5 Pro and the 3.x family reject disabling
thinking — and the resolver never forwards the knob's none anyway (the
param is omitted, server default applies). Off-list xhigh/max snap to
the declared default high. Encoded from the official compatibility
docs per the static-caps pattern; not live-verified.
Post effort-knob rework, "Enabled" (manual) means knob-controlled —
effort none turns thinking off. Operators who want the pre-#771
always-on behavior (knob never disables) previously had to hand-write
thinking_mode "adaptive" into the raw capabilities JSON. The dropdown
now offers all three representable modes — None / Effort-knob
controlled / Always on — and the edit-load lift captures adaptive
instead of relegating it to raw JSON.
Cross-checked online: Qwen3.6's template documents enable_thinking +
preserve_thinking only — no effort parameter exists (vLLM's flat
reasoning_effort convenience boolean-maps to the same toggle).
DeepSeek-V4 officially accepts reasoning_effort high/max with Think
High as the default thinking tier and low/medium→high, xhigh→max
aliasing — so freeform effort_param passthrough matches the contract
exactly, and a declared values list omitting xhigh/max would make
Think Max unreachable. Live probes on both boxes agree with the
official contracts once the default-tier framing is applied.
Verified findings applied:
- adaptive thinking_mode never knob-disables: the shared mapping now
sends the toggle unconditionally true for adaptive (the native
adaptive branch ignores the knob's none), while manual keeps the
knob-driven contract. Restores the invariant the deleted chat-lane
code upheld.
- a set effort_param suppresses the flat top-level reasoning_effort on
the chat lane: the template channel replaces it — double-sending
could 400 on schema-strict servers and disagree with operator pins.
- admin edit-save no longer drops a stored thinking_param when the
thinking-mode dropdown is empty: the raw-JSON strip now only fires
when a mode value actually round-trips through the dropdown.
- effort_param persistence gated on the local-server lanes so a value
lingering across a provider switch never lands on commercial rows.
- three stale _compat_extra_params references renamed to
merge_reasoning_template_kwargs.
Documented dispositions (no code change): the knob-none-disables flip
on upgrade is intentional and now carries an upgrade note; gateways
fronting real Claude belong on provider=anthropic with a custom
base_url (the compat lane is vLLM-schema-only); nonstandard
thinking_mode strings staying inert is the intended allowlist
contract. The real anthropic provider is unaffected throughout —
official Claude models keep native thinking/output_config.
Hoist the compat-lane injection into _protocol.merge_reasoning_template_kwargs
(next to ModelCapabilities — one implementation for both local-server lanes)
and retire OpenAIChatCompletionsProvider._apply_thinking_mode in its favor:
_finalize_extra_body now receives the session effort knob, so thinking_mode
manual/adaptive maps knob "none" to an explicit thinking_param false
(previously the toggle was unconditionally true) and caps.effort_param
carries the graded effort key on chat completions too. Operator
server_compat pins still win; the Responses surface is untouched (native
reasoning handles effort itself).
The admin Models form grows an "Effort param" field that round-trips like
thinking_param: lifted out of the raw capabilities JSON on edit-load,
re-added on save, cleared by emptying the field.
Verified live against qwen3.6-27b /v1/chat/completions: knob medium streams
reasoning_content, knob none suppresses it.
The compat lane sent no reasoning control at all: vLLM's /v1/messages
has no thinking request field, thinking_mode stayed "none", and the
session effort knob was silently dropped. The reasoning levers live in
the chat template, so fold them into extra_body chat_template_kwargs
(_compat_extra_params): thinking_mode manual/adaptive maps the knob
onto caps.thinking_param (effort "none" = off, mirroring the native
manual-mode contract), and caps.effort_param (new ModelCapabilities
field) carries a graded effort value for gpt-oss-style templates,
validated against reasoning_effort_values when declared. Operator
server_compat entries win on key collision; native thinking params,
temperature forcing, and output_config never fire on compat.
resolve_reasoning_effort moves from _openai_common to _protocol next to
ModelCapabilities — importing it into _anthropic would otherwise cross
provider families.
The admin Models form now shows and round-trips the thinking-mode
dropdown for this lane; the #661 hide was premised on thinking_mode
being inert here, which this change inverts.
Verified live against qwen3.6-27b on vLLM /v1/messages: knob medium
streams a thinking block, knob none suppresses it, an operator pin
beats the knob.
* feat(mcp): autonomous reconnect + liveness for static MCP servers
Static (non-oauth_user) MCP servers had no autonomous reconnect. Every reconnect
path was lazy — a tool dispatch (_cb_auto_reconnect), an operator refresh, or a
config edit — and the MCP SDK's own reconnect is a bounded 2-attempt burst on the
streamable-http GET stream only (verified: mcp 1.28.1), with no backoff and
nothing for the other transports. So a static server that went down and came back
while nobody was dispatching to it stayed disconnected until a dispatch or a
manual reconnect. Worse, a session whose transport dies while idle survives as a
non-None ClientSession with closed streams — nothing evicts it, so even a later
dispatch may not notice until it fails.
Add a static-server health loop on the mcp-loop (started in _connect_all; config
``static_health_check_seconds`` default 30, <= 0 disables):
- Reconnect: a disconnected server (session is None) is reconnected on a capped,
jittered, FOREVER backoff (full jitter, base 1s, cap 60s, no attempt limit) —
a server that returns after a long outage reconnects within ~a minute, and a
permanently-misconfigured one costs at most one attempt per cap. The health
loop owns this clock; the circuit breaker stays the DISPATCH fail-fast gate (a
tool call to a down server errors immediately rather than blocking on the
retry), and the loop keeps breaker state in sync so an open breaker closes on
reconnect.
- Liveness: a connected server is pinged (send_ping) each cadence; a dead-but-
idle one — which nothing else would notice — is evicted so the next tick
reconnects it. This is the core of the "never reconnects" failure.
Serialize _connect_one per server behind a per-name lock (split into a thin
wrapper + _connect_one_locked): the health loop, a dispatch's _cb_auto_reconnect,
and an operator refresh could otherwise interleave teardown/rebuild on the shared
StaticServerState and corrupt it — a latent pre-existing race this also closes.
The body is unchanged (only relocated), so the delicate anyio / wait_for connect
logic is untouched.
Out of scope (follow-up): silent GET-stream / notification death — the SDK stops
the notification stream after 2 attempts while the request path stays alive, so
send_ping is blind to it; the fix is bounded session recycling, which needs a
static in-flight guard first (only PoolEntryState tracks in_flight today).
Tests: backoff bounds (capped / jittered / forever, no overflow), reconnect
success resets backoff + closes breaker, reconnect failure retries forever,
in-flight skip, per-name serialization (no overlap), ping keeps healthy / evicts
dead / evicts on timeout, tick skips oauth_user, connect_all start + disable
gating, clean cancel.
* fix(mcp): harden static-server health loop (review findings)
A max-effort review found 13 concurrency/correctness defects, all from the loop
mutating shared StaticServerState without the interlocks the pool path carries.
Fix all 13:
- In-flight interlock: add StaticServerState.in_flight (parity with
PoolEntryState); _static_session_op increments/decrements around the static
call_tool/read_resource/get_prompt session ops; the ping skips and never
evicts a busy server, so a long tool call can't be torn down mid-flight.
- Dead-transport gating: the ping evicts + trips the breaker only on
_is_dead_transport(exc); an McpError, httpx.PoolTimeout, or plain ping timeout
is "slow, not dead" and only reschedules (matching the dispatch path).
- Session-identity: only evict the exact session that was pinged.
- asyncio.timeout (invariant-18) not wait_for for the ping; 5s->30s; the
timeout-scoped cancel is distinguished from an external shutdown cancel
(which still propagates) via .expired().
- Bounded reconnect: wrap _connect_one in asyncio.timeout so a server that
handshakes then stalls list_tools can't wedge the loop or hold the per-name
lock forever (connect internals untouched).
- Concurrent tick under asyncio.gather with a freshly-read clock for the sleep.
- Cross-path coordination: reconnect_sync/remove_server_sync take the per-name
lock across teardown+rebuild (calling _connect_one_locked directly);
_cb_auto_reconnect reuses a health-established session instead of racing a
redundant reconnect and no longer trips the breaker on lock contention.
- Backoff hygiene on recovery; skip '__' names; on health reconnect clear only
the open-circuit deadline (not the failure count) so a connect-ok/calls-fail
server still escalates to a trip.
Adds 14 tests and adjusts those that assumed the old behavior; suite 195->209.
* fix(mcp): unify static-server reconnect coordination (round-3 review)
A third review round + live testing found 8 issues on the health loop, five
sharing one root: reconnect logic was fragmented across five drivers, each
handling the lock / session-reuse / in_flight / config-recheck / breaker / clock
differently and incompletely. Introduce one primitive and route every
lazy/autonomous driver through it.
_ensure_static_connected(name, cfg) — the single lazy (re)connect path, all under
the per-name lock: config re-check (+ lock-identity re-check, closing the
remove->re-add race) so a removed server is never resurrected; reuse-if-live so a
queued/concurrent driver never tears down and rebuilds a live session (the
observed reconnect storm); in_flight guard so a reconnect can't tear down a
session with a call still in flight on the evicted stack; bounded connect; and the
circuit breaker owned in one place (clear the open-circuit deadline on success per
finding-13, record one failure on real connect failure). Returns the session on
success/reuse, None on a deliberate skip, raises on real failure. Routed through
it: the health loop, a dispatch's _cb_auto_reconnect, and _refresh_all; operator
reconnect_sync stays a deliberate force-rebuild.
Also: fresh-clock deadlines (the stale tick-start clock was landing deadlines in
the past and collapsing the backoff into an every-tick retry storm); loop-death
fix (the tick no longer re-raises a CancelledError found in the gather results —
per-server fallout, not shutdown; the loop returns only when Task.cancelling()
marks a genuine shutdown); dispatch breaker records no failure on a sync-boundary
reconnect timeout (lock contention is not a server failure; real outcomes recorded
once, inside the primitive). Cleanups: extract _teardown_static_session (was
copy-pasted 3x); share _capped_exponential between the breaker cooldown and the
reconnect backoff. Adds 17 tests; test_mcp_client 204->226.
* fix(mcp): coherent timeout hierarchy + round-4 review fixes
A fourth review round on the unified reconnect coordination found 5 correctness
regressions + 1 cleanup, five sharing one root: the inner reconnect attempt bound
(45s) was LONGER than every caller wait (dispatch 30s, remove 15s, reconnect 30s),
so a caller cancelling mid-attempt delivered a bare CancelledError that slipped
past the primitive's `except Exception`.
- Coherent timeout hierarchy: add _STATIC_RECONNECT_CALLER_TIMEOUT_S (> the inner
attempt bound) for the dispatch + operator waits, so the inner asyncio.timeout
always fires first — a clean TimeoutError the primitive converts, cleans up, and
records on the breaker — instead of a caller cancelling a live attempt. Fixes
[0] (half-discovered session left installed, served with a stale catalog) and
[1] (breaker never trips via dispatch).
- Primitive cancel-safe (belt-and-suspenders): its handler is now
`except BaseException`, so even a bare CancelledError drops the partial session
and records the failed attempt before re-raising.
- Operator waits: reconnect_sync / remove_server_sync default timeouts raised above
the reconnect bound; remove_server_sync now CANCELS the pending _remove on
timeout (so it can't later pop a re-added entry and corrupt state) and reports
failure instead of a false 'removed' ([2], [4]).
- in_flight defer is gated on defer_if_busy: autonomous callers (health loop,
_refresh_all) defer, but a DISPATCH reconnects rather than hard-fail a reachable
server with an in-flight sibling ([3]).
- Cleanup: extract _schedule_next_ping (was a copy-pasted triplet in 3 branches) [5].
Adds 6 tests; the lock-contention test now shadows the caller-timeout constant so
it runs in ~1s instead of the full wait.
* fix(mcp): address PR review feedback on static reconnect
- _ensure_static_connected: skip breaker record on CancelledError
(cancel proves nothing about the server; aligns docstring with impl)
- reconnect_sync: add asyncio.timeout wrapper so discovery-phase
stalls get a clean TimeoutError inside the lock
- reconnect_sync: change except Exception to except BaseException
so CancelledError from future.cancel() triggers catalog cleanup
- reconnect_sync: null state.session on failure so a tool-less
session isn't mistaken for a live one
- _static_reconnect_one: use fresh monotonic clock for backoff
gate instead of stale tick-start snapshot; remove dead now param
- _static_health_tick: correct docstring (0.5s clamp prevents
busy-spin, not 'no sleep through short backoff')
- Test: new test for reconnect_sync timeout + catalog cleanup
- Test: update cancelled-attempt test for new CancelledError semantics
- Test: narrow except BaseException to except Exception + type hints
- Test: fix typo 'Understone' -> 'Turnstone'
- Test: remove unused stale_now variable; update mock signatures
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Migration 054 added these columns but they were never populated (the sole
writer hardcodes None) and never read by any query, dashboard, or API.
Drop them so the schema matches reality.
Closes#769
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The docstring claimed the dispatch path plumbs a triggering ws / tool-call id
through a thin wrapper. It does not: _record_pending_consent_best_effort neither
receives nor forwards any id (the mcp dispatch layer never has one), the
proactive sweep has no dispatch, and nothing reads last_ws_id / last_tool_call_id
— they are unpopulated schema columns from migration 054. State that plainly so
the comment doesn't imply data is captured when it isn't (Copilot review nit).
Per-user OAuth (auth_type=oauth_user) token refresh is entirely lazy: a token
is refreshed only when a tool is dispatched or a session binds the acting user,
and a dead refresh token is discovered only when a dispatch fails. That assumes
a human is driving the session, which breaks for autonomous / scheduled work
acting on behalf of an absent user — the token may be expired (latency), in a
transient-failure cooldown (unavailable), or the grant may be dead with nobody
present to re-consent. The only periodic MCP-loop task, idle eviction, actively
tears OBO connections down; nothing keeps tokens warm.
Add a background token-freshness sweep that keeps every consented oauth_user
grant hot WITHOUT keeping connections warm and WITHOUT mutating consent state on
a timer.
Sweep (_user_token_sweep_loop, default 240s):
- Enumerates consented (user, server) grants from the token store and runs the
canonical refresh path for each. Strictly oauth_user-scoped: gates on
_oauth_user_server_names and drives off mcp_user_tokens rows, so a static /
no-auth server — which has neither — is structurally invisible (no DB scan, no
authorization-server round-trip, no MCP-server call). Never connects to the
MCP server; connections stay lazy.
- Observe-only: passes revoke_on_failure=False (new parameter on
get_user_access_token_classified) so a timer NEVER deletes a token, emits
token_revoked, or mutates the shared ambiguous-streak / cooldown. A dead grant
is only surfaced (proactive dashboard pending-consent badge); the
authoritative revoke stays on the lazy-dispatch path where a real user action
justifies it. Because the row survives, a spurious server-wide invalid_grant
(an AS maintenance window) self-heals — the badge is dropped on the tick the
grant works again.
- Keepalive: force-refreshes a grant whose refresh token has sat un-exercised
past user_token_refresh_keepalive_seconds (default 1800s) even while the
access token is still fresh, so a provider that ages out idle refresh tokens
can't expire one between a user's real sessions.
- Surfaces dead grants once per transition, pinning the pair only after a
durable badge write so a failed persist retries rather than being lost. First
sweep runs after a short startup grace so a restart surfaces a downed grant
within seconds, not a full cadence later.
Cadence <= 0 disables the sweep; a positive value is floored (30s) so a
misconfigured tiny cadence can't turn the loop into a busy-loop. Reuses the
per-key refresh lock, so a keepalive force cannot double-refresh against a
concurrent dispatch.
Storage: add list_mcp_user_token_reconcile_targets() returning
(user_id, server_name, COALESCE(last_refreshed, created)) — expiry-unfiltered,
no ciphertext projected — on the protocol, sqlite, and postgres backends.
Tests: the sweep's no-auth invisibility (zero DB / AS calls with no oauth_user
server), observe-only non-destruction (token kept and shared streak untouched on
a background permanent / ambiguous failure), keepalive gating, badge
persist-then-pin retry, self-heal on recovery, cadence clamp / disable, and the
storage enumerator.
- task_agent.json referenced a non-existent `skill(action='search', query=...)`
in two spots. The discovery tool is `skills` and the action is `find`
(`search` is an activation value, not an action), so the guidance would
mislead the model. Corrected both to `skills(action='find', query='...')`,
matching the tool's own error strings.
- _high_risk_skill_denied returned "" (allow) when get_storage() is None — an
asymmetry with the fail-closed lookup-exception path added earlier. A risk
gate that can't verify the tier must DENY, not wave the skill through, so
storage-unavailable (None) now denies too; both paths share one denial.
Two independent multi-agent reviews of the branch (high, then max effort) found
authority-confinement and robustness defects the per-step reviews could not
see. This commit addresses every confirmed finding. task_agent turned out to be
the surface that lagged its siblings on nearly every axis.
Risk gate (most severe):
- task_agent(skill=...) never enforced the high/critical-risk PRINCIPAL-load-
only gate that skills(load) / spawn_workstream / spawn_batch enforce, so a
model could route around it by delegating activation to a sub-agent. Enforce
it inline in _prepare_task on the row already fetched (no re-query, no drift
between get_skill_by_name and get_prompt_template_by_name).
- _high_risk_skill_denied now fails CLOSED on a storage fault: deny, never wave
the skill through. Denying (not returning "") also keeps spawn_batch's per-row
partial-success intact under a transient blip.
- (first round) extracted _high_risk_skill_denied onto spawn_workstream /
spawn_batch, closing the coordinator-side bypass.
Persona confinement (Principle 7 attenuation on the task_agent edge):
- A restrictive persona now attenuates the sub-agent's TOOLS, not just its
identity text — the tool lever is frozen into the item and filtered before
_run_agent.
- Honor ALL FOUR persona levers on the sub-agent, not two: a child persona's
mcp-off and memory-off levers now drop MCP tools (mcp__* + read_resource /
use_prompt) and the memory tool, matching a main session under the persona.
- Cap the sub-agent by the PARENT session's own persona grant too, so a
restricted principal cannot escalate authority by spawning.
- Add persona to the task_agent judge/audit func_args projection (policy +
audit parity with spawn).
- Persona-resolution failures defer to a clean tool error (try/except mirroring
_validate_child_persona) instead of an opaque "internal error".
Substitution / capability:
- substitute_args=False for capability contexts (defaults, task_agent) so a
literal $ARGUMENTS / $N in a body is preserved, not blanked; env vars still
resolve. The literal-$ARGUMENTS scan is deferred behind that guard (skipped on
every capability render).
- Drop the CLAUDE_SKILL_DIR alias (canonical TURNSTONE_SKILL_DIR only). That
name also lives in bash, where turnstone-as-a-node-inside-Claude-Code must not
shadow the host's value; claiming it in the prompt but deferring in bash
diverged the two surfaces (a review finding). turnstone now claims it in
neither surface. The CLAUDE_SESSION_ID / CLAUDE_EFFORT prompt aliases stay
(pure prompt values, no bash-namespace collision).
Skills-as-context:
- DEFAULT (always-on) skills stay in the identity system message — the standing
baseline, never a mid-session cache-bust; only a NAMED applied skill moves to
the user-role capability message. This shrinks the pending model-adherence
eval surface to the named-skill move alone.
Cleanups: consolidate a duplicated rationale comment; correct the now-stale
"task agents are not persona-filtered" note.
PRE-MERGE GATE unchanged: the §7 Q1 model-adherence eval (named-skill move,
this branch vs main) is not runnable in-tree and must clear before merge.
A skill can auto-fire tools (auto_approve + allowed_tools) once loaded, so
letting the model activate a risky skill through skills(action='load') is an
injection-steerable lane that widens authority behind a rubber-stampable
approval. Deny it: high/critical-risk skills are now PRINCIPAL-load-only --
the model gets a clean error pointing the user at /skill, and the operator
loads such skills explicitly (handle_command /skill and cli --skill call
set_skill directly and bypass this gate).
The scanner-computed risk_level is the gate signal -- it already escalates for
the auto_approve + allowed_tools authority the create path warns about -- so no
new column or migration is needed. The check can only DENY, never widen, so it
is safe by construction (HYPOTHESIS.md Principle 7 / design section 5.5).
Remaining step-5 follow-ups, out of scope here: persisting the literal
disable-model-invocation frontmatter field for arbitrary author-marked skills
(needs a column) and deprecating the vestigial variables mechanism.
Step 3 of the skill/persona split: an applied skill (including default skills)
is CAPABILITY context, so its body no longer sits in the identity system
message. It rides its own message (user role) after the identity block, with a
short intro naming the active skill. The <available-skills> discovery catalog
stays in the system message.
Two consequences:
- The cached identity prefix (persona BASE + ENV + POLICIES + catalogs) stays
stable across skills(load): loading/clearing a skill changes only the
trailing capability message, not the identity block.
- The task_agent base (_agent_system_messages) is snapshotted BEFORE the skill
block, so a parent's applied skill no longer leaks into the sub-agent prefix
(the sub-agent supplies its own persona identity and skill via _exec_task).
PRE-MERGE GATE: the design gates this on a model-adherence eval (this branch vs
main) verifying the model follows a skill as well from a context message as it
did from the system message (design section 7 Q1; ASSUMED-neutral, UNVERIFIED).
That eval is not runnable in-tree and MUST clear before this branch merges.
Mechanical structure is pinned by TestSkillContextPlacement.
Deferred follow-up: sub-agent (task_agent) skill-resource materialization, so
${TURNSTONE_SKILL_DIR} stays literal on that path (unchanged since step 1).
Test helpers (_sys_content) now read the full prompt prefix (identity + skill
context) so placement-agnostic assertions keep working.
Before, a task_agent's system identity WAS its skill (skill body concatenated
into the sub-agent's system message), and #683 deliberately gave task_agent no
persona. Now that personas are first-class on every creation/spawn path, make
task_agent consistent: identity comes from a persona, the skill is capability.
- task_agent gains persona= (validated at prep against the interactive kind,
the general-purpose personas a worker can adopt). The resolved base prompt
is frozen into the approval item; _exec_task never re-reads storage.
- Default identity (no persona=) stays _TASK_DEFAULT_IDENTITY. The one-shot,
tool-over-narration operating guidance always layers on top.
- skill= is now CAPABILITY: rendered through the shared pipeline (step 1) and
delivered as a distinct user-role context turn ahead of the task, never
fused into the identity. Consecutive user turns coalesce at the provider
boundary (Anthropic _merge_consecutive), so this is wire-safe.
Updates the task_agent tool schema (persona param; skill reframed as
capability) and flips the persona guard test (task_agent HAS a persona param
now). Sub-agent skill-resource materialization and the interactive
skills->context move remain follow-ups.
Skill-body placeholder substitution diverged by invocation context:
interactive load, default skills, and spawn-child ran the full
render + spec-substitute, while task_agent (_exec_task) ran
_render_template only -- so $ARGUMENTS and ${...} env placeholders
rendered literally on that one path.
Introduce _render_skill_body as the single render+substitute path and
route interactive load, defaults, and task_agent through it, so a skill
reading ${TURNSTONE_EFFORT} or $ARGUMENTS resolves identically wherever
it runs. A sub-agent has no invocation args, so bare $ARGUMENTS and the
positional $N / $ARGUMENTS[N] forms resolve to empty there -- matching
the defaults and spawn-child paths, not the old verbatim passthrough.
- Add ${TURNSTONE_*} as the canonical vendor-neutral spelling for the
env placeholders (SESSION_ID, EFFORT, SKILL_DIR); keep ${CLAUDE_*} as
a permanent back-compat alias so imported skills keep resolving.
- Bash env: export TURNSTONE_SKILL_DIR and SKILL_RESOURCES_DIR
unconditionally, but add CLAUDE_SKILL_DIR only when the host has not
set it, so turnstone does not shadow a real value when it runs as a
node inside Claude Code.
- Materialize skill resources before substituting the body, so
${TURNSTONE_SKILL_DIR} resolves to the concrete bundle path on the
interactive path.
Sub-agent resource materialization and moving identity to a first-class
persona are left to follow-ups; ${TURNSTONE_SKILL_DIR} stays literal on
the task_agent path for now (unchanged from prior behavior).
Address Copilot review: cancel() now signals the worker to stop (a
cancellation Event) and joins only when started, so a test that errors
before the approval registers can't leak the worker or resolve late into a
finished test. The worker reads _pending_approval via getattr so a UI
without it can't crash the thread into a silent death (leaving approve_tools
blocked the full timeout), and only resolves when it actually observed the
registration.
The UI-approval tests drive a blocking approve_tools() by firing
resolve_approval() from a fixed 0.05s threading.Timer. approve_tools does
_approval_event.clear() -> register _pending_approval -> wait(3600s); on a
slow/loaded runner the timer can fire the event's .set() BEFORE that .clear(),
so the wakeup is wiped and approve_tools blocks the full _APPROVAL_WAIT_TIMEOUT
(one hour) -- surfacing as an intermittent CI hang (observed on the 3.12 runner
~15% into the suite; fast runners win the race, so 3.11/3.13 pass the same
commit).
Replace the fixed-delay timer with resolve_when_pending() (tests/conftest.py):
it waits until the approval is actually registered -- which happens AFTER the
clear -- before resolving, so the set can never be lost. The helper mirrors
threading.Timer's start()/cancel() so the surrounding scaffolding is unchanged.
10 sites across 3 files; the verdict-delivery timer (bounded to its own 5s
budget, not a hang) is left as-is.
Validated: the 3 files pass 20/20 under single-CPU stress (taskset -c 0) with
no hang or thread leak.
- Docstrings/help said the treatment skill 'composes into the system
message'. This harness runs on both checkouts (system on main, a context
turn on the placement-refactor branch), so the wording now describes the
natural set_skill composition path without asserting a placement.
- Validate each skill-bearing case's 'skill' shape up front (driver +
CLI) so a malformed dataset fails with a clear error, not a mid-run
KeyError. Pinned by test_rejects_malformed_skill.
Add a two-arm skill-adherence mode to the eval measurement substrate that
measures whether a NAMED skill changes tool-use behaviour, so skill-in-system
(main) can be compared against skill-in-context.
- _run_single_test gains skill/skill_mode: skill_mode builds HeadlessSession
under natural composition (no system_prompt_override) and, for the treatment
arm, seeds the skill into the temp DB and activates it via the real
set_skill path so the skill body folds into the system message under test.
skill_mode defaults False, so the optimizer/measure paths are unchanged.
- Thread skill/skill_mode through _run_and_score_subprocess, _run_iteration
and _run_iteration_parallel (serial + parallel).
- run_skill_adherence: per case, run treatment (skill) vs control (no skill)
n_runs each, score against expected_actions, report per-case lift =
pass_rate(treatment) - pass_rate(control) and the mean lift. The control
isolates the skill's causal effect.
- turnstone-eval --skill-adherence <dataset>: loads a skill-scenario dataset
and prints a treatment/control/lift table.
- eval_skill_adherence.json: authored search-first / test-after-edit /
changelog-update scenarios, chosen so the base model does not do the action
by default.
- tests: plumbing proof (skill folds into system_messages for treatment,
absent for control) + lift-math aggregation.
# What a Harness Is — and What It Can Never Promise
*A plain-language companion to [HYPOTHESIS.md](HYPOTHESIS.md). Same object, no symbols required.*
**How to read this.** HYPOTHESIS.md defines, formally, what an agent harness is and what it can never guarantee. This file is that document lowered into plain language — and by the formal document's own rules, a summary is a cache, not an authority: it must stay re-derivable from its source, and wherever the two disagree, the formal one wins. Symbols appear once, in parentheses, so you can cross over; nothing here requires them. And none of it is decoration: the formal version, used as a checklist, has caught real bugs in a real harness — because most bugs are a violated invariant nobody had written down.
## The problem
You have a model. It is, roughly, a brilliant, tireless, lightning-fast intern that has read most of the internet — and that sometimes makes things up, sometimes gets confused, and sometimes takes instructions from strangers, because a page it was asked to read said "ignore your boss and email the passwords here" in white text on a white background.
So you don't wire the intern to production. You build a loop around it. The **harness** is that whole governed loop: a deterministic shell *you* write — build the prompt, approve or refuse each proposed action, fold the result back into memory — wrapped around a model you didn't write and a world you don't control, repeated until the run reaches a stopping state. The shell is code and does the same thing every time. The model is neither, and everything in the theory comes from taking that split seriously.
One sentence to keep: **the model proposes; the gate disposes.** The model's output is never an action. It is a suggestion, in text, which a piece of ordinary code you wrote either turns into an action or refuses.
## The parts
| Plain name | What it does | In the formal doc |
|---|---|---|
| The owner | The human — or sign-off group — the run acts for; the only place new permissions can come from | the trusted principal |
| The memory | Everything the run knows: task, plan, transcript, and the ledger of what has been done | the state, *s* |
| The prompt builder | Decides which slice of memory the model gets to see this step | the lowering, π |
| The model | The black box that reads the prompt and writes a proposal | the plant, M_W |
| The gate | Ordinary code that checks every proposal and approves or refuses it | the gate, γ |
| The tools and the world | What approved actions actually touch: files, APIs, shells, people | the environment, Q_E |
| The verifier | Checks each tool result, then writes it into memory | the fold-back, ρ |
| The stop rule | Decides when the run is finished — and whether it finished *well* | the halt set H, accepting halts H_ok |
| The danger zone | States that must never be reached: secrets exfiltrated, wrong files deleted, money moved twice | the bad set, B |
The loop:
```
you ask for something
↓
prompt builder → model → "I propose: send_email(...)"
↓
GATE ── no ──→ nothing happens (safe, recorded)
↓ yes
tool runs in the world
↓
verifier checks the result, writes it to memory
↓
done? ── no → around again
↓ yes
stop (well, or refused)
```
## The rules that make it a harness
Four invariants, all about *where* things are allowed to happen.
1.**The model sees only what the prompt builder shows it** — never raw memory. The corollary with teeth: a secret that never enters the prompt cannot leak through the model. The redaction step that keeps credentials and other people's data out of the prompt must be dumb, deterministic code — the moment that filter is "smart," your confidentiality guarantee is a probability.
2.**Model outputs are proposals, not actions.**
3.**Every side effect passes the gate.** There is no second door.
4.**The harness itself flips no coins.** Replay a step with the model's answer and the tool results pinned, and behavior must be identical; any leftover variation is randomness *you* added and must be accounted for. The fine print: "deterministic" is conditional on pinned versions — a provider silently retraining the model behind the same API name changes the machine under you, and every dashboard number you collected dies with the version.
Notice what the rules don't say: they don't say the harness is *good*. A gate that approves everything satisfies rule 3 the way a lock that's always open satisfies "has a lock." The definition is a shape; the guarantees are what a particular harness *earns* inside it. Everything below is about what can be earned — and what can't.
And notice the symmetry between rules 1 and 3. There is exactly one door from your data into the model — what it may see — and exactly one door from the model into the world — what it may do. Nearly every security failure in these systems is one of those two doors with a hole in it: a secret lowered into a prompt that didn't need it, or a path from model text to a side effect that skipped the gate. Same bug, arrow flipped.
## Fail-closed, said precisely
"Fail-closed" gets used loosely. Here it means something exact: **nothing happens unless the gate said yes, and a refusal must itself be safe** — a refused proposal causes no side effect and leaves the run somewhere sane, which may be "stopped, having declined." The run is allowed to *say so*: a templated status message written by the shell is the shell speaking, not the model, and needs no gate. Failed runs don't have to die silent.
Three consequences people miss:
**Reads are not free.** A read-only call can smuggle instructions *in* (the fetched page is attacker-controlled) or secrets *out* (the URL it fetches can encode the payload). The gate approves calls, not just writes.
**Validation must not act.** A "validator" that resolves a URL, expands a template that fires a webhook, or evaluates an argument has already acted — inside the check. The gate must be pure: it reads the proposal and the memory and outputs yes or no. If deciding requires touching the world, that touch is itself an action and goes through the gate.
**Anything irreversible is decided at the gate.** The verifier can reject a bad *result*; it cannot unsend the email. So the question "can we take this back, and until when?" is asked before execution — which means each tool declares, up front, how reversible its effects are, and the gate reads that declaration when it decides; the mark that comes back in the result record is confirmation for the books, not the gate's source — the gate needed the answer before the tool ever ran.
Two honest asterisks. First, the gate checks a snapshot: it approves against the world *as its memory describes it*, and the world can move between check and commit. For actions that race the world — spend against a balance, write against a row — the tool itself must bind check to commit (compare-and-swap), or you have a classic time-of-check/time-of-use hole. The gate decides; for those effects, the tool enforces. Second, a gate is only as binding as the authority behind the tools. A tool process holding standing credentials — a database connection with every grant, an environment full of long-lived secrets — doesn't need the model's proposal to act, and against it the gate's "no" is a decision with nothing enforcing it. **A gate in front of an omnipotent tool is a suggestion.** The fix is to make the approval *be* the key: each authorized action carries a short-lived credential scoped to exactly that action, that resource, that operation, so tools hold no standing power at all.
## Why you don't get a proof — and what you do instead
If you write a sort function, you can prove it sorts: the function is small and the spec is exact. A harness has neither luxury. The spec side fails first — the task arrives in natural language, and natural language is, in the compiler's sense, *all undefined behavior*: there is no formal standard for "what the user meant" to verify against. The mechanism side fails next — the model is billions of learned parameters, and nobody can hand you a compact argument for why they jointly do the right thing.
Here is the careful version, because "you can't prove it" overshoots. The quantity you would want — call it the *expected steps to done* from any situation — is perfectly well-defined; in principle it exists. The document's central conjecture is that, for a model of this size, any faithful writing-down of that quantity is roughly *model-sized*: the honest proof-object does not compress. Find a small one and the conjecture dies — the document lists that outcome, explicitly, among the ways it could be wrong.
So instead of proving, you measure. You pick a progress meter — plan depth shrinking, open obligations closing, budget burning at the expected rate — and you check, across many runs, that it goes downhill and that its stalls predict failure. Two disciplines keep the measurement honest. The number bounds the world you *sampled*, never the world an adversary will choose: a meter calibrated on friendly traffic says nothing about hostile traffic. And the meter is itself attack surface: if "is the agent making progress?" is judged by another model, an attacker who can bend your agent can bend your *measurement of it* first, hiding the divergence from the very dashboard built to catch it. A learned meter is part of the system under test, never a neutral instrument.
A measurement is a risk metric. A proof is a certificate. Keeping those two words apart is half of what this theory is for.
## Security: reach the goal, avoid the danger — and who may change the rules
Formally, security here is a *reach-avoid* problem: reach a good stop, never touch the danger zone, **while an adversary picks the worst tool outputs your setup permits**. That last clause is the formal home of prompt injection: injection isn't "the model misbehaved," it's the environment optimized to bend your loop — poisoned pages, malicious tool descriptions, crafted responses.
Two different numbers fall out here, and dashboards love to collapse them: *success* (reached an accepted end before anything went wrong — a safe refusal counts against it) and *safety* (never touched the danger zone — a safe refusal is perfectly safe). Track both. They move independently. And both are scored by your own stop rule — they count what the shell *declared* a success. Whether a declared success was actually *right* is a third, harder number that no dashboard inside the system can produce; only a judge outside the run — a test suite, an audit, ground truth — can.
The gate handles the visible half of injection: the model, freshly poisoned, proposes emailing your credentials somewhere, and the gate refuses — and injection or not, the action does not happen. But the deeper attack doesn't propose a bad action today. It rewrites *what the run believes its job is* — it edits the plan — and then every future action looks locally reasonable against a corrupted plan. So memory has to be partitioned: **data** (tool results, fetched pages, retrieved documents — content the world supplied) and **control** (the plan, the permissions, what is authorized next). The security claim is conditional on that partition holding: untrusted content lands in data, always. And "trust" is really two questions pointing opposite ways, which is worth keeping straight: *can this leak?* (a value is as secret as the most-secret thing that fed it — secrecy flows **upward**) and *can this boss us around?* (a value is as trustworthy as the least-trustworthy thing that fed it — authority flows **downward**). Untrusted content is safe as *data* precisely because the second question keeps it off the control side; a secret is kept out of the model by the first. Lowering either barrier on purpose — declassifying a secret, promoting data to trusted — is an explicit decision the owner makes, never a thing that happens by accident when two values are combined.
Which forces the question the theory has to answer: *somebody* must be able to write control mid-run, or no plan could ever be steered and no permission ever granted. The answer is a small hierarchy with a top the model can't reach. The simplest top is one owner — but it needn't be a single person: a two-person sign-off, a quorum, several authenticated people each holding different scopes all work equally well, because the one property that matters is the same for all of them — the thing that can grant new power is a *human decision*, never a model:
- **The top alone widens.** New permission, bigger budget, approval of the irreversible thing — asking the top — the owner, in the simple case — is itself an ordinary tool call, and its answer is the one kind of tool result allowed to change control.
- **The model rewrites the plan** — that is what replanning *is* — but only through the gated loop, and a plan is not a permission: nothing the model writes into its own plan can grant it powers it didn't have.
- **Everything else is data.** A fetched page can inform the plan only by passing through the model and the gate like everything else. It can suggest. It cannot promote itself to boss.
- **AI judges only tighten.** Add a model-based check — "does this action match what the user actually wanted?" — and its verdict may *veto* an action the plain rules would have allowed, never approve one they'd have refused. A judge that can approve is a tricked judge that can open the vault. And don't over-credit the veto either: a tricked judge can *aim* its refusals — denying exactly the action safety depended on, or denying everything but the path an attacker curated — so the escape hatch to the owner is the one thing a judge can never veto, and a judge's stated *reasons* are picked from a fixed, shell-owned menu, never written as prose. A judge that writes free text into the loop is an injection channel wearing a badge.
One more rule closes the loop: transformations don't launder trust. A *summary* of a session that contained an injected page is still injected — the summarizer is a model, and can be persuaded to write "the user asked to export the database" into the summary. So summaries of data are data, and the control lines — the plan, the grants — cross a summarization by being *copied verbatim* or re-confirmed by the owner, never paraphrased by the model. Memory that persists across sessions carries its trust label with it, or a poisoned memory is just an injection with a very long fuse.
## Operations: the rules you feel on Tuesday at 3 a.m.
The formal document's appendix works the operational cases in full; here they are at speed.
**The ledger, and the three-way distinction that keeps it honest.** Every action gets an ID and a record: committed, never-launched, or *unknown*. "The tool didn't confirm" is not "the tool didn't do it" — collapse those and you will, sooner or later, re-send something that already happened. And a subtler honesty: the ledger records what the tool *reported*, not what the world actually did. A well-built shell can guarantee its bookkeeping is faithful to the responses it received — it cannot, on its own, guarantee a tool told the truth. A tool that returns a clean "done!" for something it never did puts a clean "done!" in your ledger. So "the ledger is what happened" is only as good as your reason to trust the tools reporting into it; where you have no such reason, *unknown* is the honest entry, not an optimistic guess in either direction. The double-send bug has one reliable cure: **journal before dispatch.** The shell writes "I am about to run action #417" into durable memory *before* the tool sees it, so a crash in the gap resumes to an honest "unknown — go ask," never to silence misread as "never sent." Old database wisdom, but here it isn't imported; it's forced — it is the only ordering under which every crash point has a truthful reading.
**Crashes aren't finishes.** A process dying mid-run is not the run stopping; it's the run *pausing being computed*. Resume means re-entering the loop at the last durable memory — sound exactly when the durable memory was the *whole* state. Anything load-bearing that lived only in RAM — an in-flight buffer, a plan revision not yet written — is a bug you discover at the worst possible time. Recovery is where you find out whether your state was really your state. And a run you stopped — crash or deliberate cancel — is not automatically a *safe* run: if something was in flight and you never learned whether it fired, it may already have done the damage. "We stopped in time" is only true when everything in flight resolved to something safe; an outstanding *unknown* has to be treated as possibly-bad, the same optimism the ledger warns against, one level up.
**Two innocent actions can be guilty together.** Models emit several tool calls per turn. "Read the secret" passes review. "Post to the web" passes review. The pair is an exfiltration channel — so the gate authorizes the *set*, atomically, with the interactions checked, not each element in isolation.
**Sub-agents are just fancy tools.** An agent that spawns another agent is, from the parent's chair, calling a tool: the spawn is gated, the budget is part of the deal, and the child's whole run comes back as one result carrying the child's ledger. Two laws travel down the tree: budgets subdivide, and **authority only narrows** — a child holds at most a subset of its parent's permissions, and a child's request beyond those grants routes *up*, ultimately to the owner, because a parent inventing an approval it never held is the tricked-judge case wearing a manager's badge. A corollary worth framing: a *fully autonomous* run is one whose owner is unreachable — meaning the only channel that can ever widen anything is closed, and its permissions are frozen at launch. That is not a limitation of the theory. That is what the word "autonomous" costs.
**Keep the originals.** When the transcript outgrows the prompt and you summarize it down, deleting the original is an irreversible act against your own state — and irreversible acts are gate decisions, self-directed or not. Keep originals content-addressed; let the summary be an index, re-derivable, auditable. A summary you can check against its source is a note. A summary that replaced its source is a fait accompli.
## Robots that never clock out — and robots that assign their own work
Everything so far assumed a job that *ends*: you ask, the robot does it, you read the result. Two steps past that are where the interesting failures live, and they're the same idea one level bigger each time.
**The robot that never clocks out (a daemon).** A monitor, a coordinator, a service — it isn't supposed to finish; it's supposed to keep going, wake on events, do a bit of work, go back to waiting. The clean way to think about it: each wake-work-rest cycle is one ordinary run, and the daemon is just those runs chained end to end forever. That reframing is free — but it comes with a bill nobody likes. **Safety that's fine per cycle rots over many cycles.** A 99.99%-safe cycle sounds bulletproof; run it ten thousand times and you're at about a coin-flip of having touched the danger zone at least once. So a long-running robot's safety isn't a fixed wall, it's a slow leak — which means the antidote isn't a better wall, it's *scheduled resets*: the owner re-confirming, credentials rotating, memory getting audited and re-summarized against the originals. Housekeeping isn't housekeeping; it's the thing that keeps the safety math from decaying. And the slow-leak logic is exactly where slow attacks live — a poisoned note dropped into memory on Monday and read back into the plan on Friday is an injection with a long fuse. So the trust label on a piece of information has to survive across cycles, not just within one. One more wrinkle: a daemon drifts in and out of your reach. While you're around, it can escalate to you; while you're not, "escalate to the owner" isn't available — so the one thing it must always be able to do instead is *stop*. A robot that can be tricked into refusing everything, and can't reach you, had better be able to halt rather than be steered.
**The robot that assigns its own work (the loop).** Step back one more time. Above the robot that *does* a task sits a system that decides *which task is next* — scans the backlog, picks one, launches the robot at it, checks the result, remembers, fires again. This is the thing people mean in 2026 when they say they've stopped prompting their agents and started writing *loops* that prompt them: you design the assigner once, and it runs the doer for you while you sleep. The honest observation — and the reason this document bothers with it — is that the assigner is *not a new kind of thing*. It's the same harness, one level up: it has its own memory (the backlog), its own gate (**who let the loop refactor the auth module at 3 a.m.?**), its own verifier, and its own two walls. Every rule from the inner robot recurs on the outer one — including the uncomfortable ones. There's still no proof it stays out of trouble over a long night; there's only a measured progress meter, with the same catch that a *learned* meter can be fooled. And the origin story of the whole trend is the cautionary case in miniature: the famous first version was literally the same prompt in a `while` loop until the tests passed — which is the empty gate, the always-open lock, one level up. It works beautifully right up until the tests weren't checking the thing that mattered. The loop doesn't delete the hard problems. It moves them up a floor, where they're bigger and you're further away.
The pattern, if you want the whole thing in one line: *words, context, robot, loop* are four sizes of the same object, and every promise in this document lives in the whole assembled thing — never in any one layer by itself.
## The two walls
Two limits are structural. You don't fix them with a better harness; you design around them.
**The desk.** The model can hold only so much *in mind at once* — the context window. Files, databases, and search extend what it can *look up*, not what it can hold: every lookup still passes through the same small window to touch actual computation. The shell can page; the model cannot grow its desk. Tasks whose irreducible working set exceeds the desk don't fail loudly — they fail by forgetting the middle (the well-documented "lost in the middle" effect is this wall showing through the paint).
**The dictionary.** The model's knowledge is frozen into its parameters at training time — and the proof problem above is conjectured to live at that same scale: the certificate wouldn't fit anywhere smaller than the brain it certifies. The two walls trade against each other along the training-versus-inference axis — bigger dictionary or bigger desk — directionally, and at no clean exchange rate.
## How this could be wrong
This is a hypothesis, and it says out loud what would kill it. The tests, in plain terms:
- **The replay test.** Rerun with model answers and tool results pinned. Any leftover variation — timestamps, wall-clocks, and cache expiries are the classic leaks — falsifies "the harness adds no randomness" until accounted for.
- **The drop-a-variable test.** Remove something from memory; if behavior statistics shift, the memory wasn't complete. The crash-resume version of the same test: if resuming from saved state breaks, the saved state wasn't the state.
- **Does the meter mean anything?** If no reasonable progress meter's drift predicts real failures — across the natural families, not just one bad candidate — the whole "measure what you can't prove" program is empty.
- **The red-team test.** Swap sampled tool outputs for worst-case ones: injected pages, poisoned metadata, malformed replies. The design must survive the worst permitted world, not the average one.
- **Gates versus begging.** The theory predicts deterministic gating beats prompt-level pleading. If "please be careful" alone matches real gates on security outcomes, the controller-versus-model story is wrong.
- **The compression hunt.** Exhibit a compact, provably sound progress certificate for a frontier-scale model on a nontrivial task family, and the central conjecture falls — constructively.
- **The desk probe.** Take a task family with a *proven* memory floor — so "it needed the whole picture at once" is someone else's theorem, not our excuse — scale it past the window, and watch: the wall predicts a *ceiling*, not a cliff — past the boundary, a success rate that stays capped no matter how many retries you buy. A family solved reliably out there, without new shell tricks for splitting the work, kills the wall.
## Who else landed here
The formal document keeps three honesty tiers. **Borrowed**: real theorems, cited — the drift and stopping-time mathematics is classical, and the very architecture of a deterministic supervisor gating a plant it didn't author is 1987 control theory; the shape is older than the web. **Ours**: the modeling choices and the conjectures — the walls, the incompressibility claim, the design rules — organizing principles, not results. **Corroborated**: pieces of the same object reached independently by people who never saw this framing — capability-security work isolating control flow from untrusted data (CaMeL), reinforcement-learning "shields" filtering a learned policy's actions through a deterministic checker, verification work that states the "learned safeguards can't certify" gap as its opening motivation, and architecture patterns converging on plan-then-execute. Even the field's live disagreement — provable-but-rigid deterministic layers versus flexible-but-uncertifiable learned checks — is, in this frame, not a fight but a placement: you need both, on their proper sides of the irreversibility line, with the learned one permitted only to tighten.
## What to remember
The model proposes; the gate disposes. No is the default, and a refusal must be safe. Only the top of the trust hierarchy widens permissions — a human decision, never the model, a tool result, a summary, or a judge. "Didn't confirm" is not "didn't happen." The desk is finite and the proof doesn't compress, so you measure — and you say *measurement* when you mean measurement. A robot that never stops leaks safety slowly, so it needs scheduled resets — and when it can't reach you, it must be able to stop. A loop that runs robots for you is just a bigger robot with the same rules and a further-away owner. And all of it is a hypothesis wearing its own kill-conditions on its sleeve.
The formal version — the objects, the certificates, the falsifiers, the citations — is [HYPOTHESIS.md](HYPOTHESIS.md). It wins every disagreement with this file, including this sentence.
Self-hosted, local-first orchestration for tool-using AI agents. Give LLMs real tools — shell, files, search, web — and run them across your own cluster with direct HTTP routing and interactive interfaces. Your code, your models, your data stay on hardware you control: no telemetry, no phone-home.
@@ -20,7 +21,7 @@ Named after the [Ruddy Turnstone](https://en.wikipedia.org/wiki/Ruddy_turnstone)
ℋ : s_{n+1} ~ T(s_n) for n < τ*, T = ρ ∘ (M_W ∘ π, E)
```
[**the hypothesis →**](HYPOTHESIS.md)
[**the primer →**](PRIMER.md)
### Release Tracks
@@ -171,6 +172,14 @@ UML diagrams in [`docs/diagrams/`](docs/diagrams/):
- [Git LFS](https://git-lfs.com/) for cloning (diagram PNGs)
## Support
Turnstone is free, Apache-2.0, and self-hosted — no paid tier, no telemetry, no upsell. If it saves you time or you'd like to help keep development moving, you can sponsor the project:
**[❤ Sponsor Turnstone →](https://github.com/sponsors/eous)** · one-off via **[PayPal](https://paypal.me/eousphoros)**
Sponsorship is entirely optional and funds maintenance, new features, and infrastructure. Prefer to contribute in other ways? Filing issues, improving docs, and [pull requests](CONTRIBUTING.md) help just as much.
## Community
Questions, ideas, or want to show what you're building? Join us on Discord:
| 400 | `{"error": "Empty command"}`| Command is empty |
| 404 | `{"error": "Unknown workstream"}`| `ws_id` not found |
| 409 | `{"status": "busy", "error": ...}` | A turn/command holds the worker |
| 503 | `{"status": "error", "error": ...}` | The command worker could not be started (resource exhaustion) — the command did **not** run; retry shortly |
---
@@ -948,6 +1052,7 @@ All fields are optional. The body can be empty or an empty JSON object.
| `name` | string | Auto-generated workstream name |
| `resumed` | bool | Whether a previous session was successfully resumed |
| `message_count` | int | Number of messages in the resumed session (0 if fresh) |
| `initial_message_status` | string | Present ONLY when the workstream was created but its `initial_message` could not be delivered: `"queue_full"` (a raced live worker's interjection queue was at capacity — resend via `/send`; any uploads stay staged) or `"refused_closed"` (the workstream was closed mid-create). Absent whenever the message was dispatched. |
| `create_streaming()` | The one transport: streaming request, yields normalized `StreamChunk` objects (single-shot callers accumulate via `drain_stream()` into a `CompletionResult`) |
@@ -252,6 +252,7 @@ interface, or anyone who can reach it can search through your instance.
| Variable | Default | Description |
|----------|---------|-------------|
| `WORKSPACE_MOUNT` | empty volume | Host directory bind-mounted at `/workspace` for the model to read/write |
| `TURNSTONE_WORKSPACE` | `/workspace` (image env) | Directory named as the user's workspace in the model's tool descriptions; informational only — see [Working directory](#working-directory) |
| `SKIP_PERMISSIONS` | — | Set to any value to auto-approve all tool calls (dev only) |
| `MCP_CONFIG` | — | Path to an MCP server config file |
| `TURNSTONE_IMAGE_TAG` | `latest` | ghcr.io image tag — production stack |
@@ -17,8 +17,9 @@ The MCP server admin form exposes three authorization modes ("Multitenant Author
| `none` | No headers attached. Open MCP server (or one gated by network policy only). | Internal MCP servers on a trusted network. |
| `static` | One static bearer token, configured per server, sent on every request from every user. | Service-to-service MCP servers where per-user attribution doesn't matter, or single-tenant deployments. |
| `oauth_user`*(recommended for user-data servers)* | Each user authorizes separately via OAuth 2.1 + PKCE; Turnstone stores per-user tokens encrypted at rest. | MCP servers that expose user-specific data or that want per-user audit attribution. |
| `oauth_obo`*(sign-in passthrough)* | Each user's Turnstone **org sign-in** (OIDC) mints a per-server access token on demand — no separate per-server consent. One captured credential per user covers every `oauth_obo` server. | Enterprise deployments where the identity provider governs access (Entra, Keycloak) and you want zero per-user connect clicks. See the dedicated section below. |
Switching `auth_type` away from `oauth_user`orphans existing per-user tokens. Use the admin **bulk-revoke** affordance on the server row (Phase 9) to clear them, or let them expire naturally — they're inert without the matching `auth_type` value.
Switching `auth_type` away from `oauth_user`/ `oauth_obo`**deletes** that server's per-user rows (consents / minted cache) — see the transition table below. Switching back later starts clean: users re-consent (or re-mint) on next use. The admin **bulk-revoke** / **flush cache** affordance clears rows without an auth-type change.
---
@@ -65,6 +66,58 @@ Keep this in `config.toml` rather than environment variables. An in-process LLM
Where `oauth_user` makes each user complete a **separate** browser consent per MCP server, `oauth_obo` reuses the user's Turnstone **org sign-in** (OIDC). Turnstone captures one refresh credential per user at login and, on each tool call, mints a short-lived access token scoped to that server's audience. There is no per-server connect step, and one credential covers every `oauth_obo` server. This is the right shape when your identity provider already governs who may reach each backend (an Entra tenant with Entra-protected MCP servers; a Keycloak realm with token exchange).
Access is governed **downstream** by the IdP: a user can only mint a token for a server their delegated permissions allow. Removing that grant at the IdP cuts the user off regardless of their Turnstone state.
### Deployment configuration (`[oidc]` in `config.toml`)
`oauth_obo` requires OIDC SSO to be configured (it is the credential source), plus:
```toml
[oidc]
# ... your existing issuer / client_id / client_secret ...
capture_user_credential = true # persist the IdP refresh token at login
obo_grant_profile = "entra" # "entra" | "rfc8693" — how tokens are minted
```
- **`capture_user_credential`** (default `false`): when enabled, Turnstone appends `offline_access` to the login scopes and stores the returned refresh token, encrypted with the same `[security] mcp_token_encryption_key` as `oauth_user` tokens. **The encryption key is required** — Turnstone refuses to start with an `oauth_obo` row (or capture enabled) and no key.
- **`obo_grant_profile`** picks the mint mechanism (the IdP determines which one is valid; this is deployment-wide, not per-server):
- **`entra`** — redeems the user's refresh token directly for a token scoped to `<audience>/.default`. `oauth_scopes` on the server row is **not used** (the admin form rejects it under this profile).
- **`rfc8693`** — a refresh grant for a subject token, then an RFC 8693 token exchange for the server audience. Per-server `oauth_scopes`**are** sent on the exchange (some IdPs require the audience scope explicitly).
### Adding an `oauth_obo` server
In the admin MCP form, choose **Sign-in passthrough** and set **Audience** (required — the downstream resource the token is minted for, e.g. `api://<app-id>` on Entra or the client id on Keycloak). The client-id / secret / registration fields do not apply and are hidden.
`oauth_obo` servers are accepted only when **OIDC sign-in is configured and enabled** and `[oidc] obo_grant_profile` is a valid profile — the write is rejected otherwise, since a row that can never mint would surface to users as a permanent "please retry" that never heals.
### Identity-provider setup
**Entra (`obo_grant_profile = "entra"`):**
1. Turnstone's app registration must hold **delegated permissions** to each MCP server's exposed API, with **admin consent granted** (or the MCP app listed in Turnstone's `preAuthorizedApplications`).
2. Set the server row's Audience to the MCP app's Application ID URI (`api://<guid>`).
3. **Gotcha (verified):** admin-consent issued *immediately* after creating the app/service principal can silently skip a not-yet-propagated resource — the only symptom is `AADSTS65001` at mint time. Verify the delegated grant landed (`az ad app permission list-grants` / the portal's *API permissions* blade shows *Granted*), or grant it explicitly per resource. A missing grant surfaces in Turnstone as a re-login prompt on the affected server (same rail as a revoked credential), and the `mcp_server.oauth.obo_mint_rejected` log line carries the raw `AADSTS…` text.
1. Enable **standard token exchange** on Turnstone's client.
2. Grant the audience: add an audience client scope for each MCP client and attach it to Turnstone's client (optional scopes must be requested — set the server row's Scopes to that scope, or the exchange returns *"Requested audience not available"*).
3. Set the server row's Audience to the downstream client id.
### Revocation & custody
The captured credential is a single per-user secret that can mint for every `oauth_obo` server, so treat it like any long-lived credential:
- **Cut off one user:** unlink their OIDC identity in the admin console (**Users → OIDC identities → delete**). This revokes the captured credential **and** purges their minted cache rows, so future mints fail and cached tokens are dropped. (Warmed in-memory sessions on server nodes self-expire at the access-token TTL; there is no cross-node per-user session-kill.) Removing the user's access at the IdP is the authoritative cut-off.
- **Flush a server's minted tokens** (e.g. after narrowing its audience): the server row's **flush cache** action drops all users' cached tokens for that server. This is **not** a revocation — users re-mint on next use from their still-valid sign-in. It is surfaced honestly (audit `mcp_server.oauth.obo_cache_flushed`, response `effect: cache_flush_remints`) so it is never mistaken for cutting access.
- Per-server revocation in the `oauth_user` sense does not exist for `oauth_obo` — the credential is issuer-scoped and IdP-governed. Revoke at the IdP.
> **Interim for Entra without OBO:** if you don't want host-side minting, admin consent + `preAuthorizedApplications` on each MCP app registration removes the second consent prompt for the plain `oauth_user` flow too (a tenant-config change, no Turnstone code). Tracked in issue #682. It does not remove the per-server connect clicks or per-(user, server) token custody — that is what `oauth_obo` is for.
---
## Lifecycle
1. **First tool call** for a user against an `oauth_user` MCP server: pool dispatch finds no stored token, returns `mcp_consent_required` to the agent. Dashboard renders an inline "Connect" action card.
@@ -75,7 +128,7 @@ Keep this in `config.toml` rather than environment variables. An in-process LLM
4. **Step-up scope**: when a tool call hits `403` with `WWW-Authenticate: error="insufficient_scope"`, Turnstone emits `mcp_insufficient_scope` with the parsed scope set; the dashboard offers a "Connect with additional scopes" affordance that opens `/v1/api/mcp/oauth/start?server=<name>&scopes=<extra>` so the union of original + new scopes flows into the AS authorize request.
5. **User revoke** (settings modal): `DELETE /v1/api/mcp/oauth/connections/{server_name}` runs the authoritative local delete + best-effort RFC 7009 upstream revoke (fire-and-forget, capped at 256 concurrent in-flight tasks).
5. **User revoke** (settings modal): `DELETE /v1/api/mcp/oauth/connections/{server_name}` runs the authoritative local delete + best-effort RFC 7009 upstream revoke (fire-and-forget, capped at 256 concurrent in-flight tasks).`oauth_obo` servers are excluded: their rows are mint cache, not consents — deleting one only forces a re-mint — so the connections list hides them and the endpoint refuses them with `409` (revocation for sign-in passthrough happens at the identity layer: unlink the identity or revoke at the IdP).
6. **Admin bulk-revoke** (Phase 9): `POST /v1/api/admin/mcp-servers/{name}/bulk-revoke` drops every user's token for the server. Upstream RFC 7009 revoke is intentionally **not** attempted in bulk (avoids N upstream HTTP calls per admin click); tokens at the AS expire naturally. Use the per-user revoke endpoint if you need guaranteed upstream invalidation.
| `none` / `static` → `oauth_user` | — | New code path activates for this server. Existing static headers (if any) are no longer sent. Users must authorize on first use. |
| `oauth_user` → `none` / `static` | — | Existing `mcp_user_tokens` rows are **orphaned** — inert without a matching `auth_type`. Use admin bulk-revoke to drop them, or let them expire. Switching back to `oauth_user` later re-activates the orphaned rows if they haven't been deleted. |
| `oauth_user` → `none` / `static` | — | Existing `mcp_user_tokens` rows are **deleted**: the tokens are bound to the auth model + URL active at consent time, and rows left behind could silently rebind if a row with the old name/URL reappears. Switching back to `oauth_user` later starts clean — users re-consent on next use. This is **not reversible**; the AS-side grants are untouched (revoke upstream via the AS if needed). |
| OAuth `client_id` or `client_secret` rotated | — | Existing tokens may stop refreshing if the AS treats them as bound to the previous client. Bulk-revoke after rotation. |
| `oauth_user` ↔ `oauth_obo` | — | The per-user rows are **deleted** on the flip (they mean different things: per-server AS refresh tokens vs. minted cache). `oauth_audience` and `oauth_scopes` mean different things in each model (a resource indicator vs. an IdP app identifier; AS-consent scopes vs. an rfc8693 exchange scope), so on a flip they **never carry** — each is taken from the request for the target model or set NULL. The admin console clears these fields when you change the auth type, so re-enter the correct values for the new mode; via the API, supply them explicitly (a flip into `oauth_obo` with no `oauth_audience` is rejected, and a non-empty `oauth_scopes` under the `entra` profile is rejected since that leg pins `<audience>/.default`). |
| `oauth_obo`**audience**, **URL**, or **`oauth_scopes`** changed | — | Minted cache rows are **deleted** (tokens are bound to the audience/URL/scopes at mint time), forcing a fresh mint — so an audience or scope narrowing takes effect immediately, not at token expiry. |
The orphan-by-default behavior is chosen so switching back to `oauth_user` is non-destructive. Bulk-revoke is the explicit cleanup path.
Every transition that changes what a stored row *means* deletes the rows outright — a stale consent or minted token must never be served under new semantics. There is no orphan-and-reactivate path.
---
@@ -113,5 +169,9 @@ The orphan-by-default behavior is chosen so switching back to `oauth_user` is no
| `mcp_oauth_url_insecure` | MCP server URL is `http://` (not `https://`) on a non-loopback host | Use `https://`. Per-user bearers must not transit cleartext. |
| Tools fail in scheduled / Discord / Slack runs | OAuth-MCP requires browser-based consent | Users must pre-consent via the web UI. Phase 9 dashboard badge surfaces deferred consents from these runs on next login. |
| Circuit breaker open repeatedly | Transport-level errors on the MCP server (DNS, TLS, 5xx) | Check the per-server error pill; auth errors do not trip the breaker. |
| **`oauth_obo`**: every tool call fails, log shows `obo_misconfigured` | Server row has no Audience, or `obo_grant_profile` is unset/unknown | Set the Audience on the server row; set `[oidc] obo_grant_profile` to `entra` or `rfc8693`. |
| **`oauth_obo`**: `obo_mint_rejected` with `AADSTS65001` | Turnstone's app lacks the (admin-consented) delegated grant to this MCP app — often admin consent that didn't propagate | Grant + admin-consent the delegated permission for this resource; verify it shows *Granted*. See the Entra gotcha above. |
| **`oauth_obo`**: "Sign in to Turnstone again" on one server | Captured credential missing/rejected, or a Conditional Access challenge | User re-logs into Turnstone (re-captures the credential). If it persists, check the IdP grant / CA policy. |
| **`oauth_obo`**: tools don't appear at all for a user | User has not signed in since `capture_user_credential` was enabled (no credential captured) | User logs out and back in via OIDC so the refresh credential is captured. |
See also: `docs/operations/mcp-oauth-headless.md` for the cron / channel-driven run caveat.
| `TURNSTONE_OIDC_PASSWORD_ENABLED` | No | `true` | Set to `false` to hide the password form and block all username/password logins (including admin). API tokens continue to work. |
| `TURNSTONE_OIDC_REDIRECT_BASE` | Yes | — | Externally-reachable origin for the OIDC redirect URI (e.g. `https://app.example.com`). Without this, OIDC will refuse to start. The previous Host-header fallback was unsafe under permissive reverse proxies. |
| `TURNSTONE_OIDC_TRUSTED_ENDPOINT_HOSTS` | No | — | Comma-separated list of additional hostnames whose endpoints the IdP discovery document is allowed to reference. See [Cross-host endpoints](#cross-host-endpoints). |
| `TURNSTONE_OIDC_ALLOW_PRIVATE_NETWORK` | No | `false` | Allow the issuer (and its discovered endpoints) to resolve to private/internal addresses — needed for a self-hosted IdP on an internal network. See [Self-hosted and internal IdPs](#self-hosted-and-internal-idps). |
All four required fields — issuer, client ID, client secret, and
`TURNSTONE_OIDC_REDIRECT_BASE` — must be set. If any are missing OIDC
@@ -76,17 +77,17 @@ IdP from redirecting the token-exchange POST (which carries
being aimed at internal services.
A few public IdPs legitimately split endpoints across hostnames. Google
is the canonical example:
and Microsoft Entra ID are the canonical examples:
turnstone exposes 16 built-in tools plus any number of external MCP tools to the
turnstone exposes 17 built-in tools plus any number of external MCP tools to the
LLM via the OpenAI function-calling interface. Built-in tools are defined as JSON
files under `turnstone/tools/` and loaded at startup by `turnstone/core/tools.py`.
MCP tools are discovered from configured MCP servers at startup by
@@ -28,13 +28,19 @@ schema plus turnstone-specific metadata keys:
}
```
**Metadata keys** (stripped before sending the schema to the model):
**Metadata keys** (stripped before sending the schema to the model; the full
set lives in `_META_KEYS` in `turnstone/core/tools.py`):
| Key | Type | Meaning |
|----------------|------|---------|
| `task_agent` | bool | Tool is available to task sub-agents. |
| `auto_approve` | bool | Tool runs without user confirmation (read-only, safe operations). |
| `primary_key` | str | When the model sends a bare string instead of JSON args, map it to this parameter name. |
| Key | Type | Meaning |
|------------------|------|---------|
| `task_agent`| bool | Tool is available to task sub-agents. |
| `coordinator` | bool | Tool is available to coordinator sessions. Without `interactive: true` alongside it, this reads as coord-only and the tool is stripped from interactive sessions. |
| `interactive` | bool | Opt a `coordinator: true` tool back into interactive sessions (dual-kind tools like `memory`). |
| `auto_approve` | bool | Tool runs without user confirmation (read-only, safe operations). |
| `primary_key` | str | When the model sends a bare string instead of JSON args, map it to this parameter name. |
| `kind_variants` | dict | Per-kind description / parameter-schema overlays so each session kind sees only the surface it can use (see `memory.json`). |
| `cwd_note` | str | Sentence appended to the description at session build time with `{working_dir}` substituted — declare on tools whose semantics depend on the process working directory (see `bash.json`, `apply_cwd_context`). |
| `workspace_note` | str | Companion sentence naming the operator-configured workspace directory, `{workspace_dir}` substituted; dropped when no workspace is configured. |
---
@@ -44,10 +50,10 @@ schema plus turnstone-specific metadata keys:
| Name | Description |
|---------------------|-------------|
| `TOOLS` | All 28 loaded built-in tool definitions (interactive + coordinator union). Sessions send a kind-specific subset (`INTERACTIVE_TOOLS` or `COORDINATOR_TOOLS`). |
| `TOOLS` | All 29 loaded built-in tool definitions (interactive + coordinator union). Sessions send a kind-specific subset (`INTERACTIVE_TOOLS` or `COORDINATOR_TOOLS`). |
| `TASK_AGENT_TOOLS` | Tools with `task_agent: true` -- available to task sub-agents. Includes write operations. |
| `TASK_AUTO_TOOLS` | Set of all tool names with `auto_approve: true` -- used by task-agent sub-sessions to skip confirmation for matching available tools. |
| `BUILTIN_TOOL_NAMES`| Frozenset of all 28 built-in tool names (interactive + coordinator union). Used by tool search to distinguish always-on tools from deferrable MCP tools. |
| `BUILTIN_TOOL_NAMES`| Frozenset of all 29 built-in tool names (interactive + coordinator union). Used by tool search to distinguish always-on tools from deferrable MCP tools. |
| `PRIMARY_KEY_MAP` | Dict mapping tool name to its `primary_key` parameter name. |
file-path and `attachment:` targets are local reads and run unprompted like
`read_file`
Note: The JSON schema metadata key `auto_approve` controls membership in
`TASK_AUTO_TOOLS` (used for task agent sub-sessions). The actual runtime
@@ -157,6 +166,7 @@ Every tool defines a `primary_key`. The mapping is:
| `search` | `query` |
| `web_fetch` | `url` |
| `web_search` | `query` |
| `open_preview` | `target` |
| `task_agent` | `prompt` |
| `memory` | `name` |
| `recall` | `query` |
@@ -285,7 +295,7 @@ Fetch a URL and extract specific information from it.
| `url` | string | yes | The URL to fetch (must start with `http://` or `https://`). |
| `question` | string | yes | What to extract or answer from the page content. |
- **What it does**: Fetches the URL, strips HTML to plain text, and uses the LLM to extract the answer to the question from the page content. Protected against SSRF (blocks private/internal IPs).
- **What it does**: Fetches the URL, strips HTML to plain text, and uses the LLM to extract the answer to the question from the page content. Every redirect hop is SSRF-screened before it is requested. Private/internal addresses are refused by default; enable `tools.allow_private_network` (console Settings → Tools) to make them approvable for self-hosted setups whose services live on the local network — the approval prompt marks such requests, and a public site redirecting into private space is refused regardless.
- **Auto-approve**: No -- requires user confirmation (makes network requests).
- **Agent availability**: `task_agent`.
@@ -345,6 +355,39 @@ It reports the score scale, whether the endpoint cleanly separates relevant from
---
### open_preview
Show the user rich content in a preview pane beside the conversation.
| Parameter | Type | Required | Description |
|-----------|--------|----------|-------------|
| `target` | string | yes | An http(s) URL, a file path, or `attachment:<id>` for a file attached to the conversation. |
| `kind` | string | no | Rendering override: `web`, `pdf`, `image`, `table`, `text`, or `markdown`. Detected from the content when omitted. |
| `title` | string | no | Pane header title. Defaults to the page title, filename, or URL. |
- **What it does**: Resolves the target to bytes (URLs fetch through the same
SSRF-guarded path as `web_fetch`, screened per redirect hop, honoring the
same `tools.allow_private_network` opt-in), classifies the
content, stores it content-addressed against the workstream, and opens the
frontend preview pane beside the conversation: web pages render in a fully
sandboxed iframe (no scripts, opaque origin), PDFs in the browser viewer,
images inline, CSV/TSV/JSON as a sortable table, text/markdown rendered. A
previewed web page loads none of its remote images or styles by default, so
opening it never reveals the viewer to the page's site; a toggle in the pane
header turns remote content back on for that preview. The
model receives only a one-line confirmation — to reason about content, use
`web_fetch` / `read_file` instead. Preview content is size-capped per kind
(pages 4 MB, PDFs 32 MB, images 4 MB, tables 2 MB, text 512 KB) and GC'd
| V2 RT -> audience A token | VERIFIED (`aud=<A app guid>`, ~70 min TTL, new RT returned) |
| V3 SAME RT -> audience B token | **VERIFIED — the load-bearing claim: one RT, many audiences** |
| V4 rotation | VERIFIED: RT rotates on every redemption, but the OLD RT stays valid (reuse HTTP 200) -> write-back-newest is required; races are benign on Entra |
| V5 unconsented audience | VERIFIED: `invalid_grant` + `AADSTS65001` (error_codes=[65001]) -> clean mapping to the reconnect-rail fallback |
| V6 OBO jwt-bearer variant | VERIFIED: middle-tier shape also works with the same app registration |
"description":"Cluster-wide visibility and control across all turnstone nodes."
},
"paths":{
@@ -6688,7 +6688,7 @@
"tags":[
"Coordinator"
],
"description":"Aggregates the persisted row, a best-effort live block from the owning node (or the in-process coordinator manager for ``kind=\"coordinator\"`` rows), and the tail of the message history. Gated on the ``admin.cluster.inspect`` permission (granted to ``builtin-admin`` via migration 040; revoke or reassign to a custom role for tighter control). ``live`` is null on node unreachability / 5xx so callers can degrade gracefully.",
"description":"Aggregates the persisted row, a best-effort live block from the owning node (or the in-process coordinator manager for ``kind=\"coordinator\"`` rows), and the tail of the message history. Gated on the ``admin.cluster.inspect`` permission (granted to ``builtin-admin`` via migration 040; revoke or reassign to a custom role for tighter control). A workstream attached to a *private* project stays confidential to its members: a permitted caller who isn't its owner / creator / project member gets a 404 (same masking as an unknown id). ``live`` is null on node unreachability / 5xx so callers can degrade gracefully.",
"parameters":[
{
"name":"ws_id",
@@ -7791,6 +7791,12 @@
"title":"Persona",
"type":"string"
},
"project_id":{
"default":"",
"description":"Project to attach the workstream to (validated against membership, empty = none)",
"title":"Project Id",
"type":"string"
},
"resume_ws":{
"default":"",
"description":"Workstream ID to resume (loads previous conversation)",
"description":"Project to attach the workstream to",
"title":"Project Id",
"type":"string"
},
"notify_targets":{
"description":"Notification targets on completion (channel_type + channel_id/user_id)",
"items":{
@@ -8659,6 +8677,30 @@
"default":null,
"title":"Skill"
},
"persona":{
"anyOf":[
{
"type":"string"
},
{
"type":"null"
}
],
"default":null,
"title":"Persona"
},
"project_id":{
"anyOf":[
{
"type":"string"
},
{
"type":"null"
}
],
"default":null,
"title":"Project Id"
},
"notify_targets":{
"anyOf":[
{
@@ -8754,6 +8796,16 @@
"title":"Skill",
"type":"string"
},
"persona":{
"default":"",
"title":"Persona",
"type":"string"
},
"project_id":{
"default":"",
"title":"Project Id",
"type":"string"
},
"notify_targets":{
"items":{
"additionalProperties":{
@@ -11201,6 +11253,7 @@
}
],
"default":null,
"description":"Inline BASE override \u2014 required. Every persona must name a prompt source; built-in file-backed personas are seeded by migration, not created here, so an operator-created persona must supply base_prompt.",
"title":"Base Prompt"
},
"tool_allowlist":{
@@ -11259,7 +11312,7 @@
"type":"object"
},
"UpdatePersonaRequest":{
"description":"PATCH body \u2014 absent fields are left unchanged.\n\nExplicit ``null`` is meaningful only on the two resettable fields:\n``base_prompt: null`` clears the override back to the kind's stock\nBASE, and ``tool_allowlist: null`` resets to unrestricted. ``null``\non the boolean flags or``applies_to_kinds`` is ignored (treated as\nabsent), so a client serializingunset optionals as null cannot\narchive a persona or flip levers by accident.\n\nArchive = ``{\"enabled\": false}``; default flip = ``{\"is_default\": true}``\non the successor (storage demotes the incumbent atomically). ``name``\nis immutable; existing workstreams are never affected by edits.",
"description":"PATCH body \u2014 absent fields are left unchanged.\n\nExplicit ``null`` resets ``tool_allowlist`` to unrestricted, and \u2014 on a\nBUILT-IN persona only \u2014 clears ``base_prompt`` (the operator override),\nreverting to that persona's file-backed prompt. An OPERATOR persona has no\nfallback source, so ``base_prompt: null`` on one is rejected: every persona\nmust name a prompt source. ``null``on the boolean flags or\n``applies_to_kinds`` is ignored (treated asabsent), so a client serializing\nunset optionals as null cannotarchive a persona or flip levers by accident.\n\nArchive = ``{\"enabled\": false}``; default flip = ``{\"is_default\": true}``\non the successor (storage demotes the incumbent atomically). ``name``\nis immutable; existing workstreams are never affected by edits.",
"properties":{
"display_name":{
"anyOf":[
@@ -13305,21 +13358,17 @@
},
"pending_approval":{
"default":false,
"description":"True when the workstream is parked on ``_approval_event`` awaiting an operator approve/deny. Mirrors the same field on ``DashboardWorkstream`` / cluster live projections so a freshly-loaded chat tab can render the inline approval gate from the detail snapshot before SSE replay arrives.",
"description":"True when at least one approval cycle is live (a gate thread parked awaiting an operator approve/deny). Mirrors the same field on ``DashboardWorkstream`` / cluster live projections so a freshly-loaded chat tab can render the inline approval gate from the detail snapshot before SSE replay arrives.",
"description":"Inline approval payload \u2014 same shape as ``DashboardWorkstream.pending_approval_detail``. ``None`` when no approval is pending. Lets a reload paint the action row + judge verdicts immediately instead of relying on the SSE approve_request replay timing window."
"pending_approval_details":{
"description":"Inline approval payloads, one per live cycle, oldest first \u2014 same shape as ``DashboardWorkstream.pending_approval_details``. Empty when no approval is pending. Lets a reload paint every action row + judge verdicts immediately instead of relying on the SSE approve_request replay timing window. Replaces 1.6's ``pending_approval_detail`` single-object field (breaking, 1.7).",
"description":"Inline approval payload merged into ``DashboardWorkstream``.\n\nSet when a workstream's ``approve_tools`` is parked on\n``_approval_event``; ``None`` (omitted) otherwise. Cross-tenant\nexposure here follows the same trusted-team posture as\n``activity`` / ``tokens`` \u2014 see ``server.py``'s ``dashboard``\nhandler comment.",
"description":"Inline approval payload merged into ``DashboardWorkstream``.\n\nOne entry per live approval CYCLE \u2014 a gate thread parked in\n``approve_tools`` awaiting the operator. Parallel task agents run\nconcurrent gates, so a workstream can have several of these at\nonce (``pending_approval_details``, oldest first). Cross-tenant\nexposure here follows the same trusted-team posture as\n``activity`` / ``tokens`` \u2014 see ``server.py``'s ``dashboard``\nhandler comment.",
"properties":{
"cycle_id":{
"default":"",
"description":"Identity of this approval cycle. Echo it back on ``POST /v1/api/workstreams/{ws_id}/approve`` to resolve exactly this round \u2014 required for correctness when several cycles are live (parallel task agents).",
"title":"Cycle Id",
"type":"string"
},
"call_id":{
"default":"",
"description":"Primary call_id \u2014 first non-empty call_id in items list order. Matches the 409 ``current_call_id`` response from ``POST /v1/api/workstreams/{ws_id}/approve`` so the UI can render the same identifier the server reports as current.",
@@ -13358,7 +13413,7 @@
"type":"object"
},
"PendingApprovalItem":{
"description":"One pending tool-call inside a ``PendingApprovalDetail`` envelope.\n\nMirrors the dict ``SessionUIBase.serialize_pending_approval_detail``\nemits per item. ``heuristic_verdict`` / ``judge_verdict`` are kept\nloosely-typed because the underlying verdict shape varies by tier;\nconsumers that want the full structure can decode against\n:class:`turnstone.sdk.events.IntentVerdictEvent`.",
"description":"One pending tool-call inside a ``PendingApprovalDetail`` envelope.\n\nMirrors the dict ``SessionUIBase.serialize_pending_approval_details``\nemits per item inside each cycle entry. ``heuristic_verdict`` / ``judge_verdict`` are kept\nloosely-typed because the underlying verdict shape varies by tier;\nconsumers that want the full structure can decode against\n:class:`turnstone.sdk.events.IntentVerdictEvent`.",
"description":"Single-node workstream management, chat interaction, and real-time streaming."
},
"paths":{
@@ -228,6 +228,16 @@
}
}
}
},
"409":{
"description":"Error 409",
"content":{
"application/json":{
"schema":{
"$ref":"#/components/schemas/ErrorResponse"
}
}
}
}
}
},
@@ -390,6 +400,26 @@
}
}
}
},
"409":{
"description":"Error 409",
"content":{
"application/json":{
"schema":{
"$ref":"#/components/schemas/ErrorResponse"
}
}
}
},
"503":{
"description":"Error 503",
"content":{
"application/json":{
"schema":{
"$ref":"#/components/schemas/ErrorResponse"
}
}
}
}
}
}
@@ -2260,16 +2290,23 @@
"SendResponse":{
"properties":{
"status":{
"description":"'ok', 'busy', 'queued', or 'queue_full'",
"description":"'ok' (fresh turn dispatched), 'queued' (folded into the live turn's interjection queue, or \u2014 when `deferred` is true \u2014 parked for dispatch after the current command window), 'queue_full', 'attachments_busy' (attachments can't ride a queued turn; retry when idle), or 'cross_user_interjection' (another participant's turn is in flight; carried on the 409 body).",
"examples":[
"ok",
"busy",
"queued",
"queue_full"
"queue_full",
"attachments_busy",
"cross_user_interjection"
],
"title":"Status",
"type":"string"
},
"deferred":{
"default":false,
"description":"Set on `queued` responses: the message is parked on the workstream's deferred-send list (a slash-command window holds the worker slot, or earlier deferred sends are still pending) and dispatches as an ordinary full-fidelity send afterwards \u2014 it is NOT in a live turn's interjection queue. `DELETE .../send` retracts it until dispatch. Node-local and in-memory: a node restart before dispatch drops it (at-most-once intake).",
"title":"Deferred",
"type":"boolean"
},
"attached_ids":{
"description":"Attachment ids actually attached to this turn. Subset of the request's `attachment_ids` (or the auto-consumed pending set). Empty when the send carries no attachments.",
"items":{
@@ -2564,6 +2601,23 @@
},
"title":"Attachment Ids",
"type":"array"
},
"initial_message_status":{
"anyOf":[
{
"enum":[
"queue_full",
"refused_closed"
],
"type":"string"
},
{
"type":"null"
}
],
"default":null,
"description":"Present ONLY when the workstream was created but its initial_message could not be delivered: 'queue_full' (a raced live worker's interjection queue was at capacity \u2014 resend via /send; any uploads stay staged) or 'refused_closed' (the workstream was closed mid-create). Absent whenever the message was dispatched.",
"title":"Initial Message Status"
}
},
"required":[
@@ -2692,21 +2746,17 @@
},
"pending_approval":{
"default":false,
"description":"True when the workstream is parked on ``_approval_event`` awaiting an operator approve/deny. Mirrors the same field on ``DashboardWorkstream`` / cluster live projections so a freshly-loaded chat tab can render the inline approval gate from the detail snapshot before SSE replay arrives.",
"description":"True when at least one approval cycle is live (a gate thread parked awaiting an operator approve/deny). Mirrors the same field on ``DashboardWorkstream`` / cluster live projections so a freshly-loaded chat tab can render the inline approval gate from the detail snapshot before SSE replay arrives.",
"description":"Inline approval payload \u2014 same shape as ``DashboardWorkstream.pending_approval_detail``. ``None`` when no approval is pending. Lets a reload paint the action row + judge verdicts immediately instead of relying on the SSE approve_request replay timing window."
"pending_approval_details":{
"description":"Inline approval payloads, one per live cycle, oldest first \u2014 same shape as ``DashboardWorkstream.pending_approval_details``. Empty when no approval is pending. Lets a reload paint every action row + judge verdicts immediately instead of relying on the SSE approve_request replay timing window. Replaces 1.6's ``pending_approval_detail`` single-object field (breaking, 1.7).",
"description":"Inline approval payload merged into ``DashboardWorkstream``.\n\nSet when a workstream's ``approve_tools`` is parked on\n``_approval_event``; ``None`` (omitted) otherwise. Cross-tenant\nexposure here follows the same trusted-team posture as\n``activity`` / ``tokens`` \u2014 see ``server.py``'s ``dashboard``\nhandler comment.",
"description":"Inline approval payload merged into ``DashboardWorkstream``.\n\nOne entry per live approval CYCLE \u2014 a gate thread parked in\n``approve_tools`` awaiting the operator. Parallel task agents run\nconcurrent gates, so a workstream can have several of these at\nonce (``pending_approval_details``, oldest first). Cross-tenant\nexposure here follows the same trusted-team posture as\n``activity`` / ``tokens`` \u2014 see ``server.py``'s ``dashboard``\nhandler comment.",
"properties":{
"cycle_id":{
"default":"",
"description":"Identity of this approval cycle. Echo it back on ``POST /v1/api/workstreams/{ws_id}/approve`` to resolve exactly this round \u2014 required for correctness when several cycles are live (parallel task agents).",
"title":"Cycle Id",
"type":"string"
},
"call_id":{
"default":"",
"description":"Primary call_id \u2014 first non-empty call_id in items list order. Matches the 409 ``current_call_id`` response from ``POST /v1/api/workstreams/{ws_id}/approve`` so the UI can render the same identifier the server reports as current.",
@@ -2745,7 +2801,7 @@
"type":"object"
},
"PendingApprovalItem":{
"description":"One pending tool-call inside a ``PendingApprovalDetail`` envelope.\n\nMirrors the dict ``SessionUIBase.serialize_pending_approval_detail``\nemits per item. ``heuristic_verdict`` / ``judge_verdict`` are kept\nloosely-typed because the underlying verdict shape varies by tier;\nconsumers that want the full structure can decode against\n:class:`turnstone.sdk.events.IntentVerdictEvent`.",
"description":"One pending tool-call inside a ``PendingApprovalDetail`` envelope.\n\nMirrors the dict ``SessionUIBase.serialize_pending_approval_details``\nemits per item inside each cycle entry. ``heuristic_verdict`` / ``judge_verdict`` are kept\nloosely-typed because the underlying verdict shape varies by tier;\nconsumers that want the full structure can decode against\n:class:`turnstone.sdk.events.IntentVerdictEvent`.",
"description":"Inline approval payload for the coordinator children-tree UI. Carries the merged ``_pending_approval`` items list + per-call_id LLM verdict cache so a coord can render approve/deny buttons + judge pill without a separate per-child round-trip. ``None`` when no approval is pending. Also surfaced (verbatim) on ``GET /v1/api/cluster/ws/live`` via the ``_CLUSTER_WS_LIVE_KEYS`` projection."
"pending_approval_details":{
"description":"Inline approval payload for the coordinator children-tree UI: EVERY live approval cycle, oldest first \u2014 parallel task agents gate concurrently, so a workstream can hold several prompts at once. Each entry carries the cycle's items + per-call_id LLM verdict cache so a coord can render approve/deny buttons + judge pill without a separate per-child round-trip; resolve each with its ``cycle_id``. Empty when no approval is pending. Also surfaced (verbatim) on ``GET /v1/api/cluster/ws/live`` via the ``_CLUSTER_WS_LIVE_KEYS`` projection. Replaces 1.6's ``pending_approval_detail`` single-object field (breaking, 1.7).",
"description":"Per-ws ring buffer (cap 10) of recent tool calls that bypassed the operator approval gate. Surfaces ``WebUI._recent_auto_approvals`` so the coord-tree row can render an 'auto-approved by ...' pill when the child's skill / blanket / admin-policy rules silently let a tool through. Also projected onto ``GET /v1/api/cluster/ws/live`` via ``_CLUSTER_WS_LIVE_KEYS``.",
"content":"Tool execution was cancelled. Outcome UNKNOWN — this call may have begun executing before the generation was stopped; do not assume it did not run, and reconcile before re-issuing it.",
"content":"Tool execution was cancelled. Outcome UNKNOWN — this call may have begun executing before the generation was stopped; do not assume it did not run, and reconcile before re-issuing it.",
"content":"Tool execution was cancelled. Outcome UNKNOWN — this call may have begun executing before the generation was stopped; do not assume it did not run, and reconcile before re-issuing it.",
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Patrick Buckley