mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-27 06:14:48 -06:00
bfa1b104cfcbc04ebbd0ef4ea9bff1950d435821
807 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Patrick Buckley
|
d29840f985 |
fix(personas): repr the input in the not-found resolver error
Review feedback on #792: the "not found or disabled" branch
interpolated the raw input unquoted, so the whitespace-only and
trailing-space inputs the forgiving lookup explicitly handles rendered
invisibly in CLI output and logs. Use {name!r} like the other two
resolver errors already do.
(cherry picked from commit
|
||
|
Patrick Buckley
|
44c0b9c340 |
feat(personas): agent discoverability + forgiving name resolution
Coordinators and interactive agents had no way to enumerate valid
persona names: task_agent / spawn_workstream / spawn_batch described
`persona=` but nothing listed what it accepts, and resolution was an
exact case-sensitive slug match - users reaching for the display name
or a case variant got an unexplained failure.
- Inject the live persona catalog (enabled, interactive-kind; children
and sub-agents are always interactive) into the `persona` parameter
description of task_agent / spawn_workstream / spawn_batch, riding
the same render path as the model-alias injection. Rebuilt from the
pristine TOOLS base every render, so repeated renders are idempotent
and archived personas drop out instead of lingering. Entries carry
name + default marker + <=96-char description; names-only past 25
personas. spawn_batch's persona property is nested per-child under
children.items.properties (located via _persona_property, null-safe
against name-colliding MCP tools). Storage-less sessions keep the
base text: the render runs at session construction, so it gates on
is_storage_initialized() rather than get_storage(), which would
auto-init SQLite as a side effect.
- resolve_persona_for_kind - the ONE shared rule behind the HTTP
create handler, CLI --persona, the coordinator spawn precheck, and
task_agent prep - is now forgiving: exact slug, then the lowercased
input (created names are regex-enforced lowercase slugs), then a
case-insensitive display-name match accepted only when unique among
the kind's enabled personas. Duplicates refuse loudly naming the
candidate slugs; a same-label persona of another kind neither blocks
nor wins (the label the caller saw came from a kind-filtered
surface); whitespace-only input never matches blank display names
(display_name defaults to ""). Every failure now enumerates the
kind's valid names, so a stale injected list or a typo self-corrects
on the next attempt.
- The canonical slug is stamped everywhere: task_agent prep rewrites
its arg from the resolved snapshot, _validate_child_persona returns
(canonical, error) and both spawn call sites adopt it - approval
chrome, the wire, and workstream_config never carry a forgiven
variant.
- Create-persona shelf: label hint under Name explaining agents and
the CLI launch the persona by this name (case-insensitive) and the
display name is only a list label. docs/personas.md gains a "How
agents discover personas" section and drops the stale claim that
task_agent has no persona parameter.
Tests: resolver unit suite (case/display/ambiguity/cross-kind/
whitespace/disabled/storage-failure) + guards for injection content
and ordering, idempotent re-render, archive drop, the 25-persona
prose cutoff, coordinator-kind exclusion, and canonical stamping
through spawn_workstream / spawn_batch / task_agent.
(cherry picked from commit
|
||
|
Patrick Buckley
|
cdbdf3dc2b |
fix: address review — request-scoped storage in coord tenancy checks
- _coordinator_tenant_check and _coord_attachment_owner resolved storage from
the global registry (get_workstream_row / for_request without a storage arg),
which can evaluate the project-tenancy decision against a different or
auto-initialised backend and fail OPEN on a missing project row. Use
request.app.state.auth_storage explicitly, matching cluster_ws_detail and
_resolve_coordinator_or_404; fail closed (404) when it is unset.
- reject_unassignable_scopes now derives its allowed-scope error message from
ASSIGNABLE_SCOPES so validation and the message can't drift.
(cherry picked from commit
|
||
|
Patrick Buckley
|
8aabb061c2 |
fix: scope private-project workstream visibility to members, not admins
Workstreams attached to a private project were visible -- including their
conversation content -- to holders of admin.cluster.inspect / admin.coordinator
(both default builtin-admin permissions), defeating the project's confidentiality
boundary. Enforce that a private project's resources are visible only to people
IN the project (owner, workstream creator, or an explicit member), even for admins.
Surfaces closed:
- WorkstreamProjectVisibility bypass narrowed to service scope only (node->console
machine plumbing, re-filtered per-user at the console edge). No human principal
bypasses; admin.cluster.inspect gates the inspect surfaces, not tenancy. This
flows to /dashboard, session listings, the attachment row-gate, cluster_workstreams,
cluster_node_detail, and cluster_snapshot/SSE.
- cluster_ws_detail 404-masks a workstream in a private project the caller can't
see; cluster_ws_live_bulk routes such ids to the denied list (no private-project
oracle).
- Coordinator operator verbs (history/export/detail/send/approve/set_title/open/
children/tasks/attachments) now enforce project tenancy: _coordinator_tenant_check
on coord_endpoint_config, the gate in _resolve_coordinator_or_404 (children/tasks),
the tenant_check now run in make_open_handler before rehydrate, and a
project-visibility check in _coord_attachment_owner. admin.coordinator gates the
surface cluster-wide, but a non-member is 404-masked. The tenant-check mirrors the
manager-first + coordinator-kind ladder so kind-isolation is preserved.
- service scope is no longer user-assignable: admin_create_token and both
turnstone-admin CLI mint paths reject it via reject_unassignable_scopes, so an
admin.users holder cannot self-mint a service token and restore the bypass. Service
scope is minted only by ServiceTokenManager / the JWT secret.
- The events/global node proxy (service-elevated cross-tenant firehose) is gated on
admin.cluster.inspect so a plain authenticated user cannot reach it through the
console proxy.
Updates the OpenAPI description, the row-gate/tenancy-filter docstrings, and adds
tests for every surface (visibility predicate + cluster detail/bulk + coordinator
history/export/children/open/attachments + events/global proxy + scope-mint
rejection); inverts the tests that pinned the old admin-bypass contract.
(cherry picked from commit
|
||
|
Patrick Buckley
|
8bd638569f |
fix(mcp): route pool transport lifecycles through per-entry owner tasks (#788)
* fix(mcp): route static transport lifecycles through per-server owner tasks
A crash-looping MCP server drove the mcp-loop thread to a sustained,
climbing 100%+ CPU spin. Root cause: anyio cancel scopes are
host-task-bound, and the static path entered the SDK's transport /
ClientSession task-group scopes from short-lived connect tasks (every
health tick is a new task since #768). Once such a scope was cancelled
after its host task had finished - by anyio's task_done when a
transport child died with the server, or by ClientSession.__aexit__
during a cross-task teardown - CancelScope._deliver_cancellation could
never make progress (task.cancel() on a done task is a no-op) and
re-armed itself via call_soon every loop iteration, forever: ~900k
callbacks/s per zombie scope, one more per flap cycle (verified against
anyio 4.14.1; no upstream fix exists as of that release).
Fix: each static server's transport + session cms are now entered,
parked, and exited by ONE long-lived owner task
(_static_transport_owner), so scopes always have a live host and always
exit in the task that entered them. Teardown follows a one-cancel close
protocol (signal the close event before the first await, graceful
grace, then at most ONE cancel - never a second, which would abandon a
scope exit mid-flight). Connect timeouts now cancel only the waiting
caller; connect failures are delivered through a readiness future;
unrequested owner death (server died under a live session) evicts the
session immediately via a done-callback instead of waiting for the next
liveness ping. A rate-limited, mcp-loop-scoped gc-walk backstop
(_maybe_disarm_orphaned_scopes) disarms any zombie minted by paths not
yet migrated (the oauth_user pool keeps the old cross-task-close shape;
follow-up).
Also fixed: BaseExceptionGroup (BaseException-derived, as raised by
anyio task groups wrapping a stray CancelledError, e.g. an
accept-then-RST server) escaped `except Exception` in _connect_all and
killed it before the health/sweep loops were created - silently
disabling all autonomous recovery. Handled there and in the
health/sweep/eviction loops and the reconnect/refresh callers.
Verified: a live SIGKILL-flap repro went from 130%+ CPU (climbing, one
armed scope per cycle) to 0.3% flat with zero armed scopes; the RST
repro now leaves both background loops alive (previously both silently
dead). New tests: owner-lifecycle + close-protocol units (incl. an
exactly-one-cancel pin), a _connect_all BaseExceptionGroup regression,
a discriminating disarm-sweep test, and a ~10s live SIGKILL-flap smoke
test (real FastMCP subprocess, skips on environment gaps) asserting
zero armed scopes, exactly one live owner, and a post-recovery tool
call. Full 8470-test suite green; ruff+mypy clean.
* fix(mcp): route pool transport lifecycles through per-entry owner tasks
Completes the owner-task migration started for the static path: the
oauth_user pool path had the same latent anyio cancel-scope exposure
(host-task-bound scopes entered by short-lived connect tasks; a scope
cancelled after its host finished re-delivers cancellation via
call_soon forever - the 100%-CPU zombie), previously covered only by
the disarm backstop.
Each (user, server) pool entry's transport + ClientSession cms are now
entered, parked, and exited by ONE long-lived owner task
(_pool_transport_owner). The caller keeps building client_kwargs (the
per-user bearer and, when an auth-capture carrier is active, the
httpx_client_factory response hook) so 401/WWW-Authenticate capture
semantics are unchanged. Teardown is the shared one-cancel close
protocol (_teardown_pool_entry: signal before first await, graceful
grace, at most ONE cancel), used by the connect stale-guard, idle/LRU
eviction, and shutdown (parallel signal-then-reap). Unrequested owner
death evicts the session but keeps the entry and its discovered
catalog, matching the existing evict-session-keep-entry semantics the
auth_401 retry relies on.
Discovery still runs in the connecting caller while the transport is
hosted by the owner, so a transport collapse mid-discovery (e.g. the
SDK tearing its task group down on an upstream 401) cancels the OWNER,
not the caller - a bare await on the response stream would hang until
the 30s phase timeout. _await_pool_discovery races each discovery
await against owner completion and converts owner death into a prompt
ConnectionError (the owner is never cancelled there; teardown owns its
lifecycle). Carrier-first failure classification preserves auth_401
semantics for captured 401s.
With no cross-task stack closes left, _safe_close_stack and
_safe_teardown_on_connect_failure are deleted (zero callers).
Tests: new tests/test_mcp_pool_owner.py pins the pool close protocol
(graceful event-before-await close, exactly-one-cancel escalation,
owner-death eviction retaining entry+catalog, caller-cancel-mid-connect
cm-exit guarantee, factory-present-iff-capture, and the
owner-death-during-discovery fast-fail). 1010 mcp tests and the full
8471-test suite green, including the historical cross-task-anyio
sentinel test_integration_pool_reuse_401_refresh_and_retry_succeeds;
ruff+mypy clean; zero destroyed-task warnings.
* fix(mcp): harden disarm-sweep loop guard and owner BaseException arm
Review follow-ups on the owner-task migration:
- _maybe_disarm_orphaned_scopes now enforces its mcp-loop requirement
instead of trusting callers: it returns without walking (and without
advancing the rate-limit clock) unless the currently running loop IS
self._loop. A suppressed close can fire before start() or after
shutdown(), where the walk would be wasted at best and a cross-thread
reach at worst.
- The transport owner's BaseException arm now re-raises non-Exception,
non-group escapees (KeyboardInterrupt, SystemExit) after delivering
them to the readiness future - failure delivery is the arm's job;
swallowing an interpreter-level exit was not.
* fix(mcp): extend owner-death discovery fast-fail to the static path
The static connect path had the same exposure the pool's discovery race
closed: discovery runs in the connecting caller while the transport is
hosted by the owner task, so a transport collapse mid-discovery cancels
the OWNER and the caller's bare await on the response stream hung until
the caller-side attempt timeout (~45s) instead of failing promptly.
_await_pool_discovery is renamed to _await_owner_discovery (it is now
path-neutral) and wired into _connect_one_locked's four discovery
awaits. The helper also converts a discovery future that completes
CANCELLED without the race's own reap (an SDK-internal cancellation
shape) into the same ConnectionError, instead of leaking a bare
CancelledError the caller would misread as its own cancellation.
The pool transport owner's BaseException arm gains the same refinement
the static owner received in review: interpreter-level exits
(KeyboardInterrupt, SystemExit) re-raise after delivery to the
readiness future instead of being swallowed.
Tests: static owner-death-during-discovery fast-fail (<1s vs the ~45s
hang), and a direct pin on the cancelled-discovery-future conversion.
* fix(mcp): replace owner BaseException arm with targeted catch + finally delivery
The owner's failure arm now catches only (BaseExceptionGroup, Exception);
waiter delivery for everything else moves to a finally that resolves the
readiness future with a clean transport-failure ConnectionError before
the task unwinds. Interpreter exits and BaseException-derived library
control-flow escapes propagate from the owner exactly once, uncaught -
and the waiter can never be left hanging on an unresolved future (the
initial _connect_all connect has no outer bound). For SystemExit /
KeyboardInterrupt asyncio additionally stops the loop right after, so
the delivery is load-bearing for the non-exit BaseException shapes and
free for the exits.
Pinned by a test driving a BaseException-derived escape through the
owner: the waiter resolves promptly with ConnectionError while the
escape propagates unswallowed.
* fix(mcp): mirror targeted-catch + finally delivery in the pool owner
Same shape the static owner received in review: the failure arm catches
only (BaseExceptionGroup, Exception), and waiter delivery for anything
else moves to a finally that resolves the readiness future with a clean
ConnectionError before the task unwinds - interpreter exits and
BaseException-derived library escapes propagate exactly once, uncaught,
and the waiter can never be left hanging.
* test(mcp): narrow the escape test's waiter catch to explicit types
* test(mcp): narrow discovery-race waiter catches to explicit types
* refactor(mcp): make reap/synchronization awaits explicit to analyzers
Full-absorb reaps (cancel-then-drain of a future whose outcome is
deliberately consumed) become `await asyncio.gather(x,
return_exceptions=True)` - one line, self-describing, and in the
owner-died discovery reap it is also a small semantic improvement: a
caller cancellation arriving during the reap now propagates instead of
being masked by the ConnectionError. Bare synchronization awaits and
selective suppress blocks in tests keep their raise-through semantics
via throwaway assignment. Applied uniformly across the owner-task
test files, including sites introduced by the static-path PR.
(cherry picked from commit
|
||
|
Patrick Buckley
|
efd0a1d000 |
test(mcp): narrow the escape test's waiter catch to explicit types
(cherry picked from commit
|
||
|
Patrick Buckley
|
2f93c39fd3 |
fix(mcp): replace owner BaseException arm with targeted catch + finally delivery
The owner's failure arm now catches only (BaseExceptionGroup, Exception);
waiter delivery for everything else moves to a finally that resolves the
readiness future with a clean transport-failure ConnectionError before
the task unwinds. Interpreter exits and BaseException-derived library
control-flow escapes propagate from the owner exactly once, uncaught -
and the waiter can never be left hanging on an unresolved future (the
initial _connect_all connect has no outer bound). For SystemExit /
KeyboardInterrupt asyncio additionally stops the loop right after, so
the delivery is load-bearing for the non-exit BaseException shapes and
free for the exits.
Pinned by a test driving a BaseException-derived escape through the
owner: the waiter resolves promptly with ConnectionError while the
escape propagates unswallowed.
(cherry picked from commit
|
||
|
Patrick Buckley
|
20a61b692b |
fix(mcp): route static transport lifecycles through per-server owner tasks
A crash-looping MCP server drove the mcp-loop thread to a sustained,
climbing 100%+ CPU spin. Root cause: anyio cancel scopes are
host-task-bound, and the static path entered the SDK's transport /
ClientSession task-group scopes from short-lived connect tasks (every
health tick is a new task since #768). Once such a scope was cancelled
after its host task had finished - by anyio's task_done when a
transport child died with the server, or by ClientSession.__aexit__
during a cross-task teardown - CancelScope._deliver_cancellation could
never make progress (task.cancel() on a done task is a no-op) and
re-armed itself via call_soon every loop iteration, forever: ~900k
callbacks/s per zombie scope, one more per flap cycle (verified against
anyio 4.14.1; no upstream fix exists as of that release).
Fix: each static server's transport + session cms are now entered,
parked, and exited by ONE long-lived owner task
(_static_transport_owner), so scopes always have a live host and always
exit in the task that entered them. Teardown follows a one-cancel close
protocol (signal the close event before the first await, graceful
grace, then at most ONE cancel - never a second, which would abandon a
scope exit mid-flight). Connect timeouts now cancel only the waiting
caller; connect failures are delivered through a readiness future;
unrequested owner death (server died under a live session) evicts the
session immediately via a done-callback instead of waiting for the next
liveness ping. A rate-limited, mcp-loop-scoped gc-walk backstop
(_maybe_disarm_orphaned_scopes) disarms any zombie minted by paths not
yet migrated (the oauth_user pool keeps the old cross-task-close shape;
follow-up).
Also fixed: BaseExceptionGroup (BaseException-derived, as raised by
anyio task groups wrapping a stray CancelledError, e.g. an
accept-then-RST server) escaped `except Exception` in _connect_all and
killed it before the health/sweep loops were created - silently
disabling all autonomous recovery. Handled there and in the
health/sweep/eviction loops and the reconnect/refresh callers.
Verified: a live SIGKILL-flap repro went from 130%+ CPU (climbing, one
armed scope per cycle) to 0.3% flat with zero armed scopes; the RST
repro now leaves both background loops alive (previously both silently
dead). New tests: owner-lifecycle + close-protocol units (incl. an
exactly-one-cancel pin), a _connect_all BaseExceptionGroup regression,
a discriminating disarm-sweep test, and a ~10s live SIGKILL-flap smoke
test (real FastMCP subprocess, skips on environment gaps) asserting
zero armed scopes, exactly one live owner, and a post-recovery tool
call. Full 8470-test suite green; ruff+mypy clean.
(cherry picked from commit
|
||
|
Patrick Buckley
|
4da7c3b91c |
fix(redact): harden credential redaction and restore JS/backend parity
Address review findings on the client-side credential redactor and mirror
each fix into the backend output guard so both surfaces censor identically:
- Detect and redact single-quoted JSON secrets such as
{'Authorization': 'Bearer ...'} (Python dict reprs / JS object literals),
which the double-quote-only pattern silently bypassed on both sides.
- Cover mongodb+srv://, rediss:// and amqps:// connection strings.
- Match the Bearer auth scheme case-insensitively (RFC 7235).
- Redact prefixed key/token assignments (api_key=, secret_key=,
access_token=) as a whole rather than chewing the tail into a garbled
"api_[REDACTED:api_key]", while still covering bare key=/token=. A word
boundary was rejected because it would drop coverage for <prefix>_key=.
- Remove the redundant |Authorization alternative (covered by /i) and swap
the manual value-slicing helper for a capture-group substitution.
The backend edits touch both detection sites and both redaction pipelines,
so single-quoted secrets are flagged (and therefore sanitized), not merely
rewritten. Adds JS runtime-smoke and backend unit coverage for every case.
(cherry picked from commit
|
||
|
Patrick Buckley
|
012f4e3e16 |
Add client-side credential redaction for tool call cards
New shared ES6+ module (redact_credentials.js) provides comprehensive
visual credential censorship matching the backend output guard patterns:
- PEM private key blocks, connection strings, Bearer tokens
- OpenAI / GitHub / AWS / Google API key formats
- Query-string and JSON-style credential values
- JSON secret keys (api_key, password, token, authorization, etc.)
- ENV secret lines (SECRET_KEY=, DATABASE_URL=, etc.)
Integrated into both frontend surfaces:
- interactive.js: replaces legacy minimal _redactApiKeys function
- conversation.js::buildConvResult (shared substrate, used by coordinator)
- coordinator.js::renderToolOutput fallback paths
Backend parity: added 'authorization' to the JSON secret regex in
output_guard.py so the output guard flags and redacts Authorization
headers in JSON tool output.
Tests: ported the runtime smoke test from the removed _redactApiKeys
to import the new module directly; added redact_credentials.js to the
var-free and const-reassign guard bundles.
(cherry picked from commit
|
||
|
Patrick Buckley
|
3636724848 |
fix(core): scrub credentials and control chars from tool-args log preview
`tool_args_preview` feeds `stream.tool_args_malformed` (WARNING) and
`wire.tool_args_legalized` (DEBUG), and tool arguments are model/user
controlled — they can carry secrets (a token in a bash command, a password in a
connection string) or raw CR/LF that break log lines. Route the preview through
`output_guard.redact_credentials` over the full value first (before the 120-char
cap, so a secret straddling the cut isn't half-shown past the pattern's reach),
then collapse every control char to a space, mirroring `audit._scrub_string`.
Addresses the PR review comments.
(cherry picked from commit
|
||
|
Patrick Buckley
|
eeda5ac312 |
fix(core): legalize malformed tool-call arguments before the wire
A tool call whose `arguments` is not a JSON-object string (an unterminated
string from a non-`length` truncation, or an empty `""` from a no-arg call)
was committed verbatim and replayed on every subsequent send. Strict renderers
that re-parse arguments at render time (vLLM's `deepseek_v4`
`_postprocess_messages` runs `json.loads` on them) reject the whole request
with HTTP 400, wedging the conversation. The only prior guard dropped partial
tool calls on `finish_reason == "length"`; a `stop`/`tool_calls` finish reason
carrying invalid JSON slipped through, and its synthetic "retry" result kept it
from being an orphan, so the orphan-repair pass never touched it.
Add `sanitize_tool_call_arguments`, a wire-neutral legalize pass in lowering
(fold, legalize, repair), normalizing any non-JSON-object `arguments` to `{}`
on the transient wire copy only. The canonical trajectory keeps the raw model
output, so a wedged session self-recovers on its next send. A
`wire_valid_arguments` predicate is shared with a non-destructive
`stream.tool_args_malformed` warning at the stream accumulator, which surfaces
the model-quality problem at production time.
Convert `lowering.py` to structlog so the new pass emits structured events.
(cherry picked from commit
|
||
|
Patrick Buckley
|
be872b840f |
fix(ui): make pane hotkeys work off macOS and match across surfaces
The pane/workstream accelerators only worked on macOS. They were bound to
Ctrl, which on Windows/Linux IS the browser's own accelerator: Ctrl+T,
Ctrl+W and Ctrl+1-9 were swallowed by the browser (new tab / close tab /
switch tab) and never reached the page. macOS browsers own Cmd instead, so
Ctrl was free there and everything appeared to work.
On top of that the shortcuts were declared in three places that had drifted
apart — the "?" overlay, each app.js keydown handler, and the tab-menu
badges in shell.js. The console fell to convTabMenu's node-proxy fallback
lane, which dropped every shortcut badge (and Fork), so its tab menu showed
no accelerators and Ctrl+W there just closed the browser tab.
Choose the modifier per platform (Ctrl on macOS, Alt on Windows/Linux) and
make shell.js the single source of truth for the per-pane accelerators: a
stable accel registry drives both the platform-aware badge and one shared
keydown handler that invokes the ACTIVE pane's own menu item, so a badge
can't advertise a chord the handler ignores and each surface contributes
only what it supports (the console omits Fork; it has no fork surface yet).
Each surface's app.js keeps only its global accels (new / switch /
dashboard); the console regains switch + dashboard to match. Mod+W now
uniformly means Close pane (drop the tab, session keeps running), matching
its badge and the universal Ctrl+W convention — previously the standalone's
Ctrl+W stopped the session. The previously-dead "Refresh title / Ctrl+Shift+R"
is wired, and Ctrl+T / Ctrl+D yield to text editing while a field is focused
(macOS transpose / delete-forward).
(cherry picked from commit
|
||
|
Patrick Buckley
|
3615f98c19 |
Fix send button stuck disabled by pruning orphaned approval cycles (#775)
* Fix send button stuck disabled by pruning orphaned approval cycles When a DOM wipe (clear_ui / replay_truncated / replaceChildren) detaches approval card elements while an approve_request event is processed between the wipe and refetch-restore, the matching approval_resolved may never arrive. The orphaned cycle entry in approvalCycles keeps pendingApproval=true and the send button disabled forever. The fix adds a pruning pass at the top of _syncApprovalState(): cycles whose blockEls are all .isConnected === false are deleted from the Map. This runs on every register/resolve/rebuild so orphans are cleaned up promptly. Also fixes an ordering bug in showInlineToolBlock discovered during review: the block element was appended to the DOM after _registerApprovalCycle, so the new isConnected prune would kill the just-registered cycle before it took effect. * Fix comment inaccuracy in showInlineToolBlock append-before-register guard The comment said 'blockEls.every(el => el.isConnected)' but the actual prune check is '!blockEls.some(el => el.isConnected)' - no block elements are connected, not every element. |
||
|
Patrick Buckley
|
6f8efaa44e |
test(golden): freeze the anthropic-compatible reasoning-effort wire
The wire-payload golden matrix had no anthropic-compatible coverage —
both AnthropicProvider rows are the native lane (compat=False), so the
distinct compat wire shape (reasoning control in
extra_body.chat_template_kwargs, never the native thinking param) was
unfrozen. Add the compat lane across all eight representative fixtures
with a manual-mode capability (the lane has no static table, so caps
ride in as a model definition would supply them) and reasoning_effort=
high: every golden now pins {enable_thinking: true, reasoning_effort:
high} in chat_template_kwargs, asserts the native thinking param is
absent, and preserves temperature (no forced 1.0). _capture gains an
optional caps override to support the no-static-table lane.
|
||
|
Patrick Buckley
|
530958e06b |
fix(providers): the session effort level always reaches the local-lane wire
Local lanes dropped the knob's graded value unless the operator declared
reasoning_effort_values (and, on the template channel, an effort key) —
picking Max sent a bare thinking toggle and the effort select
degenerated into seven positions that all meant 'on'. The user's
setting now always rides:
- openai-compatible: the flat reasoning_effort param carries the knob
verbatim (effort_passthrough on the lane default); declared values
still snap ordinally, and a declared effort_param still claims the
template channel and suppresses the flat param.
- anthropic-compatible: the graded value rides chat_template_kwargs
alongside the toggle whenever reasoning control is engaged — under
the operator's effort_param, else the conventional fallback key
(reasoning_effort); templates that don't reference the kwarg ignore
it. thinking_mode=none still injects nothing.
- Commercial lanes untouched: empty declared values still mean 'no
effort control' (o1-mini) and the ordinal snap is unchanged.
Golden writer now pins ensure_ascii=False: the baselines' literal em
dashes came from a hand edit (
|
||
|
Patrick Buckley
|
e136237b63 |
fix(providers): openai-compatible never consults the commercial table
Local-lane model ids are operator-chosen strings (vLLM --served-model-name), so a prefix collision with a cloud model id inherited that model's sampling and effort contract: a box named o3-distill silently lost temperature support, and one named gpt-5.5-my-finetune was sent gpt-5.5's snapped reasoning_effort values it never declared. Both surfaces of the lane now return plain defaults (OPENAI_COMPAT_DEFAULT in _openai_common): the chat class directly, and the responses pin via a compat-mode OpenAIResponsesProvider mirroring AnthropicProvider(compat=True). Everything beyond the defaults is declared by the operator on the model definition, matching the anthropic-compatible lane and lookup_model_capabilities' documented 'no static table for local models' contract. The commercial openai lane (Responses-only) is untouched. Pre-split tests that reached commercial rows through the chat-class OpenAIProvider alias now source them from lookup_openai_capabilities; their subject (registry rows + shared gating helpers) is unchanged. |
||
|
Patrick Buckley
|
41e9907803 |
fix(console): available-models rows always carry effort_ladder
The except path for a malformed capabilities column appended the row without the key, so clients had to null-check a field the happy path guarantees. Initialize each entry with an empty ladder and let the try block overwrite it — the response schema is stable per row. |
||
|
Patrick Buckley
|
7f0e0406b3 |
test(approvals): concurrency matrix + suite migration to the cycle model
New regression matrix for the release blockers: cross-approval independence, lost-wakeup at gate entry, FIFO selector-less resolution, resolve-all sweep, double-resolution no-op, cards/legacy view tracking, and the generation-exactness set — stale delivery rejection, Smart-Approvals origin check, purge keep_origin, the purge-to-register window eviction, late cross-generation "superseded" stamping, concurrent smart+human gates, and the pre-delivered-verdict fast path. Plus sub-agent judge wiring (agent_gate off the main slot, close() firing all generations) and endpoint tests for cycle pinning and the Approve+Always race guard. Gate threads run under one shared mock-patch harness — mock.patch start/stop of the same target from concurrent threads corrupts the patcher's restore stack — with a sweep-until-dead teardown so the conftest leak guard can't trip. Existing suites migrate off the singleton fields to cycle assertions and the pending_approval_details wire shape. |
||
|
Patrick Buckley
|
3607517814 |
fix(providers): registry effort truth — o-series/gpt-5.5/codex-max/sonnet-5; forward declared none
Capability-registry corrections verified against the official OpenAI reasoning guide, the Azure reasoning-models matrix (2026-06 revision), and the Anthropic models-overview/effort/migration pages (2026-07): OpenAI (vocabulary confirmed none/minimal/low/medium/high/xhigh — no "max" level exists; knob max rides the xhigh ceiling via the ordinal snap): - o1/o3/o3-mini/o3-pro/o4-mini declare low/medium/high (every o-series model except o1-mini) — without declared values the session knob was silently dropped for these models. o1-mini stays effort-free. - gpt-5.5 default corrected none -> medium (5.5 reasons by default, unlike 5.1-5.4). - gpt-5.1-codex-max gets an explicit row: it prefix-matched the gpt-5.1 row (no xhigh), capping the knob's xhigh at high on the one model xhigh was introduced for. Anthropic (effort-page matrix): - claude-sonnet-5 row added — it previously fell through to _ANTHROPIC_DEFAULT (manual budgets, 200k ctx, no effort), all wrong: adaptive-by-default thinking (manual budgets are a 400), sampling params rejected, 1M ctx / 128k out, effort low..max incl. xhigh. - claude-sonnet-4-6 gains its documented "max" effort level (knob xhigh now rides max, not high) and the stale 64k max_output becomes the documented 128k. - fable-5 / opus-4-8 / opus-4-7 / opus-4-6 / opus-4-5 rows verified correct as declared. Knob semantics completed: resolve_reasoning_effort now forwards the knob's "none" position verbatim when the model DECLARES an explicit none level (gpt-5.1+, grok-4.3) — omitting the param there leaves a reasoning-on server default (gpt-5.5: medium) in charge of a knob that promises off. Models without a declared none still omit, and none is never a snap target. Parity harness swaps its synthetic openai shape for the real gpt-5.5 registry row. |
||
|
Patrick Buckley
|
a0e04a8588 |
fix(providers): effort snapping is ordinal — round up, cap at the ceiling
The knob domain grew xhigh/max after the snapping fallbacks were written, which silently inverted their semantics: off-list meant "unrecognized string" then, but now usually means "above the model's ceiling", where falling back to the default tier is directionally wrong (grok-4.3 at knob max got low; values low/medium/high at knob xhigh got medium; Anthropic manual mode gave xhigh/max a 4096 budget while high got 16384). One rule everywhere now, via snap_reasoning_effort in _protocol: exact match wins; otherwise the smallest declared level ranking at or above the knob; above the ceiling, the ceiling. "none" is never a snap target, and default_reasoning_effort only catches values the ordinal snap cannot rank. - resolve_reasoning_effort (flat chat / responses / validated effort_param lanes) snaps ordinally: xhigh over (low, medium, high) now sends high; xhigh over DeepSeek-style (high, max) sends max — matching DeepSeek's official xhigh-to-max aliasing, so a declared values list now reproduces that contract instead of defeating it. - _map_reasoning_to_effort (native output_config) rounds up too: knob xhigh on Opus 4.6 (low, medium, high, max) rides max instead of silently dropping output_config. - EFFORT_BUDGET_MAP is monotone across the whole knob domain: minimal/low 1024 (API floor), medium 4096, high 16384, xhigh 32768, max 65536. Unknown strings still fall to the 4096 default. Google defaults are unaffected (ceiling and default coincide at high); wire goldens unchanged. Parity harness caught the budget clamp interacting with its own max_tokens during development — capture budget raised above the largest manual budget. |
||
|
Patrick Buckley
|
ffe8214cfe |
test(providers): ladder-to-wire effort parity harness across all lanes
Proves the effort-ladder projection against the real request path instead of against the mapping helpers it shares with it. For 22 (provider lane x capability shape) points — both Anthropic lanes, openai-compatible on both API surfaces, openai, google (default and template-override hybrid), xai (default and inert-override), and the DeepSeek/qwen template contracts — every knob position is driven through the actual provider create_streaming against a recording fake client, and two invariants are asserted per shape: 1. each ladder token decodes to an expected effort wire subset (toggle / template effort / flat param / thinking budget / output_config) that must equal the captured kwargs exactly; 2. two knob positions carry equal tokens iff they produce identical effort-relevant wire payloads — the grouping promise the UI annotations lean on. The RecordingClient SDK-seam stub moves from the wire-payload golden harness into tests/_wire_capture.py so both suites capture at the same seam. Verified the harness catches the bug class it was built for: re-adding xai to _CHAT_LANES fails xai-template-override-inert. |
||
|
Patrick Buckley
|
1f63f622c9 |
fix(providers): xai effort ladder is flat-only; share the suppression rule
Second external audit round on the ladder. Verified and fixed:
- xai was in _CHAT_LANES on the false premise that XAIProvider
subclasses the chat provider. It subclasses OpenAIResponsesProvider,
whose surface ignores extra_body entirely, so a thinking_mode /
effort_param override never changes an xai request — but the ladder
claimed a template toggle ("on+low") that does not exist on the
wire. xai now projects through the flat channel only, like openai.
- The flat-param suppression rule (a declared effort_param claims the
template channel) was encoded independently in
apply_temperature_and_effort and the ladder. Extracted into
flat_effort_suppressed() in _protocol so the request path and the
projection cannot drift.
- admin_effort_ladder logs the swallowed resolver exception before
returning 400 (a genuine bug would otherwise hide as a silent 400).
- list_available_models reads server_compat with .get() instead of
destructively popping it out of the parsed capabilities dict.
- models_changed SSE now re-annotates the skill launch-config effort
select after invalidating the models cache instead of leaving a
stale ladder until the next keystroke.
- Capabilities JSON textarea placeholder hints the two effort fields
that have no structured control (reasoning_effort_values,
default_reasoning_effort).
|
||
|
Patrick Buckley
|
f4701bf0f9 |
test(golden): re-baseline Google wire payloads for the effort knob
The Gemini effort fix (
|
||
|
Patrick Buckley
|
06cc184227 |
fix(console): address verified external-audit findings on the effort ladder
The one that mattered: /v1/api/models passed the capabilities column — a JSON STRING (sa.Text) — straight into effort_ladder_for_model, whose field filter calls .items() on it; the per-row guard swallowed the AttributeError, so effort_ladder was silently absent from every row and the sklc annotation could never fire. The endpoint now parses the JSON and splits the namespaced server_compat exactly like the model_registry loader, and a regression test seeds a string-capabilities row. Projection fidelity: effort_ladder_for_model threads api_surface (the responses surface ignores extra_body — flat-param-only ladder, matching create_provider's request-time divergence); google/xai route through the chat-lane projection they actually inherit (_finalize_extra_body + flat param); the native-Anthropic branch reflects that output_config gates on supports_effort alone, independent of thinking_mode; budget clamping to per-request max_tokens is documented as out of scope. Hardening and symmetry: admin endpoint 400s (not 500s) on non-dict JSON bodies and gained HTTP tests; the admin edit-load strips effort_param from the raw JSON only for the lanes whose save path re-adds it, so non-compat rows can't silently lose a stored key; the empty-model early return bumps the ladder sequence so stale in-flight responses can't re-annotate; alias labels only reference positions the target select actually offers (the skill shelf omits none/minimal); the sklc models cache no longer pins a rejected promise and is invalidated on the models_changed event; debounces unified at 500ms; merge_reasoning_template_kwargs now always returns a fresh dict for non-empty input; the shared budget constants are public. |
||
|
Patrick Buckley
|
59a527f2f2 |
feat(console): surface each model's effective effort ladder
Seven knob positions render as seven behaviors in the UI, but the real
ladder depends on the lane and the model: qwen3.6 has two (off/on),
DeepSeek-V4 three, Claude 4.6 five. Operators had no way to see which
positions alias — the confusion class behind silently-equal effort
levels.
providers/effort_ladder.py projects the knob domain through the same
mapping functions the providers use at request time (resolve_reasoning_
effort, reasoning_template_kwargs, the manual budget map — hoisted to a
shared constant so the projection can't drift), yielding
{value, effective} rows where equal tokens promise identical requests.
/v1/api/models rows now carry the ladder (guarded per row), and
POST /v1/api/admin/models/effort-ladder computes it for the admin
modal's unsaved edits.
The admin per-model effort select and the skill launch-config effort
select annotate aliased positions ("Max (= high)", "None (model
default)") with a sends-tooltip; annotations refresh as thinking-mode /
effort-param / capabilities fields change. The ladder describes what
Turnstone sends — server-side templates may alias further (DeepSeek-V4
folds low/medium into its default high tier).
|
||
|
Patrick Buckley
|
d7941c88be |
fix(providers): thread the session effort knob to Gemini
_GOOGLE_DEFAULT declared no reasoning_effort_values, so resolve_reasoning_effort returned None and the session effort knob was silently dropped for every Gemini model — the same bug class this branch fixed on the local lanes. Gemini's OpenAI-compat surface documents a flat reasoning_effort (2.5: thinking_budget mapping; 3.x: thinking_level), so declaring values lights up the inherited chat-completions path. Values are the safe cross-model set (minimal/low/medium/high): "none" is excluded because 2.5 Pro and the 3.x family reject disabling thinking — and the resolver never forwards the knob's none anyway (the param is omitted, server default applies). Off-list xhigh/max snap to the declared default high. Encoded from the official compatibility docs per the static-caps pattern; not live-verified. |
||
|
Patrick Buckley
|
2cf23b6fe2 |
fix(providers): address high-effort review of the reasoning-knob branch
Verified findings applied: - adaptive thinking_mode never knob-disables: the shared mapping now sends the toggle unconditionally true for adaptive (the native adaptive branch ignores the knob's none), while manual keeps the knob-driven contract. Restores the invariant the deleted chat-lane code upheld. - a set effort_param suppresses the flat top-level reasoning_effort on the chat lane: the template channel replaces it — double-sending could 400 on schema-strict servers and disagree with operator pins. - admin edit-save no longer drops a stored thinking_param when the thinking-mode dropdown is empty: the raw-JSON strip now only fires when a mode value actually round-trips through the dropdown. - effort_param persistence gated on the local-server lanes so a value lingering across a provider switch never lands on commercial rows. - three stale _compat_extra_params references renamed to merge_reasoning_template_kwargs. Documented dispositions (no code change): the knob-none-disables flip on upgrade is intentional and now carries an upgrade note; gateways fronting real Claude belong on provider=anthropic with a custom base_url (the compat lane is vLLM-schema-only); nonstandard thinking_mode strings staying inert is the intended allowlist contract. The real anthropic provider is unaffected throughout — official Claude models keep native thinking/output_config. |
||
|
Patrick Buckley
|
68b22adfa3 |
feat(providers): share the effort-knob→chat_template_kwargs mapping with the openai-compatible lane
Hoist the compat-lane injection into _protocol.merge_reasoning_template_kwargs (next to ModelCapabilities — one implementation for both local-server lanes) and retire OpenAIChatCompletionsProvider._apply_thinking_mode in its favor: _finalize_extra_body now receives the session effort knob, so thinking_mode manual/adaptive maps knob "none" to an explicit thinking_param false (previously the toggle was unconditionally true) and caps.effort_param carries the graded effort key on chat completions too. Operator server_compat pins still win; the Responses surface is untouched (native reasoning handles effort itself). The admin Models form grows an "Effort param" field that round-trips like thinking_param: lifted out of the raw capabilities JSON on edit-load, re-added on save, cleared by emptying the field. Verified live against qwen3.6-27b /v1/chat/completions: knob medium streams reasoning_content, knob none suppresses it. |
||
|
Patrick Buckley
|
9289693730 |
fix(providers): drive reasoning via chat_template_kwargs on the anthropic-compatible lane
The compat lane sent no reasoning control at all: vLLM's /v1/messages has no thinking request field, thinking_mode stayed "none", and the session effort knob was silently dropped. The reasoning levers live in the chat template, so fold them into extra_body chat_template_kwargs (_compat_extra_params): thinking_mode manual/adaptive maps the knob onto caps.thinking_param (effort "none" = off, mirroring the native manual-mode contract), and caps.effort_param (new ModelCapabilities field) carries a graded effort value for gpt-oss-style templates, validated against reasoning_effort_values when declared. Operator server_compat entries win on key collision; native thinking params, temperature forcing, and output_config never fire on compat. resolve_reasoning_effort moves from _openai_common to _protocol next to ModelCapabilities — importing it into _anthropic would otherwise cross provider families. The admin Models form now shows and round-trips the thinking-mode dropdown for this lane; the #661 hide was premised on thinking_mode being inert here, which this change inverts. Verified live against qwen3.6-27b on vLLM /v1/messages: knob medium streams a thinking block, knob none suppresses it, an operator pin beats the knob. |
||
|
metaclassing
|
deff44bcea |
Addendum to Entra ID's... proclivities (#772)
* OIDC entra capture * copilot being nitpicky --------- Co-authored-by: pow3rtool <root@pow3rtools> |
||
|
Patrick Buckley
|
217d3a3a9b |
feat(mcp): autonomous reconnect + liveness for static MCP servers (#768)
* feat(mcp): autonomous reconnect + liveness for static MCP servers Static (non-oauth_user) MCP servers had no autonomous reconnect. Every reconnect path was lazy — a tool dispatch (_cb_auto_reconnect), an operator refresh, or a config edit — and the MCP SDK's own reconnect is a bounded 2-attempt burst on the streamable-http GET stream only (verified: mcp 1.28.1), with no backoff and nothing for the other transports. So a static server that went down and came back while nobody was dispatching to it stayed disconnected until a dispatch or a manual reconnect. Worse, a session whose transport dies while idle survives as a non-None ClientSession with closed streams — nothing evicts it, so even a later dispatch may not notice until it fails. Add a static-server health loop on the mcp-loop (started in _connect_all; config ``static_health_check_seconds`` default 30, <= 0 disables): - Reconnect: a disconnected server (session is None) is reconnected on a capped, jittered, FOREVER backoff (full jitter, base 1s, cap 60s, no attempt limit) — a server that returns after a long outage reconnects within ~a minute, and a permanently-misconfigured one costs at most one attempt per cap. The health loop owns this clock; the circuit breaker stays the DISPATCH fail-fast gate (a tool call to a down server errors immediately rather than blocking on the retry), and the loop keeps breaker state in sync so an open breaker closes on reconnect. - Liveness: a connected server is pinged (send_ping) each cadence; a dead-but- idle one — which nothing else would notice — is evicted so the next tick reconnects it. This is the core of the "never reconnects" failure. Serialize _connect_one per server behind a per-name lock (split into a thin wrapper + _connect_one_locked): the health loop, a dispatch's _cb_auto_reconnect, and an operator refresh could otherwise interleave teardown/rebuild on the shared StaticServerState and corrupt it — a latent pre-existing race this also closes. The body is unchanged (only relocated), so the delicate anyio / wait_for connect logic is untouched. Out of scope (follow-up): silent GET-stream / notification death — the SDK stops the notification stream after 2 attempts while the request path stays alive, so send_ping is blind to it; the fix is bounded session recycling, which needs a static in-flight guard first (only PoolEntryState tracks in_flight today). Tests: backoff bounds (capped / jittered / forever, no overflow), reconnect success resets backoff + closes breaker, reconnect failure retries forever, in-flight skip, per-name serialization (no overlap), ping keeps healthy / evicts dead / evicts on timeout, tick skips oauth_user, connect_all start + disable gating, clean cancel. * fix(mcp): harden static-server health loop (review findings) A max-effort review found 13 concurrency/correctness defects, all from the loop mutating shared StaticServerState without the interlocks the pool path carries. Fix all 13: - In-flight interlock: add StaticServerState.in_flight (parity with PoolEntryState); _static_session_op increments/decrements around the static call_tool/read_resource/get_prompt session ops; the ping skips and never evicts a busy server, so a long tool call can't be torn down mid-flight. - Dead-transport gating: the ping evicts + trips the breaker only on _is_dead_transport(exc); an McpError, httpx.PoolTimeout, or plain ping timeout is "slow, not dead" and only reschedules (matching the dispatch path). - Session-identity: only evict the exact session that was pinged. - asyncio.timeout (invariant-18) not wait_for for the ping; 5s->30s; the timeout-scoped cancel is distinguished from an external shutdown cancel (which still propagates) via .expired(). - Bounded reconnect: wrap _connect_one in asyncio.timeout so a server that handshakes then stalls list_tools can't wedge the loop or hold the per-name lock forever (connect internals untouched). - Concurrent tick under asyncio.gather with a freshly-read clock for the sleep. - Cross-path coordination: reconnect_sync/remove_server_sync take the per-name lock across teardown+rebuild (calling _connect_one_locked directly); _cb_auto_reconnect reuses a health-established session instead of racing a redundant reconnect and no longer trips the breaker on lock contention. - Backoff hygiene on recovery; skip '__' names; on health reconnect clear only the open-circuit deadline (not the failure count) so a connect-ok/calls-fail server still escalates to a trip. Adds 14 tests and adjusts those that assumed the old behavior; suite 195->209. * fix(mcp): unify static-server reconnect coordination (round-3 review) A third review round + live testing found 8 issues on the health loop, five sharing one root: reconnect logic was fragmented across five drivers, each handling the lock / session-reuse / in_flight / config-recheck / breaker / clock differently and incompletely. Introduce one primitive and route every lazy/autonomous driver through it. _ensure_static_connected(name, cfg) — the single lazy (re)connect path, all under the per-name lock: config re-check (+ lock-identity re-check, closing the remove->re-add race) so a removed server is never resurrected; reuse-if-live so a queued/concurrent driver never tears down and rebuilds a live session (the observed reconnect storm); in_flight guard so a reconnect can't tear down a session with a call still in flight on the evicted stack; bounded connect; and the circuit breaker owned in one place (clear the open-circuit deadline on success per finding-13, record one failure on real connect failure). Returns the session on success/reuse, None on a deliberate skip, raises on real failure. Routed through it: the health loop, a dispatch's _cb_auto_reconnect, and _refresh_all; operator reconnect_sync stays a deliberate force-rebuild. Also: fresh-clock deadlines (the stale tick-start clock was landing deadlines in the past and collapsing the backoff into an every-tick retry storm); loop-death fix (the tick no longer re-raises a CancelledError found in the gather results — per-server fallout, not shutdown; the loop returns only when Task.cancelling() marks a genuine shutdown); dispatch breaker records no failure on a sync-boundary reconnect timeout (lock contention is not a server failure; real outcomes recorded once, inside the primitive). Cleanups: extract _teardown_static_session (was copy-pasted 3x); share _capped_exponential between the breaker cooldown and the reconnect backoff. Adds 17 tests; test_mcp_client 204->226. * fix(mcp): coherent timeout hierarchy + round-4 review fixes A fourth review round on the unified reconnect coordination found 5 correctness regressions + 1 cleanup, five sharing one root: the inner reconnect attempt bound (45s) was LONGER than every caller wait (dispatch 30s, remove 15s, reconnect 30s), so a caller cancelling mid-attempt delivered a bare CancelledError that slipped past the primitive's `except Exception`. - Coherent timeout hierarchy: add _STATIC_RECONNECT_CALLER_TIMEOUT_S (> the inner attempt bound) for the dispatch + operator waits, so the inner asyncio.timeout always fires first — a clean TimeoutError the primitive converts, cleans up, and records on the breaker — instead of a caller cancelling a live attempt. Fixes [0] (half-discovered session left installed, served with a stale catalog) and [1] (breaker never trips via dispatch). - Primitive cancel-safe (belt-and-suspenders): its handler is now `except BaseException`, so even a bare CancelledError drops the partial session and records the failed attempt before re-raising. - Operator waits: reconnect_sync / remove_server_sync default timeouts raised above the reconnect bound; remove_server_sync now CANCELS the pending _remove on timeout (so it can't later pop a re-added entry and corrupt state) and reports failure instead of a false 'removed' ([2], [4]). - in_flight defer is gated on defer_if_busy: autonomous callers (health loop, _refresh_all) defer, but a DISPATCH reconnects rather than hard-fail a reachable server with an in-flight sibling ([3]). - Cleanup: extract _schedule_next_ping (was a copy-pasted triplet in 3 branches) [5]. Adds 6 tests; the lock-contention test now shadows the caller-timeout constant so it runs in ~1s instead of the full wait. * fix(mcp): address PR review feedback on static reconnect - _ensure_static_connected: skip breaker record on CancelledError (cancel proves nothing about the server; aligns docstring with impl) - reconnect_sync: add asyncio.timeout wrapper so discovery-phase stalls get a clean TimeoutError inside the lock - reconnect_sync: change except Exception to except BaseException so CancelledError from future.cancel() triggers catalog cleanup - reconnect_sync: null state.session on failure so a tool-less session isn't mistaken for a live one - _static_reconnect_one: use fresh monotonic clock for backoff gate instead of stale tick-start snapshot; remove dead now param - _static_health_tick: correct docstring (0.5s clamp prevents busy-spin, not 'no sleep through short backoff') - Test: new test for reconnect_sync timeout + catalog cleanup - Test: update cancelled-attempt test for new CancelledError semantics - Test: narrow except BaseException to except Exception + type hints - Test: fix typo 'Understone' -> 'Turnstone' - Test: remove unused stale_now variable; update mock signatures * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Stefano Maffeis
|
bcf509a440 |
Drop dead last_ws_id/last_tool_call_id columns from mcp_pending_consent
Migration 054 added these columns but they were never populated (the sole writer hardcodes None) and never read by any query, dashboard, or API. Drop them so the schema matches reality. Closes #769 Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
Patrick Buckley
|
acc262c405 |
feat(mcp): proactively keep consented OAuth (OBO) tokens fresh for unattended work
Per-user OAuth (auth_type=oauth_user) token refresh is entirely lazy: a token is refreshed only when a tool is dispatched or a session binds the acting user, and a dead refresh token is discovered only when a dispatch fails. That assumes a human is driving the session, which breaks for autonomous / scheduled work acting on behalf of an absent user — the token may be expired (latency), in a transient-failure cooldown (unavailable), or the grant may be dead with nobody present to re-consent. The only periodic MCP-loop task, idle eviction, actively tears OBO connections down; nothing keeps tokens warm. Add a background token-freshness sweep that keeps every consented oauth_user grant hot WITHOUT keeping connections warm and WITHOUT mutating consent state on a timer. Sweep (_user_token_sweep_loop, default 240s): - Enumerates consented (user, server) grants from the token store and runs the canonical refresh path for each. Strictly oauth_user-scoped: gates on _oauth_user_server_names and drives off mcp_user_tokens rows, so a static / no-auth server — which has neither — is structurally invisible (no DB scan, no authorization-server round-trip, no MCP-server call). Never connects to the MCP server; connections stay lazy. - Observe-only: passes revoke_on_failure=False (new parameter on get_user_access_token_classified) so a timer NEVER deletes a token, emits token_revoked, or mutates the shared ambiguous-streak / cooldown. A dead grant is only surfaced (proactive dashboard pending-consent badge); the authoritative revoke stays on the lazy-dispatch path where a real user action justifies it. Because the row survives, a spurious server-wide invalid_grant (an AS maintenance window) self-heals — the badge is dropped on the tick the grant works again. - Keepalive: force-refreshes a grant whose refresh token has sat un-exercised past user_token_refresh_keepalive_seconds (default 1800s) even while the access token is still fresh, so a provider that ages out idle refresh tokens can't expire one between a user's real sessions. - Surfaces dead grants once per transition, pinning the pair only after a durable badge write so a failed persist retries rather than being lost. First sweep runs after a short startup grace so a restart surfaces a downed grant within seconds, not a full cadence later. Cadence <= 0 disables the sweep; a positive value is floored (30s) so a misconfigured tiny cadence can't turn the loop into a busy-loop. Reuses the per-key refresh lock, so a keepalive force cannot double-refresh against a concurrent dispatch. Storage: add list_mcp_user_token_reconcile_targets() returning (user_id, server_name, COALESCE(last_refreshed, created)) — expiry-unfiltered, no ciphertext projected — on the protocol, sqlite, and postgres backends. Tests: the sweep's no-auth invisibility (zero DB / AS calls with no oauth_user server), observe-only non-destruction (token kept and shared streak untouched on a background permanent / ambiguous failure), keepalive gating, badge persist-then-pin retry, self-heal on recovery, cadence clamp / disable, and the storage enumerator. |
||
|
Patrick Buckley
|
62034378c6 |
fix(skills): address Copilot review — task_agent doc refs + gate fail-closed on missing storage
- task_agent.json referenced a non-existent `skill(action='search', query=...)` in two spots. The discovery tool is `skills` and the action is `find` (`search` is an activation value, not an action), so the guidance would mislead the model. Corrected both to `skills(action='find', query='...')`, matching the tool's own error strings. - _high_risk_skill_denied returned "" (allow) when get_storage() is None — an asymmetry with the fail-closed lookup-exception path added earlier. A risk gate that can't verify the tier must DENY, not wave the skill through, so storage-unavailable (None) now denies too; both paths share one denial. |
||
|
Patrick Buckley
|
bcb8c5ab88 |
fix(skills): harden task_agent / persona / skill activation from whole-PR review
Two independent multi-agent reviews of the branch (high, then max effort) found authority-confinement and robustness defects the per-step reviews could not see. This commit addresses every confirmed finding. task_agent turned out to be the surface that lagged its siblings on nearly every axis. Risk gate (most severe): - task_agent(skill=...) never enforced the high/critical-risk PRINCIPAL-load- only gate that skills(load) / spawn_workstream / spawn_batch enforce, so a model could route around it by delegating activation to a sub-agent. Enforce it inline in _prepare_task on the row already fetched (no re-query, no drift between get_skill_by_name and get_prompt_template_by_name). - _high_risk_skill_denied now fails CLOSED on a storage fault: deny, never wave the skill through. Denying (not returning "") also keeps spawn_batch's per-row partial-success intact under a transient blip. - (first round) extracted _high_risk_skill_denied onto spawn_workstream / spawn_batch, closing the coordinator-side bypass. Persona confinement (Principle 7 attenuation on the task_agent edge): - A restrictive persona now attenuates the sub-agent's TOOLS, not just its identity text — the tool lever is frozen into the item and filtered before _run_agent. - Honor ALL FOUR persona levers on the sub-agent, not two: a child persona's mcp-off and memory-off levers now drop MCP tools (mcp__* + read_resource / use_prompt) and the memory tool, matching a main session under the persona. - Cap the sub-agent by the PARENT session's own persona grant too, so a restricted principal cannot escalate authority by spawning. - Add persona to the task_agent judge/audit func_args projection (policy + audit parity with spawn). - Persona-resolution failures defer to a clean tool error (try/except mirroring _validate_child_persona) instead of an opaque "internal error". Substitution / capability: - substitute_args=False for capability contexts (defaults, task_agent) so a literal $ARGUMENTS / $N in a body is preserved, not blanked; env vars still resolve. The literal-$ARGUMENTS scan is deferred behind that guard (skipped on every capability render). - Drop the CLAUDE_SKILL_DIR alias (canonical TURNSTONE_SKILL_DIR only). That name also lives in bash, where turnstone-as-a-node-inside-Claude-Code must not shadow the host's value; claiming it in the prompt but deferring in bash diverged the two surfaces (a review finding). turnstone now claims it in neither surface. The CLAUDE_SESSION_ID / CLAUDE_EFFORT prompt aliases stay (pure prompt values, no bash-namespace collision). Skills-as-context: - DEFAULT (always-on) skills stay in the identity system message — the standing baseline, never a mid-session cache-bust; only a NAMED applied skill moves to the user-role capability message. This shrinks the pending model-adherence eval surface to the named-skill move alone. Cleanups: consolidate a duplicated rationale comment; correct the now-stale "task agents are not persona-filtered" note. PRE-MERGE GATE unchanged: the §7 Q1 model-adherence eval (named-skill move, this branch vs main) is not runnable in-tree and must clear before merge. |
||
|
Patrick Buckley
|
fec5067fcd |
feat(skills): gate model-initiated load of high/critical-risk skills
A skill can auto-fire tools (auto_approve + allowed_tools) once loaded, so letting the model activate a risky skill through skills(action='load') is an injection-steerable lane that widens authority behind a rubber-stampable approval. Deny it: high/critical-risk skills are now PRINCIPAL-load-only -- the model gets a clean error pointing the user at /skill, and the operator loads such skills explicitly (handle_command /skill and cli --skill call set_skill directly and bypass this gate). The scanner-computed risk_level is the gate signal -- it already escalates for the auto_approve + allowed_tools authority the create path warns about -- so no new column or migration is needed. The check can only DENY, never widen, so it is safe by construction (HYPOTHESIS.md Principle 7 / design section 5.5). Remaining step-5 follow-ups, out of scope here: persisting the literal disable-model-invocation frontmatter field for arbitrary author-marked skills (needs a column) and deprecating the vestigial variables mechanism. |
||
|
Patrick Buckley
|
b0ed67aa60 |
refactor(skills): move applied-skill body out of the identity system message
Step 3 of the skill/persona split: an applied skill (including default skills)
is CAPABILITY context, so its body no longer sits in the identity system
message. It rides its own message (user role) after the identity block, with a
short intro naming the active skill. The <available-skills> discovery catalog
stays in the system message.
Two consequences:
- The cached identity prefix (persona BASE + ENV + POLICIES + catalogs) stays
stable across skills(load): loading/clearing a skill changes only the
trailing capability message, not the identity block.
- The task_agent base (_agent_system_messages) is snapshotted BEFORE the skill
block, so a parent's applied skill no longer leaks into the sub-agent prefix
(the sub-agent supplies its own persona identity and skill via _exec_task).
PRE-MERGE GATE: the design gates this on a model-adherence eval (this branch vs
main) verifying the model follows a skill as well from a context message as it
did from the system message (design section 7 Q1; ASSUMED-neutral, UNVERIFIED).
That eval is not runnable in-tree and MUST clear before this branch merges.
Mechanical structure is pinned by TestSkillContextPlacement.
Deferred follow-up: sub-agent (task_agent) skill-resource materialization, so
${TURNSTONE_SKILL_DIR} stays literal on that path (unchanged since step 1).
Test helpers (_sys_content) now read the full prompt prefix (identity + skill
context) so placement-agnostic assertions keep working.
|
||
|
Patrick Buckley
|
c023272b16 |
refactor(skills): task_agent identity from persona, skill demoted to capability
Before, a task_agent's system identity WAS its skill (skill body concatenated into the sub-agent's system message), and #683 deliberately gave task_agent no persona. Now that personas are first-class on every creation/spawn path, make task_agent consistent: identity comes from a persona, the skill is capability. - task_agent gains persona= (validated at prep against the interactive kind, the general-purpose personas a worker can adopt). The resolved base prompt is frozen into the approval item; _exec_task never re-reads storage. - Default identity (no persona=) stays _TASK_DEFAULT_IDENTITY. The one-shot, tool-over-narration operating guidance always layers on top. - skill= is now CAPABILITY: rendered through the shared pipeline (step 1) and delivered as a distinct user-role context turn ahead of the task, never fused into the identity. Consecutive user turns coalesce at the provider boundary (Anthropic _merge_consecutive), so this is wire-safe. Updates the task_agent tool schema (persona param; skill reframed as capability) and flips the persona guard test (task_agent HAS a persona param now). Sub-agent skill-resource materialization and the interactive skills->context move remain follow-ups. |
||
|
Patrick Buckley
|
3568a6db50 |
refactor(skills): unify skill-body substitution across invocation contexts
Skill-body placeholder substitution diverged by invocation context:
interactive load, default skills, and spawn-child ran the full
render + spec-substitute, while task_agent (_exec_task) ran
_render_template only -- so $ARGUMENTS and ${...} env placeholders
rendered literally on that one path.
Introduce _render_skill_body as the single render+substitute path and
route interactive load, defaults, and task_agent through it, so a skill
reading ${TURNSTONE_EFFORT} or $ARGUMENTS resolves identically wherever
it runs. A sub-agent has no invocation args, so bare $ARGUMENTS and the
positional $N / $ARGUMENTS[N] forms resolve to empty there -- matching
the defaults and spawn-child paths, not the old verbatim passthrough.
- Add ${TURNSTONE_*} as the canonical vendor-neutral spelling for the
env placeholders (SESSION_ID, EFFORT, SKILL_DIR); keep ${CLAUDE_*} as
a permanent back-compat alias so imported skills keep resolving.
- Bash env: export TURNSTONE_SKILL_DIR and SKILL_RESOURCES_DIR
unconditionally, but add CLAUDE_SKILL_DIR only when the host has not
set it, so turnstone does not shadow a real value when it runs as a
node inside Claude Code.
- Materialize skill resources before substituting the body, so
${TURNSTONE_SKILL_DIR} resolves to the concrete bundle path on the
interactive path.
Sub-agent resource materialization and moving identity to a first-class
persona are left to follow-ups; ${TURNSTONE_SKILL_DIR} stays literal on
the task_agent path for now (unchanged from prior behavior).
|
||
|
Patrick Buckley
|
9bf8d5699b |
fix(test): harden resolve_when_pending — cancellable worker (review)
Address Copilot review: cancel() now signals the worker to stop (a cancellation Event) and joins only when started, so a test that errors before the approval registers can't leak the worker or resolve late into a finished test. The worker reads _pending_approval via getattr so a UI without it can't crash the thread into a silent death (leaving approve_tools blocked the full timeout), and only resolves when it actually observed the registration. |
||
|
Patrick Buckley
|
c0ff00a1ff |
fix(test): eliminate lost-wakeup race in approval-prompt tests
The UI-approval tests drive a blocking approve_tools() by firing resolve_approval() from a fixed 0.05s threading.Timer. approve_tools does _approval_event.clear() -> register _pending_approval -> wait(3600s); on a slow/loaded runner the timer can fire the event's .set() BEFORE that .clear(), so the wakeup is wiped and approve_tools blocks the full _APPROVAL_WAIT_TIMEOUT (one hour) -- surfacing as an intermittent CI hang (observed on the 3.12 runner ~15% into the suite; fast runners win the race, so 3.11/3.13 pass the same commit). Replace the fixed-delay timer with resolve_when_pending() (tests/conftest.py): it waits until the approval is actually registered -- which happens AFTER the clear -- before resolving, so the set can never be lost. The helper mirrors threading.Timer's start()/cancel() so the surrounding scaffolding is unchanged. 10 sites across 3 files; the verdict-delivery timer (bounded to its own 5s budget, not a hang) is left as-is. Validated: the 3 files pass 20/20 under single-CPU stress (taskset -c 0) with no hang or thread leak. |
||
|
Patrick Buckley
|
45010f5890 |
fix(eval): address Copilot review — checkout-agnostic docs + skill validation
- Docstrings/help said the treatment skill 'composes into the system message'. This harness runs on both checkouts (system on main, a context turn on the placement-refactor branch), so the wording now describes the natural set_skill composition path without asserting a placement. - Validate each skill-bearing case's 'skill' shape up front (driver + CLI) so a malformed dataset fails with a clear error, not a mid-run KeyError. Pinned by test_rejects_malformed_skill. |
||
|
Patrick Buckley
|
845df69031 |
feat(eval): skill-adherence measurement mode
Add a two-arm skill-adherence mode to the eval measurement substrate that measures whether a NAMED skill changes tool-use behaviour, so skill-in-system (main) can be compared against skill-in-context. - _run_single_test gains skill/skill_mode: skill_mode builds HeadlessSession under natural composition (no system_prompt_override) and, for the treatment arm, seeds the skill into the temp DB and activates it via the real set_skill path so the skill body folds into the system message under test. skill_mode defaults False, so the optimizer/measure paths are unchanged. - Thread skill/skill_mode through _run_and_score_subprocess, _run_iteration and _run_iteration_parallel (serial + parallel). - run_skill_adherence: per case, run treatment (skill) vs control (no skill) n_runs each, score against expected_actions, report per-case lift = pass_rate(treatment) - pass_rate(control) and the mean lift. The control isolates the skill's causal effect. - turnstone-eval --skill-adherence <dataset>: loads a skill-scenario dataset and prints a treatment/control/lift table. - eval_skill_adherence.json: authored search-first / test-after-edit / changelog-update scenarios, chosen so the base model does not do the action by default. - tests: plumbing proof (skill folds into system_messages for treatment, absent for control) + lift-math aggregation. |
||
|
Patrick Buckley
|
dbf389783e |
refactor(personas): file-backed built-in prompts, explicit source column
Built-in persona base prompts move from inline DB text / base.md into prompts/personas/<slug>.md — code-owned, PR-reviewable, drift-proof. base.md / base_coordinator.md become personas/engineer.md / orchestrator.md. Prompt source is now explicit in storage instead of inferred in app logic: a new base_prompt_file column plus CHECK (base_prompt IS NOT NULL OR base_prompt_file IS NOT NULL) — two nullable columns, never both empty. Resolution is a coalesce (base_prompt else load(base_prompt_file)), frozen into the workstream stamp at creation. base_prompt_file marks a persona as built-in (code-only, un-archivable); an operator override on a built-in is allowed and wins over the file. "Inherit the kind default" is a workstream-creation act (is_default), not a persona-row state. Migration 063: - seeds reference their file (base_prompt NULL); no runtime file reads — the backfill's frozen prompt text is inlined as a point-in-time snapshot so migration history stays self-contained and reproducible. - every existing workstream is stamped by kind (creative -> writer, else the kind default), set-based (INSERT..SELECT via temp tables) with the persona column added after the bulk writes to shorten its lock window. Storage guards (both backends): operators must supply base_prompt; built-ins can't be archived or have base_prompt_file set via the API; clearing an operator persona's only source is rejected. Follow-ups reviewed alongside (#756): soft-set visibility docstring scoped to per-process; _apply_persona_snapshot / _current_persona_snapshot own the stamp round-trip; spawn approval-header args (skill/name/target_node) flattened+capped like persona; server-side tool injection generalized to replace-only (client-def gated, incl. the xAI include forwarding). Seed copy revised (researcher soft; de-costumed prose; engineer de-biased). New test_schema_parity asserts create_all matches the alembic head. Closes #683 groundwork; ruff + strict mypy clean, full suite green. |
||
|
Patrick Buckley
|
75c2e6c364 |
fix(personas): apply PR review feedback
The roster persona merge distinguishes key-absence (pre-persona node in a rolling upgrade — preserve) from present-but-empty (authoritative unstamped — accept), so a stale in-memory value can never mask the snapshot on an immutable field. The node create route caps the persona slug at 64 like the console proxy, keeping oversized values out of the storage lookup and the reflected 400 text. The four persona admin handlers drop their redundant function-local asyncio imports, and the DELETE-route test moves its request out of the assert statement. |
||
|
Patrick Buckley
|
09c05733c6 |
test(personas): harden the guard suite — real paths over scripted events
The rank guard now derives needs_approval through the real _prepare_tool on a bash call under an allowlisting persona instead of scripting the flag, and asserts the approval gate actually fires. The row-shape guard's source-grep is replaced with behavioral collector tests driving both ws_created lanes (poll-diff and SSE relay), plus a proxy-forward twin and a saved-list value assertion that would catch positional column mix-ups. Receiving-side stamping gets its first HTTP coverage: create with an explicit persona under workstreams.create only (selection needs no persona perm), kind-mismatch and unknown-name 400s, omitted-persona default stamping, the 503 on a failed default lookup, and the clean-None legacy lane. Resume adoption is pinned end to end: a corrupt target stamp leaves the session fully intact, an MCP-on stamp is refused when the client was persona-gated at construction, and an MCP-off stamp drops the live surface (listeners deregistered, toolsets reset). Soft-set tool_search expansion recomposes the prompt exactly once; legacy sessions never recompose. Compaction legs run real flows now: spill plus the recall-pointer variant under memory-off with recall visible vs hidden, and a full stamp surviving compaction-then-resume. Migration 063 gains the downgrade config-cleanup case (stamps removed, creative_mode preserved) and the conversion idempotency case (already-stamped creative rows don't crash the upgrade). RBAC coverage goes cross-perm: read-only and write-only principals hit every verb (a wrong-perm-name regression in any handler is now visible), archive and default-flip succeed through PATCH, persona.* strings round-trip the role editors and the overrides overlay, and the production route table is asserted directly (no DELETE registered). Endpoint/storage fixtures move off migration-seed names; storage hardening tests cover the size caps, corrupt-row reads, the TypeError-to-ValueError ordering, the duplicate-name race mapping, and the single-default backstop. Shell asserts pin the new picker surfaces and drop the last persona-as-kind wording. |
||
|
Patrick Buckley
|
5d1d34cd82 |
fix(personas): close review findings across the envelope, resume, and RBAC lanes
Provider search gating (replace-only): native web search now stands in for
a client web_search def that survived the persona visibility filter — on
both OpenAI surfaces and both injection lanes (web_search_options, the
server_side_tools loop, and _convert_tools' capability lane). A scribe or
any envelope hiding web_search stays search-free on search-capable models;
coordinators and tool-less utility calls stop receiving search too.
Resume stamp discipline: resume() loads config and parses the target's
stamp BEFORE touching session identity/history, so a corrupt stamp raises
with the session intact instead of half-adopting and then 'repairing' the
target's stamp on the next config save. The MCP lever now follows the
stamp on mid-session adoption: an MCP-off stamp drops the live surface in
place (listeners deregistered, toolsets reset); adopting an MCP-on stamp
into a session whose persona gated the client off is refused loudly (the
surface cannot be rebuilt post-construction). The REPL /resume handler
reports these errors instead of crashing the CLI.
Fail-closed default lane: a FAILED default-persona lookup at create is a
503 (routes) / clear exit (CLI) instead of silently degrading to the
unstamped stock envelope; a clean 'no default configured' still creates
legacy. resolve_persona_for_kind reports storage-unavailable distinctly
from unknown-persona.
Soft-set governance: tool_search expansion under a persona visibility set
recomposes the system prompt so tool-gated policy segments land with the
tool they gate. MCP resource/prompt catalogs gate on read_resource /
use_prompt visibility. Spawn judge/audit projections carry persona (the
human approval header already did). Active-list rows carry persona like
their project_id twin.
RBAC catalogs: persona.{create,read,write} join _VALID_PERMISSIONS and
the roles-editor sections, making the documented grant-outward path real.
Storage hardening: default-persona invariants move to a shared _utils
helper (validate + demote) with a pg advisory xact lock serializing
promotions and a post-promote single-default assertion; create maps the
unique-name race to the same ValueError as the pre-check; reads validate
JSON shape loudly (naming the persona); serialize enforces size caps;
field validation runs before invariant checks so malformed input is a 400,
never a TypeError-500. org_id guards explicit null and caps at 64.
Also: base_override='' means 'no override' at the compose boundary;
persona tag flattened/capped before the spawn approval header; /creative
redirect resolves the writer persona before advertising it; memory-nudge
gating unified through _nudges_enabled.
Provider/row-shape tests updated to the new contracts (the old ones
pinned the injection hole and the pre-persona row shape).
|
||
|
Patrick Buckley
|
e2dcd2bd6b |
fix(personas): apply review findings — stamp adoption on fork/restore, PATCH semantics, gating
Review pass over the branch surfaced real defects, all fixed here with regression guards: - Fork-resume (resume_ws) adopts the SOURCE workstream's stamp, resolved pre-construction so all four levers (including the construction-time MCP gate) bind the fork; a corrupt source stamp is a loud 400, an unstamped legacy source forks unstamped — never the kind default. Watch-restore and CLI --resume thread the stamp the same way, closing an MCP leak where a restored MCP-off workstream re-merged the catalog. - SessionManager.open parses the stamp inside the install guard so a corrupt stamp releases the reserved slot; a retry reproduces the loud error instead of 'already tracked'. - Mid-session resume() adopting a stamp rebuilds the tool_search pathway to match (hard set drops it, soft set force-constructs it); soft persona sets survive the global tool-search setting being off. - Memory nudges gate on actual memory-tool VISIBILITY, not just the memory lever, so an allowlist that hides the tool also silences the nudges that point at it; post-compaction resume gets a no-recall nudge variant when the pointer would dangle. - Console PATCH: explicit null flags from UpdatePersonaRequest no longer archive the persona or flip levers on a rename; multi-kind personas survive a shelf edit; admin list ships the per-kind tool_inventory so the shelf checklist tracks the server inventory instead of a hardcoded JS list; admin CRUD moved off the event loop. - Migration 063 converts legacy creative_mode workstreams to the full writer stamp (downgrade removes all persona keys). - REPL: /new passes the persona; /workstreams unpacks the widened row. - Shared resolve_persona_for_kind is the single eligibility rule for the HTTP handler, CLI, and spawn precheck; spawn_batch memoizes the persona lookup; ToolSearchManager.is_expanded gives the visibility tail an O(1) probe. |
||
|
Patrick Buckley
|
9706fc5d9c |
test(personas): guard suite — rank guard, levers, spawn, RBAC, immutability
The 15 guards from the design brief: the approval path is untouched under any persona (rank guard); empty-toolset personas compose no tools block and put zero definitions on the wire; the tool_search escape hatch is soft when included (discovered tools union with the allowlist) and hard when omitted (pathway disabled, including native defer_loading); memory-off suppresses recall injection, the memory tool, and memory-directed nudges while behavioural nudges and task-agent tools survive; MCP-off is session-wide and refresh-proof; spawn validates persona at prep time and never inherits the parent's; task_agent's schema stays persona-free; the stamp is immutable, survives SessionManager.open threading, and corrupt stamps fail construction loudly; mandatory prompt policies compose under every persona; CLI resolution (extracted to resolve_cli_persona_kwargs for testability) loads seeds, exits clearly on unknown names, and adopts the resume target's stamp; persona edits/archives never touch stamped workstreams; and the row-shape contract twins carry the persona field. RBAC endpoint coverage: admin CRUD 403s without persona.* and succeeds with it; the picker feed needs no persona permission and hides archived personas; invariant violations surface as 400s; DELETE is 405. |