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
Patrick Buckley