48 Commits

Author	SHA1	Message	Date
Patrick Buckley	8605c9783d	feat(models): rfc8693_obo auth mode, per-alias exchange scopes, identity-keyed mint cache ... Adds the dedicated `rfc8693_obo` model auth mode (#955): model definitions gain an `obo_scopes` column (migration 069), the mint threads the scopes to the token-exchange leg (RFC 8693), and every dynamic mode pins its grant leg — a mode is a dialect commitment, not a hint the deployment profile resolves. Exchange-capable IdPs refuse an audience whose scope was not requested; this closes the structurally unmintable model-OBO path on token-exchange deployments. The model mint-cache is identity-keyed on the owning definition's alias (`__model_obo__:<alias>` per user, `__model_app__:<alias>` under the shared app principal), matching the MCP discipline where rows key on the unique server name. The bearer's shape lives in the row's audience/scopes columns and the freshness gate compares it on every read, so a re-aimed alias refuses its old row and overwrites the same key in place. Admin lifecycle (rename, re-aim, scope change, delete) purges a definition's own rows through one shared helper — sound because one definition owns each key; a sibling's rows are untouchable by construction. Cooldown and backoff additionally key on the dispatch shape, so an operator's config repair is an instant clean slate. Cause records, cooldowns, locks and memoization are per-alias end to end, and the session heartbeat reads refusal causes under the same keys. Console: default-deny write gating for dynamic rows (value-diff over the full column ladder, admin.mcp escalation, a never-blockable pure-disable carve-out), a two-tier validator (audience allow-list on every write; deployment-posture checks when the pair is chosen), one shared scopes parser whose omit-unchanged arm keeps over-cap DB-direct residue rows disarmable without ungating real changes, and served constraints (dynamic/scopes/app-identity mode lists, mode-to-profile pairing) so the shelf tracks the registry by data. The admin shelf gains the mode option, a scopes input with residue affordances, pairing-aware option greying, and a derived auth badge. Registry load refuses control characters in alias, audience, and scopes — including the C0 separator block that str.split() would silently collapse — and the C0/DEL class has one exported spelling shared by every surface. Profile-mismatch visibility warns at reload and boot with the mode-correct cause, gated on OIDC being enabled. Breaking: a stored `entra_obo` alias on a deployment whose `[oidc] obo_grant_profile` is `rfc8693` (or the inverse pairing) no longer mints via the profile-driven overload — the mint refuses before any IdP traffic with cause `grant_profile_mismatch`, and the `model.auth_fail_closed` policy governs static fallback. Such rows never minted usefully on scope-gating IdPs; the shelf now surfaces the pairing and the per-turn heartbeat names the refusal cause. Live-verified end to end: scoped token exchange mints, the warm cache serves with zero IdP calls, and the mode/profile mismatch refuses with zero IdP traffic (scripts/obo-e2e/keycloak_e2e.sh); the refresh-redemption profile's E1-E7 hold via scripts/obo-e2e/entra_e2e.py. Closes #955.	2026-08-04 05:19:03 -07:00
Patrick Buckley	1e34e19d48	refactor: single-style module imports and narrowed JSON body typing ... Consolidates the repeated function-local model_registry imports onto one from-style module import per test file (the module object stays available for monkeypatching), converts the e2e script's mcp_oauth import to match, and reads the request body as Any before the isinstance narrow so the declared dict type is earned rather than asserted. Addresses the automated review feedback on the pull request; the two code-scanning flags are dismissed as false positives separately (the missing-key refusal log names config knobs and carries no secret value; the URL assertion is a test expectation, not a sanitizer).	2026-08-03 20:11:28 -07:00
Patrick Buckley	33ace975d2	feat(models): default-deny governance and admin UI for per-alias backend auth ... Follow-up to the per-alias Entra OBO/app-identity backend auth: the console write path now applies default-deny field classification, the admin shelf gains full backend-auth support, and the session/registry rebind machinery is hardened for config changes landing under live sessions. Console write gate: - Default-deny classification: any non-neutral change to a row that is or becomes dynamic requires admin.mcp plus validation; the provably auth-neutral columns are enumerated (MODEL_AUTH_NEUTRAL_FIELDS) and a live-schema classification test forces every future column to be classified. The derivation is a pure function (_derive_auth_gate) with unit-pinned exclusivity invariants. - Two-tier validation mirroring the MCP oauth_obo validator: the row tier (audience allow-list) runs on every gated write; the posture tier (OIDC configured, token store present) runs on pair changes and on enable-arming. - Pure-disable carve-out: disabling a dynamic row is de-escalation and is never blocked — admin.models suffices and validation is skipped, including for rows with corrupt or skewed stored values. - Capabilities are compared canonically (key order, integral floats), the audience compare normalizes both sides, and staging an audience on a static row is refused on both write twins. - Calibrate writes the capabilities column under an enforced confinement invariant with a compare-and-swap persist. Admin shelf: - Backend-auth section with a per-open constraints fetch (GET /model-definitions/auth-constraints: audience allow-list, grant profile, dynamic modes), datalist audience suggestions, server-defined modes preserved on round-trip, and permission-aware visibility built on cache-skew-safe helpers shared through auth.js. - Refused live-registry swaps surface as an amber registry_warning on the write, delete, reload, and calibrate responses; audit rows carry auth_gated / auth_disarmed markers visible in the audit view. Registry and sessions: - The encryption-key requirement for dynamic auth is enforced inside ModelRegistry.reload() itself — nodes refuse with 503 and the console records coord_registry_error — and reload bumps the generation before the map swap so a racing reader can never pair a stale generation with new maps. - resolve()/resolve_binding() return the generation from inside the registry lock; sessions rebind per send on generation change with atomic client/provider/config commits, fallback-first handling of removed or unconstructable aliases, and judge/limiter resets only when the binding actually changed. - Mint refusals record per-user causes surfaced in the per-turn heartbeat logs; misconfiguration warnings are deduplicated with bounded state. Verification: 10417 tests (99 added on this branch), a 71-scenario browser harness over the real admin shelf, and a live rfc8693 token-exchange e2e run (MCP legs verified end to end; the model-leg scope gap is tracked as #955 under a narrow known-gap signature). Closes #950.	2026-08-03 20:11:28 -07:00
metaclassing	9adde920d4	feat(models): per-alias backend auth via Entra OBO and app identity (#898) ... Adds a per-alias `auth_mode` on model definitions so a model backend can authenticate to an Entra-fronted gateway with a per-request minted token instead of one shared static API key, letting the gateway attribute calls to the actual user or to the app as a machine identity. - `static` (default, unchanged) sends the stored `api_key`. - `entra_obo` mints a per-user On-Behalf-Of token for `obo_audience` from the caller's captured refresh credential. - `entra_app` mints an app-identity token via the client-credentials grant, and covers userless turns that OBO cannot. Reuses the existing OBO grant legs, refresh-token rotation CAS, cluster advisory lock and the `mcp_user_tokens` mint-cache, keyed under synthetic `__model_obo__:<audience>` / `__model_app__:<audience>` rows. The token binds at the call site through `client.with_options(api_key=...)` so each SDK emits it on its own auth path rather than through header injection. Migration 068 adds `auth_mode` and `obo_audience`. Both are additive and existing rows default to `static`, so behaviour is unchanged unless an alias opts in. Operator controls: `model.auth_audience_allowlist` is an exact-match allow-list that gates which audiences may be configured and denies all by default, and changing a mode or audience requires `admin.mcp`. `model.auth_fail_closed` decides whether a failed mint may fall back to an explicitly configured static key. A delegated call with no user, or a dynamic alias with no real static key, always refuses. Two changes here apply regardless of whether any alias opts in: - Storage and app state are now wired into the console MCP client manager. This fixes per-user `oauth_user` / `oauth_obo` dispatch for coordinator-hosted sessions, which previously raised `RuntimeError` on first call because `set_app_state` was only ever called on the node. - Unattended watch restores and `--resume` resolve the persisted workstream owner instead of constructing the session under an empty principal. A workstream with no owner is now a permanent refusal rather than an anonymous, auto-approved run.	2026-08-02 15:15:02 -07:00
Patrick Buckley	3a28dc2f16	fix(providers): review round 5 — same-id fragment merge, post-finish blip tolerance, chat finish shim ... Correctness: - ToolCallSlotter's reannounce split is gated to ID-LESS deltas: id equality proves the same call, so compat servers that repeat the id+name header on every argument fragment merge back into one call with valid JSON (round 4's ungated heuristic split them into duplicate half-JSON calls — execution-confirmed by the review). The residual id-less repeat-name-per-fragment shape is documented as inherently ambiguous; ids are the only disambiguator. - drain_stream keeps a completed result when the transport blips AFTER the finish reason (trailing usage chunk / citation footer window): the generation is in hand, so forfeit the trailing metadata instead of discarding a fully-delivered verdict or re-paying a compaction. - The chat iterator shims finish_reason="stop" when a stream ends CLEANLY after delivering content or tool calls — the deleted non-streaming `or "stop"` default for lax finish-reason-less servers, now safe to restore because abrupt deaths surface as httpx.TransportError (round 4) rather than clean exhaustion. This supersedes the round-3 keep-the-gate ruling: the httpx catch changed the calculus, and the Anthropic/Responses lanes already got their marker-based shims. Empty/reasoning-only streams still fail the complete-or-error gate. Two streaming tests gained the shim chunk. Dispositions held: o1-era stream-rejecting models (third re-report) stay a release-note remediation per the earlier ruling. Cleanup: the two Responses terminal branches collapse into one path (status derived from the event type when the payload is missing — also fixes the end-of-stream debug log reporting finish_reason=None for completed lax streams); the annotations walk is one shared helper (the two copies had already diverged on None-content guarding); _raise_responses_failure is annotated NoReturn; scripts/livepass.py drops the phantom supports_streaming key; test_model_registry's capture helpers ride scripted_chat_client; _openai_stream_chunk points at its fake_chat_stream shape-twin for future consolidation.	2026-07-13 22:39:19 -07:00
Patrick Buckley	3ffa8b9057	fix(providers): review round 2 — complete-or-error drain, code-gated retries, truncation-safe blocks ... Correctness (3 confirmed + 2 plausible, all fixed): - drain_stream now raises typed, retryable IncompleteStreamError when a stream exhausts without any finish reason — every adapter emits one on a healthy stream, so its absence means the generation died mid-response behind a cleanly-closing proxy. This restores the retired transport's complete-or-error contract (a half-generated compaction summary was previously returned as finish=stop and stored, silently replacing real history) and DELETES round 1's suffix-info fold: with no finish-less success path there is nothing to classify, so a trailing status ping can never be stored as content either. - Index-degenerate parallel tool calls get distinct slots: a delta whose id differs from its slot's opens a new call (id-less fragments still follow their index's current call), so historical compat servers that emit every parallel call at index 0 no longer fuse distinct calls into concatenated garbage arguments. Result order stays index-sorted (stable) like the retired array parse. - response.failed retryability is code-gated: only transient codes (server_error, rate_limit_exceeded) raise the retryable typed error; deterministic rejections (invalid prompt, image fetch, policy) raise plain RuntimeError and stop retry loops on attempt zero instead of running the full backoff ladder against a doomed request. - Terminal Responses events rebuild provider_blocks from response.output when present: the item being generated at max_output_tokens truncation never receives output_item.done, and storing a reasoning item without its required following item made the next turn's replay a 400. - merge_usage's base case uses dataclasses.replace so a future UsageInfo field can't be silently zeroed on drained lanes. Cleanup: run_abortable_with_deadline bundles the three-point abort wiring (ref + cancel_ref + on_abandon) so it cannot be half-wired — both judges converted; scripted_chat_client hoists the 14 chat-lane fake_create closures (call scripts + .calls recording replace per-test counter cells); fake_chat_stream gains reasoning=, collapsing the reasoning-capture suite's hand-rolled chunk shape; FakeAnthropicBlock hoists the duplicated _Block test class; the class and judge PlantUML diagrams drop the retired create_completion flow. Also converts test_model_registry's agent-model fakes, which returned legacy response objects that iterated as EMPTY streams — they only passed through the old drain's silent finish=stop default, exactly the hazard the new gate exists to catch.	2026-07-13 22:39:19 -07:00
Patrick Buckley	e8a17921bf	fix(model-turn): round-3 review — complete the scheme rollout to the main loop, coordinator role, admin save path, and CLI switch ... - The main streaming loop now applies the in-code model-definition rung (caps.default_reasoning_effort) exactly like model_turn does, so the same alias samples identically between chat and every auxiliary lane (resolve_lane's stated contract). This also unblocks operator temperature on gpt-5.x aliases whose declared default is "none" — the main loop previously sent neither knob while aux lanes sent both. - coordinator.reasoning_effort default "medium" -> "" (the missed unset sentinel): coordinators inherit like every other lane; the role rung fires only when the operator stored a value. - admin webux: _onSettingChange no longer hides the save button for a blanked nullable number input, so the blank-means-inherit save path is actually reachable from the field it decorates. - /model switch on STORE-LESS sessions (the CLI) keeps the user's explicit --temperature//reason knobs when the target alias declares no override — the current knobs are the only authority there (mirrors the max_tokens fallback). Store-backed sessions still re-resolve. - ModelLane docstring no longer documents the removed caller-default effort rung; CLI status line shows any resolved effort ("medium" is no longer a hidden code default); dead `u = usage` alias dropped; three test docstrings re-pointed from the deleted ChatSession._maybe_synth_reasoning_block to model_turn.synth_reasoning_block.	2026-07-13 08:48:27 -07:00
Patrick Buckley	b6391d1f90	fix(model-turn): one sampling-knob assignment scheme — alias > config > model definition > omit ... Round-2 review fixes. The round-1 de-pinning collided with ConfigStore.get's default-on-miss semantics: the registry defaults (temperature 1.0, effort "medium") were manufactured onto every store-backed lane's wire, making the documented "unset -> omit" terminal unreachable. Unset is now representable end to end, and one scheme governs every lane: per-model alias value > operator-stored global setting > in-code model definition (effort only: caps declaration) > field omitted, inference engine's default rules. - settings_registry: model.temperature default None, model.reasoning_effort default "" — the registered defaults ARE the unset sentinels, so the admin UI and the wire agree. Admin webux renders nullable floats blank ("(inherit model default)") and maps blank-save to reset; the "" effort choice reads "(inherit)". - model_turn: resolve_temperature_setting/resolve_effort_setting are the ONE pair of operator-rung resolvers, shared by resolve_lane, both session factories, and the /model switch (the 4th-copy mirror is gone; the switch no longer leaks the previous model's override on store-less sessions). The caps rung moved out of the lane into model_turn's effective computation, below a new request-shaped default_reasoning_effort parameter (utility + output guard pass "low": budget coherence with their small token caps, not sampling policy — any operator or model-definition value beats it). The hidden "medium" terminal is gone. - providers: Protocol + all adapters take reasoning_effort: str | None = None (the Protocol-signature "medium" was the same manufactured pin one layer down); ModelCapabilities.default_reasoning_effort defaults "" — commercial rows all declare theirs explicitly, so only local lanes and Anthropic change, both to match their real serving defaults (Anthropic manual-thinking models no longer get implicit thinking-on-medium). reasoning_template_kwargs distinguishes unset (inject nothing; template default rules) from the explicit "none" off-switch. apply_temperature skips temperature unless reasoning is EXPLICITLY off on none-declaring models (unset leaves the server default in charge, possibly reasoning-on). - session: ctor takes temperature: float | None / reasoning_effort: str | None = None; _save_config/resume round-trip unset as "" (the str(None) era guarded); _run_agent relays session temperature AND effort on the same-alias fall-through only (a task alias's configured knobs stay reachable in both directions). - optimizer: the five meta lanes are decoupled from --temperature/ --reasoning-effort (test-model knobs, per their documented meaning); registry-less meta lanes omit both fields. - cli: --temperature/--reasoning-effort default unset and fall through the model config instead of pinning 0.5/"medium" for every CLI session. - cleanup from the review's below-cap findings: dead resolve_server_type deleted (tests re-pointed at _server_type_of), stale ChatSession comments in _openai_responses fixed, _store_get_or_none extracted, eval system-turn conversion hoisted out of the per-turn loop, dead _provider_extra_params patch removed, test_perception uses the shared mock_completion_result, effort_ladder uses apply_capability_overrides instead of a SimpleNamespace fake config. Wire goldens regenerated: the only drift is the manufactured "medium" effort vanishing from unset-effort requests (Responses reasoning.effort, Chat/Google reasoning_effort, Anthropic output_config.effort) — pure removals, no additions. Ladder tests now fake ConfigStore with the REAL get() semantics (registry default on miss) so a forgiving fake can't mask this class of bug again.	2026-07-13 08:48:27 -07:00
Patrick Buckley	09fd17f2da	fix(model-turn): reasoning effort rides the ladder too; round-1 review fixes ... Patrick's rulings applied from the round-1 high review: - reasoning_effort loses every code pin, same as temperature: ModelLane resolves the ladder (ModelConfig.reasoning_effort → global model.reasoning_effort setting → the lane capabilities' default_reasoning_effort), model_turn takes str | None, and the pins in both judges, all five optimizer lanes, _utility_completion's signature default, and model_turn's own "medium" default are gone. Explicit relays of user/operator knobs (session effort on the agent seam and web-fetch, harness knobs in eval) stay relays. Effort's terminal is the caps default, not wire omission — it gates thinking modes, so unset ≠ the explicit "none" value. - model.temperature setting default 0.5 → 1.0 (safer for modern models; several providers no longer accept temperature at all — those drop it via capabilities regardless). No judge-specific knob: a judge alias with a per-model override is the remediation path. - The agent seam keeps the alias ladder (configured → inherited global → none), per ruling; the ModelLane docstring no longer documents the removed session-relay convention. - Optimizer lanes get real operator knobs: the existing --temperature / --reasoning-effort CLI flags now relay into all five internal LLM steps (previously they reached only the eval sessions, leaving the deleted pins with no replacement mechanism). Round-1 cleanups: create_streaming widened to float | None (the Protocol's two entry points agree; all callers pass explicitly); model_turn's provider invocation is a direct keyword call again (strict mypy re-checks it); perception threads the caller's already-resolved capabilities (one config generation across gate and wire); redundant extra_params pre-resolution dropped at utility/agent/eval; the synth source-tag joins the one-fetch-per-call cfg chain; effort_ladder delegates its capability merge to resolve_capabilities; stale _maybe_synth_reasoning_block pointers fixed in the providers package. Wire goldens regenerated: the only drift is the hidden "temperature": 0.5 pin vanishing from unset-temperature requests.	2026-07-13 08:48:27 -07:00
Patrick Buckley	a029849724	fix(models): keep raw exception text out of client-construction 503s ... Review: the wrapped ValueError is echoed in 503 bodies, and arbitrary SDK exception text can embed filesystem paths. Echo the exception type only; log the full exception with traceback at the raise site.	2026-07-06 21:10:27 -07:00
Patrick Buckley	9c2e809b26	fix(models): surface client-construction failures as factory misconfig ... SDK client construction can fail on environment problems the config never sees (e.g. httpx resolving a certifi CA path deleted by a venv rebuild). Those escaped as bare exceptions and turned every workstream open/create into an opaque 500; re-type them as ValueError in ModelRegistry.get_client so routes answer 503 with the message and the alias.	2026-07-06 21:10:27 -07:00
Patrick Buckley	8f347da653	fix(registry): normalize config.toml context_window=0 to auto-detect ... context_window=0 is the documented auto-detect sentinel -- "inherit the CLI-detected window." The DB loader applies it (row.get(k, 0) or context_window); the config.toml loader did not (entry.get(k, default) only substitutes a MISSING key), so an explicit context_window = 0 leaked a literal 0 downstream, zeroing every budget that reads ModelConfig.context_window -- judge lowering, session compaction. The DB loader's comment even claimed config.toml shared "the same fallback chain," which was false. Match the DB loader at the source. The judge-side _positive_window coercion stays as defense-in-depth, but its comments no longer misframe 0 as garbage -- it's a valid sentinel, now normalized at load.	2026-07-02 22:00:19 -07:00
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.	2026-06-28 04:09:30 -07:00
Patrick Buckley	68ed0b6c15	feat(rerank): per-model calibration on detect + calibrate endpoint ... Phase 3. Stores reranker calibration per-model on the model definition's capabilities (rerank_threshold/rerank_scale/rerank_separated; a non-empty rerank_scale is the "has been calibrated" marker) instead of a single global threshold, populated automatically when a reranker endpoint is detected. - ChatSession._bm25_rerank_threshold precedence: the active reranker model's calibrated rerank_threshold (when separated) wins; calibrated-but-not- separated -> 0 (no floor); else the global tools.rerank_bm25_threshold fallback. Reads the raw caps dict, in-memory per turn. - Detect (admin_detect_model) calibrates a supports_rerank endpoint and autopopulates the three fields like context window; the create-model UI shows a verdict chip (calibrated / no-clean-separation / not-calibrated). - POST /api/admin/model-definitions/{id}/calibrate backs the Re-calibrate button; calibration runs off the event loop (run_in_executor, bounded by asyncio.timeout(90)), persists the fields + refreshes the registry, and is graceful on a down/slow endpoint (never 500). Shared merge_calibration_into_caps helper used by the endpoint and the CLI. - turnstone-admin rerank-calibrate is now per-model: --model <alias> required; --apply writes that model's caps (not the global setting). A no-separation result records the marker (calibrated, no floor) consistently across CLI and endpoint. The serving lesson stays documented: Qwen3-Reranker needs vLLM --chat-template or its scores are near-random (live-validated 0.6B + 4B: the calibrated floor came out 0.95 vs 0.33 for the same task -- why per-model calibration exists). Negative-tested: the floor-precedence branches (calibrated+separated -> per- model, calibrated+!separated -> 0, uncalibrated -> global, no-alias -> global, registry-must-not-be-consulted), endpoint persist/refresh + graceful failure, detect-skips-non-rerankers, the CLI per-model write, and the caps-merge preserving supports_rerank. Chip states verified via headless Chrome.	2026-06-01 15:47:14 -07:00
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.	2026-05-31 19:54:43 -07:00
Patrick Buckley	cc4d9b1a8d	fix(server): boot nodes with no models in a degraded state ... A node refused to start with no model configured and no LLM reachable, so a fresh cluster couldn't come up to be configured. load_model_registry now accepts allow_empty and returns an empty registry; ModelRegistry permits the empty state (default unset); the server passes allow_empty so a node registers and shows in the console, then picks up models added in the admin UI live. The CLI keeps failing fast — a REPL with no model is unusable.	2026-05-31 16:05:21 -07:00
Patrick Buckley	c58df26a30	style(tests): ruff-format provider/registry empty-string tests	2026-05-31 13:06:24 -07:00
chrismuzyn	85db6895e3	make sure empty strings don't get passed to the openapi sdk which don't ... allow env var fallback	2026-05-31 13:06:24 -07:00
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.	2026-05-24 17:49:27 -07:00
Patrick Buckley	ee163c0ae4	fix(session): prevent LLM bypass of per-role plan/task model overrides ... The LLM was passing ``task_agent(model="default")`` (and the same for plan_agent) and routing to whichever backend the auto-created ``default`` alias was attached to at boot — flatspark in the verified case (ws_id 7dde674) — silently bypassing the operator-configured ``model.task_alias`` / ``model.plan_alias`` (gh200). Root fix: - ``load_model_registry`` only synthesises the back-compat ``default`` alias when neither DB nor ``[models.*]`` populate the registry. The shim was only ever meant for single-CLI-model setups; with a multi- model DB it became a phantom routing target aliasing ``LLM_BASE_URL``. - ``_render_agent_tool_descriptions`` filters ``default`` out of the LLM-visible alias list. The English reading of "default" trips the model into picking it explicitly even when the description tells it to omit ``model=`` for the per-role default. Defense-in-depth at the validator chokepoint (``_validate_agent_model_override``): explicit rejection of ``alias == "default"`` (post-strip) with corrective guidance; ``default`` filtered out of the unknown-alias retry list so an LLM probing with a bogus alias can't enumerate it back; the no-alternatives wording is distinguished from the no-registry-configured wording. The render path also always rewrites tool descriptions instead of returning early on filter-empty, so a reload that drops the registry to only ``default`` clears stale alias names left over from a prior render.	2026-05-11 16:53:50 -07:00
Patrick Buckley	33865ca9d2	fix(reasoning): apply full-stack review findings ... Multi-stage /review on the full Phase 1+2+3+4 stack surfaced 9 findings (0 critical, 3 major, 5 minor, 1 nit, 1 uncertain). All applied. Major * perf-1 (session_routes.py:2402): make_history_handler ran sync storage.load_workstream_config inside async def history on the cold- workstream path, blocking the event loop on every dashboard /history request for non-resident workstreams. Every other storage call in the same handler correctly used asyncio.to_thread. Wrap the sync call in asyncio.to_thread (preserving the existing try/except so a DB failure still degrades to the conservative-default branch instead of bubbling out). * q-2 (test_reasoning_audit_log_discipline.py): the security-sensitive test (reasoning text never lands at INFO+ severity) only covered the 4 Phase 1 surfaces. Phase 2 added the strip predicate in AnthropicProvider._convert_messages and Phase 3 added 3 more code paths that touch reasoning text — none guarded. Added 4 parallel tests using the existing capture-and-walk infrastructure: OpenAIResponsesProvider.extract_reasoning_text, OpenAIChatCompletionsProvider.extract_reasoning_text, ChatSession._stream_response (drives the synth-block stamp via a fake reasoning-emitting stream), AnthropicProvider._convert_messages with replay_reasoning_to_model=False (drives the Phase 2 strip predicate). * q-1 (model_registry.py:42): the persist_reasoning flag name implied storage-control but actually gates UI rehydration only — operators flipping it could reasonably expect "stop persisting reasoning" but storage of reasoning bytes happens in provider_data regardless. Renamed everywhere to surface_persisted_reasoning: ModelConfig field, migration 052 column (renaming in-place since 052 is not yet on main), schema, MODEL_DEFINITION_MUTABLE allowlist, _postgresql.py + _sqlite.py CRUD impls, _protocol.py create_model_definition signature, 3 console_schemas Pydantic models, console/server.py admin POST + PUT, model_registry row mapper, history_decoration.py helper parameter, server.py _build_history local var, session_routes.py make_history_handler local var, sdk/events.py HistoryEvent docstring, admin.js form id + override pill label, index.html form input id + UI label + tooltip, coordinator.js (none needed), and every test that referenced the old field name. The admin tooltip now reads "Storage of reasoning bytes is unaffected by this flag — they ride in provider_data regardless" so the decoupling stays explicit at the operator surface. Minor * bug-1 (history_decoration.py:336): dispatcher discriminated on provider_content[0]["type"] only. Anthropic's redacted_thinking blocks (sealed by the safety system) can appear before, after, or interleaved with regular thinking blocks per the API docs. When a redacted block lands first, the dispatcher returned "" and the UI silently lost the surrounding thinking text. Registered "redacted_thinking" as a second key in _BLOCK_TYPE_PROVIDER_FACTORY pointing at the same AnthropicProvider factory — the existing extractor's type=="thinking" filter already correctly skips redacted blocks while walking the full list. Regression test added. * q-3 (_protocol.py:155): replay_reasoning_to_model defaults split across 9 sites — operator-side defaults to False (matches DB server_default), provider-API defaults to True (back-compat with direct callers). Original "pick False everywhere" fix would have silently flipped behaviour for any direct provider caller. Instead documented the intentional bifurcation in the Protocol's create_streaming docstring. * q-4+q-5 (_protocol.py:107 + 3 providers): MAX_REASONING_DISPLAY_BYTES was enforced via Python str slicing which counts code points, not UTF-8 bytes — 4-byte CJK/emoji glyphs would blow past the byte ceiling. Renamed to MAX_REASONING_DISPLAY_CHARS to match actual behaviour. Hoisted the 4-line truncation pattern into a shared _join_reasoning_with_cap helper in _protocol.py; each provider's extractor becomes a single line at the tail. * q-6 (tests/_session_helpers.py): _NullUI + _make_session were duplicated verbatim between test_session_replay_reasoning.py and test_session_synth_reasoning_block.py. Hoisted to a shared tests/_session_helpers.py module (importable, leading underscore so pytest doesn't try to collect it). test_model_registry.py's _make_session has a different signature (registry/model_alias args + _FakeUI) and is not a candidate for sharing. Nit * q-7 (history_decoration.py:286): _make_provider_factory used a dict-as-cell workaround for closure read-only scope. Replaced with the more idiomatic nonlocal pattern. Lint + test gate * ruff check + ruff format -- clean. * mypy -- no issues across all 191 source files. * pytest -m 'not live' -- 6115 passed (3 deselected). Net +5 tests (4 audit-log discipline + 1 redacted_thinking dispatcher). Refinements vs the dedupe output (caught during sanity rendering the report) * perf-1 fix preserved the try/except wrapper. The original "wrap in to_thread" one-liner would have let an OperationalError bubble out instead of degrading to the fallback branch. * q-3 fix explicitly documented the bifurcation rather than collapsing both sides to False. "Pick False everywhere" would silently flip back-compat behaviour for direct provider callers. * q-1 fix included the admin.js:5292 fallback site (m.persist_reasoning !== false) that the original threaded-change list missed. * q-6 fix verified the third _make_session in test_model_registry.py is structurally different (different signature + different UI helper) and intentionally NOT a dedupe target.	2026-05-09 02:45:13 -07:00
Patrick Buckley	1873e7a758	feat(reasoning): persist reasoning text on history payload (Phase 1) ... Surface stored Anthropic thinking blocks on /history responses so refreshing the page rehydrates the reasoning bubble. Wire payloads unchanged. Per-model operator knobs added to model_definitions for both UI rehydration and (Phase 2) wire-build replay. Why now: reasoning is already round-tripped via _provider_content for Anthropic-with-thinking turns, but never surfaces on the history wire, so a tab reload showed only the final answer with no rationale. Operators also have no per-model lever to opt out of UI display or to opt in to replay-to-model on subsequent calls. What this change does * Migration 052 adds two boolean columns to model_definitions: persist_reasoning (default 1) controls UI rehydration; replay_ reasoning_to_model (default 0) reserved for Phase 2's wire-build shape filter. Mirrors the enabled column pattern (NOT NULL + integer server_default). * LLMProvider Protocol gains extract_reasoning_text(provider_blocks) with concrete impls on AnthropicProvider (walks type=='thinking' blocks, joins with newline, caps at 64 KiB) and no-op stubs on OpenAIChatCompletionsProvider + OpenAIResponsesProvider. Google inherits the no-op via OpenAIChat. Phase 3 will wire the OpenAI Responses extractor once include=['reasoning.encrypted_content'] is requested. * turnstone.core.history_decoration gains a structural dispatcher extract_reasoning_text_from_provider_content keyed off the first block's type field (Anthropic 'thinking' / OpenAI Responses 'reasoning' / Gemini 'thought' are non-overlapping by API design). Both history surfaces use it: _build_history calls the dispatcher directly (the SSE-replay path builds entry dicts from scratch), and the lifted make_history_handler runs the list-helper variant in the existing to_thread block. * make_history_handler resolves persist_reasoning via three tiers: live session -> workstream_config.model_alias (the same key SessionManager uses to rehydrate the original model after process restart) -> conservative True default. Operator flag-flip takes effect uniformly on both warm and cold workstreams. * Frontend: app.js replayHistory and coordinator.js role==='assistant' branch each call the existing reasoning-bubble construction (for app.js, the document.createElement pattern from the live SSE handler; for coord, the appendMsg('reasoning') helper) when msg.reasoning is non-empty. Reasoning bubbles render before the content bubble, matching live SSE order. * Admin UI: two checkboxes ('Persist reasoning', 'Replay reasoning to model') in the model edit modal, plus override-pill display in the model row when set to non-default values. What is intentionally out of scope * Phase 2 -- ANTHROPIC_VALID_BLOCK_TYPES shape filter at _anthropic.py:312-316, _convert_messages replay_reasoning_to_model parameter, thinking-strip branch, _msg_text_chars token-calibration extension. The replay flag is stored but not consumed on the wire. * Phase 3 -- OpenAI Responses include=['reasoning.encrypted_content'], Gemini include_thoughts spike, ModelCapabilities.supports_ reasoning_replay. * Phase 4 -- Local-model / chat-template reasoning persistence (session.py:3486 reasoning_parts accumulator). Tests * AnthropicProvider.extract_reasoning_text -- 13 unit tests covering None / empty / mixed / multi-block / cap / malformed / non-list inputs plus other-provider no-op verification (real provider instances, no mocks). * extract_reasoning_for_history -- 10 dispatcher tests including block-type discriminator routing (thinking vs reasoning vs unknown), strip-when-flag-false, empty / non-dict guards, and cross-role isolation. * _build_history -- 6 boundary tests through the real Anthropic extractor with stub sessions, including the registry-lookup failure default-True branch. * make_history_handler -- 5 round-trip tests through real storage: the storage layer's reconstruct_messages decodes provider_data into _provider_content, and the helper extracts through the real AnthropicProvider. Includes the live-session flag honoring path, the cold-workstream workstream_config lookup path, and the no-alias default-True fallback path. * Audit-log discipline -- 4 structural mock-and-assert tests that capture every Logger.info / warning / error call across the pipeline (extractor, dispatcher, list-helper, _build_history) and assert no captured payload contains a marker reasoning string. * model_definitions storage -- 6 round-trip tests: default flags, explicit create with both flags, individual update of each flag, and list-includes-flags assertion. * model_registry -- 4 tests: dataclass defaults, dataclass with explicit flags, DB-row-mapping with both flags, and pre-052 legacy-row default-fallback. Edge cases pinned by the test suite * Pre-052 DB rows missing the new columns degrade to dataclass defaults (test_db_reasoning_flags_default_when_absent). * Live session in memory has its flag honored (test_history_handler_ with_persist_flag_false_via_live_session). * Cold workstream resolves the flag via workstream_config + app.state.registry (test_history_handler_cold_workstream_resolves_ via_workstream_config) -- this closes the gap where a process restart would have silently un-honored an operator flag-flip. * Cold workstream without persisted model_alias falls through to default True (test_history_handler_cold_workstream_no_alias_ defaults_true). * Foreign / unknown / missing block types degrade silently to no reasoning field rather than misroute or crash. Lint + test gate * ruff check + ruff format -- clean. * mypy -- no issues across all 191 source files. * pytest -m 'not live' -- 6030 passed (3 deselected).	2026-05-09 02:45:13 -07:00
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.	2026-05-08 18:23:38 -07:00
Patrick Buckley	32fd8f29c7	feat(providers): api_surface toggle + mistral medium reasoning fix (#469) ... * feat(providers): api_surface toggle + mistral medium reasoning fix Mistral medium open-weights served by vLLM expects reasoning_effort via the Responses API (`reasoning.effort`), not as a `chat_template_kwargs` entry on Chat Completions. The session was unconditionally injecting `{"reasoning_effort": ...}` into `chat_template_kwargs` for every openai-compatible request, which corrupted the prompt rendering for any backend whose chat template didn't consume that key (Mistral medium, Mistral cloud, Groq, OpenRouter). Changes: - Add `api_surface` ("chat" | "responses") to `ModelConfig.server_compat` and thread it through `create_provider` / `model_registry.get_provider`. `openai-compatible` defaults to Chat Completions; operators can flip individual aliases to Responses for endpoints that support it. - New `vllm-mistral-medium` profile that pre-fills api_surface=responses on Detect for known Mistral medium model ids. - Drop the unconditional `reasoning_effort` injection into `chat_template_kwargs`. Operators running gpt-oss-style local templates that consume `reasoning_effort` from the chat template now opt in via `server_compat.extra_body.chat_template_kwargs`. - New "API Surface" select in the Models admin tab; allowlist-validated server-side at create/update time; pre-filled by Detect via the profile suggestion. - Evict the cached provider singleton in `ModelRegistry.reload()` when api_surface changes (previously only cfg.provider triggered eviction). - Fix `_run_agent` fallback path to inherit the session's primary alias for capability and server_compat resolution; previously the fallback passed `alias=None`, which silently dropped per-model caps on the agent path. Tests: 5117 passed (-m "not live"); ruff + mypy clean. * fix(providers): don't auto-suggest Responses for Mistral medium vLLM's Responses API surface for Mistral medium open-weights doesn't wire up the Mistral tool-call parser as of vLLM 0.x — tool calls leak into the response as ``[TOOL_CALLS]<name>{...}`` text instead of structured tool_calls. Chat Completions on the same engine handles tools cleanly via ``--tool-call-parser mistral``, and reasoning can be turned on via the vLLM CLI ``--reasoning-parser`` flag. Drop the auto-suggest mapping so Detect falls back to the generic ``vllm`` profile. Keep the ``vllm-mistral-medium`` profile definition in place so an operator who specifically wants per-request effort and accepts the tool-calling limitation can still pick "Responses API" manually in the admin UI. * fix(providers): address Copilot review on PR #469 - providers/__init__.py: drop the redundant *_responses_provider / *_chat_provider names; have create_provider use _openai_provider and _openai_compat_provider directly so they're not flagged as unused globals. - console/server.py: tighten _validate_api_surface to a strict equality match against the canonical {"chat", "responses"} set. The previous strip().lower() membership check accepted ' Responses '/'CHAT' but stored the raw string verbatim, which then failed to round-trip through the admin <select>. - console/static/admin.js: gate the entire server_compat block (server type, api_surface, extra_body) on provider == "openai-compatible" at save time so toggling provider away can't leave a stale hidden surface selection in the persisted capabilities JSON. - tests/test_session.py: splat the bad kwarg via **dict so CodeQL no longer flags the call as a wrong-name keyword (the point of the test is the runtime contract, not the static type). - tests/test_admin_model_registry_refresh.py: add endpoint-level tests for the api_surface validation on both create and update — covers the bogus-value rejection, non-canonical-string rejection, and the happy path persisting through to the refreshed registry.	2026-05-03 13:37:50 -07:00
Patrick Buckley	3308e3645a	fix(core): scope rehydrate fallback to manager, fix resume orphan ... Address Copilot feedback on PR #465: 1. The has_alias fallback in both session_factories silently rewrote any unknown caller-supplied alias to the default, including on the fresh-create path where the create handler maps the factory's ValueError to a 503 with operator-friendly text. A typo in body.model would now silently start a workstream on the default instead of telling the caller their requested model could not be resolved. Move the fallback out of the factories: each factory raises again on unknown aliases, and SessionManager filters stale aliases out of the rehydrate path via a new ``model_validator`` constructor kwarg (production wiring passes ``registry.has_alias`` on both interactive and coordinator). 2. ChatSession.resume()'s elif branch flipped self.model to the persisted model name even when the alias was unresolvable, leaving the session paired with the constructor's default provider/client but a removed model name — a broken state whose next API call fails. Drop the model copy: keep the constructor's coherent default (provider + model + capabilities) and just log the unreachable saved values so the missing alias is auditable. Tests: - Move stale-alias coverage from the factory level into SessionManager (tests/test_session_manager.py): validator drops stale aliases before reaching build_session; live aliases pass through unchanged. - tests/test_sessions.py renamed test_resume_restores_model → test_resume_keeps_defaults_when_alias_unresolvable to match the new contract.	2026-05-02 19:02:48 -07:00
Patrick Buckley	0a43bed3d5	fix(core): preserve workstream model + config on rehydrate ... SessionManager.open() was calling build_session(ws) without a model arg on the rehydrate path. The session_factory then resolved the *current* default alias, ChatSession.__init__'s _save_config() (INSERT OR REPLACE per-key) clobbered the persisted workstream_config with those defaults, and the subsequent resume() "restored" what was now the default — silently resetting model_alias, model, temperature, reasoning_effort, max_tokens, skill, creative_mode, instructions, token_budget, and notify_on_complete on every reopen and every service restart, for both interactive and coordinator workstreams. Three layers: 1. SessionManager.open() now reads workstream_config via self._storage.load_workstream_config(ws_id) and threads the saved model_alias into build_session(ws, model=saved_alias). 2. ChatSession.__init__ now skips its initial _save_config() when a workstream_config row already exists for self._ws_id — protects every other persisted knob without having to plumb each one through the adapter signature, and catches any future construction path that forgets to thread model through build_session. 3. Both session_factories (server.py interactive, console session_factory.py coordinator) now treat an unknown caller- supplied alias the same as an unset alias: fall back to the runtime default rather than raising. Without this, a workstream pinned to an alias an operator has since removed from the registry would 500 on every reopen — defeating the "best effort restore, default if the original is gone" contract this fix is meant to deliver. Mirrors _effective_default_alias's existing has_alias guard against a stale ConfigStore default.	2026-05-02 19:02:48 -07:00
Patrick Buckley	e3f2237c36	refactor(coord): hygiene pass on coord_registry refresh — async + selective teardown + test cleanup ... Hygiene follow-ups from the multi-stage code review on #453. perf-1 — sync helper called from async route handlers ``_refresh_coord_registry`` runs two sync DB reads and a registry reload that takes ``_client_lock``; calling it directly from an async handler held the event loop for the duration. All four call sites now ``await asyncio.to_thread(_refresh_coord_registry, ...)``, matching the pattern from commit ``1f7d6ad`` (offloaded ``tenant_check``). perf-3 — ModelRegistry.reload() tore down all clients unconditionally The reload always closed every cached client and provider, even when the changed fields (``model``, ``temperature``, ``context_window``) didn't touch the connection target. Now selective: clients drop only when alias removed or ``(base_url, api_key, provider)`` differs; providers drop only when alias removed or ``provider`` string differs. Keeps connection pools warm across the common admin-edit case where only metadata changed. Two new ``test_model_registry`` cases lock the keep-warm vs drop-on-change behaviour, and the existing ``test_reload_clears_clients`` was updated (it asserted the old overly-aggressive contract) into ``test_reload_keeps_clients_when_connection_target_unchanged``. q-5 — helper rename ``_refresh_console_coord_registry`` → ``_refresh_coord_registry``. The ``console_`` prefix was redundant given the function lives in ``turnstone/console/server.py`` and sibling helpers there (``_notify_nodes_model_reload``, ``_publish_config_change``, ``_collect_model_status``) all omit it. q-1 — shared test middleware ``tests/test_admin_model_registry_refresh`` now imports the header-driven ``_AuthMiddleware`` from ``tests/_coord_test_helpers`` and sets default ``X-Test-User`` / ``X-Test-Perms`` headers on the ``TestClient``. The local hardcoded variant duplicated infrastructure the helper module exists to centralise. q-3 — multi-alias test registry ``_make_registry`` extracted a ``_make_config`` helper and gained an ``extras={alias: model}`` param so multi-alias scenarios stop hand-building ``ModelConfig`` literals. ``test_delete_endpoint_refreshes_registry`` now uses the helper. 310 tests pass across the related coordinator + model surfaces.	2026-04-29 18:25:40 -07:00
Patrick Buckley	48c9ad2a40	refactor(core): split SessionKindAdapter Protocol into construction +… (#412) ... * refactor(core): split SessionKindAdapter Protocol into construction + emission (Stage 2 P3) The single ``SessionKindAdapter`` Protocol that ``SessionManager`` takes is split into two: * ``SessionKindAdapter`` — kind / build_ui / build_session / cleanup_ui. Required for every kind. The shared lifecycle manager always delegates here for construction + cleanup. * ``SessionEventEmitter`` — emit_created / emit_state / emit_rehydrated / emit_closed. **Optional**, wired through a new ``event_emitter: SessionEventEmitter | None = None`` kwarg on ``SessionManager``. Reserved for future kinds whose lifecycle transitions don't fan out anywhere; both production kinds wire one today. Both production adapters implement both Protocols. The interactive lifespan (``server.py``) and console lifespan (``console/server.py``) pass their adapter as both ``adapter`` and ``event_emitter`` — production behaviour is unchanged. Six lifecycle sites in ``SessionManager`` (create / open eviction / open rehydrate / close / set_state / close_idle / _reserve_and_install_locked unwind) now call ``self._event_emitter.emit_*(...)`` guarded by ``if self._event_emitter is not None``. InteractiveAdapter asymmetry preserved + documented: * ``emit_closed`` stays load-bearing — it's the **sole** transport path for ``ws_closed`` onto the process-wide global SSE queue (Stage 1 consolidated emission from the create handler here so there's exactly one emission point; ``name`` powers the frontend's eviction toast). * ``emit_created`` / ``emit_state`` / ``emit_rehydrated`` are documented no-op stubs (``del ws[, state]``). Those events fire from out-of-band paths — the create HTTP handler enqueues ``ws_created`` directly onto ``global_queue`` *after* attachment validation (so a rejected upload doesn't surface a phantom create→close pair); ``WebUI._broadcast_state`` emits the full ``ws_state`` payload (tokens + context_ratio + activity) via the ``SessionUI.on_state_change`` callback chain. The stubs exist solely to satisfy ``SessionEventEmitter`` Protocol so the adapter can be wired as the manager's ``event_emitter`` for the ``emit_closed`` path. Each stub has a 1-line inline rationale to match the in-repo convention (``coordinator_adapter.py:210``). Test scaffolding: * ``tests/test_session_manager.py`` — ``_make_manager`` and ``_make_with_writer`` wire ``FakeAdapter`` as both ``adapter`` and ``event_emitter`` for production parity; the standalone ``test_create_uses_configured_node_id`` does the same. ``FakeAdapter.emit_rehydrated`` now records as ``_Event("rehydrated", ...)`` rather than conflating with ``"created"``, and ``test_open_resurrects_closed_state`` asserts against ``events_of("rehydrated")`` so a regression where the manager fires the wrong call on the open path actually fails. * ``tests/_coord_test_helpers.py`` and ``tests/test_coordinator_end_to_end.py`` — wire ``CoordinatorAdapter`` as both args. * Six interactive test fixtures (``test_skills.py``, ``test_prompt_templates_runtime.py`` x2, ``test_model_registry.py``, ``test_server_authz.py``, ``test_server_attachments_on_create.py``) — wire ``event_emitter=adapter`` so they match the production wiring, removing the footgun where a future contributor adds a ``gq.get_nowait()`` assertion and silently loses the only ``ws_closed`` transport. * ``tests/test_interactive_adapter.py`` — drops the three tautological no-op-emit_* tests (``test_emit_created_is_noop``, ``test_emit_state_is_noop``, ``test_emit_rehydrated_is_noop``); keeps the four ``emit_closed`` tests (real behaviour). Lint + mypy clean. 4475 tests passing. * docs(core): correct SessionKindAdapter + SessionEventEmitter docstrings to match implementation Two Copilot review threads on PR #412 caught the same real discrepancy: my P3 docstrings on ``SessionKindAdapter`` and ``SessionEventEmitter`` described an *intent* — "interactive doesn't implement ``SessionEventEmitter``; the manager skips emit calls when no emitter is wired" — that doesn't match the actual wiring. ``InteractiveAdapter`` does implement both Protocols and ``server.py`` does pass it as ``event_emitter``; only the three no-op stubs (``emit_created`` / ``emit_state`` / ``emit_rehydrated``) are dead, while ``emit_closed`` is load-bearing. Updated both docstrings to: * State that both production adapters implement both Protocols. * Explain the asymmetry is in *which* emit methods carry real bodies (coord: 4; interactive: 1, with 3 documented stubs because the out-of-band paths — create handler ``ws_created`` after attachment validation, ``WebUI._broadcast_state`` carrying the richer ``ws_state`` payload — fire those events). * Clarify the ``if self._event_emitter is not None`` guard exists for the kwarg-omitted case (tests that don't care about events, reserved for future kinds whose transitions don't fan out anywhere). Docstring-only change. Lint + mypy clean; the 75 tests in test_session_manager + test_interactive_adapter + test_coordinator_adapter pass. Resolves the two Copilot review threads on PR #412 (commits PRRC_kwDORcMomM67VyPD, PRRC_kwDORcMomM67VyPI).	2026-04-25 22:52:39 -07:00
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 a46dab1 removed row-level ownership gates — so _active_coords is now just coord_ws_id → ui. perf-6: _enqueue_on_ui was doing {**payload, "ws_id": coord_ws_id} on every dispatch. The dispatch path owns payload and doesn't reuse it — mutate in place. * test(coord): add adapter tests for worker dispatch + children registry + fan-out Fills the coverage gap on CoordinatorAdapter — the review (q-3) flagged the coord-specific concurrency paths ported from the deleted CoordinatorManager as untested. Three new test classes: - TestCoordinatorAdapterWorkerDispatch: _spawn_worker reuse gate, queue.Full backpressure, concurrent-call bug-1 reproducer (two threads → exactly one worker via ws._lock + _worker_running), finally-clears-flag. - TestCoordinatorAdapterChildrenRegistry: registry seed on emit_created vs emit_rehydrated rebuild, _pop_coord_registry_locked reverse-index cleanup, _merge_child_ids_locked idempotency, _prime_children_from_snapshot merge. - TestCoordinatorAdapterDispatchChildEvent: unknown-parent drop, ws_created fan-out, cluster_state / ws_closed reverse-index routing, perf-6 in-place ws_id stamp. * fix: regressions flagged by ultrareview Verify stage of the cloud review surfaced 6 confirmed regressions from Stage 1's adapter layer. Fixing together since they share the same root cause (plumbing moved into adapters without retiring the old emission paths). - Interactive adapter emit_created / emit_state / emit_rehydrated become no-ops. The create_workstream HTTP handler still fires ws_created (after attachment validation, per the pre-Stage-1 "no phantom events on rejected upload" contract); WebUI _broadcast_state still fires ws_state with the full payload (tokens + context_ratio + activity). Firing from the adapter too was duplicating both events. Also closes the phantom-ws-created regression (adapter fired before attachment validation ran). - emit_closed Protocol gains a ``name`` kwarg; the adapter is the sole emitter for ws_closed on interactive now, and the frontend eviction toast needs the name. Manager passes ws.name from close() / create()+open() eviction / close_idle paths. - _idle_cleanup_thread stops firing its own reason="idle" ws_closed — close_idle already fires via the adapter with reason="closed", and the frontend never differentiated the two anyway. - close_workstream_endpoint fix: "Cannot close last workstream" 400 was a stale error (the guard went away with the default-startup workstream). Return 404 on close() == False (which now means the ws was already closed or unknown). Also switches the audit actor from _require_ws_access's stored owner to _auth_user_id — the stored owner is metadata post-#400, so attributing actions to it misrepresents who actually did them. - CLI /ws close mirrors the same stale-error fix. - SessionManager.close now calls storage.delete_workstream_override alongside update_workstream_state, same as the old WorkstreamManager.close did. Without it overrides leak until tombstone cleanup. close_idle does the same. - SessionManager._reserve_and_install_locked records the eviction on turnstone.core.metrics so the global eviction counter keeps working. Old WSM did this inline; the unification dropped it. - ConsoleCoordinatorUI.on_rename now fans out to the cluster collector via a new class attribute ``_collector`` (set at console startup alongside ``_coord_mgr``). The old ``_on_rename_observer`` plumbing went away with CoordinatorManager and the "adapter emit_console_ws_rename runs from whichever code path renames" comment was aspirational — nothing actually did it. Full pytest: 4375 passed (tests/live + test_server_live.py excluded; both pre-existing live-backend failures unrelated to this branch). Ruff + mypy clean. * refactor(ui): extract SessionUIBase for shared UI scaffolding Direct response to review feedback that the unification wasn't merging enough of the two workstream kinds. WebUI (node) and ConsoleCoordinatorUI (console) both: - Keep a per-UI list of SSE listener queues guarded by a lock - Block a worker thread on _approval_event / _plan_event - Fan enqueued events out with the same ws_id-stamping pattern - Resolve approvals / plans with the same broadcast-then-signal pattern All of that now lives once in turnstone/core/session_ui_base.py. Both UIs subclass SessionUIBase; kind-specific bodies (WebUI's per-UI metrics + _broadcast_state + intent-verdict bookkeeping, ConsoleCoordinatorUI's collector fan-out) stay in the subclasses. WebUI.resolve_approval still overrides the base (it adds intent- verdict updates) but now calls super() for the shared broadcast + event-set steps. Same shape as the other approval/plan hooks: subclasses extend, base provides skeleton. Net file-level: +156 LOC for the base, -144 LOC across the two subclasses. The raw number is unexciting — but there's now a single source of truth for the listener + blocking-gate machinery, and bugs (like the duplicate ws_created / ws_state events that prompted this refactor) can't arise from the two implementations drifting. Full pytest: 4375 passed. Ruff + mypy clean. * refactor(ui): move metrics + verdict bookkeeping into SessionUIBase Second pass at unifying the two UIs. Per-workstream metrics accumulators (token counts, tool-call counts, context ratio, activity tracking), intent-judge verdict cache + pending-decision list, and the verdict-persistence path all move to SessionUIBase. Before: WebUI tracked all of it; ConsoleCoordinatorUI tracked none of it (a comment on the old on_intent_verdict literally admitted the deferral — "skip the persistence + late-decision plumbing that WebUI does"). Coord sessions never got verdict rows in storage, never had a user_decision stamped, and the dashboard had no way to show coord token usage because the data wasn't captured. Now the base class captures the data and persists the rows for every kind. Kind-specific broadcast (WebUI's _broadcast_state with rich per-UI payloads) stays on WebUI; prometheus counters on the node (_metrics.record_judge_verdict) stay on WebUI's on_intent_verdict override. Everything else shared. Behaviour change worth flagging: coord sessions now write intent_verdicts and output_assessments rows for every judge call and every output-guard warning. Previously silent; the storage rows now exist and any future coord-dashboard surface can read them. Shape of the unification: - resolve_approval: was overridden on WebUI (intent-verdict decision propagation); now lives on the base. Both kinds inherit unchanged. - on_intent_verdict: WebUI overrides only to add _metrics.record_*; rest of the body is the base. - on_output_warning: was on both separately; fully base-shared now. Full pytest: 4375 passed. Ruff + mypy clean. * fix: regressions flagged by second-pass review Three confirmed findings with direct fixes + a dedicated test file for SessionUIBase (was previously uncovered). bug-1 — Coord approve_tools didn't reset _last_verdict_decision or clear _llm_verdicts between approval rounds. WebUI did (inline). Coord inherited SessionUIBase.on_intent_verdict which stamps via the decision flag, so after the first resolve every subsequent round's verdicts were stamped with the prior round's user_decision before the user had decided the new round. Fix: add SessionUIBase._reset_approval_cycle() clearing both under _ws_lock; call from the top of both subclass approve_tools methods. Single-source invariant — can't drift again. sec-1, sec-2 — delete_workstream_endpoint and open_workstream's rehydrate path recorded the audit row under the stored ws.user_id ("owner_uid") rather than the authenticated caller. With row-level ownership gating gone (a46dab1), any team member acting on a peer's workstream produced an audit row naming the victim as the actor. Fix: pass _auth_user_id(request) as the audit actor, matching the pattern close_workstream already follows. q-2 — SessionUIBase had no direct tests. The new tests/test_session_ui_base.py covers listener fan-out, approval + plan blocking gates, intent-verdict cache + FIFO eviction, verdict persistence paths, output-guard persistence, the reset-between-rounds invariant (bug-1 regression test), a cross-subclass test that verifies BOTH WebUI.approve_tools and ConsoleCoordinatorUI.approve_tools call _reset_approval_cycle (verified it fails without the fix), and a concurrent enqueue/register smoke. Full pytest: 4395 passed (+20 new). Ruff + mypy clean. * fix: PR #408 review findings from copilot + code-quality Three substantive fixes + mechanical side-effect-in-assert cleanup. Copilot findings: - session_ui_base.py: on_intent_verdict had a race with resolve_approval. Previously acquired _ws_lock twice (read decision → release → if unset, acquire again to append). resolve_approval could interleave between the two acquisitions, swap-and-clear the pending list and set the decision — our verdict then got appended to the fresh (empty) list and stamped with the NEXT round's decision on the following resolve. Fix: decision-check + append under ONE acquisition; storage UPDATE (if decision already set) runs outside the lock. New regression test counts lock acquisitions during on_intent_verdict and fails if the two-phase pattern returns. - server.py close_workstream_endpoint: comment said "treat as already-closed success" but handler returned 404. Comment rewritten to match the 404 behaviour ("the ws isn't tracked here" is the only reachable meaning for close() → False now). - test_session_ui_base.py concurrency smoke: the test ended with ``pytest.assume = lambda ...`` — a leftover that mutates pytest globals and can surprise other tests. Replaced with explicit ``not is_alive()`` assertions so the "threads completed cleanly" intent survives -O optimization stripping. Code-quality (assert side-effects): Six ``assert mgr.open(...)`` / ``assert mgr.close(...)`` in test_session_manager.py stripped under ``python -O``. Mechanical fix: extract to local before asserting. Ignored the two "Protocol method body is `...`" flags — that's the standard Protocol idiom; replacing with ``pass`` or ``NotImplementedError`` changes typing semantics. Full pytest: 4396 passed.	2026-04-24 14:28:51 -07:00
Patrick Buckley	551fc43c15	feat: per-call model selection on plan_agent / task_agent (#361) ... * feat: per-call model selection on plan_agent / task_agent The calling LLM can now pass `model="<alias>"` to plan_agent or task_agent to override the operator-configured per-kind model for that one invocation. Useful when subtask difficulty varies within a session: the model can downgrade to a cheap alias for trivial work and reach for a stronger one when the problem is hard. Tool descriptions list the live registered aliases (refreshed when the operator hits "sync to nodes" / internal_model_reload), so the calling LLM always sees the current options. Bad aliases return a corrective error dict with the available choices so the LLM retries cleanly rather than failing silently. No whitelist — any alias the registry knows is acceptable; cost control is intentionally ceded to the model. No per-call effort override (out of scope; effort stays operator-configured). Resolution precedence in _run_agent: explicit per-call agent_alias override > registry per-kind (plan_model/task_model) > legacy agent_model > session model. The plan retry path (when _validate_plan fails) reuses the same alias so coaching reflects real model behaviour rather than a different model masking the signal. Implementation: - plan_agent.json / task_agent.json: optional `model` parameter. - ChatSession._validate_agent_model_override extracts and validates the arg; mirrors the existing empty-prompt error pattern. - _prepare_plan / _prepare_task stash the override in item["model_override"]; _exec_* pass it through. - _run_agent gains agent_alias kwarg with defence-in-depth ValueError on unknown alias. - _render_agent_tool_descriptions deep-copies plan/task entries before mutating description so the module-level TOOLS constant stays untouched across sessions; rebuilds the BM25 tool-search index when active so its text matches what the LLM sees. - server._broadcast_agent_tool_schema_refresh walks active workstreams on internal_model_reload so descriptions update without restart. * fix: clarify no-registry placeholder + avoid double BM25 rebuild Addresses Copilot feedback on PR #361. 1. plan_agent.json / task_agent.json placeholder said the parameter falls back to the "operator-configured plan/task model". That text is what no-registry sessions see (registry-bearing sessions get the templated description with the live alias list); for those single-model sessions, omitting the param falls back to the current session model, not an operator-configured one. Reword so the no-registry user gets accurate guidance. 2. _on_mcp_tools_changed already calls _rebuild_tool_search after merging MCP tools. _render_agent_tool_descriptions also rebuilt the BM25 index when active, so the MCP refresh path was rebuilding twice per refresh. Move the BM25 rebuild out of the private render helper into the public refresh_agent_tool_schemas wrapper — _on_mcp_tools_changed keeps calling the render helper directly (no double rebuild), and registry-reload callers go through the wrapper which still keeps the index in sync.	2026-04-16 11:50:13 -07:00
Patrick Buckley	6c026710ff	feat: ConfigStore + admin UI for plan/task agent model and effort (#360) ... * feat: ConfigStore + admin UI for plan/task agent model and effort Per-kind sub-agent routing was added in #359 but only via config.toml. Operators can now switch the plan_agent / task_agent model and reasoning effort at runtime from the admin Model tab without restarting. Adds four ConfigStore-backed settings: model.plan_alias — alias for plan_agent model.task_alias — alias for task_agent model.plan_effort — reasoning effort for plan_agent model.task_effort — reasoning effort for task_agent Server startup and internal_model_reload both apply these as overrides on top of the registry's config.toml-loaded values; the new logic computes "effective" values for all five model-routing fields and only calls registry.reload() when at least one differs. Admin UI: extracts ALIAS_SETTING_KEYS to a const used by both the dynamic-alias-choice injection and the empty-option label rendering. Adds INHERIT_EMPTY_LABEL_KEYS so plan_effort / task_effort show "(inherit)" for empty — distinct from the literal "none" choice (which actually disables reasoning, very different from leaving unset). Also fixes Copilot review feedback from #359: - _validate_effort treats empty / whitespace as unset rather than warning on benign explicit-empty configs (with .strip().lower() normalisation; "HIGH" and " low " now parse correctly) - turnstone.example.toml's reasoning_effort comment lists the full set of accepted values (none, minimal, low, medium, high, xhigh, max) * fix: apply routing overrides on config-reload + skip no-op model-reload Addresses Copilot feedback on PR #360. 1. Admin settings updates fan out via /_internal/config-reload, which only reloaded the ConfigStore — plan/task routing changes weren't visible until a model-reload or restart, defeating the runtime configurability this PR is meant to add. 2. /_internal/model-reload always called registry.reload(), churning cached clients even when nothing changed. Risky when fanned out across nodes (could close in-flight clients). Extracts two helpers in server.py: - _effective_routing(cs, ...) pure function: overlay CS values on base - _apply_routing_overrides(reg, cs) reload only when something differs Used by the startup path, config_reload (new), and model_reload (now short-circuits with a noop response when models + routing are unchanged).	2026-04-16 11:08:26 -07:00
Patrick Buckley	54dd557476	feat: split plan_model and task_model, configurable agent reasoning effort ... plan_agent and task_agent previously shared a single agent_model knob and plan_agent hardcoded reasoning_effort="high" in three call sites. They have different cost/latency profiles — plan is rare and benefits from a stronger model, task is frequent and benefits from a cheaper one — so sharing the knob undertunes both. ModelRegistry gains plan_model, task_model, plan_effort, task_effort. Per-kind overrides win over the legacy agent_model, which still works as the single-knob fallback for both. resolve_agent_alias(kind) and resolve_agent_effort(kind) centralise the resolution; PLAN_DEFAULT_EFFORT captures the back-compat "high" default in one place rather than at every call site. session._run_agent delegates resolution by label ("plan" vs "task"). The three hardcoded reasoning_effort="high" arguments are removed — behaviour is identical when no plan_effort is configured. Loader validates effort against {none,minimal,low,medium,high,xhigh,max} and warns + drops typos rather than passing them to the provider. ConfigStore parity and admin UI for the new knobs are deferred to a follow-up — config.toml-only is enough for the backend split.	2026-04-16 10:26:19 -07:00
Patrick Buckley	934cb075d6	feat: per-model sampling parameters (temperature, max_tokens, reasoni… (#350) ... * feat: per-model sampling parameters (temperature, max_tokens, reasoning_effort) Model sampling parameters were global-only settings applied uniformly to all models. Different models have fundamentally different requirements (o-series needs no temperature, Anthropic needs temp=1.0 with thinking, local models may need different max_tokens). This adds per-model overrides with global fallback so each model definition can specify its own defaults. Migration 036 adds nullable temperature, max_tokens, reasoning_effort columns to model_definitions. NULL inherits the global default from ConfigStore. The session factory and /model switch command both resolve per-model override → global fallback consistently. The admin UI model create/edit modal now has dedicated form fields for these parameters with client-side validation, a visual section divider, and per-model override hints in the model table rows. Removes vestigial model.name and model.context_window global settings (now handled per-model by the model registry) with startup warnings for existing config.toml users. * fix: defensive parsing for config.toml per-model sampling params Wrap temperature/max_tokens conversions in try/except with range validation. Invalid values log a warning and fall back to None (inherit global default) instead of aborting registry load.	2026-04-13 17:14:58 -07:00
Patrick Buckley	2b93598d68	feat: multi-model health tracking with runtime default and DB-only st… (#304) ... * feat: multi-model health tracking with runtime default and DB-only startup Replace active-probe circuit breaker with passive per-backend health tracking. Backends are marked degraded after consecutive failures and recover when a request succeeds — requests are never blocked. - Add model.default_alias ConfigStore setting for runtime default model - Make load_model_registry CLI args optional for DB-only startup - Per-(provider, base_url) health trackers via HealthTrackerRegistry - Two-pass fallback: prefer healthy backends, then try degraded - Remove BackendHealthMonitor, CircuitState, probe threads, cooldown - Remove circuit_state from API schema, SDK events, metrics, frontends * feat: add "Set Default" button to Model Definitions admin panel Show a "default" badge on the current default model alias and a "set default" action button on all other models. Clicking it writes model.default_alias via the settings API. The list endpoint now includes default_alias in the response so the UI can highlight it. * fix: address review feedback — metric scoping, effective default, session alias - Move turnstone_backend_up metric out of BackendHealthTracker into server callback; only the effective default backend drives the gauge - _build_health_dict resolves effective default via ConfigStore override - session_factory computes selected_alias once before registry.resolve - admin model-definitions endpoint returns effective default (not just override) so UI shows correct badge when ConfigStore is empty - Rename circuitTitle → healthTitle in console JS - Fix ruff SIM117 lint in test * fix: validate effective default against enabled models, degraded label, log normalization - admin model-definitions endpoint validates default_alias against enabled models using same fallback rules as load_model_registry - UI text "backend down" → "backend degraded" to match advisory semantics - Health tracker log uses normalized base_url from key, not raw argument	2026-04-04 23:44:29 -07:00
Patrick Buckley	f978e7facd	fix: skip chat_template_kwargs for commercial OpenAI API (#297) ... * fix: skip chat_template_kwargs for commercial OpenAI API OpenAI rejects chat_template_kwargs as an unknown parameter — it's only meaningful for local model servers (vLLM, llama.cpp, SGLang). Split OpenAIProvider into separate singletons for "openai" vs "openai-compatible" so _provider_extra_params can gate on provider_name instead of inspecting base_url. Also deduplicates agent inline code into the same method and fixes pre-existing test pollution where get_capabilities was mutated on the singleton without cleanup. * feat: add OpenAI Responses API provider for commercial models Split the OpenAI provider into three concrete implementations behind the LLMProvider protocol: - _openai_chat.py: Chat Completions API for local model servers (vLLM, llama.cpp, SGLang) - _openai_responses.py: Responses API for commercial OpenAI (GPT-5.x, O-series) - _openai_common.py: shared capability table, temperature/reasoning gating, cache retention, citations, usage extraction The Responses API handles reasoning_effort as a {"effort": value} dict, system messages as an instructions field, and tool format translation at the provider boundary. ChatSession is unchanged — the provider abstracts the API difference. Also fixes diff_file direction when comparing against provided content. * fix: Responses API input format and local model provider routing - Assistant input messages use plain string content (not output_text) - Tool call argument deltas match on item_id, not call_id - Auto-detect openai-compatible provider for non-api.openai.com URLs - Fix diff_file direction when comparing against provided content * fix: resolve env vars before provider auto-detection in config.toml models Config-file model entries using ${ENV_VAR} placeholders in base_url were not resolving env vars before _resolve_openai_provider(), causing commercial OpenAI configs to be misclassified as openai-compatible.	2026-04-04 18:36:37 -07:00
Patrick Buckley	2bb55590bf	feat: replace Redis MQ with direct HTTP transport (Phase 1) ... Delete the entire turnstone/mq/ package (broker, bridge, protocol, client) and turnstone/sim/ package. Remove Redis as a dependency. Channel gateway and console now communicate with server nodes via direct HTTP (httpx + httpx-sse) instead of Redis pub/sub and queues. Single-node deployments work with zero infrastructure beyond the database. Key changes: - Channel adapters use httpx POST for create/send/approve/close and httpx-sse for per-workstream event streaming - Console collector discovers nodes via services table instead of Redis SCAN - Console scheduler dispatches tasks via HTTP POST with DB-based leader election - Server registers in services table with 30s heartbeat - Server accepts optional ws_id in create request (for Phase 2 console-generated routing) - SDK events gain IntentVerdictEvent and OutputWarningEvent types - All docs, examples, bootstrap wizard updated 63 files changed, -5968 net lines (Redis transport fully removed)	2026-03-30 20:30:05 -07:00
Patrick Buckley	23fed785c4	feat: auto-detect model changes when LLM backend swaps models (#255) ... * feat: auto-detect model changes when LLM backend swaps models The BackendHealthMonitor already probes /v1/models every 30s but discarded the response. Now compares the detected model against the last known one and triggers a registry reload when it changes. - Extract _extract_context_window() helper for reuse across detect_model, probe_model_endpoint, and the health monitor - BackendHealthMonitor: new provider/initial_model/on_model_changed params; _check_model_change() fires callback on model swap - Server: wire _handle_model_change callback that updates cli_model_args and calls registry.reload(); guarded by _user_specified_model flag so --model overrides are never auto-replaced - Session: _refresh_model_from_registry() called at top of send(); two string compares when nothing changed, full re-resolve on swap - 7 new tests for _extract_context_window and model change detection * fix: address Copilot review on model re-detection - server: update cli_model_args only after successful reload (not before), add finally block for new_reg.shutdown(), guard against cli_model_args not yet initialized - session: wrap registry lookup in try/except for concurrent reload race, reset judge on model change, recompute tool_truncation when context_window changes in auto mode	2026-03-30 05:43:53 -07:00
Patrick Buckley	381651049b	fix: detect context window from vLLM max_model_len field (#252) ... * fix: detect context window from vLLM max_model_len field vLLM exposes the context window as max_model_len on the model object, not meta.n_ctx_train (llama.cpp format). Both detect_model() and probe_model_endpoint() now check max_model_len first, falling back to meta.n_ctx_train for llama.cpp. Fixes 32768 fallback on vLLM servers that report 262144+ token context windows. * test: add vLLM max_model_len detection tests Copilot review: new vLLM context window path had no test coverage. Add tests for probe_model_endpoint (max_model_len detected, preferred over meta.n_ctx_train) and detect_model (vLLM model object with max_model_len).	2026-03-30 04:19:24 -07:00
Patrick Buckley	497984b452	feat: database-backed model definitions with admin UI (#212) ... Add model_definitions table (migration 028) enabling model management via the admin console without SSH access or server restarts. Models defined in the database coexist with config.toml models through a per-node merge strategy — config.toml overrides DB for the same alias, DB-only models coexist alongside, no cross-node contamination. Storage layer: - model_definitions table with CRUD (SQLite + PostgreSQL) - MODEL_DEFINITION_MUTABLE allowlist, admin.models permission ModelRegistry integration: - load_model_registry() merges DB + config.toml + CLI models - context_window=0 auto-detects from provider capability table or inherits CLI-detected value (same as config.toml behavior) - ModelRegistry.reload() with validation, TOCTOU-safe accessors - internal_model_reload + internal_model_status server endpoints Admin API + UI: - 6 console endpoints (list, create, get, update, delete, reload) with admin.models permission, audit trail, provider validation - Models tab in System group with sky blue (--blue) accent color - Provider badges (openai/anthropic), source badges (config/db) - Write-only API keys (never readable, "***" sentinel on update) - Sync-pending indicator, mobile responsive, focus-trapped modal Also changes is_secret settings from write-blocked (403) to write-only across all settings, making judge.api_key configurable via admin UI.	2026-03-29 01:58:08 -07:00
Patrick Buckley	f74aa2264e	refactor: add is_error to on_tool_result protocol, remove text heuris… (#207) ... * refactor: add is_error to on_tool_result protocol, remove text heuristics Add is_error keyword arg to SessionUI.on_tool_result() so tools report errors structurally. Server and JS client no longer guess from output text prefixes — each tool sets the flag at the source. Bash tool: exit code >= 2 is error, exit code 1 is ambiguous (grep no-match). History reconstruction keeps text heuristic as fallback for pre-migration data. Update SDKs (Python + TypeScript), test mocks, docs, and diagrams. * fix: infinite recursion in _report_tool_result, signal exits, stale docs * fix: add _tool_error_flags to test_load_skill ChatSession stubs	2026-03-28 22:09:52 -07:00
Patrick Buckley	19c3a48b10	fix: server startup stampede — timeout model detection, non-fatal PG … (#132) ... * fix: server startup stampede — timeout model detection, non-fatal PG migrations detect_model() blocked the main thread for up to 400s when the LLM backend was unreachable (OpenAI SDK default: 600s read timeout × 2 retries × TCP retransmit). Cap startup detection at 10s with no retries — the BackendHealthMonitor handles ongoing availability probing after startup. PostgreSQL migrations via _run_with_pg_lock() crashed the server on lock contention when 10 containers stampeded the advisory lock simultaneously. Wrap in try/except matching the SQLite path — the entrypoint script already runs migrations before the server process starts. Health check start_period increased from 15s to 60s to accommodate the startup sequence under load. * fix: address review — narrow PG migration except, add detect_model test Narrow the PG migration except clause to (OSError, EOFError) so DDL errors still propagate. Add two unit tests for detect_model() verifying with_options(timeout=10, max_retries=0) is called and that connection errors in non-fatal mode return (None, None).	2026-03-20 20:01:21 -07:00
Patrick Buckley	5378b33641	feat: output guard — evaluate tool results before they enter context (#109) ... * feat: output guard — evaluate tool results before they enter context Add turnstone/core/output_guard.py — a time-budgeted heuristic that evaluates tool execution results after execution but before they enter the conversation context window. Priority-ordered detection (5s budget, highest priority first): 1. Prompt injection: override phrases, role injection, instruction override markers, meta-injection patterns 2. Credential leakage: API keys (OpenAI/GitHub/AWS/Google), PEM private key blocks, connection strings, .env secret format 3. Encoded payloads: script data URIs, hex shellcode sequences 4. Adversarial URLs: cloud metadata endpoints, credential query params 5. System info disclosure: private IPs, sensitive file paths Annotates and optionally redacts (credentials → [REDACTED:<type>]). Does NOT gate — surfaces warnings via on_output_warning callback. Integration: - Wired into session.py tool result loop via _evaluate_output() - JudgeConfig gains output_guard + redact_secrets fields (both default true) - SessionUI protocol gains on_output_warning callback - 25 compiled regex patterns, pure function, no I/O 29 tests covering all detection categories, benign output false positive checks, credential redaction, and time budget behavior. * fix: address PR #109 review — protocol, config, and guard fixes Copilot review feedback: - Replace _CLEAN singleton with _clean() factory to prevent mutable shared state (OutputAssessment has list fields) - Remove redundant second _CREDENTIAL_PATTERNS loop in _check_credentials - Evaluate text parts of list outputs (images) not just string outputs - Wire output_guard + redact_secrets through ConfigStore settings registry and _build_judge_config() so operators can configure via admin Settings tab - Remove --no-output-guard CLI flag claim from docs (use Settings tab) Typecheck fix: - Add on_output_warning to all SessionUI implementations: NullUI (eval, 5 test files), WebUI (server — emits SSE event), TerminalUI (CLI — ANSI colored warning), RecordingUI, FakeUI	2026-03-16 16:22:10 -07:00
Patrick Buckley	376da3d084	feat: prompt template tech debt — tests, read-only endpoints, double-… (#67) ... * feat: prompt template tech debt — tests, read-only endpoints, double-load fix, server creation modal Close test coverage gaps for prompt templates: - Resume with deleted template: verifies graceful degradation (template_content=None, warning logged) - Threading safety: concurrent set_template/init_system_messages with no race conditions - Factory passthrough: template kwarg propagation through WorkstreamManager.create() Add read-only template listing endpoints (read scope, no content exposed): - GET /v1/api/templates — prompt template summaries (name, category, is_default, origin) - GET /v1/api/ws-templates — enabled workstream template summaries (name, description, model) - Available on both server and console; Python + TypeScript SDK methods added - Console creation modal switched from admin endpoint to read-scope endpoint Eliminate double-load inefficiency in workstream creation: - Template validation moved before mgr.create() (no create-then-rollback on invalid template) - template kwarg plumbed through WorkstreamManager.create() and session factory - _SessionFactory Protocol added for proper mypy typing Add workstream creation modal to server web UI: - Name, model, template dropdown, ws_template/profile dropdown - Instrument panel aesthetic: gradient top border, blur backdrop, amber accent - Focus trap, Escape/Enter keyboard handling, loading state, error display - WCAG AA contrast compliance, reduced-motion support * fix: add list_ws_templates SDK methods + regenerate OpenAPI snapshots Add list_ws_templates() to Python SDK (async + sync) and listWsTemplates() to TypeScript SDK for the new GET /v1/api/ws-templates server endpoint. Add WsTemplateSummary + ListWsTemplateSummaryResponse TypeScript types. Regenerate openapi-server.json and openapi-console.json snapshots. Addresses Copilot review feedback on PR #67. * fix: skip template pre-validation when resuming a workstream When resume_ws is set, the request's template field is irrelevant — resume() restores the template from workstream_config. Pre-validating a stale template name would incorrectly return 400 before the resume even runs. Addresses Copilot review feedback on PR #67.	2026-03-15 02:09:31 -07:00
Patrick Buckley	8895bf07eb	feat: admin Settings tab — form-based editor replacing "coming soon" … (#60) ... * feat: admin Settings tab — form-based editor replacing "coming soon" stub Section-grouped layout with collapsible headers for all ~40 ConfigStore settings (model, session, tools, server, mcp, ratelimit, health, judge, memory). Type-appropriate inputs: CSS toggle for bools, number with min/max/step, select for choices, text for strings. Secret fields shown read-only. Source badge (storage/default), amber restart indicator. Inline save per field with dirty detection, row flash on success, reset to default via styled confirm modal. Full WCAG keyboard accessibility (Enter/Space on section headers, aria-labels, focus-visible). Mobile responsive single-column at <700px. Reduced-motion safe. * fix: Settings tab polish — help tooltips, context_window auto-detect, UX fixes Settings UI: - Help tooltips: ? button on ~25 settings with plain-English explanations and optional reference links (arXiv, Fowler, MCP spec). Click to toggle popover, Escape to dismiss, aria-expanded for accessibility. - Sections start collapsed for scannable overview. - Restart badge: hidden by default, shows when dirty, persists after save with amber glow. Positioned left of source badge. - Secret row alignment fixed (transparent border matches input box model). - Docs link in toolbar → Swagger UI Settings section. - Number inputs: spin buttons hidden (Firefox/WebKit), empty value guard, numeric dirty detection (0.1 vs 0.10 no longer false positive). - Secret reset button enabled when source=storage (clear legacy overrides). - Space key repeat guard on section headers. - Sidebar: sticky + max-height:100vh, no longer stretches with content. Backend: - context_window default changed from 131072 to 0 (auto-detect). Fallback lowered from 131K to 32K (realistic for local models when detection fails). Session normalizes 0→32768 defensively. - Settings registry: help + reference_url fields on SettingDef, richer descriptions for model/session/tools/judge/memory settings. - Schema API includes help + reference_url. - Bootstrap system prompt: added Runtime Settings section. Docs: tab counts updated to 13 across README, architecture, console, governance.	2026-03-14 13:12:40 -07:00
Patrick Buckley	fb190f8977	Normalize session_id into ws_id as sole persistent identity (#29) ... * Normalize session_id into ws_id as sole persistent identity Eliminate the separate session_id concept. The workstream ID (ws_id) is now the single identity used for both real-time routing and conversation persistence, removing a layer of indirection that was 1:1 in practice and buggy on resume (stale pointers, orphaned rows). Schema changes (migration 006): - Drop sessions table; add alias/title columns to workstreams - Rename conversations.session_id → ws_id - Rename session_config table → workstream_config (ws_id column) - Data migration remaps existing conversations to ws_id Storage/API renames: - register_session → register_workstream (already existed, merged) - save_message/load_messages now keyed by ws_id - resolve_session → resolve_workstream - ChatSession.session_id property → ws_id - ChatSession.resume_session() → resume() - resume_session field → resume_ws - SessionResumedEvent → WorkstreamResumedEvent - /api/sessions → /api/workstreams/saved - /sessions slash command → /workstreams - --session-retention-days → --retention-days Channel eviction recovery simplified: reuses old ws_id directly instead of get_session_id_by_ws() reverse lookup. * Fix Copilot review feedback: stale session wording in docs, regenerate OpenAPI spec - docs/channels.md: "resumes the session" → "resumes the workstream", "Session resumed:" → "Resumed:", "old session was pruned" → "old workstream was pruned" - docs/api-reference.md: "Each session object" → "Each saved workstream object", field descriptions updated, removed stale node_id field - sdk/typescript/openapi-server.json: fully regenerated from Python models — removes all stale session_id properties from WorkstreamInfo, DashboardWorkstream, CreateWorkstreamResponse schemas	2026-03-07 12:49:07 -08:00
Patrick Buckley	a20a058c59	Add cluster-scale schema, fix console proxy UX, harden SDK sync runner (#22) ... * Add cluster-scale schema, fix console proxy UX, harden SDK sync runner Schema redesign for multi-node deployments: - New `workstreams` table with node_id, state, lifecycle tracking - Add node_id + ws_id columns to sessions table with indexes - Full UUID (32 hex) for session_id and ws_id (was truncated 12/8) - Server generates and owns node_id, bridge retrieves via /health - Bridge retries with exponential backoff, fatal on auth errors - WorkstreamManager persists workstreams and state changes to storage - /health endpoint exposes node_id for bridge discovery Console proxy UX fixes: - Remove duplicate turnstone branding from proxy banner - Same-tab navigation for Open Node UI and workstream deep links SDK _SyncRunner fix: - Sentinel pattern for StopAsyncIteration across thread boundary Remove misplaced PNGs from docs/diagrams/ (correct copies in png/ subdir). * Address PR #22 review feedback - Fix CLI session_factory signature (ws_id param) — CI typecheck failure - First-phase eviction in create() now calls _cleanup_ui + record_eviction - close() persists "closed" state to storage via update_workstream_state - Fix noqa comment in test to pragma: no cover	2026-03-04 06:04:59 -08:00
Patrick Buckley	d28879208f	Add multi-provider LLM adapter with model capability flags (#12) ... * Add multi-provider LLM adapter with model capability flags Introduce a provider abstraction layer between ChatSession and LLM SDK clients, enabling native support for Anthropic alongside OpenAI-compatible APIs. Each provider translates at the API boundary while the internal message format remains OpenAI-like throughout session history and persistence. - LLMProvider protocol with StreamChunk/CompletionResult normalized types - OpenAIProvider: GPT-4o, GPT-5.x, O-series capability tables with conditional temperature, reasoning_effort, and token param handling - AnthropicProvider: native streaming, message/tool format conversion, adaptive vs manual thinking modes, effort parameter for 4.6 models - ModelCapabilities per-model flags: temperature support, token param name, thinking mode, effort levels, context window, max output - Smart auto-detect: latest Opus for Anthropic, latest base GPT for OpenAI - --provider CLI flag for both turnstone and turnstone-server - anthropic SDK as optional dependency (pip install turnstone[anthropic]) - 56 new provider tests, 672 total passing - Updated architecture docs and 4 PlantUML diagrams * Fix Copilot PR #12 review: reasoning_effort gating, Anthropic thinking, provider factory - Default reasoning_effort_values to () so unknown/local models don't receive unsupported top-level reasoning_effort param. Models that need it (GPT-5.x, search models) have explicit capability declarations. - Fix Anthropic _reasoning_params: "none" and "" effort now return {} instead of enabling thinking with 4096 budget. - Use create_provider("openai") singleton instead of OpenAIProvider() in ChatSession fallback for consistency with registry path. - Add 8 parameter gating tests: unknown model no reasoning_effort, GPT-5 no temperature, GPT-5.1 conditional temperature, O-series no temperature, Anthropic none/empty/low effort.	2026-03-03 16:52:02 -08:00
Patrick Buckley	2c48f694db	Add multi-model support with ModelRegistry, fallback routing, and per… (#9) ... * Add multi-model support with ModelRegistry, fallback routing, and per-workstream selection Introduces a ModelRegistry that holds named model configurations loaded from [models.*] sections in config.toml. Each workstream can select its model at creation time or switch mid-session via /model <alias>. When the primary model is unreachable, a configurable fallback chain tries alternative models. Sub-agents (plan/task) can optionally use a cheaper model via the agent_model setting. Core changes: - New turnstone/core/model_registry.py: ModelConfig (frozen, api_key redacted from repr), ModelRegistry (thread-safe lazy client creation, resolve, fallback chain), load_model_registry() with backwards-compatible config loading - session.py: registry/model_alias params, /model show+switch command, fallback in _create_stream_with_retry (extracted _try_stream), agent model override in _run_agent - workstream.py: factory signature accepts optional model_alias, create() gains model param - cli.py + server.py: build registry, updated session factories, banner, shutdown - protocol.py: model field on CreateWorkstreamMessage - bridge.py: pass model through workstream creation chain Frontend: - MODEL column added to dashboard tables in both server and console UIs - Responsive: hidden alongside NODE at narrow viewports - ARIA labels include model info, title attributes for truncated text - SSE connected event includes model_alias Documentation: - README: architecture tree, Multi-Model Support section, config keys - docs/architecture.md: module map, Multi-Model Registry subsection - docs/api-reference.md: model field in workstream creation, model_alias in SSE - PlantUML diagrams 02 + 03 updated with ModelRegistry Tests: 43 new tests (576 total), mypy clean, ruff clean. * Fix Copilot PR #9 review: model_alias property, preserve manual tool_truncation - Expose model_alias as a public @property on ChatSession instead of accessing the private _model_alias from server.py and tests - Track _manual_tool_truncation flag so /model switch only recomputes tool_truncation when it was auto-derived, preserving --tool-truncation overrides - Update PlantUML diagram to reflect the public property	2026-03-02 20:58:35 -08:00