mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
main
22 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Patrick Buckley
|
7a06f5e8bc |
refactor(session): make ModelLane the provider boundary (#979) (#989)
* refactor(session): make ModelLane the provider boundary (#979) ## Summary This closes the model-lane ownership gap left by #832: `ChatSession` no longer stores raw provider/client handles. `ResolvedModelBinding` now carries the provider, client, model, capabilities, registry generation, and backend-auth configuration as one coherent snapshot. - Atomically rebind existing sessions after model-registry changes while pinning each in-flight send, fallback, judge, output guard, task agent, title, compaction, perception, and voice operation to its initiating principal and binding. - Fence UI publication, canonical trajectory folds, durable writes, streams, retries, child scopes, and judge work by generation. Stop can hand off to a successor without accepting late state; cancelled tools retain typed effect receipts, and concurrent approval batches resolve by exact cycle or call. - Make create, fork, open, close, and delete race-safe with hidden `creating` reservations, incarnation-aware state tails, and an ACL-rechecked transaction that clones checkpoint-bounded history, configuration, project/persona state, and attachment references. - Extend REST/OpenAPI and Python/TypeScript SDK contracts for create/fork inputs, routed-create metadata, live-workstream probes, targeted approvals, and structured cancellation results. - Update architecture, storage, authentication, judge, channel, console, API, and SDK documentation, including regenerated architecture diagrams and OpenAPI artifacts. ## Validation - SQLite suite: 11,188 passed, 9 skipped, 10 deselected - PostgreSQL suite: 11,195 passed, 2 skipped, 10 deselected - Live backend: 3 passed - SSE recovery: 6 passed; browser recovery harness passed all scenarios - Ruff: clean; 595 files correctly formatted - mypy: 243 source files clean - TypeScript: typecheck/build and 35 tests passed - OpenAPI artifacts fresh; all 14 changed diagrams reproduce byte-for-byte - `git diff --check` and Git LFS integrity clean Closes #979. * fix(deps): update nanoid for GHSA-2v37-7h3g-55p8 Refresh the transitive lock entry admitted by PostCSS so the TypeScript security gate no longer resolves the vulnerable custom-generator implementation. Validation: - npm ci - npm audit --audit-level=moderate: 0 vulnerabilities - TypeScript typecheck and build - TypeScript tests: 35 passed * fix(test): assert canonical model registry URLs Replace prefix checks with exact canonical base URL assertions so the tests do not model incomplete URL validation. Validation: tests/test_model_registry.py (185 passed); Ruff check/format; mypy. |
||
|
Patrick Buckley
|
026c646116 |
test(sse): normalize session_ui_base imports to a single style
github-code-quality flagged 8 spots where tests imported
turnstone.core.session_ui_base both as `from ... import` and `import ... as
suib` (the alias was only there to monkeypatch the module-level batch
constants). Drop the alias and patch via string target
(`monkeypatch.setattr("turnstone.core.session_ui_base._TOKEN_BATCH_WINDOW_SECS", ...)`),
which resolves to the same module global — behavior-identical. The one test
that READS the constant imports the symbol directly. Test-only, no
production change.
|
||
|
Patrick Buckley
|
5083f67e96 |
fix(sse): batch fast-stream tokens and recover overflowed listeners
A long live session driven by a fast local model (500-2000 tok/s) showed
corrupted / missing spans of assistant text while the backend stayed
healthy. Root cause: on_content_token/on_reasoning_token enqueued one SSE
event per model delta, so the per-listener queue (cap 500) overflowed
against any slow consumer; put_nowait on a full queue silently dropped the
newest event. Once saturated, drops scatter (the consumer keeps freeing
single slots), so the client's lastEventId sails past the holes and
reconnect-replay (eid > last_event_id) can never heal them. A dropped
fence-closer reshapes all downstream markdown -> reads as heavy corruption.
Fix B (primary) - emit-time micro-batching:
Coalesce content/reasoning fragments over a ~25 ms window (or 4 KB) into
one _enqueue, cutting the wire event rate ~10-20x at local-inference
speeds. A batch is assembled before it gets an _event_id, so it is one
ordinary ring entry no cursor can fall inside (unlike the forbidden
in-ring coalesce). Two conditions are load-bearing and pinned:
1. The inflight-buffer append and the enqueue are one _ws_lock section,
so a snapshot's snap_seq stays a true high-water mark for its text.
Splitting them lets a straddling snapshot double-render (the client
content path is a blind +=, no dedup).
2. Every non-token emit flushes the pending batch first, enforced at the
single _enqueue choke point, so stream_end/tool_*/state_change can't
overtake trailing content and repaint it into a new bubble.
Fix A (recovery net) - poison-at-first-overflow:
_ListenerQueue latches `poisoned` atomically at the FIRST rejected put and
refuses every later put, freezing its contents as a contiguous prefix; the
drain loop closes the stream after an id-less stream_overflow frame and the
native EventSource reconnect replays the whole gap from the ring buffer.
Poisoning at the first full (not after N) is required: any deliver-while-
dropping window advances lastEventId past interior holes that reconnect
can't replay. A ws teardown that races the overflow sets an out-of-band
`closing` flag (mark_closing), checked inside the drain loop's poison
branch: a poisoned+closing queue returns clean (no false overflow frame),
while a healthy closing queue still drains its full tail FIFO to the in-band
ws_closed sentinel -- so a slow-but-unpoisoned client never loses the turn's
final content batch + stream_end at teardown.
Client (interactive.js):
- Reconnect storm guard: after 3 overflow closes in 60 s the pane drops to
a degraded catch-up (stop live streaming, "connection is slow" state,
reconnect after a doubling 15->120 s cooldown that resyncs from the ring
or the uncapped /history floor). The cooldown ladder is keyed off a
last-trip timestamp, not the overflow-window array (which the trip
clears), so the escalation survives its own backoff.
- Close-on-hide / replay-on-show: a visibilitychange handler closes the
EventSource on tab-hide (a throttled hidden tab is the likeliest slow
consumer) and reconnects with the saved Last-Event-ID on show. The
factory recovery beat defers when hidden, and giveUp() detaches the
handler, so a dead or backgrounded controller can't reopen a stream.
- Drop-vs-render-wedge counters distinguish this bug (server overflow
closes) from the handler-wedge class (render/finalize throws) in the
field. No global gap-detector: live ids are not strictly monotonic
across concurrent tool+content emit, so a naive id!=last+1 check would
false-positive; recovery is server-signalled instead.
Corrects the stale _resolve_event_buffer_max comment that justified the
50k ring on a "PR-G closes connections on hide" mitigation that never
existed (the close-on-hide handler above is the real one).
Negative-tested (revert the guarantee, confirm the pin fails, restore):
per-token inflight append -> snapshot straddle double-render; removed
choke-point flush -> stream_end split; no poison latch -> silent drops;
top-of-loop closing check -> healthy-close tail loss; missing mark_closing
wiring / drain closing check -> clean close mis-reported as overflow;
_noteStreamOverflow cooldown reset -> ladder never escalates; removed
hidden-tab recovery guard / giveUp handler removal -> hidden-tab reconnect.
|
||
|
Patrick Buckley
|
7f0e0406b3 |
test(approvals): concurrency matrix + suite migration to the cycle model
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. |
||
|
Patrick Buckley
|
c0ff00a1ff |
fix(test): eliminate lost-wakeup race in approval-prompt tests
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. |
||
|
Patrick Buckley
|
77cb76c006 |
feat(task-agent): recall sub-trajectory + per-agent read isolation
Final chunk of the task_agent modernization: rebuild a finished task
agent's card from /history (reload / reopen while the workstream is in
memory) and isolate each sub-agent's file-read tracking.
Recall: _project_agent_steps projects a sub-agent's trajectory into step
items (FIFO-per-call_id pairing via _iter_agent_tool_results, shared with
_cancel_ledger; output/arguments/count capped); _stash_agent_trajectory
keeps them on the UI in an LRU-bounded store; make_history_handler
attaches them as agent_steps to each task_agent tool_call, and
replayHistory/_replayAgentCard rebuild the collapsed card. In-memory only
(durable persistence deferred); a cold/evicted entry renders the flat
parent row ("not retained"), never a fabricated 0-step card.
Read isolation: _read_files (the blind-overwrite guard's memory) is now
per-sub-agent via the _active_read_files contextvar -- _exec_task copies
the parent's set on spawn and merges the agent's reads back on
completion, so a sibling in the 4-wide pool can't suppress another
agent's guard.
Also: _exec_task now self-reports the task_agent tool_result on every
path (the parent loop only reports error/denied results centrally) --
without it the live card never completed and a failed task recorded
is_error=False in the canonical trajectory. is_error flows from
_tool_error_flags to the recalled step; on_info suppression is per-thread
so a parallel sibling tool's progress isn't dropped.
|
||
|
Patrick Buckley
|
ca7958329a |
feat(task-agent): nest sub-tool steps in an expandable card
Route a task agent's sub-tool events (tool_pending / approve_request, tagged with parent_call_id) into a collapsible card under the task_agent row, replacing the blue on_info turn-legs. - conversation.js / interactive.js: buildAgentCardBody + _routeAgentItems / _ensureAgentCard nest steps by parent_call_id. Collapsed by default (a task agent can run 100+ steps and the parent fans out many in parallel); the label carries the live count + state. Auto-expand when a nested approval is pending so the blocking prompt can't hide behind the toggle. - session.py / session_ui_base.py: on_agent_step paints auto-tool step rows; namespace child call_ids by parent so the 4-wide task pool can't collide on local sequential ids (call_0); suppress sub-agent on_info on the web pane (no call_id to nest by — the card carries steps + result). - cli.py: on_agent_step prints a dim step leg (no card on the CLI, which keeps its on_info). - livepass.py: task-agent card harness driving the real InteractivePane. |
||
|
Patrick Buckley
|
65eaacb341 |
feat(task-agent): Turn-IR sub-harness + parent-tagged step events
Rebuild the task_agent sub-harness on the canonical Turn trajectory (build list[Turn], lower via dicts_from_turns at the wire boundary) instead of hand-rolled OpenAI dicts; the cancel-ledger helpers read Turns. Tag each sub-tool's events with parent_call_id via a lock-guarded child registry stamped centrally in SessionUIBase._enqueue, so a later UI can nest a task agent's steps under its card. Getattr-guarded on the session side so CLI/eval/test UIs are unaffected. Behaviour-preserving (same wire shape, same cancellation semantics); the parent tag is wire-invisible and unconsumed until the frontend card lands. |
||
|
Patrick Buckley
|
effdb8f365 |
fix(judge): persist superseded late verdicts for the audit trail
ChatSession._on_verdict guards on judge-generation identity so a stale verdict can't ride a reused call_id into the Smart-Approvals cache — but it dropped those verdicts entirely, before persistence. Every ruling the sequential judge delivered after the next turn began left intent_verdicts claiming the judge never answered. Route superseded verdicts to a new persist-only hook (SessionUIBase.on_superseded_intent_verdict): the row lands with user_decision="superseded" while every live surface stays untouched (no SSE, no replay cache, no pending-decision park). The hook is duck-typed; display-only UIs (CLI/eval) don't define it and keep the plain drop. upsert_intent_verdict already excludes user_decision from its on-conflict SET, so a superseded fallback upgrading its heuristic row in place cannot clobber a decision already stamped there. |
||
|
Patrick Buckley
|
110d44b07e |
refactor(tools): remove man, math, and plan_agent built-in tools
`man` and `math` duplicated capabilities already reachable through `bash`; `plan_agent` is better expressed as a `task_agent` running a planning skill, and carried a large amount of special-case machinery (plan-review gate, refinement loop, per-kind model routing). Removing all three shrinks the tool surface and cuts per-call token cost. Also removed, as dead-once-the-tools-are-gone: - the `math` sandbox executor (`turnstone.core.sandbox`) and its `[sandbox]` extra; the eval analyst now runs bash-only - the read-only `AGENT_TOOLS` sub-agent tool set and the `agent` tool-metadata key (`task_agent`/`TASK_AGENT_TOOLS` retained) - the plan-review protocol end to end: the `on_plan_review` UI hook, `resolve_plan`, `POST /v1/api/plan` + `POST /v1/api/route/plan`, the `plan_review`/`plan_resolved` SSE events, and their Python SDK / TypeScript SDK / OpenAPI / frontend / Discord+Slack bindings - the `model.plan_alias` / `model.plan_effort` settings and the registry `plan_model` / `plan_effort` routing fields TOOLS 31->28, TASK_AGENT_TOOLS 13->11; COORDINATOR_TOOLS unchanged. BREAKING CHANGE: removes the `man`, `math`, `plan_agent` tools, the plan-review SSE/HTTP/SDK surface, and the plan_* model-routing settings from the experimental 1.6 line. |
||
|
Patrick Buckley
|
44f19f1401 |
feat(judge,console,ui): paint pending tool calls before the intent verdict
The intent-validation judge runs before the approval gate resolves, and Smart Approvals (judge.smart_approvals) parks approve_tools on the async LLM verdict for up to judge.timeout — so the tool-call card never reached the UI until the judge had ruled. An operator could not see a committed call, let alone Stop it, during that window. approve_tools now emits a tool_pending event carrying the serialized batch at the top of the gate, before the tool-policy lookup, the verdict wait, and the human prompt. It is a UI paint only — no persistence, audit, or verdict bookkeeping — so it cannot perturb the gate's accounting. The authoritative tool_info / approve_request / tool_result events that follow upgrade the same construct in place, keyed by call_id, and the Last-Event-ID replay slice reconstructs it on reconnect. A ToolPendingEvent joins the SDK registry. Coordinator: appendToolBatch was already idempotent on call_ids; the new handler reuses the --running placeholder it already upgrades, with an "Evaluating" kicker that swaps to "Running" on the auto-approve upgrade. Interactive: showInlineToolBlock was create-only, so a second card would duplicate. Added announceToolBlock + _takeAnnouncedBlock to reuse the announced shell (matched on its call_id set) instead. The announced rail is dashed amber and must out-specify the .msg.ts-approval--inline cyan-hold (specificity 0,2,0) — at 0,1,0 it rendered cyan, indistinguishable from a normal card — so the announced card is the one visually distinct surface in the stream. Screen-reader parity: the early paint announces politely through dedicated off-screen aria-live regions on both surfaces (the messages log is aria-live=off mid-stream, so the appended shell alone is inaudible), and the announced shell carries aria-busy until the upgrade clears it. Polite, not assertive — the human gate keeps its assertive announcement. Tests cover the gate ordering (tool_pending precedes tool_info and the Smart Approvals gate) plus string-guards on both UIs' wiring, the announced-rail specificity, and the screen-reader regions. |
||
|
Patrick Buckley
|
948e413f66 |
feat(judge): add Smart Approvals (auto-approve trusted judge verdicts)
Opt-in judge.smart_approvals (default off): when the intent-validation LLM judge returns a high-confidence "approve" verdict, the tool batch is approved automatically with no operator prompt. review/deny recommendations, low confidence, judge errors (llm_fallback), and a deterministic heuristic deny/critical finding all still require a human. Requires judge.enabled. - Batch-atomic: a parallel tool batch auto-approves only if every call qualifies; one non-qualifying call holds the whole batch for a human. - Gate: tier==llm + recommendation==approve + confidence >= judge.confidence_threshold (default raised 0.7 -> 0.95), with a floor that never clears an explicit heuristic deny/critical verdict. - approve_tools waits for the async LLM verdicts, finalises the audit trail (AutoApproveReason.smart_approval), and re-emits verdicts after the card so the live chip updates; the auto-approved row renders the LLM verdict rather than the cautious heuristic carry-over. - judge: always deliver exactly one verdict per call (fallback on error); reject non-finite confidence so NaN can't clear the bar. - Drop verdicts from a superseded judge generation so a reused call_id from a prior turn's still-running daemon can't satisfy the gate's wait. Config plumbed through the server/console/CLI builders and the live _judge_cfg; admin Judge tab renders the toggle. Docs + example config updated. ~35 tests covering the gate matrix, batch-atomicity, the heuristic floor, audit stamping, the streaming re-emit, NaN/duplicate-id defenses, and the cross-turn generation guard. |
||
|
Patrick Buckley
|
3233719856 |
feat(judge): output_guard LLM stage with capability gate (#560 mitigation #1)
Adds a second, LLM-driven stage to the output guard so domain-camouflaged prompt-injection payloads that the regex stage misses (arXiv:2605.22001 — Llama 3.1 8B evades the existing regex set on ~90% of camouflaged prompts) get caught before the tool output lands in the assistant's context. ## Surface * New `OutputGuardJudge` in `turnstone/core/output_guard_judge.py` — synchronous, single-shot LLM call. Inlines the alias-resolution + client-config + JSON-parsing helpers (copied verbatim from `IntentJudge` at `judge.py:917-969` / `1604-1659`) rather than going through a shared module — when `IntentJudge` lifts its own helpers, both copies move together. * JSON-in-content verdict with a 3-strategy parser (direct / markdown fence / balanced braces). `IntentJudge` ships a 4th regex-field fallback; OutputGuardJudge deliberately doesn't, because strategy-4 hits on broken LLM output can extract a "verdict" from the model's reasoning quote that lands in storage looking identical to a clean strategy-1 result. Failure of all three returns `error="unparseable_verdict"` and the heuristic stage stands. * `OutputJudgeVerdict` is a frozen dataclass with: `risk_level` (none/low/medium/high — normalises `critical`→`high` and `info[rmational]`→`low` for IntentJudge-echo safety), `flags: tuple[str, ...]`, `reasoning`, `confidence: float` (0.0-1.0, parsed + clamped from the LLM's self-report; pass-through to audit, no threshold gating), `judge_model`, `latency_ms`, `error`. * Real wall-clock timeout via `ThreadPoolExecutor.shutdown(wait=False, cancel_futures=True)` on the timeout/cancel path — `with ... as ex:` would block return until the worker drained. 1s `cancel_event` poll mirrors `IntentJudge._run_judge` at `judge.py:1117-1118`. * HTTP client lazy-init + reuse for the judge instance's lifetime. Session-side model swap drops the entire judge, dropping the client with it. * Untrusted tool output wrapped in per-call random-nonced `<tool_output_NONCE>...</tool_output_NONCE>` fence. Closing-tag substrings in the raw text are case-insensitively backslash-escaped first (`</tool_output` → `<\/tool_output`) so an attacker can't break out even if they guess the nonce. System prompt classifies the fenced region as UNTRUSTED DATA so directives inside are evaluated as content, not obeyed. * Judge user prompt carries the heuristic verdict (risk + flags + annotations), the tool description (looked up from the session's tools registry), and the tool args (truncated to 500 chars, also classified UNTRUSTED in the system prompt since they may be caller-supplied). Lets the judge defer to the regex on credential leaks and focus on injection signals the regex set misses; also enables output-vs-request plausibility reasoning. ## Session integration * `_evaluate_output(call_id, output, func_name, *, tool_args="")` — heuristic always runs; LLM stage runs when `judge.output_guard_llm` is enabled. When the LLM produces a usable verdict and the heuristic didn't detect credentials, the LLM verdict is acted on; otherwise the heuristic stands. * Credential redaction is a regex-only signal. When `heuristic. sanitized` is non-None, the heuristic owns the acted assessment regardless of what the LLM said — an LLM asked about prompt- injection can correctly label a credential-bearing output as "none" risk for injection, but the secret still needs redaction. * `_batch_evaluate_outputs` runs the per-tool guard concurrently (4-worker pool) when LLM is enabled and there are ≥2 string outputs — collapses N×LLM-latency to ⌈N/4⌉×latency on the common 5-20 tool-calls-per-turn turn. * Per-session `TokenBucket(rate=1.0, burst=60)` caps adversarial LLM-fan-out cost at 60 calls/min/session. * Pre-truncation: the per-tool loop truncates output before the judge sees it, so the judge evaluates exactly what enters the assistant's context (no wasted tokens on text that won't land). * Both heuristic and LLM tier rows persisted to `output_assessments` when the LLM ran (audit completeness); heuristic-only rows skip when matched-clean to keep the table focused. ## Storage Migration 057 extends `output_assessments` with five LLM-tier columns: `tier` (`heuristic` / `llm`, backfilled to `heuristic`), `reasoning`, `judge_model`, `latency_ms`, `confidence`. Tie-break on `(created DESC, tier='llm' first)` so downstream consumers see the acted verdict first when the two rows tie at second resolution. `StorageBackend.record_output_assessment` + sqlite/pg implementations + `SessionUIBase.record_output_assessment` + `SessionUI` protocol + the test stub overrides (cli, eval, 9 test files) all take the new LLM-tier kwargs. ## Config surface Three new judge.* settings in `settings_registry`: * `judge.output_guard_llm` (bool, default False) — capability gate. Default off; operators opt in once a small/fast model is pointed at `output_guard_model`. * `judge.output_guard_model` (str, default "") — alias for the LLM stage. Empty inherits the session model (same fallback shape as `judge.model`). * `judge.output_guard_llm_timeout` (float, default 30.0, min 1.0) — wall-clock budget per call. Both `server.py` and `console/session_factory.py` wire these into the `JudgeConfig` they hand to `ChatSession`. ## Notes * No backwards-compatibility shims — the LLM stage is purely additive. * No reasoning/threshold gating on confidence; it rides as an audit-only signal per maintainer direction. Surface it in the `on_output_warning` dict so live UI / cluster broadcast can sort flagged outputs by judge certainty. * Tests: 392 lines of judge-only coverage (`test_output_guard_judge. py`) + 629 lines of session-integration coverage in `test_session. py`, plus the storage and stub-shape updates. |
||
|
Patrick Buckley
|
08f6f146bc |
feat(sse): per-ws ring buffer + Last-Event-ID replay foundation
Adds the server-side foundation for SSE reconnect-with-replay (PR-D in issue #540's sequencing): a per-ws monotonic ring buffer that holds the last N events for replay against a client's `Last-Event-ID` header (or `?last_event_id=N` query-param fallback for manual reconnect paths that can't set custom headers). Per-ws lane (SessionUIBase + make_events_handler): - `_event_buffer` deque (cap 2000, env-overridable via `TURNSTONE_SSE_EVENT_BUFFER_MAX`) holds (event_id, event_dict) tuples; `maxlen` evicts the oldest automatically. - Existing `_ws_inflight_seq` renamed to `_event_id` and lifted to live alongside the listeners — one monotonic counter drives both the new replay slice AND the existing `_seq`/`snap_seq` snapshot dedup (byte-identical contract on token events). - `_enqueue` now stamps every event with `_event_id` (and `_seq` on `content`/`reasoning` token events) under `_listeners_lock`, so the buffer append + listener fan-out + new listener registration are all atomic against each other. - New `register_listener_with_replay` returns (queue, replay_events, status, lost_count, earliest_id) where status ∈ {replay_ok, truncated}. `make_events_handler` reads `Last-Event-ID` (header or query), branches three ways (fresh / replay_ok / truncated), and emits the SSE `id:` field on every event sourced from the buffer. On `replay_ok` the in-progress snapshot is skipped (the buffered events already cover it); on `truncated` an explicit envelope precedes the fresh-style recovery path. - Every events stream emits a jittered `retry:` in [2500, 4500] ms on first yield so 6-pane reconnects don't lockstep on EventSource's default ~3 s interval. Global lane (server.py / _global_fanout_thread / global_events_sse): - Parallel buffer + counter on `app.state.global_event_buffer` and `app.state.global_event_id_holder`; fanout thread stamps each event with `_event_id` and appends to the buffer under `global_listeners_lock`. `global_events_sse` branches on `Last-Event-ID` with the same three shapes. Tests: - 16 new tests in `tests/test_sse_reconnect_replay.py` cover the ring buffer semantics (empty-listeners hold, last_event_id slicing, truncation, atomic registration), the counter invariants (monotonic under concurrent writers, no skip on queue.Full, persists across turn boundaries, cross-thread consistency), and the handler branching (retry on first yield, id: on buffered events, snapshot-skip on replay_ok, envelope on truncated, query-param fallback, malformed header → fresh). - Existing `tests/test_session_ui_base.py` updated for the `_ws_inflight_seq` → `_event_id` rename and the new `_event_id` field on enqueued events. Backward-compat: all consumers that don't send `Last-Event-ID` (today's browser, Python SDK, TypeScript SDK, channel adapter) see behaviour identical to pre-PR — the server change is purely additive on the request side. |
||
|
Patrick Buckley
|
13ed62d200 |
fix(judge): UPSERT intent_verdicts so llm_fallback upgrades land
Async LLM-tier "llm_fallback" verdicts (judge.py:1073, judge.py:1131
via _deliver_fallbacks) deliberately reuse the heuristic verdict's
``verdict_id`` so the row gets "upgraded in place" from heuristic →
llm_fallback when the LLM judge times out, is cancelled, or returns
no content. The consumer ``_persist_intent_verdict`` was doing a
plain INSERT via ``create_intent_verdict``, hitting the
``intent_verdicts_pkey`` constraint on every llm_fallback delivery.
Postgres logged the duplicate-key error; the application try/except
swallowed it at log.debug — so the row never actually got upgraded
and the LLM judge's annotation ("(LLM judge did not return a
verdict)") was lost.
The collision rate exploded on stable/1.5 smoke tests because
PR #527 (just merged) added two new heuristic-INSERT paths in the
auto-approve early-return branches of ``approve_tools`` — previously
those branches dropped heuristic verdicts on the floor, leaving no
row for the fallback to collide with.
Fix:
- New ``upsert_intent_verdict`` method on the storage protocol +
sqlite + postgres impls, using dialect-specific
``insert(...).on_conflict_do_update(index_elements=["verdict_id"],
set_={...})``. Set_ clause updates ONLY the three fields that
genuinely change between heuristic and llm_fallback: ``tier``,
``reasoning``, ``judge_model``.
- Every other column is excluded from set_: identity columns
(verdict_id, ws_id, call_id, func_name, func_args), carried-
verbatim columns (intent_summary, risk_level, confidence,
recommendation, evidence, latency_ms), and ``user_decision``.
- ``user_decision`` exclusion is load-bearing: ``IntentVerdict
.to_dict()`` doesn't project it, so a fallback verdict reaching
``_persist_intent_verdict`` carries the kwarg's ``"pending"``
default. If the operator already resolved the approval between
heuristic INSERT and fallback delivery, the row's user_decision
has been stamped to ``"approved"``/``"denied"``/``"timeout"`` (or
an auto-approve reason at heuristic-INSERT time per PR #527).
Including ``user_decision`` in set_ would silently clobber that
back to ``"pending"``.
- ``_persist_intent_verdict`` switched from ``create_*`` to
``upsert_*``. Bulk path ``create_intent_verdicts_bulk`` stays as
plain INSERT — every heuristic ``verdict_id`` is freshly minted
in ``judge.evaluate`` so in-turn dups can't happen. The inverse
race (daemon-judge verdict lands BEFORE the bulk write) IS
reachable today but its observable behavior is unchanged by the
per-row UPSERT switch; documented at the bulk site for a future
hardening pass.
Test coverage:
- TestIntentVerdictUpsert × 4 — fresh-id insert, conflict-upgrade,
user_decision preservation across heuristic→approved→fallback,
identity + carried-field preservation.
- Existing tests in test_session_ui_base.py updated to mock the
new upsert method instead of create_intent_verdict.
|
||
|
Patrick Buckley
|
59c116f9eb |
fix(judge): explicit user_decision vocabulary (no more empty strings)
Auto-approved tool calls left intent_verdict rows with `user_decision=""`,
indistinguishable from rows still pending manual review. Real misdiagnosis
incident: a coord with `recommendation="review"` and `user_decision=""` was
read as "stuck waiting for approval" when in fact the tools had been
auto-approved and the child was running normally.
New vocabulary at the storage API boundary (column server_default stays
`""` so pre-fix legacy rows are still distinguishable as such):
- `pending` — at insert, before any resolution
- `approved` / `denied` — manual user resolution
- `timeout` — approval-event timeout (split from `denied` so the
audit column alone tells them apart; the feedback
string used to carry this distinction)
- `policy` / `blanket` / `skill` / `always` / `auto_approve_tools` —
auto-approve reasons (mirror `AutoApproveReason`)
Heuristic verdicts on the two auto-approve early-return branches are now
persisted with `user_decision=<reason>` (previously dropped on the floor).
Late LLM verdicts for already-auto-approved call_ids look up the reason via
a TTL-pruned `_auto_approve_reasons` map (lazy 60s prune at write time, so
no fixed cap can silently regress the fix on the N+1th auto-approve; LLM-
disabled sessions don't leak entries because prune fires whenever auto-
approves happen).
Bug fixes caught during review:
- `on_intent_verdict` early-returns when the verdict already carries an
auto_reason — without this, a manual `resolve_approval` on a mixed batch
would overwrite the auto-stamped row with `approved`/`denied`.
- `_record_auto_approves` runs BEFORE `_persist_auto_approved_heuristic_*`
so the lookup map is populated before any concurrent LLM verdict can
fire and miss it.
- `resolve_approval(timeout=True, approved=True)` now raises ValueError
to make the split-brain shape unrepresentable.
- Approval-timeout feedback string derives from `_APPROVAL_WAIT_TIMEOUT`
rather than the hardcoded "1 hour".
|
||
|
Patrick Buckley
|
f519ef1036 |
fix(sse): always advance _ws_inflight_seq on emit, even past cap
Copilot caught a real bug in the cap+seq interaction: the previous shape only advanced ``_ws_inflight_seq`` when the buffer actually appended, on the theory that "every _seq corresponds to a buffered fragment" was a useful invariant. It wasn't — once the buffer hit its cap, seq stalled at the high-water-pre-cap, so a subscriber that registered AFTER the cap was hit would capture ``snap_seq == stalled_seq``, and every subsequent live token (also tagged with the stalled seq) would be filter-dropped by the events handler's ``seq <= snap_seq`` dedup. Silent loss of the entire post-cap stream for refresh-past-cap tabs. Fix: advance seq on every emit, regardless of buffer cap. The cap is a buffer-size limit, not a stop-streaming signal. Past-cap tokens are absent from the snapshot's text payload (the buffer was truncated at cap) but the live stream past them is now correctly delivered — refresh-after-cap renders snapshot-up-to-cap then live tokens past it, with a visual gap equal to the past-cap chunk and no silent drop of subsequent tokens. Test ``test_inflight_seq_increments_only_on_actual_append`` enforced the buggy invariant and is renamed/flipped to ``test_inflight_seq_advances_on_every_emit_even_at_cap``. Added ``test_subscriber_after_cap_hit_receives_subsequent_tokens`` (and the reasoning equivalent) as direct regressions for the silent-token-loss scenario. |
||
|
Patrick Buckley
|
29b850919f |
feat(sse): refresh-resume for mid-stream page reloads
Refreshing a coordinator or interactive workstream pane while the LLM is mid-stream now restores the partial assistant text + reasoning immediately and flips the composer back to stop-mode, instead of showing nothing until the response completes. Per-turn inflight buffers (`_ws_inflight_content`, `_ws_inflight_reasoning`, `_ws_inflight_seq`) on `SessionUIBase` are kept separate from the existing multi-turn `_ws_turn_content` buffer that drives the dashboard's IDLE-piggyback payload. New `on_turn_start` (top of send-loop, defensive) and `on_turn_committed` (right after `messages.append(assistant_msg)`, primary) lifecycle hooks reset inflight at turn boundaries. The seq counter is monotonic across turns so a long-lived subscriber's `snap_seq` cutoff stays valid for the lifetime of the connection — resetting per-turn would silently drop turn N+1's first M tokens (M = whatever was streamed pre-snapshot in turn N). `snapshot_and_consume_state_payload` also drains inflight at idle/error so cancel and exception paths don't leak stale text. New `register_listener_with_in_progress_snapshot` atomically registers a listener and snapshots the inflight buffers; `make_events_handler` emits a `state_change` event (so the JS busy machine flips to stop-mode) followed by a one-shot `in_progress_snapshot` after the kind-specific replay, then strips the internal `_seq` field from yielded live events while filtering against `snap_seq`. A per-listener shallow `dict` copy in the live drain prevents the multi-tab race where one listener's `del event["_seq"]` would corrupt another listener's filter view. `_synthesize_cancelled_results` now emits synthetic `on_tool_result` events for each cancelled tool so live coord tabs can drop the newly-additive `coord-tool-batch--running` indicator cleanly. The indicator now coexists with `--auto`/`--approved` (applied on `tool_info` and `approval_resolved` approved; removed when every row in the batch has a result), making live tool execution visually parallel to the replay-time orphan rendering. Frontend handlers in `app.js` (interactive) and `coordinator.js` (coord) absorb EventSource auto-reconnect re-replays via a length-based prefix check on the in-progress buffer. New `InProgressSnapshotEvent` + `StateChangeEvent` dataclasses in the Python and TypeScript SDKs with type guards. `_MAX_TURN_CONTENT_CHARS` lifted 256 KiB → 512 KiB (single constant for both buffers — headroom for current commercial models). Regression tests cover race-free composition under concurrent writers, seq-filter dedup invariants, the cross-turn seq monotonic invariant, idle/error inflight drain, synthesized `on_tool_result` on cancel (including UI-hook failure isolation), and the multi-listener shared-dict invariant. |
||
|
Patrick Buckley
|
9b5096fe3c |
fix(approve): visibility for child tool calls bypassing operator gate (#430)
* fix(approve): visibility for child tool calls bypassing operator gate
When a coord LLM spawns a child with `skill="X"`, the skill template's
`allowed_tools` JSON list silently populates the child UI's
`auto_approve_tools` set. Tool calls whose names are in that set
short-circuit the approval gate without prompting the operator —
matching the user-reported bug "tool calls of children occasionally
getting approved instead of waiting for approve/deny".
The auto-approve paths themselves are unchanged (Option C — visibility
only). Surfaces:
- Per-item annotations: each pending tool gets `auto_approved=True` +
`auto_approve_reason` ("skill" / "always" / "policy" / "blanket" /
"auto_approve_tools") at the four gate-bypass paths.
- Per-ws ring buffer (cap 10) of recent bypasses, exposed via
`/dashboard` and the cluster live-bulk projection so the coord-
tree row can render an "auto-approved by ..." pill.
- `tool.auto_approved` audit row per `approve_tools` call —
forensic durability beyond the in-memory ring buffer.
- Per-ws WebUI page: inline "auto: <reason>" badge next to each
tool name, so an operator who clicks through from the coord tree
to the child's page sees the same bypass signal.
Persistence across UI rebuilds:
- The ring buffer is in-memory only; a saved-workstream rehydrate /
coord→node click-through / process restart all build a fresh UI.
`replay_recent_auto_approvals_from_audit` runs at the end of
`SessionUIBase.__init__` and re-seeds the buffer from recent
`tool.auto_approved` audit rows scoped to this ws_id.
- Adds `resource_id` filter to `list_audit_events` (protocol +
SQLite + Postgres) so the replay is a single indexed query.
Source provenance:
- `_auto_approve_tools_source: dict[str, str]` per UI tracks which
writer added each tool name to `auto_approve_tools` ("skill" at
skill-template setup time, "always" on Approve+Always click).
Lets the dashboard pill distinguish a skill-driven bypass from
an explicit operator-Always click — those are very different
signals that previously rendered the same.
Magic-string drift mitigation:
- `AutoApproveReason` constants in `core/session_ui_base.py` lift
the five reason strings into a single source of truth.
- `KNOWN_AUTO_APPROVE_REASONS` JS constant + validator render
unknown reasons as "unknown" with a console.warn instead of
rendering raw (a typo would otherwise silently desync wire ↔
pill).
Recording-leak fixes (q-2 from review):
- Policy `allow` partial-resolve now records the policy-tagged
items at two previously-leaking branches: the early-return-on-
deny path and the still_pending-non-empty fall-through to the
prompt path.
Other review fixes:
- Heuristic verdict surfaces consistently as `heuristic_verdict`
in both `_serialize_approval_items` and the dashboard
serializer (was inconsistent: one emitted `verdict`, the other
`heuristic_verdict`). app.js updated to read either key for
mid-deploy compatibility.
- `_tag_auto_approved` helper on SessionUIBase replaces the
verbatim tag loops previously copy-pasted across WebUI and
ConsoleCoordinatorUI.
* fix(approve): apply Copilot review feedback on PR #430
- coordinator_ui: use ``approval_label or func_name`` for the
``auto_approve_tools`` subset check, matching WebUI. Pre-fix
an "Approve + Always" entry whose approval_label differs from
func_name (skill__name, mcp_resource__uri) wouldn't match on
the coord page and the operator would be re-prompted.
- _parse_audit_timestamp: treat naive ISO strings as UTC. Audit
rows are written via ``datetime.now(UTC).strftime(...)`` with
no timezone marker; ``datetime.fromisoformat`` returns a naive
datetime, and ``.timestamp()`` on a naive datetime interprets
it in the server's local timezone — wrong on any non-UTC
server. Stamp UTC explicitly before converting.
- server.py: drop the dead ``pending = []`` after the blanket
tag — the function returns inside the same block without
reading ``pending`` again.
- _protocol.py: fix docstring reference from
``_replay_recent_auto_approvals`` to
``replay_recent_auto_approvals_from_audit`` (the actual
method name).
|
||
|
Patrick Buckley
|
7e33fc68bb |
fix(approve): apply /review feedback on inline child approvals
Critical:
- coordinator.js RISK_SEVERITY accepted 'crit' only; production
emits 'critical' (per turnstone/core/judge.py:1556 + heuristic
seeds). A risk_level=='critical' verdict ranked as 0 and
rendered with .risk.low (green) styling, never triggering
the crit-risk auto-expand. Now accepts both aliases. Unknown
risk_level falls back to rank 2 ('high') so future schema
drift fails *safe* (over-alert) instead of silently
downgrading. Pill ternary handles both 'crit' and 'critical'
alias to the existing .risk.crit class.
Major:
- Urgent live-badge flush now coalesces N urgent calls in the
same JS tick into one bulk request via queueMicrotask, instead
of firing N single-id fetches. The motivating 10-children-
pending-bash scenario in the design doc now lands on one bulk
/v1/api/cluster/ws/live request.
- Test coverage gap: added test_session_ui_base.py cases for
POLICY-BLOCKED (item.error + needs_approval=False) and
judge-unavailable (no verdict + no judge_pending) matrix rows.
Added literal-string assertions to the smoke list in
test_coordinator_page.py so a refactor dropping either branch
surfaces at test-time.
Minor batch (4 coord.js + 1 CSS + 1 fake-divergence):
- 409 stale-call_id path re-enables both buttons before return
(urgent fetch is best-effort; could also fail).
- judgePending pill no longer conflicts with a present heuristic
verdict — guard changed from !judge to !verdict.
- Empty <div class="approval-reasoning"> no longer appended when
reasoning is absent but evidence is present (evidence still
renders inside the disclosure).
- Dead .ch-row .approval-pill.rec-* CSS rules removed (JS never
combines those classes). Recommendation chip in the disclosure
footer now has its own scoped rules so the chip is actually
styled.
- _FakeUI.serialize_pending_approval_detail call_id selection
aligned to the real impl's "first non-empty" semantics.
- liveBadgeCache reconnect cleanup now preserves permanent
(403/404) entries — denied users no longer pay one wasted
bulk fetch per denied id per reconnect.
All 4465 non-live tests pass. Ruff + mypy clean. node --check OK.
|
||
|
Patrick Buckley
|
fbb9be27f9 |
feat(approve): expose pending_approval_detail on /dashboard + guard stale call_id
Lays the server-side groundwork for inline approve/deny buttons + judge verdict on the coordinator children-tree UI. Two surgical changes: 1. SessionUIBase.serialize_pending_approval_detail() merges the active _pending_approval items[] with per-call_id verdicts from _llm_verdicts. The dashboard handler embeds this on every per-ws row so cluster live-bulk callers can render inline UI without an extra per-child round-trip. 2. make_approve_handler now returns 409 when the body sends a call_id that doesn't match any currently-pending item. Closes the stale call_id race where an operator clicks approve on a row showing call A while the child has rolled over to call B. Empty/missing call_id preserves backwards compatibility with CLI + channel adapters that don't track it. Cross-tenant exposure on /dashboard is consistent with the trusted-team posture already in place for activity / tokens — documented in the new method's docstring so the choice survives the next reviewer. Plan: docs/design/inline-child-approvals.md (chunk 1 of 4). |
||
|
Patrick Buckley
|
c837e3fa6d |
feat(core): Stage 1 SessionManager unification (#408)
* feat(core): scaffold SessionManager + SessionKindAdapter Protocol Stage 1 step 1 — pure addition, no production wiring. Defines the shape later steps will port the shared mechanics onto: slot accounting, per-ws-id refcounted rehydrate locks, kind-agnostic lifecycle; kind-specific event transport + session construction on the adapter. Pruned from the earlier Protocol draft (see design brief): per-kind permission_scope (static handler map is simpler), allows_child_spawn / quota_policy (deleted in #403), on_child_spawned (coordinator tool owns children registry), allows_active_focus / active_id / switch (frontend owns the active-tab state). * feat(core): port shared session-lifecycle mechanics onto SessionManager Stage 1 step 2. Adds create / open / close / set_state / close_idle / get / list_all / count on top of the Step 1 scaffolding. Pure addition — still no production wiring; the new class doesn't replace any call sites yet. Concurrency shape is ported from CoordinatorManager (the more- complete side): single-phase slot reservation under the manager lock, per-ws refcounted open-lock to serialize concurrent lazy rehydrate, placeholder workstreams count toward max_active but can't evict each other. WSM's two-phase eviction outside the lock is not carried over; it had a window where a burst of creates could silently exceed max_active. Deletions (vs. the union of the two old managers): - "refuse to close last workstream" guard — handled by the dashboard; only existed to protect the now-deleted default startup workstream. - active_id / switch / get_active — frontend owns focus; server-side duplicate state is gone. - _active_coords presence cache — defer measurement to Step 4; if it pays for itself at realistic cluster sizes, the CoordinatorAdapter can maintain it by observing emit_* calls. - Children registry + reverse index — coordinator tool owns this, manager stays kind-agnostic. Skill resolution (name → template_id + applied_version) is now shared via SessionManager._resolve_skill, so WSM's pre-resolve-at- callsite pattern and CM's internal-lookup pattern converge. Callers pass the skill name; the manager does the lookup once. 26 smoke tests cover create eviction + overflow, concurrent-create cap, persist/session rollback, open for missing/deleted/wrong- kind/wrong-user rows, concurrent-open serialization, close unblocks UI + emits closed, set_state + storage + adapter observer, close_idle, list_all ordering, count, eviction fires adapter transport, node_id passthrough. * feat(core): add InteractiveAdapter for SessionManager Stage 1 step 3. Adapter that bridges SessionManager to the node's interactive transport: - emit_created/state/closed → pushes onto the process-wide SSE global_queue (same shape current server.py handlers produce inline) - cleanup_ui → ports WorkstreamManager._cleanup_ui body: unblock _approval_event / _plan_event / _fg_event, broadcast ws_closed to per-UI listener queues (with full-queue fallback), cancel + close the session - build_ui/build_session → delegate to injected factories (ui_factory builds WebUI, session_factory is the existing closure from server.py with judge_model + memory_config captures) Also extends SessionKindAdapter.build_session with **extra passthrough so interactive callers can pass judge_model per-call without polluting the manager API; and adds a reason= kwarg to emit_closed so the frontend's "evicted" special-case keeps working (frontend doesn't differentiate "idle" from "closed", so close_idle collapses into close()). 14 new adapter tests cover wire payload shape, queue.Full tolerance, cleanup_ui event unblocking + listener broadcast + queue-full fallback, session cancel+close, graceful handling of stub UIs / None session, and kwarg passthrough to the session factory. * feat(console): add CoordinatorAdapter for SessionManager Stage 1 step 4. Coordinator-side SessionKindAdapter implementation: - emit_created/state/closed → delegate to the existing ClusterCollector.emit_console_ws_* methods (same wire shape the old CoordinatorManager emitted inline) - cleanup_ui → ports the listener-queue + approval/plan event unblocks from CoordinatorManager._cleanup, with queue-full fallback so an unresponsive browser tab can't wedge close - build_ui/build_session → delegate to injected factories; session factory doesn't accept client_type so we strip it at the adapter boundary Collector emission exceptions are swallowed (same policy as today's inline fan-out — dashboard lag on one tick is preferable to breaking the lifecycle path). Intentionally out of scope: the children registry (_children / _child_to_coord) stays in the coordinator tool when wired in Step 5; the _active_coords lock-free presence cache is deferred pending a measurement at realistic cluster sizes. 10 new tests cover transport payloads, collector-exception tolerance, cleanup_ui event unblock + listener broadcast + queue-full eviction, construction passthrough. * feat(server): wire interactive server.py to SessionManager Stage 1 step 5a. Production-path swap: WorkstreamManager → SessionManager(InteractiveAdapter(...)). - Construction at server startup: build the adapter with the process-wide global_queue, a WebUI ui_factory closure, and the existing session_factory. SessionManager gets storage + max_active. - Default startup workstream wiring removed (the CLI-REPL leftover flagged in the handoff's "Convergence is also a pruning opportunity" section). --resume now lazily creates a workstream scoped to the resumed content; no workstream at all if --resume isn't given. The dashboard handles the 0-ws state. - HTTP handler mgr.create() calls switched to the new kw-only signature (user_id, name, model, skill, ws_id, client_type, judge_model, parent_ws_id). ui_factory/skill_id/skill_version/kind no longer threaded through — adapter handles UI construction and manager resolves skill internally. - Dropped the mgr.last_evicted block in the /new handler (adapter emits ws_closed:evicted automatically on capacity eviction). - mgr.max_workstreams → mgr.max_active. - Added active_id / switch / switch_by_index / get_active / index_of / eviction_count to SessionManager because turnstone/cli.py uses them extensively; the handoff's "delete unless there's a live caller" rule flips here — CLI is a live caller. Test fixtures across 9 files updated to build SessionManager + InteractiveAdapter rather than WorkstreamManager. test_workstream.py stays unchanged (it tests WSM directly; it'll be deleted in step 5d alongside the class itself). Full pytest: 4528 passed. Ruff + mypy clean. Next: 5b (console-side wiring, with the children-registry relocation to the coordinator tool). * feat(console): wire console server to SessionManager Stage 1 step 5b. Production-path swap: CoordinatorManager → SessionManager(CoordinatorAdapter(...)). - CoordinatorAdapter now owns the coord-specific bits that were bolted onto the old CoordinatorManager: the children registry (forward + reverse index), the lock-free active-coords presence cache, the cluster-event fan-out thread, and the worker-dispatch path (send / _spawn_worker). The shared SessionManager stays kind-agnostic. - Added CoordinatorAdapter.attach(mgr) for late-binding the owning manager (the manager's ctor takes the adapter, so the dependency has to break here). Used inside _rebuild_children_registry for the tenant- filtered SQL query, inside send/dispatch for mgr.get(ws_id), and inside the fan-out seed path for mgr.list_all(). - emit_created now seeds the children registry + active-coords slot AND calls _rebuild_children_registry (covers both create — empty query — and open/rehydrate, where the subtree is persisted). emit_closed drops both entries. Collapses the three old call-sites in CoordinatorManager's create/open/close into one per-event hook. - Console server.py builds the manager via: coord_adapter = CoordinatorAdapter(collector=..., ...) coord_mgr = SessionManager(coord_adapter, storage=..., max_active=..., node_id=ClusterCollector.CONSOLE_PSEUDO_NODE_ID) coord_adapter.attach(coord_mgr) ConsoleCoordinatorUI._coord_mgr = coord_mgr app.state.coord_adapter = coord_adapter - HTTP handler call-site updates: - coord_mgr.create drops initial_message; the handler now calls coord_adapter.send(ws.id, initial_message) after create so the worker spawn stays out of the shared manager. - coord_mgr.open_admin(ws_id) → coord_mgr.open(ws_id, user_id="", admin=True). Matches SessionManager.open's unified signature. - coord_mgr.list_for_user(uid) inlined as a list comp on list_all() (SessionManager doesn't expose the filter; two callers). - coord_mgr.children_snapshot / send → coord_adapter.*. - coord_mgr.cancel stays (now lives on SessionManager from 5a). - ConsoleCoordinatorUI.on_state_change now flows state transitions through ConsoleCoordinatorUI._coord_mgr.set_state, mirroring the WebUI pattern. The old _on_state_observer / _on_rename_observer closures the manager used to install are dead code now; leaving the fields in place for 5d cleanup. - Lifespan shutdown calls coord_adapter.shutdown() (was coord_mgr. shutdown()) and resets ConsoleCoordinatorUI._coord_mgr on teardown. Test fixture updates in _coord_test_helpers, test_coordinator_end_to_end, test_coordinator_endpoints, test_phase6_endpoints: build SessionManager + CoordinatorAdapter in _build_mgr, set app.state.coord_adapter, switch mgr.register_children / mgr.children_snapshot tests to mgr._adapter.*, and rewrite test_open_admin_uses_open_admin to assert the unified open(user_id="", admin=True) call shape. Full pytest: 4486 passed. Ruff + mypy clean. Next: 5d (remove CoordinatorManager + WorkstreamManager class bodies and their test files). * feat(core): delete WorkstreamManager + CoordinatorManager classes Stage 1 step 5c + 5d. Final step of the unification — the legacy classes and their test files go away now that every production caller has been ported. - Delete turnstone/console/coordinator.py entirely (CoordinatorManager class + the _enqueue_on_ui helper, which CoordinatorAdapter now hosts its own copy of). - Trim turnstone/core/workstream.py to just the Workstream dataclass + WorkstreamKind + WorkstreamState. ~385 lines of WorkstreamManager logic gone; the remaining shape is pure data types shared by both managers. - Delete tests/test_workstream.py (WSM-specific) and tests/test_coordinator_manager.py (CM-specific). - Wire turnstone/cli.py to SessionManager + InteractiveAdapter, same pattern as turnstone/server.py. The CLI's WorkstreamTerminalUI uses manager.set_state + manager.active_id — both preserved on SessionManager (CLI is a live caller that keeps the focus API honest, per the handoff's "delete unless it pulls its weight" rule). - Add an optional manager-level ``_on_state_change`` observer hook restored for the CLI's background-attention notification (the web path uses the adapter's emit_state; this hook covers callers that don't consume SSE). - Drop dead ``_on_state_observer`` / ``_on_rename_observer`` fields from ConsoleCoordinatorUI — the old CoordinatorManager installed them; SessionManager/CoordinatorAdapter handle fan-out directly. Vulture @ 80% confidence: zero unused symbols across the new SessionManager + adapter files. Ruff + mypy clean (170 files). Full pytest (excluding tests/live): 4414 passed. Net across the whole Stage 1 branch: one unified SessionManager + adapter Protocol replaces two ~500-line parallel managers + a ~600-line CoordinatorManager, and the interactive + coordinator transports stay cleanly separated at the adapter boundary. * refactor(auth): drop workstream row-level ownership gates Turnstone is a trusted-team tool (per #400). user_id stays as metadata for audit + display; it no longer rejects requests. Scope- level auth via admin.workstreams / admin.coordinator tokens is the only gate now. Solves sec-1 (cross-tenant delete via collision on caller-supplied ws_id, because the gate was half-implemented) and sec-2 (blank-sub JWT bypass on empty-owner rows). Net: 359 lines of defensive empty-string comparisons and admin=True bypass plumbing deleted. * fix(core): serialize set_state vs close + worker spawn Three concurrency fixes from the multi-stage review: - bug-3: set_state now looks up ws under self._lock and gates its storage write on ws._closed (a new tombstone flag). close() sets ws._closed=True and does its storage write under ws._lock. A set_state that acquires ws._lock after close sees the tombstone and skips its write instead of resurrecting the closed row. - bug-1: _spawn_worker wraps the check-and-spawn in ws._lock so two concurrent send() HTTP requests can't both observe "no live worker" and start duplicate worker threads on the same ChatSession. - bug-2: replaces Thread.is_alive() as the reuse gate with an explicit ws._worker_running flag. The flag is set before the worker thread starts and cleared in its finally block — both under ws._lock. Using is_alive() left a narrow window where the worker could exit between the check and a queue_message call, stranding the user's message with no consumer. perf-2 (lock-held-across-DB-write) is accepted as-is: per-ws serialization of state transitions behind a DB round-trip is real cost but bounded — a given ws's state flips happen sequentially on its worker thread anyway. Dropping ws._lock around the DB write would reintroduce the bug-3 race. Full pytest: 4401 passed. Ruff + mypy clean. * refactor(core): drop _resolve_skill from SessionManager Skill resolution (name → template_id + applied_version) moves out of the shared manager and back to the HTTP handlers that own the create request. The interactive handler already resolved skill_data + applied_skill_version for other purposes (model override, judge config, post-create session seed) and was passing the name to SessionManager which then redundantly re-resolved via get_skill_by_name + count_skill_versions — two wasted DB round-trips per create on a user-visible latency path. - SessionManager.create: accepts skill_id + skill_version as already-resolved kwargs; _resolve_skill helper deleted. - turnstone/server.py create_workstream: passes the skill_id / applied_skill_version it already computed. - turnstone/console/server.py coordinator_create: pre-resolves inline (parity with interactive) before calling coord_mgr.create. Fixes perf-1 (redundant skill queries per create), q-4 (divergent skill-version computation between manager and handler), q-5 (coordinator-specific lookup on the shared manager surface). Full pytest: 4401 passed. Ruff + mypy clean. * refactor(adapters): extract shared cleanup_ui + drop dead child-registry methods Both InteractiveAdapter.cleanup_ui and CoordinatorAdapter.cleanup_ui (plus their _broadcast_ws_closed_to_listeners helpers) were byte-identical. Pull them into turnstone/core/adapters/_ui_cleanup.py:cleanup_session_ui so the two adapters delegate to one implementation. Also drop CoordinatorAdapter.register_children (only test callers — now use _seed_children in tests/_coord_test_helpers.py) and _add_child (zero callers anywhere). * refactor(adapters): symmetric attach() + fail-loud on unattached manager Add InteractiveAdapter.attach(manager) + .manager property mirroring the coord-side pattern. CLI (cli.py) now uses cli_adapter.attach(manager) instead of the _mgr_ref list-ref late-binding hack; server.py picks up the same call for consistency. CoordinatorAdapter.send / _rebuild_children_registry / _prime_children_from_snapshot no longer silently return when self._manager is None — raise RuntimeError so a forgotten attach() at startup fails loud instead of dropping the whole fan-out. * docs: replace stale WorkstreamManager / CoordinatorManager references Both classes were deleted in 965e0b6; prose docstrings across the codebase still named them. Update to SessionManager (or describe the collapsed-into-one-class architecture where the distinction matters). Leaves the 'Ported from …' historical markers in session_manager.py / coordinator_adapter.py / interactive_adapter.py intact — those are deliberate pointers back to the pre-unification code. * fix(core): atomic close_if_idle + batch pop under one lock bug-5: SessionManager.close_idle re-checked ws.state == IDLE outside the lock, so a pending tool result could flip state IDLE→RUNNING between the snapshot and close() acquiring self._lock. Add _close_if_idle_locked that tests state + pops under self._lock. perf-5: drop the per-victim self._lock acquisition; collect + pop the whole batch in one acquisition, then run cleanup_ui / storage write / emit_closed outside the lock. * perf(coord): split emit_created / emit_rehydrated to skip storage query on fresh creates CoordinatorAdapter.emit_created was unconditionally calling _rebuild_children_registry (storage.list_workstreams with parent_ws_id=... limit=10001) on every create, even for fresh-create paths that provably have zero children. Add emit_rehydrated to the SessionKindAdapter Protocol. SessionManager .create still calls emit_created; .open (lazy rehydrate) now calls emit_rehydrated. CoordinatorAdapter.emit_created seeds the registry + fan-out but skips the rebuild; emit_rehydrated seeds + rebuilds + fans out. InteractiveAdapter.emit_rehydrated delegates to emit_created (no children-registry on the interactive transport). * perf(coord): fold _active_coords into _children_lock + mutate payload in place perf-4: _active_coords used a copy-on-write dict-swap pattern so the fan-out dispatch could read it lock-free, but _dispatch_child_event already re-validates the parent under _children_lock anyway — the lock-free snapshot was premature. Replace with a plain dict read+write both under _children_lock; install and remove collapse to one-liners. Value also drops the user_id half — dead after |