Compare commits

...

29 Commits

Author SHA1 Message Date
Patrick Buckley ac1fd67137 chore: bump version to 1.5.3 2026-04-30 13:34:35 -07:00
Patrick Buckley 1b40ae79f9 fix(storage): address PR #457 review feedback
Three issues from the Copilot review on PR #457:

1. SQLite race in bulk_close_stale_orphans (Copilot): the SELECT-then-
   UPDATE flow doesn't re-apply the eligibility predicates on the
   UPDATE, so a row that gets touch_workstream-bumped (or set_state-
   transitioned) between the two statements would still be flipped
   to closed.  Postgres dodges this via UPDATE...RETURNING (one atomic
   statement); SQLite needs the explicit re-application.  Fix: rebuild
   the WHERE conditions list once, apply on both SELECT and UPDATE,
   then SELECT-back by ``state='closed' AND updated=now`` to get the
   accurate closed-id list.  A row that became fresh between the two
   statements skips the UPDATE entirely.

2. SQLite IN-clause bind-parameter limit (Copilot): default 999 cap
   could be exceeded on a backlog reap (e.g. after a long outage).
   Chunked the candidate id list at 500 — same chunk size
   prune_workstreams (line 453) uses for the same reason.

3. Wall-clock-dependent test asserts (Copilot, two locations): the
   tests asserted ``updated > '2024-01-01T00:00:00'`` which is fragile
   on systems with skewed clocks or pre-2024 dates.  Replaced with
   ``updated != stale_seed`` — captures the same intent (the value
   was bumped) without depending on wall-clock date.

Two ``...``-as-no-op flags from github-code-quality were false
positives — ``...`` is the standard Python idiom for Protocol method
bodies and matches every other method in _protocol.py.  No code change.
2026-04-30 13:34:15 -07:00
Patrick Buckley b078ddccf0 fix(session_manager): scope orphan reaper by services.last_heartbeat
Replaces the ``node_id == self_node_id`` orphan-scoping heuristic from
earlier on this branch with liveness-based scoping using
``services.last_heartbeat``.  The heuristic was wrong for the post-#384
world: PR #384 (refactor: replace hash-ring rebalancer with rendezvous
hashing) deleted the rebalancer that used to keep workstreams.node_id
pointing at a live node.  Without it, ``workstreams.node_id`` is now
stamped at create time and never updated, so in containerized
deployments with dynamic hostnames a dead pod's rows have ``node_id``
matching no surviving service — they'd accumulate forever under the old
heuristic.

services.last_heartbeat is the same primitive the rendezvous router
uses for routing.  Reusing it here keeps reap scoping aligned with
routing: dead pods' rows fall out of the live set after the heartbeat
window and become reapable; alive pods' rows stay protected as long as
they heartbeat.

Mechanics:

- ``bulk_close_stale_orphans`` parameter renamed
  ``node_id: str | None`` → ``live_node_ids: list[str] | None``.  The
  WHERE clause becomes ``(node_id IS NULL OR node_id NOT IN
  live_node_ids)``.  ``None`` skips the filter entirely (single-process
  / tests / operator backfill).  ``[]`` treats every row as
  unprotected.
- ``SessionManager.close_idle`` pass 2 calls
  ``storage.list_services(self._service_type)`` to enumerate live
  peers, passes their service_ids as ``live_node_ids``.  ``_service_type``
  is derived from ``self.kind`` (INTERACTIVE→"server",
  COORDINATOR→"console") via a module-level mapping — no constructor
  param, so production wiring can't miswire the kind/service_type
  pairing.
- list_services failure → pass 2 is skipped this tick (conservative;
  never reap when liveness state is unknown).  Pass 1 still runs.
- ``workstreams.node_id`` with NULL value is always eligible — defends
  against ANSI ``NULL NOT IN (...)`` evaluating to NULL (not TRUE) and
  silently protecting orphans forever.
- Migration 048 simplified to ``(kind, updated)``; the new query's
  ``NOT IN (small list)`` predicate against an unbounded-cardinality
  column doesn't index well, so leading ``node_id`` would just add
  write cost.

Tests cover the live-services protection (own/dead/null cases), the
empty-peers reap-all case, the list_services-failure conservative
fallback, both kind/service_type pairings (interactive→"server",
coordinator→"console"), and the combined live_node_ids +
exclude_ws_ids filter matrix.
2026-04-30 13:34:15 -07:00
Patrick Buckley 4b6c93a0e9 perf(storage): partial composite index for the orphan reaper query
bulk_close_stale_orphans runs every min(300s, idle_timeout/4) on
every server and console process.  Its WHERE shape is:

    WHERE kind = ?
      AND state IN ('idle','thinking','attention','running')
      AND updated < ?
      AND node_id = ?           -- multi-node interactive only

At current scale the existing single-column indexes are sufficient —
idx_workstreams_state prunes to non-closed and the planner filters the
rest sequentially.  At 100k+ rows that filter becomes a tablescan-
shaped cost.

A partial index covering only BULK_CLOSE_STATE_VALUES rows matches the
reaper's query exactly while staying tiny — closed rows (typically
95%+ of the table) and error rows are excluded, so the index is
roughly 5% the size a full multi-column index would be.  Write
amplification only kicks in for transitions touching one of the four
covered states.

Column order (node_id, kind, updated): node_id is the most selective
filter for multi-node interactive (each server prunes to its own
node's rows), kind second so coord-only and interactive-only queries
within a node still get index-only scans, updated last so the range
comparison rides the trailing column.

Postgres uses CREATE INDEX CONCURRENTLY so the build is non-blocking
on a live system; SQLite has no concurrent concept and the table-
level write lock already serializes, so a plain CREATE INDEX is fine.
2026-04-30 13:34:15 -07:00
Patrick Buckley 9d283e951f fix(console): periodic idle cleanup for the coordinator pool
The console's coord SessionManager had no idle thread — close_idle was
never called for coordinator workstreams.  This is the worse half of
the lifecycle leak: the dashboard filters via the in-memory pool, so
DB-only orphan coords were invisible.  At empirical diagnosis,
coord closure was 16% (10 closed / 64 total) vs interactive 63%.

Adds _coord_idle_cleanup_thread mirroring turnstone/server.py's
_idle_cleanup_thread but skipping the rate-limiter / global-queue arms
the console doesn't have.  Started from the lifespan when coord_mgr is
constructed and server.workstream_idle_timeout > 0 (reuses the
existing setting — same cadence works for both kinds).

Initial sweep runs INSIDE the thread before the first sleep, not
synchronously in the lifespan: cold-start orphans are reaped without
blocking Starlette boot.  Important because cold start with many DB
orphans (the precise condition this code targets) is exactly when the
UPDATE is most likely to be slow.

Helper takes an optional stop_event parameter purely for tests —
production callers pass None and the daemon runs for process lifetime.
This avoids the SystemExit-from-stub + module-wide filterwarnings
fragility a previous iteration relied on.

Four tests: initial sweep runs before first sleep, ticks fire each
loop, exceptions don't kill the thread, stop_event exits cleanly.
2026-04-30 13:34:15 -07:00
Patrick Buckley 4e407e7d4f fix(session_manager): close DB-orphan workstreams in close_idle
Real bug: workstream rows accumulate in non-closed states (idle,
thinking, attention, running) when their owning process restarts or
crashes.  Empirical diagnosis on a live deployment found ~60 stuck
coord rows in DB invisible to the in-memory-keyed dashboard, plus
100+ interactive rows older than the 2h timeout (one stuck "thinking"
for 2 weeks — impossible across a process restart).

Root cause: close_idle iterates self._workstreams.values() — only the
loaded subset.  Anything left behind by a prior process incarnation
sits in DB forever because nothing ever re-loads it.

This commit gives close_idle a second pass.

Pass 1 (existing, unchanged): close loaded IDLE rows whose
ws.last_active (monotonic) is past timeout.  IDLE-only so legitimately-
attentive rows (waiting for user response) stay live.

Pass 2 (new): bulk-close DB rows of this manager's kind whose updated
is past the wall-clock cutoff and which aren't currently loaded.
Closes the broader BULK_CLOSE_STATE_VALUES set — any matching row is
by definition not loaded by any process and cannot be in a live
interaction.  Scoped by self._node_id so a sibling node can't reap
rows we own (multi-node interactive correctness).  No emit_closed —
never-loaded rows have no SSE listeners expecting them.

Lock invariant: pass 1 holds self._lock briefly to snapshot victims
and pop them (existing behavior).  Pass 2 holds self._lock briefly to
snapshot the loaded keys, then releases before the DB UPDATE so a slow
reaper query can't block create/get/set_state.

Also fixes a same-process race in open(): the rehydrate path read DB,
released the manager lock, then re-acquired to install — a concurrent
pass 2 between the two acquisitions snapshots loaded keys without the
in-flight ws_id, and could clobber its DB row to closed.  open() now
calls touch_workstream(ws_id) on rehydrate so the row's updated is
fresh against any pass-2 cutoff.  Pure timestamp write is safe against
concurrent close() (close still wins on the state column).

Three new tests cover the DB orphan pass (basic, exclude-loaded, kind
filter) plus node_id scoping (own/foreign rows, None-skips-filter) and
the open() rehydrate touch.
2026-04-30 13:34:15 -07:00
Patrick Buckley 7ab24e500b fix(storage): add bulk_close_stale_orphans + touch_workstream primitives
Two new methods on the StorageBackend Protocol, with implementations on
both Postgres (UPDATE ... RETURNING) and SQLite (SELECT-then-UPDATE in
one transaction).  No callers yet — wiring lands in subsequent commits.

bulk_close_stale_orphans(kind, cutoff, exclude_ws_ids, node_id=None)
flips rows in BULK_CLOSE_STATE_VALUES (idle/thinking/attention/running)
to closed when their updated timestamp is lex-older than cutoff.  The
node_id filter scopes the reap to a single node's partition — required
for multi-node interactive deployments where each node only has
authority over its own workstreams.node_id rows.  Excludes loaded ids
so the in-memory pass owns those.

touch_workstream(ws_id) bumps updated without changing state.  Used by
the open() rehydrate path to defend against the orphan reaper clobbering
a freshly-loaded row whose DB updated is older than the cutoff.  Pure
timestamp write is safe against concurrent close() because close still
wins on the state column.

BULK_CLOSE_STATE_VALUES is centralized in workstream.py so the two
backend implementations and FakeStorage all agree; if a new transient
state is added to WorkstreamState, deciding whether it joins this set
is part of the change rather than an after-the-fact audit across three
files.

Storage tests (run against both backends via the conftest fixture) cover
the kind/state/cutoff/exclude/node_id matrix plus touch_workstream.
2026-04-30 13:34:15 -07:00
Patrick Buckley 5bcbcb73b9 chore: bump version to 1.5.2 2026-04-30 03:15:59 -07:00
Patrick Buckley af6749421a fix(metacog): drop duplicate [repeat: tool()] info line
The themed ``tool_reminder`` bubble below the tool block already
shows the metacog text, and the tool block immediately above it
carries the tool name — so a separate gray ``[repeat: list_workstreams()
called with same arguments]`` info line was just duplicate visual
noise (operator-visible in the screenshot below the bubble).

Drop the ``ui.on_info`` call inside ``_apply_post_execute_advisories``
that emitted the diagnostic line.  Update the docstring to reflect
that the bubble is the canonical signal.  Rename
``test_emit_repeat_ui_line_on_streak_fire`` →
``test_no_legacy_repeat_info_line_on_streak_fire`` and invert the
assertion.
2026-04-30 03:15:22 -07:00
Patrick Buckley 4d6cb77075 fix(cli): add on_user_reminder + on_tool_reminder to TerminalUI
CI typecheck failed because ``WorkstreamTerminalUI(TerminalUI)``
inherits from ``SessionUI`` (the Protocol), and the Protocol's
``on_user_reminder`` / ``on_tool_reminder`` declarations have empty
bodies — mypy treats those as implicitly abstract, so the subclass
became un-instantiable.

Add real implementations on ``TerminalUI`` that render reminders as
``[metacognition · type] text`` lines in yellow.  This also restores
the metacog signal on the CLI surface (the legacy
``[metacognition: nudge injected — …]`` info-line went away with
``_emit_nudge_ping``; without this commit the CLI showed no signal
at all for metacog nudges).  Tool-channel and user-channel render
identically because terminal output is anchored by stdout flow
rather than by DOM anchor — the line lands directly after the
message it advises.
2026-04-30 03:15:22 -07:00
Patrick Buckley 5c225ef39b docs(metacog): align comments with side-channel + tool-channel scope
Address Copilot's review feedback on PR #456 — the docstrings and
inline comments hadn't all caught up with the architectural shift
across the branch:

  - ``_apply_reminders_for_provider`` docstring: "every user message"
    → role-agnostic, since tool messages also carry ``_reminders``
    (tool_error / repeat).
  - ``_mark_reminders_delivered`` docstring: same role-agnostic
    update; explicitly note both channels.
  - ``_append_user_turn`` callsite comment near
    ``_attach_pending_user_reminders``: still described splicing
    ``<system-reminder>`` blocks into user content; updated to
    reflect the side-channel attach + transient-copy splice at the
    provider boundary.
  - ``_build_history`` block comment: was user-message-only; now
    mentions tool messages and both ``user_reminder`` /
    ``tool_reminder`` SSE events.
  - ``_build_history`` propagation comment: same role-agnostic note
    on the per-entry surface.
  - ``app.js`` ``user_reminder`` SSE handler comment: said the
    bubble renders "above" the user message, but
    ``insertAdjacentElement('afterend', el)`` drops it BELOW.
  - ``app.js`` ``replayHistory`` comment: said "insertBefore drops
    the reminder directly above the just-rendered user bubble";
    same fix — bubble lands BELOW.

No behaviour change.
2026-04-30 03:15:22 -07:00
Patrick Buckley f5a843f44a fix(metacog): drop write-success-clear so sequential same-call streaks fire
The repeat-detection block in ``_apply_post_execute_advisories`` had
a leftover "clear streak when a write tool succeeded" branch from
when ``RepeatDetector`` tracked cumulative counts.  With the
consecutive-streak semantics introduced earlier in the branch the
branch became:

  1. Redundant — any different (name, args) signature already resets
     the streak via ``RepeatDetector.record``, so an intervening
     read/write naturally breaks the streak.
  2. Actively wrong — the clear runs ONCE at the top of each
     ``_apply_post_execute_advisories`` call, before the per-result
     loop records sigs.  In a single parallel batch
     ``[bash, bash, bash]`` the clear runs once and then three
     ``record`` calls accumulate to count=3 in the same call → fires.
     But across three sequential turns, each turn calls
     ``_apply_post_execute_advisories`` fresh, the clear runs at the
     top of each call, and only one ``record`` per call follows — so
     the count never gets above 1 and the canonical
     "small local model stuck on ``bash('echo test')``" pattern
     never triggered the nudge.

The asymmetry only existed for successful calls — failures don't
satisfy the ``not _tool_error_flags.get(tc["id"])`` predicate, so
the clear didn't fire and sequential failures already worked.  The
fix is to drop the clear entirely; ``RepeatDetector``'s
consecutive-streak semantics handle every case uniformly.

Tests:

  - ``test_successful_write_clears_streak`` →
    ``test_intervening_different_call_resets_streak`` —
    rewords the assertion to reflect the actual mechanism (any
    different sig resets, write-or-otherwise) since "writes clear"
    was the bug, not the contract.
  - ``test_failed_write_does_not_clear_streak`` →
    ``test_sequential_bash_failures_fire_repeat`` — same shape, just
    framing fixed.
  - New ``test_sequential_bash_same_command_fires_repeat`` —
    regression for the bug user hit (three sequential successful
    ``bash('echo test')`` calls now correctly fire the nudge).
2026-04-30 03:15:22 -07:00
Patrick Buckley cf44841624 feat(metacog): themed reminder bubble unifies user + tool channels
The yellow themed reminder card introduced for user-channel nudges
(correction / denial / resume / start / completion) now also fronts
tool-channel nudges (tool_error / repeat).  Pre-fix the tool channel
shipped its reminders inside the tool-result envelope via
``wrap_tool_result``, leaking the ``<system-reminder>`` block into
``self.messages`` content (same problem the user channel had before
the side-channel refactor) and surfacing the legacy gray
``[metacognition: nudge injected — …]`` info line as the only
operator-visible signal — duplicated alongside the new themed bubble
for user-channel nudges.

Tool-channel parity:

  - ``_collect_advisories`` now returns
    ``(persistent_advisories, metacog_reminders)``.  Persistent
    advisories (``GuardAdvisory`` / ``UserInterjection``) keep
    riding ``wrap_tool_result`` because they ARE conversation
    history.  Metacognitive reminders extract to the second tuple
    element; the caller attaches them to the tool message dict's
    ``_reminders`` side-channel and emits ``on_tool_reminder``.
  - ``_apply_reminders_for_provider`` already handles ``_reminders``
    on any role, so the tool-channel splice into wire content is
    free.  ``_build_history`` also already propagates
    ``entry["reminders"]`` regardless of role, so reload renders the
    bubble too.
  - ``SessionUI`` Protocol gains ``on_tool_reminder(reminders,
    tool_call_id)``; ``SessionUIBase`` enqueues a ``tool_reminder``
    SSE event with the ``tool_call_id`` anchor.
  - ``_emit_nudge_ping`` had no remaining callers and was removed —
    the themed bubble (live SSE + ``/history`` reload) is the
    canonical operator signal for both channels now.

UI polish (the four fixes the screenshot caught for the user
channel + their tool-channel mirror):

  - Bubble renders BELOW the message it advises (semantically: a
    hint to the model right before its turn).  ``addUserReminder``
    swaps ``insertBefore`` for ``insertAdjacentElement('afterend',
    el)``; ``addToolReminder`` anchors below the ``.ts-approval``
    block whose tool result triggered the batch's reminder.
  - Label uses the full feature name ``metacognition`` (was the
    ``metacog`` shorthand).
  - Card width / alignment inherits from the base ``.msg`` rule —
    ``align-self: flex-end`` and the explicit ``max-width`` are
    gone, so the card matches the user / assistant column instead
    of pinning right-aligned narrow.
  - The legacy ``[metacognition: nudge injected — …]`` gray info
    line is gone for both channels.

Frontend additions:

  - ``Pane.prototype.addToolReminder(reminders, toolCallId)``
    anchors below the ``.ts-approval`` block (live: by
    ``data-call-id``; replay: by "last block in messagesEl"
    fallback, which is correct because messages render in order).
  - SSE switch case ``"tool_reminder"`` calls ``addToolReminder``.
  - ``replayHistory``'s tool-message branch now calls
    ``addToolReminder`` when ``msg.reminders`` is present.
  - ``addUserReminder`` advances its anchor on each loop iteration
    so multiple reminders stack in queued order rather than
    reversed.

Coord console parity:

  - ``coordinator.js`` gains ``appendReminderBubble`` /
    ``appendUserReminderLive`` / ``appendToolReminderLive`` mirroring
    the interactive UI.  The tool-channel anchor walks
    ``toolRows[callId].batch`` to attach below the
    ``.coord-tool-batch`` construct (one bubble per dispatch turn,
    matching the "one nudge per batch even with many failing tools"
    drain).
  - SSE switch handles ``user_reminder`` and ``tool_reminder`` on
    the coord conversation surface.
  - ``/history`` replay propagates ``msg.reminders`` for user and
    tool messages — same wire shape as the interactive pane.
  - ``.msg.user-reminder`` styles moved to
    ``shared_static/chat.css`` so both surfaces inherit the same
    yellow themed bubble from the shared base.

Defensive read on ``_apply_reminders_for_provider`` (per Copilot
review on the closed PR): a malformed ``_reminders`` entry (string,
None, etc. — corruption / partial state) used to abort ``send`` via
AttributeError on the ``.get("text", "")`` call.  Filter to dicts
before building the block, mirroring the same filter
``_build_history`` already applies on the wire-out side; an
all-malformed list passes through as no-reminders.

Tests:

  - ``test_collect_advisories_drains_tool_buffer_on_last_result``
    rewritten to assert the ``(persistent, metacog)`` tuple shape
    and that ``MetacognitiveAdvisory`` no longer appears in the
    persistent list.
  - ``test_collect_advisories_holds_*`` and ``_drops_*`` updated for
    tuple return.
  - ``test_attach_emits_visibility_ping`` /
    ``test_collect_advisories_emits_visibility_ping`` inverted to
    assert the legacy gray line is gone on both channels.
  - ``TestSessionUIBaseToolReminderHook`` covers the new SSE event
    shape with the ``tool_call_id`` anchor.
  - ``test_malformed_reminders_filtered_out`` and
    ``test_all_malformed_reminders_passes_through`` cover the
    Copilot-flagged defensive filter.
2026-04-30 03:15:22 -07:00
Patrick Buckley 7ffab6a272 fix(session): metacog reminders ride a side-channel, not user content
User-channel metacognitive nudges (correction, denial, resume, start,
completion) used to be spliced into ``user_msg["content"]`` permanently,
which leaked the ``<system-reminder>`` envelope into every consumer of
``self.messages`` — UI replay (mitigated by a regex strip in /history),
compaction, title generation, and any future channel adapter that
echoes conversation context.  The /history strip was a band-aid;
compaction and title-gen still saw the raw spliced text.

Switch to a side-channel: ``_attach_pending_user_reminders`` writes the
rendered reminder list to ``user_msg["_reminders"]`` (sibling key,
leading-underscore convention shared with ``_attachments_meta`` /
``_provider_content``).  At the provider boundary, a new
``_apply_reminders_for_provider`` builds a transient shallow-copy with
the reminder spliced into ``content``; the original message dict
stays clean.  ``sanitize_messages`` drops the sibling key on the wire.

Once-per-session-not-per-turn semantics for the wire: after stream
success the loop calls ``_mark_reminders_delivered``, which flips a
``_reminders_delivered`` flag on every user message that carried
reminders into that call.  ``_apply_reminders_for_provider`` skips
already-delivered messages so the model sees each reminder exactly
once (the turn it advised).  ``_build_history`` ignores the delivered
flag entirely, so reconnecting tabs render the same nudge bubble the
originating tab saw via the live ``user_reminder`` SSE event.

UI surface:

  - ``SessionUIBase.on_user_reminder`` enqueues a
    ``{type: "user_reminder", reminders: [...]}`` SSE event with the
    same shape ``_build_history`` surfaces.
  - ``app.js`` renders a ``.msg.user-reminder`` bubble (yellow accent,
    pill-styled) anchored above the user message it advises, both
    live and on history replay.
  - ``replayHistory`` renders ``addUserMessage`` before
    ``addUserReminder`` so the anchor lookup finds the just-rendered
    turn (not a prior one).
  - Multi-tab caveat documented inline: non-originating tabs receive
    no ``user_message`` SSE event today, so a reminder may anchor to
    a stale prior bubble until ``/history`` reload corrects it.

Pre-existing bug surfaced by the audit: cancel handlers
(``GenerationCancelled`` / ``KeyboardInterrupt`` / generic
``Exception``) in ``ChatSession.send`` cleared
``_pending_tool_advisories`` but not the user-channel buffer.  Both
now drain through a shared ``_drain_pending_advisories`` helper.

Removed the ``/history`` regex strip — the side-channel approach
makes it redundant.  Hoisted ``escape_wrapper_tags`` +
``render_system_reminder`` imports to module top (called 2-3× per
turn).

Tests:

  - ``TestApplyRemindersForProvider`` — pass-through-by-reference,
    string + list content splice, escape on user-typed wrapper tags,
    multi-reminder ordering, source-untouched invariant, delivered
    flag skip path, fallback for unexpected content shape.
  - ``TestMarkRemindersDelivered`` — flag idempotency, no-reminders
    no-flag, only marks user messages with reminders.
  - ``TestUpdateTokenTableMsgsParam`` — calibration uses pre-built
    msgs when provided, falls back when not.
  - ``TestUserAdvisoryCancelClear`` — all three cancel branches drain
    the user buffer.
  - ``TestReminderSidechannelIsolation`` — compaction's
    ``_format_messages_for_summary`` and the title-gen extraction
    loop cannot see reminders by construction.
  - ``TestSessionUIBaseUserReminderHook`` — ``on_user_reminder``
    enqueues the right SSE shape.
  - ``TestBuildHistoryReminderPropagation`` — ``entry["reminders"]``
    propagation, absent / empty / multi / coexist-with-attachments
    cases, malformed input filtering, all-malformed elision.
  - ``test_sanitize_messages_strips_underscore_sibling_keys`` covers
    ``_reminders`` and ``_reminders_delivered``.
2026-04-30 03:15:22 -07:00
Patrick Buckley ba3bc9d989 fix(metacog): N>=3 streak detector + drop redundant error-prefix list
Cleanup pass on the metacognitive nudge stack — restores pre-split
errored-counts-toward-repeat behaviour and tightens the is_error
plumbing through the per-batch advisory hook.

The per-batch hook in ``_run_loop`` was duplicating the is_error
signal: ``self._tool_error_flags`` (set by ``_report_tool_result``)
and a string-prefix tuple (``Error`` / ``JSON parse error`` / …).
Two truth sources is what got us here — bash commands that exit
non-zero with normal stdout matched the flag but not the prefix,
the deny path matched the prefix but not the flag, and the result
was that stuck-loop detection silently broke for the most common
failure mode (the model bashing the same broken command).

Single source of truth now:

- ``_execute_tools.run_one`` deny branch routes through
  ``_report_tool_result(is_error=True)`` so denied calls populate
  ``_tool_error_flags`` like every other error path.
- The error-prefix tuple is gone; the write-success-clear gate and
  the tool-error-nudge gate both read ``_tool_error_flags`` only.

Repeat-detection state moves from a ``set[str]`` (fired on the second
identical call, ignored errors entirely) to a ``RepeatDetector``
helper in ``metacognition.py`` with consecutive-streak semantics:

- Threshold raised from 2 to 3 — two-in-a-row was noisy on
  legitimate transient retries; three is the cheapest stuck-loop
  signal.
- Recording a different signature resets the count, so [A, A, B, A]
  is two short streaks of 2 and not a streak of 4. Bounded by O(1)
  state regardless of session length.
- Errored calls now count toward the streak (the split into a
  separate metacog module unintentionally introduced a "skip errors"
  branch — restored).

While there:

- ``metacognition._COOLDOWN_SECS`` default aligned to 300s (matches
  ``MemoryConfig.nudge_cooldown`` and the ``memory.nudge_cooldown``
  config-store default; was set to 30 by an earlier investigation).
- The per-batch advisory block (~80 lines of mixed orchestration
  inside ``_run_loop``) is extracted to
  ``ChatSession._apply_post_execute_advisories`` so the wired
  behaviour is testable without driving ``_run_loop`` end-to-end.
  Producer extraction to a dedicated module is deferred to a
  follow-up; advisory producers all live on ``ChatSession`` for
  now per existing convention.
- Frontend ``appendToolOutput`` (turnstone/ui/static/app.js) now
  skips rendering when the parent approval block is denied or
  the output starts with ``Denied by user`` / ``Blocked``,
  mirroring the history-replay guard at ``_build_history``.
  Previously the live SSE path didn't need this guard because
  the deny path never emitted a ``tool_result`` event; the
  is_error routing change above means it does now, so without
  this guard the badge from ``resolveApproval`` and the SSE
  output would both render.

Tests: 8 unit tests for ``RepeatDetector`` covering streak,
threshold, clear, and intervening-sig reset; 9 integration tests
for ``_apply_post_execute_advisories`` covering the wired
behaviour (3-identical fires warning + advisory + UI line, errored
calls count toward streak as a regression guard, intervening sig
resets streak, successful write clears, failed write does not,
JSON outputs tracked but not inline-warned, tool_error nudge gates
on memory_count, repeat UI line emitted on streak fire).
2026-04-30 03:15:22 -07:00
Patrick Buckley dbe023b4dd chore: bump version to 1.5.1 2026-04-29 20:21:12 -07:00
Patrick Buckley 3d3a8b7367 docs(coord): tighten handleChildState comment per Copilot review
The pre-existing comment said pending_approval_detail "rides on
every ws_state event" — that overstated the case.  The node-side
emit is gated on ``_pending_approval is not None`` so the field is
absent on the steady-state broadcast and possibly null on a node
mid-rolling-upgrade.  The handleChildState fallback already
handles both cases; only the comment was wrong.
2026-04-29 20:20:38 -07:00
Patrick Buckley 99eff73a97 feat(coord): pass pending_approval_detail on child_ws_state SSE events
Inline child approve/deny in the coord tree UI was rendering downstream
of the bulk-live cache (``GET /v1/api/cluster/ws/live``), not the SSE
stream. ``child_ws_state`` events were tiny notifications that fired
an urgent live-bulk fetch on every activity_state transition into/out
of "approval", just to pick up the rich ``pending_approval_detail``
payload. With multiple coord tabs and multi-child workstreams, that
urgent-fetch pattern compounded the SSE-executor pressure Shape A
is unwinding.

Thread the field through every layer so the SSE event itself carries
the rich payload — browser mutates ``liveBadgeCache`` directly,
no urgent fetch:

  1. Node ``WebUI._broadcast_state`` emits ``pending_approval_detail``
     on ``ws_state`` events. Gated on ``_pending_approval is not None``
     so the per-broadcast verdict-cache deepcopy only runs when there
     is actually an approval pending. ``_build_node_snapshot`` also
     projects the field so the console's reconnect-via-snapshot
     resync path delivers it (without this the new collector
     forwarding would never see the field on a snapshot row).

  2. Console ``ClusterCollector._apply_delta`` (live ``ws_state``
     forwarding) and ``_reconcile_node`` (snapshot resync diff) both
     forward the field on the emitted ``cluster_state`` event, AND
     ``_apply_delta`` persists it on the cached ``ws`` dict so the
     ``get_node_detail`` / ``get_snapshot`` endpoints between
     reconciliations don't render stale approve/deny buttons.

  3. ``CoordinatorAdapter._dispatch_child_event`` re-emits the field
     on the ``child_ws_state`` event sent to coord listener queues.

  4. Frontend ``handleChildState`` reads ``ev.pending_approval_detail``
     and writes it directly into ``liveBadgeCache``, tagging the
     entry with ``sseUpdatedAt``. ``flushLiveFetches`` honors that
     tag for ``SSE_AUTHORITATIVE_MS`` (3s) — the upstream
     ``/dashboard`` cache has its own ~2s TTL, so a bulk-poll
     landing right after a transition can otherwise clobber the
     fresh SSE-set state with pre-transition data.

The pre-fix ``enteredApproval`` / ``leftApproval`` urgent-fetch
branch is removed. The 409 stale-call_id retry path keeps its own
urgent fetch — that's a different scenario.

Tests cover the forwarding contract at every layer, the broadcast
gate (event includes the field when an approval is pending,
omits it otherwise, and clears after resolution), and the
``flushLiveFetches`` merge-guard structural shape so a refactor
that keeps the symbols but inverts the comparison or drops the
``prev.live`` check can't pass silently.
2026-04-29 20:20:38 -07:00
Patrick Buckley 99fcd30299 fix(console): offload sync DB calls in coord children/tasks handlers
``coordinator_children`` was calling ``storage.list_workstreams``
directly on the event loop, ``coordinator_tasks`` did the same with
``load_task_envelope``, and ``_resolve_coordinator_or_404`` (called
from both handlers, plus ``coordinator_history`` and
``_resolve_coord_session``) did the same with
``storage.get_workstream`` on its cold-cache path.

The cold-cache resolver path is hit on every console restart,
coordinator eviction, and console proxy hop — exactly when the
event loop is most contended. Three coord tabs reconnecting after a
brief network blip = three serial event-loop blocks per call site.
Other lifted handlers in this file already use
``asyncio.to_thread``; bring all four call sites onto the same
pattern.

Convert ``_resolve_coordinator_or_404`` to ``async def`` and update
its four call sites to ``await``. Exception flow is unchanged.
2026-04-29 20:20:38 -07:00
Patrick Buckley 423c2e80b7 fix(console): isolate coord SSE polling on a dedicated 200-thread pool
Each coord ``events`` SSE listener parks a thread on
``client_queue.get(timeout=5)`` for the connection lifetime. The
console's coord endpoint was wiring no ``sse_executor_lookup`` on
``coord_endpoint_config``, so those parks landed on Python's default
ThreadPoolExecutor (~min(32, cpu_count+4)) and competed with every
other ``asyncio.to_thread`` caller (storage, router, audit). A few
coord tabs against a multi-child workstream would stall new request
handlers waiting for a worker thread.

Mirror the interactive-side precedent (the ``sse_executor`` /
``sse_executor_lookup`` pattern in ``turnstone/server.py``) — build a
dedicated 200-thread ``coord_sse_executor`` in the console lifespan
and wire ``sse_executor_lookup`` onto ``coord_endpoint_config``.
Drain order matters: shut the pool down AFTER ``coord_adapter.shutdown()``
so no new listeners arrive at a dying pool. ``cancel_futures=True``
discards queued-but-not-started futures during teardown.

Update the stale comment on the interactive-side wiring that claimed
"coord wires None and falls back to the default executor" — it now
points at the console's matching wire.
2026-04-29 20:20:38 -07:00
Patrick Buckley a0eb77360d fix(coord): tighten coord_registry refresh logging + comments per round-2 review
Three follow-ups from Copilot's round-2 review on #453.

ValueError logging surfaced the wrong reason
The catch-all ``except ValueError:`` logged ``reason=no_enabled_rows``
unconditionally, but ``ModelRegistry.__init__`` raises ValueError for
five distinct config issues (empty models, default / fallback / agent /
plan / task alias not present).  Operator looking at logs for a
config.toml typo would see the wrong cause.  Switch to
``log.warning("...reason=%s", exc)`` so the actual error message
threads through.  Behavior unchanged — existing registry still
preserved on every ValueError path.

Misleading shutdown() comment
The ``finally`` comment claimed shutdown() was closing clients the
throwaway registry created during DB load.  ``load_model_registry`` only
constructs ModelConfigs and the bare ``ModelRegistry(...)``;
``ModelRegistry.__init__`` leaves ``_clients`` / ``_providers`` empty
and they populate lazily on first resolve.  Today shutdown() iterates
empty dicts.  Comment now says so explicitly while keeping the call
(and its try/except) for forward-compat against an eager-init future.

Stale "probe" wording in test docstring
``test_helper_preserves_registry_when_db_probe_fails`` →
``test_helper_preserves_registry_when_strict_load_fails``.  The
explicit probe was removed in commit 1ba17ed when the helper switched
to ``load_model_registry(..., strict=True)``; the test name and
docstring still talked about a probe.  Updated wording reflects that
the loader's strict-mode re-raise is what the helper catches now.

132 tests pass.
2026-04-29 20:20:38 -07:00
Patrick Buckley 3dd0e196fe 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 20:20:38 -07:00
Patrick Buckley 19c3db5329 test(coord): lock the empty-body gate with a refresh-call spy
bug-3 / q-2 from the multi-stage review on #453: the previous test
``test_update_endpoint_with_empty_body_does_not_blow_up`` asserted only
that the registry's model name was unchanged after an empty PUT, which
holds whether or not the refresh ran (DB row matches registry → refresh
is idempotent).  A regression that always called
``_refresh_console_coord_registry`` — exactly the gate this test was
meant to lock — would have left the assertion green.

Rename to ``test_update_endpoint_skips_refresh_on_empty_body`` and spy
on the helper via ``monkeypatch.setattr``.  Empty-body PUT must register
zero calls; any future change that drops the ``if updates:`` gate now
fails loudly.
2026-04-29 20:20:38 -07:00
Patrick Buckley 0bea72019e fix(coord): strict-mode loader + guarded shutdown for coord_registry refresh
Two correctness follow-ups from the multi-stage code review on #453.

bug-2 / perf-2 (DB probe was theatre + double scan)
The previous probe defended nothing the loader didn't already swallow
on the next line: ``load_model_registry``'s row-loop catches Exception
internally, so a transient DB error after the probe still degrades to
a config.toml-only registry that ``existing.reload()`` would apply,
silently dropping every DB-sourced alias.  And on the happy path each
CRUD paid for two scans of ``model_definitions``.

Add a ``strict: bool = False`` flag to ``load_model_registry``.  When
strict, the row-loop's except re-raises instead of swallowing.  The
helper passes ``strict=True`` and drops the probe — single DB scan,
real failure isolation, the loader's silent fallback can no longer
mask a partial-result regression.  Default ``strict=False`` so CLI /
lifespan callers keep their boot-with-config-fallback behaviour.

bug-1 (shutdown could escape after a successful reload)
``ModelRegistry.shutdown()`` calls ``client.close()`` unguarded, and the
helper's ``finally`` block ran it outside the try/except.  A raising
close() after a successful ``existing.reload()`` would surface as 500
with the registry already mutated and the audit row already recording
success.  Wrap ``new_registry.shutdown()`` in its own try/except that
matches the helper's belt-and-suspenders error policy elsewhere.

The helper's docstring also drops the obsolete probe paragraph; the
``if existing is None: return`` branch gets a one-line inline comment
about the boot-from-empty case (the multi-paragraph version restated
behaviour the line itself documents).

129 tests pass (test_admin_model_registry_refresh + test_model_registry).
2026-04-29 20:20:38 -07:00
Patrick Buckley b9ff52d582 fix(coord): tighten coord_registry refresh — DB probe + accurate boot-from-empty docstring
Two follow-ups from Copilot review of #453:

1. ``load_model_registry`` swallows storage read errors internally
   (logs + continues with config.toml-only models).  Without a strict
   probe in the helper, a transient DB outage on an admin CRUD would
   apply a truncated registry that drops every DB-sourced alias —
   silently, since the loader returns a non-empty registry built from
   ``[models.*]`` config.toml entries.  Add an explicit
   ``storage.list_model_definitions(enabled_only=True)`` probe before
   the loader call so the failure is visible here and the existing
   registry is preserved on outage.

2. The previous docstring claimed ``admin_model_reload`` "has its own
   boot-from-empty story."  It doesn't — it just calls this helper,
   which no-ops when ``coord_registry`` is None.  When no model rows
   existed at boot, lifespan leaves the entire coord subsystem
   uninitialized (no ``coord_mgr``, no ``coord_adapter``, no
   ``session_factory``), and a console restart remains required after
   the operator adds the first row.  Tighten the docstring to admit
   that limitation rather than overstating the helper's reach.

New test ``test_helper_preserves_registry_when_db_probe_fails``
monkeypatches ``list_model_definitions`` to raise and asserts the
existing registry stays intact.
2026-04-29 20:20:38 -07:00
Patrick Buckley 961f999c93 fix(coord): auto-refresh console coord_registry on model-definition changes
The console builds ``app.state.coord_registry`` once at lifespan startup
and the coordinator session factory closes over that exact instance.
Until now, the model-definition admin endpoints (create/update/delete)
wrote to the DB but never touched the in-process registry — and the
explicit reload button only fanned out to nodes via HTTP, also leaving
the console's own registry stale.

Symptom: an operator who changed the underlying model name behind a
local-LLM alias (same alias, same endpoint) saw the DB row update
immediately, but coordinator sessions kept calling the prior model
name until the console process was restarted.

Fix: a new helper ``_refresh_console_coord_registry`` rebuilds a fresh
ModelRegistry from DB and applies it to ``app.state.coord_registry``
via the existing thread-safe ``ModelRegistry.reload()`` — in-place
mutation preserves object identity so the factory closure keeps
working, and active coord sessions auto-pick up the swap on their
next ``send()`` via ``ChatSession._refresh_model_from_registry``.

Wired into four endpoints in ``console/server.py``:

- ``admin_create_model_definition`` — after the DB write
- ``admin_update_model_definition`` — after the DB write, gated on
  ``if updates:`` so a no-op PUT skips the rebuild
- ``admin_delete_model_definition`` — after the DB write
- ``admin_model_reload`` — between ``_publish_config_change`` and
  ``_notify_nodes_model_reload`` so the console mirrors what the
  reload broadcasts to nodes

Failure isolation: a load or reload error leaves the existing registry
intact (logged + swallowed). Coord stays usable while the operator
investigates; the explicit reload remains the user-facing recovery path.

No node fan-out on CRUD — the explicit reload button continues to gate
cluster-wide HTTP propagation, preserving today's UX semantics on shared
clusters.

Tests in ``tests/test_admin_model_registry_refresh.py`` cover:

- helper-level: rebuild from DB, identity preservation, no-op when
  registry is None, preservation on load failure / no-enabled-rows /
  reload validation error
- endpoint-level: create / update / delete / explicit-reload all
  refresh the registry; an empty PUT skips the rebuild
2026-04-29 20:20:38 -07:00
Patrick Buckley 8bdb916064 fix(coord): raise wait_for_workstream message cap to 10 KiB
Production fan-outs are frequently hitting the 6 KiB per-child cap by
just 1-2 KiB, forcing the coordinator into a follow-up inspect_workstream
round-trip per truncated child to recover the tail. Bumping the cap to
10 KiB absorbs the common overshoot without changing the truncation
semantics — truncated=True still fires for genuinely oversized messages,
and inspect_workstream remains the unbounded follow-up.

Worst-case context impact: a 32-child fan-out at the cap is now ~320 KiB
(was ~192 KiB), still well within commercial model context windows.
Typical fan-outs of 1-5 children land at 10-50 KiB.

LAST_ERROR_MAX_LEN (1 KiB) is unchanged — it's intentionally smaller
than the wait cap so error truncation happens at write time, and
1 KiB still sits well below 10 KiB.

WAIT_MESSAGE_MAX_BYTES is referenced by name (not literal 6144) in the
truncation test, so no test value needs updating.
2026-04-29 20:20:38 -07:00
Patrick Buckley 25fe4e728a fix(coord): make coordinator fan out independent work by default
The coordinator system message was descriptive about parallelism rather
than prescriptive — "while multiple children run in parallel" framed
fan-out as incidental, and "a tasks entry, a child to own it" primed
singular delegation. The spawn_batch example (benchmark A, benchmark B,
prototype the winner) showed dependent work under a fan-out framing,
teaching the wrong shape.

In practice the coordinator failed to decompose enumerable requests
("top stories on HN, Lobsters, /r/programming, …") without explicit
"please fan this out" instructions, on both GPT-5.5 and Claude Opus.

base_coordinator.md
- Replace singular "a tasks entry, a child to own it" with plural
  "enumerate the independent units of work, spawn one child per unit,
  run them in parallel by default. Sequential only when one child's
  output feeds the next."
- Tighten the delegation paragraph.

tools_coordinator.md
- Drop the persona repetition that duplicated base_coordinator.md.
- Drop the prescriptive "## Workflow shape" section (the cost note is
  already in wait_for_workstream's tool description; the edit-X
  redirect is already in the persona).
- Drop "in one approval" / "single approval" mentions to avoid
  surfacing approval mechanics to the model.
- Replace the misleading spawn_batch example with truly independent
  items; drop "(up to 10)" which overstated the cap (it's per-call,
  not global, and is documented in the tool schema).
- Add a course-correction example to send_to_workstream — the pattern
  coordinators most often replace with cancel-and-respawn.
- Drop the read action from the tasks examples to keep the lifecycle
  (add → update → remove) coherent.

Coord system message ~16% shorter (4440 → 3722 chars). Both GPT-5.5
and Claude Opus now naturally decompose the news-board prompt without
explicit fan-out instructions. 29 prompt-composition tests pass.
2026-04-29 20:20:38 -07:00
Robert DeAngelis 0d1a32ff65 fix(server): accept --skip-permissions CLI flag (#450)
The server's --help epilog and compose.yaml both reference
--skip-permissions, but the argparser never defined it, so any
container started with SKIP_PERMISSIONS=1 exited with
"unrecognized arguments: --skip-permissions".

Wire the flag through to app.state.skip_permissions, OR-ing it
with the existing tools.skip_permissions config-store setting so
the stored value still works on its own.
2026-04-29 20:20:38 -07:00
41 changed files with 4142 additions and 386 deletions
+1 -1
View File
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
[project]
name = "turnstone"
version = "1.5.0"
version = "1.5.3"
description = "Multi-node AI orchestration platform with tool use, agent routing, and cluster simulation."
readme = "README.md"
license = "BUSL-1.1"
+397
View File
@@ -0,0 +1,397 @@
"""Console-side coord_registry auto-refresh on model-definition CRUD + reload.
The console builds ``app.state.coord_registry`` once at lifespan startup
and the coordinator session factory closes over that exact instance.
Without these refresh hooks, an admin who edits a model definition
through the UI sees the DB change immediately but coordinator sessions
keep calling the prior model name — the on-disk truth diverges from the
in-process registry until the console is restarted.
These tests cover both the helper (``_refresh_coord_registry``)
and the four wired endpoints (create / update / delete / explicit reload)
to lock in:
- in-place mutation: ``coord_registry`` object identity is preserved
across refreshes (factory closure must not be invalidated);
- failure isolation: a load or reload failure leaves the existing
registry intact rather than tearing down a working coordinator;
- no-op safety: the helper short-circuits when ``coord_registry`` is
``None`` so a coord-less console (no model rows at boot) doesn't
500 on routine model-definition CRUD.
"""
from __future__ import annotations
from typing import Any
from unittest.mock import MagicMock
import pytest
from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.routing import Route
from starlette.testclient import TestClient
from tests._coord_test_helpers import _AuthMiddleware
from turnstone.console.server import (
_refresh_coord_registry,
admin_create_model_definition,
admin_delete_model_definition,
admin_model_reload,
admin_update_model_definition,
)
from turnstone.core.model_registry import ModelConfig, ModelRegistry
from turnstone.core.storage._sqlite import SQLiteBackend
# ---------------------------------------------------------------------------
# Fixtures
# ---------------------------------------------------------------------------
@pytest.fixture
def storage(tmp_path: Any) -> SQLiteBackend:
return SQLiteBackend(str(tmp_path / "models.db"))
def _seed_model_def(
storage: SQLiteBackend,
*,
definition_id: str,
alias: str,
model: str,
base_url: str = "http://localhost:8000/v1",
enabled: bool = True,
) -> None:
"""Insert a model definition row directly via the storage API."""
storage.create_model_definition(
definition_id=definition_id,
alias=alias,
model=model,
provider="openai-compatible",
base_url=base_url,
api_key="sk-test",
context_window=8192,
capabilities="{}",
enabled=enabled,
created_by="admin",
)
def _make_config(alias: str, model: str) -> ModelConfig:
return ModelConfig(
alias=alias,
base_url="http://localhost:8000/v1",
api_key="sk-test",
model=model,
context_window=8192,
provider="openai-compatible",
source="db",
)
def _make_registry(
*,
alias: str = "local",
model: str = "old-model",
extras: dict[str, str] | None = None,
) -> ModelRegistry:
"""Build a real ModelRegistry seeded with ``alias`` (the default) plus
any ``extras`` (alias → model). ``ModelRegistry.__init__`` rejects an
empty model dict so tests that exercise the helper need at least one
entry; pass ``extras`` for multi-alias scenarios (e.g. delete-by-alias).
"""
configs = {alias: _make_config(alias, model)}
for extra_alias, extra_model in (extras or {}).items():
configs[extra_alias] = _make_config(extra_alias, extra_model)
return ModelRegistry(configs, default=alias)
class _AppState:
"""Shim mirroring Starlette's ``app.state`` for direct helper tests."""
coord_registry: ModelRegistry | None = None
# ---------------------------------------------------------------------------
# Helper-level tests — ``_refresh_coord_registry`` semantics
# ---------------------------------------------------------------------------
def test_helper_rebuilds_registry_from_db(storage: SQLiteBackend) -> None:
"""Helper pulls the latest DB rows into the existing registry."""
_seed_model_def(storage, definition_id="m1", alias="local", model="new-model")
state = _AppState()
state.coord_registry = _make_registry(alias="local", model="old-model")
_refresh_coord_registry(state, storage)
assert state.coord_registry is not None
assert state.coord_registry.get_config("local").model == "new-model"
def test_helper_preserves_object_identity(storage: SQLiteBackend) -> None:
"""The factory closes over the registry object — refresh must mutate
in place rather than swap the attribute."""
_seed_model_def(storage, definition_id="m1", alias="local", model="new-model")
state = _AppState()
state.coord_registry = _make_registry()
before = id(state.coord_registry)
_refresh_coord_registry(state, storage)
assert id(state.coord_registry) == before
def test_helper_noop_when_coord_registry_none(storage: SQLiteBackend) -> None:
"""Console boot with no model rows leaves coord_registry = None.
The helper must not 500 in that state — CRUD that lands the FIRST
row would otherwise fail before the operator can recover."""
_seed_model_def(storage, definition_id="m1", alias="local", model="m")
state = _AppState()
state.coord_registry = None
_refresh_coord_registry(state, storage) # must not raise
assert state.coord_registry is None
def test_helper_preserves_registry_when_load_fails(
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
) -> None:
"""An unexpected error from ``load_model_registry`` (e.g. config.toml
parse failure, programming bug) must not tear down a working
registry — log + leave the existing instance intact."""
state = _AppState()
state.coord_registry = _make_registry(alias="local", model="old-model")
def _boom(**_kw: Any) -> ModelRegistry:
raise RuntimeError("simulated loader failure")
monkeypatch.setattr("turnstone.core.model_registry.load_model_registry", _boom)
_refresh_coord_registry(state, storage)
assert state.coord_registry is not None
assert state.coord_registry.get_config("local").model == "old-model"
def test_helper_preserves_registry_when_strict_load_fails(
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
) -> None:
"""``load_model_registry`` normally swallows storage read errors and
would return a config.toml-only registry on a transient DB outage —
applying that via ``reload()`` would silently drop every DB-sourced
alias. The helper passes ``strict=True`` so the loader re-raises
instead, the helper's outer except catches it, and the existing
registry survives intact."""
_seed_model_def(storage, definition_id="m1", alias="local", model="db-model")
state = _AppState()
state.coord_registry = _make_registry(alias="local", model="db-model")
def _broken(**_kw: Any) -> Any:
raise RuntimeError("simulated transient DB outage")
monkeypatch.setattr(storage, "list_model_definitions", _broken)
_refresh_coord_registry(state, storage)
assert state.coord_registry is not None
# Existing registry untouched — strict=True surfaced the storage
# error to the helper before the loader's silent fallback could
# produce a truncated registry for reload().
assert state.coord_registry.get_config("local").model == "db-model"
def test_helper_preserves_registry_when_no_enabled_rows(storage: SQLiteBackend) -> None:
"""All rows disabled/deleted: ModelRegistry.__init__ rejects an empty
model dict (raises ValueError). Helper must catch and preserve the
existing registry so coord stays usable while admin restores rows."""
_seed_model_def(storage, definition_id="m1", alias="local", model="m", enabled=False)
state = _AppState()
state.coord_registry = _make_registry(alias="local", model="cached-model")
_refresh_coord_registry(state, storage)
assert state.coord_registry is not None
assert state.coord_registry.get_config("local").model == "cached-model"
def test_helper_preserves_registry_on_reload_validation_error(
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A reload that raises mid-mutation (e.g. validation guard) must
leave the existing registry instance functional."""
_seed_model_def(storage, definition_id="m1", alias="local", model="new-model")
state = _AppState()
state.coord_registry = _make_registry(alias="local", model="old-model")
def _broken_reload(*_a: Any, **_kw: Any) -> None:
raise ValueError("simulated reload validation failure")
monkeypatch.setattr(state.coord_registry, "reload", _broken_reload)
_refresh_coord_registry(state, storage)
# Existing registry still reachable; the broken reload was a no-op
# at the public-facing level.
assert state.coord_registry is not None
assert state.coord_registry.get_config("local").model == "old-model"
# ---------------------------------------------------------------------------
# Endpoint-level integration tests — verify wiring
# ---------------------------------------------------------------------------
def _make_client(storage: SQLiteBackend, registry: ModelRegistry | None) -> TestClient:
"""Build a TestClient wired to the four model-definition endpoints.
Uses the shared header-driven ``_AuthMiddleware`` from
``tests/_coord_test_helpers``; default headers below grant
``admin.models`` permission so the endpoint gate passes.
"""
app = Starlette(
routes=[
Route(
"/v1/api/admin/model-definitions",
admin_create_model_definition,
methods=["POST"],
),
Route(
"/v1/api/admin/model-definitions/reload",
admin_model_reload,
methods=["POST"],
),
Route(
"/v1/api/admin/model-definitions/{definition_id}",
admin_update_model_definition,
methods=["PUT"],
),
Route(
"/v1/api/admin/model-definitions/{definition_id}",
admin_delete_model_definition,
methods=["DELETE"],
),
],
middleware=[Middleware(_AuthMiddleware)],
)
app.state.auth_storage = storage
app.state.coord_registry = registry
# Reload endpoint also touches these — stub them so the test focuses
# on the registry-refresh behaviour without dragging in a full
# collector / proxy_client wiring.
app.state.collector = MagicMock()
app.state.collector.get_all_nodes.return_value = []
app.state.proxy_client = MagicMock()
app.state.config_store = MagicMock()
client = TestClient(app)
client.headers.update({"X-Test-User": "admin", "X-Test-Perms": "admin.models"})
return client
def test_create_endpoint_refreshes_registry(storage: SQLiteBackend) -> None:
"""POST /api/admin/model-definitions bumps the in-process registry
so newly-spawned coord sessions see the new alias immediately."""
# Pre-existing alias (registry needs at least one row)
_seed_model_def(storage, definition_id="m1", alias="local", model="m")
registry = _make_registry(alias="local", model="m")
client = _make_client(storage, registry)
resp = client.post(
"/v1/api/admin/model-definitions",
json={
"alias": "fast",
"model": "fast-model",
"provider": "openai-compatible",
"base_url": "http://localhost:9000/v1",
"api_key": "sk-x",
"context_window": 4096,
},
)
assert resp.status_code == 200, resp.text
assert registry.has_alias("fast")
assert registry.get_config("fast").model == "fast-model"
def test_update_endpoint_refreshes_registry(storage: SQLiteBackend) -> None:
"""PUT swaps the underlying model name behind a stable alias — the
user's reported regression."""
_seed_model_def(storage, definition_id="m1", alias="local", model="old-model")
registry = _make_registry(alias="local", model="old-model")
client = _make_client(storage, registry)
resp = client.put(
"/v1/api/admin/model-definitions/m1",
json={"model": "new-model"},
)
assert resp.status_code == 200, resp.text
assert registry.get_config("local").model == "new-model"
def test_update_endpoint_skips_refresh_on_empty_body(
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
) -> None:
"""An empty PUT body must skip the registry refresh — the
``if updates:`` gate exists because ``load_model_registry`` is
non-trivial and a no-op refresh on every PUT would burn cycles
rebuilding state that hasn't changed. Spy on the helper to lock
the gate down: a regression that drops the conditional would
register a call here and trip the assertion.
"""
from turnstone.console import server as server_module
_seed_model_def(storage, definition_id="m1", alias="local", model="locked-in")
registry = _make_registry(alias="local", model="locked-in")
client = _make_client(storage, registry)
calls: list[tuple[Any, Any]] = []
def _spy(app_state: Any, storage: Any) -> None:
calls.append((app_state, storage))
monkeypatch.setattr(server_module, "_refresh_coord_registry", _spy)
resp = client.put("/v1/api/admin/model-definitions/m1", json={})
assert resp.status_code == 200, resp.text
assert calls == [] # gate held: empty body did not trigger a refresh
def test_delete_endpoint_refreshes_registry(storage: SQLiteBackend) -> None:
"""DELETE drops the alias from the in-process registry too — a
coord session that tried to resolve the deleted alias would
otherwise hit a stale cached client."""
_seed_model_def(storage, definition_id="m1", alias="local", model="m")
_seed_model_def(storage, definition_id="m2", alias="extra", model="x")
registry = _make_registry(alias="local", model="m", extras={"extra": "x"})
client = _make_client(storage, registry)
resp = client.delete("/v1/api/admin/model-definitions/m2")
assert resp.status_code == 200, resp.text
assert not registry.has_alias("extra")
assert registry.has_alias("local") # default alias unaffected
def test_reload_endpoint_refreshes_registry(
storage: SQLiteBackend, monkeypatch: pytest.MonkeyPatch
) -> None:
"""The explicit reload button must refresh the console's own
registry — until this PR it only fanned out to nodes."""
_seed_model_def(storage, definition_id="m1", alias="local", model="initial")
registry = _make_registry(alias="local", model="initial")
client = _make_client(storage, registry)
# Bypass the CRUD endpoints to mimic an out-of-band DB change (e.g.
# an operator psql session) and verify the explicit reload path
# still pulls the change in.
storage.update_model_definition("m1", model="reloaded-model")
# Stub the async cluster fan-out helpers — they require a fully-wired
# collector / proxy_client which is orthogonal to the helper under test.
async def _noop_publish(_request: Any) -> None:
return None
async def _noop_notify(_request: Any) -> dict[str, Any]:
return {}
monkeypatch.setattr("turnstone.console.server._publish_config_change", _noop_publish)
monkeypatch.setattr("turnstone.console.server._notify_nodes_model_reload", _noop_notify)
resp = client.post("/v1/api/admin/model-definitions/reload")
assert resp.status_code == 200, resp.text
assert registry.get_config("local").model == "reloaded-model"
+76
View File
@@ -314,6 +314,48 @@ class TestCollectorSnapshot:
assert event["ws_id"] == "ws1"
assert event["state"] == "running"
def test_apply_snapshot_state_change_forwards_pending_approval_detail(self):
"""Reconnect-via-snapshot is the resync path after every console
restart or network blip. Without forwarding the field here,
a child sitting in approval-pending across the gap renders as
``activity_state=approval`` with no buttons until the next
state change — broken UX during the most common re-sync event."""
c = _make_collector()
c._nodes["node-a"] = NodeSnapshot(
node_id="node-a",
server_url="http://a:8080",
workstreams={"ws1": {"id": "ws1", "name": "same", "state": "idle"}},
)
q: queue.Queue[dict] = queue.Queue()
c.register_listener(q)
detail = {
"items": [{"call_id": "c1", "header": "tool x"}],
"judge_pending": False,
}
c._apply_snapshot(
"node-a",
{
"type": "node_snapshot",
"node_id": "node-a",
"workstreams": [
{
"id": "ws1",
"name": "same",
"state": "running",
"activity_state": "approval",
"pending_approval_detail": detail,
}
],
"health": {},
"aggregate": {},
},
)
event = q.get_nowait()
assert event["type"] == "cluster_state"
assert event["pending_approval_detail"] == detail
def test_apply_snapshot_skips_empty_id_workstream(self):
c = _make_collector()
c._nodes["node-a"] = NodeSnapshot(node_id="node-a", server_url="http://a:8080")
@@ -359,6 +401,40 @@ class TestCollectorDelta:
# Verify in-memory state was updated
assert c._nodes["node-a"].workstreams["ws1"]["state"] == "running"
def test_apply_delta_ws_state_forwards_pending_approval_detail(self):
"""The rich approval payload now travels on the cluster bus so
coord tabs can render inline approve/deny buttons in lockstep
with the activity_state transition. Collector must forward
the field verbatim — the adapter does the child-routing on
top, but the bus carries the data."""
c = _make_collector()
c._nodes["node-a"] = NodeSnapshot(
node_id="node-a",
server_url="http://a:8080",
workstreams={"ws1": {"id": "ws1", "name": "test", "state": "idle"}},
)
q: queue.Queue[dict] = queue.Queue()
c.register_listener(q)
detail = {
"items": [{"call_id": "c1", "header": "tool x"}],
"judge_pending": False,
}
c._apply_delta(
"node-a",
{
"type": "ws_state",
"ws_id": "ws1",
"state": "running",
"activity_state": "approval",
"pending_approval_detail": detail,
},
)
event = q.get_nowait()
assert event["type"] == "cluster_state"
assert event["pending_approval_detail"] == detail
def test_apply_delta_ws_created(self):
c = _make_collector()
c._nodes["node-a"] = NodeSnapshot(node_id="node-a", server_url="http://a:8080")
+106
View File
@@ -0,0 +1,106 @@
"""Tests for the console's coordinator idle-cleanup thread helper.
The helper itself is a tiny loop wrapping ``mgr.close_idle``; the heavy
lifting is in ``SessionManager.close_idle`` (covered in
``test_session_manager.py``) and ``bulk_close_stale_orphans`` (covered
in ``test_storage_sqlite.py``). These tests verify the glue:
- the helper runs an initial sweep BEFORE its first sleep (cold-start
cleanup without blocking the lifespan),
- the helper swallows exceptions so a transient DB blip can't kill the
daemon thread,
- the helper exits cleanly when ``stop_event`` is set.
The ``stop_event`` parameter is exclusively for tests — production
callers pass ``None`` and the daemon runs for process lifetime.
"""
from __future__ import annotations
import threading
from unittest.mock import patch
from turnstone.console.server import _coord_idle_cleanup_thread
class _StubMgr:
def __init__(
self, *, stop_event: threading.Event, expected_calls: int, raise_after: int = -1
) -> None:
self.calls: list[float] = []
self.sleep_calls_at_each_close: list[int] = []
self._stop_event = stop_event
self._expected = expected_calls
self._raise_after = raise_after
self._sleep_count = 0
def close_idle(self, timeout_sec: float) -> list[str]:
# Snapshot how many sleeps preceded this close — lets the
# "initial sweep" test verify the first close_idle ran with
# zero preceding sleeps.
self.sleep_calls_at_each_close.append(self._sleep_count)
self.calls.append(timeout_sec)
try:
if 0 <= self._raise_after < len(self.calls):
raise RuntimeError("simulated DB blip")
finally:
# Set stop after the helper has been exercised enough,
# regardless of whether this call raised.
if len(self.calls) >= self._expected:
self._stop_event.set()
return []
def record_sleep(self, _seconds: float) -> None:
self._sleep_count += 1
def _run_until_done(mgr: _StubMgr, stop_event: threading.Event, timeout_sec: float) -> None:
with patch("turnstone.console.server.time.sleep", mgr.record_sleep):
thread = threading.Thread(
target=_coord_idle_cleanup_thread,
args=(mgr, timeout_sec, stop_event),
daemon=True,
)
thread.start()
thread.join(timeout=2.0)
assert not thread.is_alive(), "helper failed to exit on stop_event"
def test_coord_idle_cleanup_runs_initial_sweep_before_sleep() -> None:
"""The first close_idle call must happen BEFORE the first time.sleep —
otherwise cold-start orphans wait one ``check_every`` interval (~30 min
on default 2h timeout) for the first reap. Crucial because the
lifespan no longer does a synchronous initial sweep."""
stop_event = threading.Event()
mgr = _StubMgr(stop_event=stop_event, expected_calls=1)
_run_until_done(mgr, stop_event, timeout_sec=120.0)
assert mgr.sleep_calls_at_each_close == [0], "first close_idle should run before any sleep"
def test_coord_idle_cleanup_calls_close_idle_each_tick() -> None:
stop_event = threading.Event()
mgr = _StubMgr(stop_event=stop_event, expected_calls=3)
_run_until_done(mgr, stop_event, timeout_sec=120.0)
assert len(mgr.calls) == 3
assert all(t == 120.0 for t in mgr.calls)
def test_coord_idle_cleanup_survives_close_idle_exceptions() -> None:
"""A transient DB error must not kill the daemon thread — the next
tick should still fire close_idle. Without the try/except, a single
blip would silently leak orphans forever."""
stop_event = threading.Event()
mgr = _StubMgr(stop_event=stop_event, expected_calls=4, raise_after=1)
_run_until_done(mgr, stop_event, timeout_sec=120.0)
# All four calls must have fired despite calls 2-4 raising.
assert len(mgr.calls) == 4
def test_coord_idle_cleanup_exits_cleanly_on_stop_event() -> None:
"""The stop_event mechanism is the test contract; verify the thread
actually exits when the event is set, without needing exceptions or
daemon-process termination."""
stop_event = threading.Event()
mgr = _StubMgr(stop_event=stop_event, expected_calls=2)
_run_until_done(mgr, stop_event, timeout_sec=120.0)
assert stop_event.is_set()
+51
View File
@@ -558,3 +558,54 @@ class TestCoordinatorAdapterDispatchChildEvent:
}
)
assert recorder.enqueued[0]["ws_id"] == "coord-a"
def test_dispatch_cluster_state_forwards_pending_approval_detail(self) -> None:
"""The rich approval payload now rides on child_ws_state directly so
the browser can mutate liveBadgeCache without a separate live-bulk
fetch. Drift here means the inline approve/deny buttons would
regress to chasing the dashboard cache (the load-storm pattern
Shape A is unwinding)."""
adapter, recorder, _ = self._setup()
with adapter._children_lock:
adapter._merge_child_ids_locked("coord-a", ["child-a1"])
detail = {
"items": [{"call_id": "c1", "header": "tool x"}],
"judge_pending": False,
}
adapter._dispatch_child_event(
{
"type": "cluster_state",
"ws_id": "child-a1",
"state": "running",
"activity_state": "approval",
"pending_approval_detail": detail,
}
)
assert len(recorder.enqueued) == 1
payload = recorder.enqueued[0]
assert payload["type"] == "child_ws_state"
assert payload["activity_state"] == "approval"
assert payload["pending_approval_detail"] == detail
def test_dispatch_cluster_state_pending_approval_detail_none_passes_through(
self,
) -> None:
"""Missing pending_approval_detail (no approval pending, or pre-fix
node mid-rolling-upgrade) must forward as None — not raise, not
omit — so the browser's handleChildState treats it as "no SSE-
supplied detail, fall back to cached value"."""
adapter, recorder, _ = self._setup()
with adapter._children_lock:
adapter._merge_child_ids_locked("coord-a", ["child-a1"])
adapter._dispatch_child_event(
{
"type": "cluster_state",
"ws_id": "child-a1",
"state": "running",
"activity_state": "tool",
}
)
assert len(recorder.enqueued) == 1
payload = recorder.enqueued[0]
assert "pending_approval_detail" in payload
assert payload["pending_approval_detail"] is None
+92
View File
@@ -152,3 +152,95 @@ def test_coordinator_js_exposes_inline_approval_helpers():
# any prior denial. bug-1 / bug-3 from the second /review pass.
assert "Denied by user" in body
assert "callOutcomes" in body
def test_coordinator_js_handle_child_state_reads_sse_pending_approval_detail():
"""Lock the Shape A behavior change: child_ws_state SSE events now
carry ``pending_approval_detail`` directly so the browser mutates
``liveBadgeCache`` without firing an urgent live-bulk fetch on
every activity_state transition into/out of approval. A refactor
that re-introduces the urgent-fetch path on routine transitions
(or drops the SSE-source merge guard in flushLiveFetches) would
re-open the load-storm pattern this PR is fixing.
Structural assertions (regex against multi-line source) — symbol-
presence alone wouldn't catch a guard that keeps the names but
inverts the comparison or drops the ``prev.live`` check. This
codebase has no JS test framework, so locking the guard's shape
here is the next-best thing to a behavioral test."""
import re
from pathlib import Path
coord_js = Path(__file__).resolve().parent.parent / (
"turnstone/console/static/coordinator/coordinator.js"
)
body = coord_js.read_text(encoding="utf-8")
# handleChildState now reads the SSE-supplied detail.
assert "ev.pending_approval_detail" in body
# The pre-fix urgent-fetch on activity_state transitions is
# gone (the 409 retry path keeps its own ``{ urgent: true }``
# for stale-call_id refresh — that's a different scenario).
assert "enteredApproval" not in body
assert "leftApproval" not in body
# SSE-authoritative window constant is defined and used.
assert re.search(r"\bconst\s+SSE_AUTHORITATIVE_MS\s*=\s*\d+", body), (
"SSE_AUTHORITATIVE_MS constant must be defined as a numeric literal"
)
# handleChildState writes sseUpdatedAt = Date.now() into the cache
# entry it sets. This is the SSE-source tag; without it, the
# merge guard in flushLiveFetches has nothing to gate on.
assert re.search(
r"sseUpdatedAt:\s*Date\.now\(\)",
body,
), "handleChildState must write sseUpdatedAt: Date.now() onto liveBadgeCache entries"
# flushLiveFetches' merge guard structure: SSE-set pending_approval
# / _detail wins over a stale bulk-poll snapshot when (live) AND
# (prev exists) AND (prev.sseUpdatedAt set) AND (within window)
# AND (prev.live exists). Inverting the comparison or dropping
# any of these guards reopens the clobber bug.
merge_guard = re.search(
r"if\s*\(\s*live\s*&&\s*prev\s*&&\s*prev\.sseUpdatedAt\s*&&\s*"
r"now\s*-\s*prev\.sseUpdatedAt\s*<\s*SSE_AUTHORITATIVE_MS\s*&&\s*"
r"prev\.live\s*\)",
body,
)
assert merge_guard is not None, (
"flushLiveFetches merge guard must be the conjunction "
"(live && prev && prev.sseUpdatedAt && now - prev.sseUpdatedAt < "
"SSE_AUTHORITATIVE_MS && prev.live). An inverted comparison or "
"missing prev.live check would let a stale bulk-poll clobber a "
"fresh SSE-set approval."
)
# The merge body must preserve BOTH pending_approval and
# pending_approval_detail from prev — preserving only one would
# render a row with a phantom badge but no buttons (or vice versa).
merge_body = re.search(
r"mergedLive\s*=\s*Object\.assign\(\s*\{\}\s*,\s*live\s*,\s*\{"
r"[^}]*pending_approval:\s*prev\.live\.pending_approval[^}]*"
r"pending_approval_detail:\s*prev\.live\.pending_approval_detail",
body,
)
assert merge_body is not None, (
"Merge body must preserve both pending_approval AND "
"pending_approval_detail from prev.live — preserving only one "
"creates a half-rendered approval row."
)
# flushLiveFetches must forward sseUpdatedAt onto the new cache
# entry so the SSE-source tag survives the bulk-poll write back —
# without this, every bulk-poll resets the window and the next
# late-arriving poll silently clobbers.
assert re.search(
r"sseUpdatedAt:\s*prev\s*\?\s*prev\.sseUpdatedAt",
body,
), (
"flushLiveFetches must forward prev.sseUpdatedAt onto the new "
"cache entry (preserving the SSE-source window across bulk-poll "
"cycles) — without this, the second bulk-poll after an SSE "
"transition silently clobbers."
)
+68
View File
@@ -8,6 +8,7 @@ from turnstone.core.metacognition import (
NUDGE_RESUME,
NUDGE_START,
NUDGE_TOOL_ERROR,
RepeatDetector,
detect_completion,
detect_correction,
format_nudge,
@@ -308,3 +309,70 @@ class TestRepeatNudge:
"""Repeat nudge should fire even with zero memories."""
state: dict[str, float] = {}
assert should_nudge("repeat", state, message_count=5, memory_count=0) is True
class TestRepeatDetector:
"""Repeat-detection streak machine — fires only when the same signature
is recorded ``threshold`` times *consecutively* (default 3). Recording
any different signature resets the streak, so an interrupted repeat
isn't flagged as a stuck loop."""
def test_below_threshold_does_not_fire(self):
det = RepeatDetector()
assert det.record("a") is False
assert det.record("a") is False # second call still under threshold
def test_at_threshold_fires(self):
det = RepeatDetector()
det.record("a")
det.record("a")
assert det.record("a") is True
def test_continues_to_fire_past_threshold(self):
# Caller is responsible for clearing after a fire — until they do,
# subsequent identical calls keep returning True.
det = RepeatDetector()
det.record("a")
det.record("a")
assert det.record("a") is True
assert det.record("a") is True
def test_clear_resets_count(self):
det = RepeatDetector()
det.record("a")
det.record("a")
det.clear()
assert det.record("a") is False # back to 1 after clear
def test_intervening_sig_resets_streak(self):
# The streak is consecutive: recording any other sig mid-streak
# discards the in-progress count. An alternating pattern like
# [A, A, B, A, A] is two short streaks of 2, not a streak of 4.
det = RepeatDetector()
det.record("a")
det.record("a")
assert det.record("b") is False # b at count 1; a's streak is gone
assert det.record("a") is False # a starts fresh at 1
assert det.record("a") is False # a at 2
assert det.record("a") is True # a hits 3 — fresh streak completes
def test_errored_signature_counts_toward_repeat(self):
# Regression: when metacog was split out of the system message,
# the error-output skip got reintroduced and stuck-loop detection
# silently broke for tools that kept failing. Detector itself is
# signature-only — error vs. success is the caller's policy.
det = RepeatDetector()
# Caller records an errored call's sig the same as a successful one;
# the streak is what matters.
for _ in range(3):
last = det.record("bash:ls /nonexistent")
assert last is True
def test_custom_threshold(self):
det = RepeatDetector(threshold=2)
assert det.record("a") is False
assert det.record("a") is True
def test_threshold_one_fires_immediately(self):
det = RepeatDetector(threshold=1)
assert det.record("a") is True
+66 -6
View File
@@ -770,16 +770,76 @@ class TestRegistryReload:
assert reg.has_alias("b")
assert reg.default == "b"
def test_reload_clears_clients(self) -> None:
models = {"a": ModelConfig("a", "http://x/v1", "key", "m")}
def test_reload_keeps_clients_when_connection_target_unchanged(self) -> None:
"""Selective teardown: a model edit that leaves base_url / api_key /
provider intact (e.g. admin tweaks the underlying ``model`` name or
``temperature``) keeps the cached HTTP client warm — no need to
re-establish TLS+pool when the endpoint is the same."""
models = {"a": ModelConfig("a", "http://x/v1", "key", "m1", provider="openai")}
reg = ModelRegistry(models=models, default="a")
# Force client creation
reg.get_client("a")
assert "a" in reg._clients
client_before = reg._clients["a"]
provider_before = reg.get_provider("a")
# Same endpoint (base_url, api_key, provider), only ``model`` changed.
new_models = {"a": ModelConfig("a", "http://x/v1", "key", "m2", provider="openai")}
reg.reload(new_models, "a")
assert "a" in reg._clients
assert reg._clients["a"] is client_before
assert "a" in reg._providers
assert reg._providers["a"] is provider_before
def test_reload_drops_client_when_base_url_changes(self) -> None:
"""A ``base_url`` change drops the cached client (different
endpoint = new connection) but keeps the cached provider —
``LLMProvider`` is keyed only on the provider string, which
didn't change."""
models = {"a": ModelConfig("a", "http://x/v1", "key", "m", provider="openai")}
reg = ModelRegistry(models=models, default="a")
reg.get_client("a")
provider_before = reg.get_provider("a")
new_models = {"a": ModelConfig("a", "http://y/v1", "key", "m", provider="openai")}
reg.reload(new_models, "a")
# Reload with same models — clients should be cleared
reg.reload(dict(models), "a")
assert "a" not in reg._clients
assert "a" in reg._providers
assert reg._providers["a"] is provider_before
def test_reload_drops_provider_when_provider_string_changes(self) -> None:
"""A provider-type swap (e.g. openai → anthropic) drops both the
client AND the provider so the next resolve picks up the right
``LLMProvider`` implementation against the new SDK."""
models = {"a": ModelConfig("a", "http://x/v1", "key", "m", provider="openai")}
reg = ModelRegistry(models=models, default="a")
reg.get_client("a")
reg.get_provider("a")
new_models = {"a": ModelConfig("a", "http://x/v1", "key", "m", provider="anthropic")}
reg.reload(new_models, "a")
assert "a" not in reg._clients
assert "a" not in reg._providers
def test_reload_drops_clients_for_removed_aliases(self) -> None:
"""Aliases removed from the registry must release their cached
clients — otherwise a deleted endpoint's connection pool would
outlive the alias indefinitely."""
models = {
"a": ModelConfig("a", "http://x/v1", "key", "m"),
"b": ModelConfig("b", "http://y/v1", "key", "m"),
}
reg = ModelRegistry(models=models, default="a")
reg.get_client("a")
reg.get_client("b")
# Drop "b" entirely.
new_models = {"a": ModelConfig("a", "http://x/v1", "key", "m")}
reg.reload(new_models, "a")
assert "a" in reg._clients # unchanged endpoint, kept warm
assert "b" not in reg._clients
def test_reload_validates_default(self) -> None:
models_a = {"a": ModelConfig("a", "x", "x", "m")}
+19
View File
@@ -224,6 +224,25 @@ class TestOpenAIProvider:
sanitize_messages([original])
assert original["content"] is None
def test_sanitize_messages_strips_underscore_sibling_keys(self) -> None:
"""Internal sibling metadata (``_reminders``, ``_reminders_delivered``,
``_attachments_meta``, ``_provider_content``) must be stripped
before the wire — the OpenAI-compat APIs reject unknown fields."""
msgs = [
{
"role": "user",
"content": "hi",
"_reminders": [{"type": "correction", "text": "watch"}],
"_reminders_delivered": True,
"_attachments_meta": [{"kind": "image"}],
}
]
result = sanitize_messages(msgs)
assert result == [{"role": "user", "content": "hi"}]
assert "_reminders" not in result[0]
assert "_reminders_delivered" not in result[0]
assert "_attachments_meta" not in result[0]
# -- sanitize_messages: orphan detection -----------------------------------
def test_sanitize_orphaned_tool_call_synthesized(self) -> None:
+837 -114
View File
File diff suppressed because it is too large Load Diff
+285 -1
View File
@@ -18,13 +18,19 @@ from __future__ import annotations
import threading
import time
from dataclasses import dataclass
from datetime import UTC, datetime
from typing import Any
from unittest.mock import MagicMock
import pytest
from turnstone.core.session_manager import SessionKindAdapter, SessionManager
from turnstone.core.workstream import Workstream, WorkstreamKind, WorkstreamState
from turnstone.core.workstream import (
BULK_CLOSE_STATE_VALUES,
Workstream,
WorkstreamKind,
WorkstreamState,
)
# ---------------------------------------------------------------------------
# Test fixtures
@@ -156,6 +162,8 @@ class _Row:
kind: str
state: str = "idle"
parent_ws_id: str | None = None
updated: str = ""
node_id: str | None = None
class FakeStorage:
@@ -164,8 +172,19 @@ class FakeStorage:
def __init__(self) -> None:
self.rows: dict[str, _Row] = {}
self.state_updates: list[tuple[str, str]] = []
self.touch_calls: list[str] = []
self.register_raises = False
self.lock = threading.Lock()
# Live-services lookup target for close_idle pass 2. Map
# service_type → list of live service_ids. Tests that exercise
# liveness scoping populate this directly; default empty means
# "no peers alive" (every row unprotected by liveness).
self.live_services: dict[str, list[str]] = {}
self.list_services_raises = False
@staticmethod
def _now_iso() -> str:
return datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
def register_workstream(
self,
@@ -178,6 +197,8 @@ class FakeStorage:
parent_ws_id: str | None = None,
skill_id: str = "",
skill_version: int = 0,
state: str = "idle",
updated: str | None = None,
) -> None:
if self.register_raises:
raise RuntimeError("register forced failure")
@@ -188,14 +209,66 @@ class FakeStorage:
user_id=user_id or "",
name=name,
kind=kind_str,
state=state,
parent_ws_id=parent_ws_id,
updated=updated if updated is not None else self._now_iso(),
node_id=node_id,
)
def touch_workstream(self, ws_id: str) -> None:
with self.lock:
self.touch_calls.append(ws_id)
if ws_id in self.rows:
self.rows[ws_id].updated = self._now_iso()
def update_workstream_state(self, ws_id: str, state: str) -> None:
with self.lock:
self.state_updates.append((ws_id, state))
if ws_id in self.rows:
self.rows[ws_id].state = state
self.rows[ws_id].updated = self._now_iso()
def bulk_close_stale_orphans(
self,
kind: WorkstreamKind | str,
cutoff: str,
exclude_ws_ids: list[str],
live_node_ids: list[str] | None = None,
) -> list[str]:
kind_str = kind.value if isinstance(kind, WorkstreamKind) else str(kind)
excluded = set(exclude_ws_ids)
live_set = set(live_node_ids) if live_node_ids else set()
now = self._now_iso()
closed: list[str] = []
with self.lock:
for ws_id, row in self.rows.items():
if (
row.kind == kind_str
and row.state in BULK_CLOSE_STATE_VALUES
and row.updated < cutoff
and ws_id not in excluded
):
# Liveness gate: when live_node_ids was provided AND
# non-empty, protect rows whose owner is in the live
# set. NULL node_id is always eligible. When
# live_node_ids is None or empty, no protection
# (mirror of the real backends).
if live_node_ids and row.node_id is not None and row.node_id in live_set:
continue
row.state = "closed"
row.updated = now
self.state_updates.append((ws_id, "closed"))
closed.append(ws_id)
return closed
def list_services(self, service_type: str, max_age_seconds: int = 120) -> list[dict[str, str]]:
if self.list_services_raises:
raise RuntimeError("list_services forced failure")
with self.lock:
return [
{"service_id": sid, "service_type": service_type}
for sid in self.live_services.get(service_type, [])
]
def get_workstream(self, ws_id: str) -> dict[str, Any] | None:
with self.lock:
@@ -228,6 +301,7 @@ def _make_manager(
max_active: int = 5,
storage: FakeStorage | None = None,
event_emitter: Any = _EMITTER_DEFAULT,
node_id: str | None = None,
) -> tuple[SessionManager, FakeAdapter, FakeStorage]:
"""Build a SessionManager wired to a FakeAdapter for both Protocols.
@@ -246,6 +320,7 @@ def _make_manager(
storage=storage,
max_active=max_active,
event_emitter=emitter,
node_id=node_id,
)
return mgr, adapter, storage
@@ -579,6 +654,24 @@ def test_open_resurrects_closed_state() -> None:
assert ws_id in [e.ws_id for e in adapter.events_of("rehydrated")]
def test_open_touches_workstream_on_rehydrate() -> None:
"""Rehydrating a workstream must bump its ``updated`` so a concurrent
close_idle pass-2 in this same process can't clobber the freshly-loaded
row to ``closed`` because its DB ``updated`` is older than the cutoff.
The touch is best-effort (try/except in open()) but must fire on the
happy path."""
mgr, _, storage = _make_manager()
ws = mgr.create(user_id="u1")
ws_id = ws.id
mgr.close(ws_id)
storage.touch_calls.clear() # only care about touches from rehydrate
reopened = mgr.open(ws_id)
assert reopened is not None
assert ws_id in storage.touch_calls
def test_open_ignores_owner_mismatch() -> None:
# Turnstone is a trusted-team tool; row-level ownership is
# metadata, not an access boundary. ``open`` no longer cares
@@ -827,6 +920,197 @@ def test_close_idle_on_empty_manager_returns_empty_list() -> None:
assert mgr.close_idle(max_age_seconds=1.0) == []
def test_close_idle_runs_db_orphan_pass() -> None:
"""DB rows of this kind that aren't loaded into the manager get
bulk-closed when their ``updated`` is older than the cutoff. Catches
the orphan-after-process-restart case the original close_idle missed."""
mgr, _, storage = _make_manager()
# Orphan rows live in storage but were never loaded via mgr.create.
storage.register_workstream(
"orphan-1",
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
storage.register_workstream(
"orphan-2",
kind=WorkstreamKind.INTERACTIVE,
state="thinking",
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert set(closed) == {"orphan-1", "orphan-2"}
assert ("orphan-1", "closed") in storage.state_updates
assert ("orphan-2", "closed") in storage.state_updates
assert storage.rows["orphan-1"].state == "closed"
assert storage.rows["orphan-2"].state == "closed"
def test_close_idle_excludes_loaded_workstreams_from_db_pass() -> None:
"""A workstream loaded into memory must NOT be reaped by the DB
orphan pass even when its storage ``updated`` is stale the
in-memory pass owns those. Verifies the exclude_ws_ids plumbing."""
mgr, _, storage = _make_manager()
ws = mgr.create(user_id="u1")
# Force the storage row's ``updated`` to look stale. In practice
# ``set_state`` would bump it, but we're simulating a long-running
# active workstream whose updated drifted older than the cutoff.
storage.rows[ws.id].updated = "2020-01-01T00:00:00"
# Huge timeout so the in-memory IDLE pass skips it (stays loaded).
closed = mgr.close_idle(max_age_seconds=10_000.0)
assert ws.id not in closed
assert mgr.get(ws.id) is not None
assert storage.rows[ws.id].state == "idle"
def test_close_idle_filters_db_orphans_by_kind() -> None:
"""An interactive manager's close_idle must not touch coordinator
rows in storage and vice versa. Without this filter, both managers
would race to close each other's rows."""
mgr, _, storage = _make_manager() # interactive by default
storage.register_workstream(
"coord-orphan",
kind=WorkstreamKind.COORDINATOR,
updated="2020-01-01T00:00:00",
)
storage.register_workstream(
"interactive-orphan",
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert "interactive-orphan" in closed
assert "coord-orphan" not in closed
assert storage.rows["coord-orphan"].state == "idle"
assert storage.rows["interactive-orphan"].state == "closed"
def test_close_idle_protects_rows_owned_by_live_services() -> None:
"""Multi-node correctness: rows whose ``node_id`` matches a service
with a recent heartbeat must NOT be reaped, even when *this* manager
is on a different node the alive peer may legitimately have them
loaded. Liveness is the rendezvous router's primitive (post-PR-#384);
using it here keeps reap scoping aligned with routing.
Default ``_make_manager`` uses an INTERACTIVE adapter, which derives
``service_type='server'`` so live_services seeded under "server"
are what the manager queries."""
mgr, _, storage = _make_manager()
storage.live_services["server"] = ["node-b"] # only node-b is alive
storage.register_workstream(
"ours-from-dead-node",
node_id="node-a", # dead pod (not in live_services)
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
storage.register_workstream(
"theirs-still-alive",
node_id="node-b",
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert closed == ["ours-from-dead-node"]
assert storage.rows["ours-from-dead-node"].state == "closed"
assert storage.rows["theirs-still-alive"].state == "idle"
def test_close_idle_protects_live_services_for_coordinator_kind() -> None:
"""Coord-side parity: a coordinator manager derives
``service_type='console'``, so live_services seeded under "console"
are what gets queried. Mirrors the interactive test to ensure both
halves of the production wiring are exercised."""
coord_adapter = FakeAdapter(kind=WorkstreamKind.COORDINATOR)
mgr, _, storage = _make_manager(coord_adapter)
storage.live_services["console"] = ["console"] # console is alive
storage.register_workstream(
"alive-console-coord",
node_id="console",
kind=WorkstreamKind.COORDINATOR,
updated="2020-01-01T00:00:00",
)
storage.register_workstream(
"dead-console-coord",
node_id="dead-console-instance", # not in live set
kind=WorkstreamKind.COORDINATOR,
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert closed == ["dead-console-coord"]
assert storage.rows["alive-console-coord"].state == "idle"
assert storage.rows["dead-console-coord"].state == "closed"
def test_close_idle_reaps_rows_with_null_node_id() -> None:
"""A row with no ``node_id`` has no owner identity — age alone gates
the reap. Defends against a NULL silently propagating through ``NOT
IN (live)`` and protecting orphans forever."""
mgr, _, storage = _make_manager()
storage.live_services["server"] = ["node-a"]
storage.register_workstream(
"no-owner",
node_id=None,
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert closed == ["no-owner"]
def test_close_idle_reaps_all_orphans_when_no_peers_alive() -> None:
"""When ``list_services`` returns an empty list (no heartbeating
peers), every stale orphan is unprotected and gets reaped. This is
the cold-start / single-process / dead-cluster-recovery case."""
mgr, _, storage = _make_manager()
# storage.live_services["server"] left empty — no peers heartbeating
storage.register_workstream(
"any-node-1",
node_id="node-a",
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
storage.register_workstream(
"any-node-2",
node_id="node-b",
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert set(closed) == {"any-node-1", "any-node-2"}
def test_close_idle_skips_pass_2_when_list_services_fails() -> None:
"""Conservative fallback: if list_services fails we can't enumerate
live owners safely, so pass 2 must skip rather than reap blind. Pass
1 (in-memory IDLE) still runs."""
mgr, _, storage = _make_manager()
storage.list_services_raises = True
storage.register_workstream(
"would-be-orphan",
node_id="node-a",
kind=WorkstreamKind.INTERACTIVE,
updated="2020-01-01T00:00:00",
)
closed = mgr.close_idle(max_age_seconds=0.0)
assert closed == []
assert storage.rows["would-be-orphan"].state == "idle"
def test_list_all_returns_creation_order() -> None:
mgr, _, _ = _make_manager()
a = mgr.create(user_id="u1")
+275
View File
@@ -4,6 +4,10 @@ from __future__ import annotations
from typing import Any
import sqlalchemy as sa
from turnstone.core.storage._schema import workstreams
# -- Workstream registration ---------------------------------------------------
@@ -664,6 +668,277 @@ class TestBatchPrimitives:
assert result == {"never-seen": 0}
# -- bulk_close_stale_orphans --------------------------------------------------
def _force_updated(backend: Any, ws_id: str, updated: str) -> None:
"""Stamp a workstream row's ``updated`` column directly.
The public surface only sets ``updated`` to ``now``, which makes it
impossible to fabricate a stale row through register/update calls.
Reaches into ``backend._engine`` same access pattern conftest uses
for cross-backend cleanup.
"""
with backend._engine.connect() as conn:
conn.execute(
sa.update(workstreams).where(workstreams.c.ws_id == ws_id).values(updated=updated)
)
conn.commit()
class TestBulkCloseStaleOrphans:
def test_closes_stale_non_terminal_rows_of_kind(self, backend):
backend.register_workstream("stale-idle", kind="interactive")
backend.register_workstream("stale-thinking", kind="interactive")
backend.update_workstream_state("stale-thinking", "thinking")
backend.register_workstream("fresh-idle", kind="interactive")
_force_updated(backend, "stale-idle", "2020-01-01T00:00:00")
_force_updated(backend, "stale-thinking", "2020-01-01T00:00:00")
# fresh-idle stays at registration time (effectively now)
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert set(closed) == {"stale-idle", "stale-thinking"}
rows = backend.get_workstreams_batch(["stale-idle", "stale-thinking", "fresh-idle"])
assert rows["stale-idle"]["state"] == "closed"
assert rows["stale-thinking"]["state"] == "closed"
assert rows["fresh-idle"]["state"] == "idle"
def test_skips_already_closed(self, backend):
backend.register_workstream("already-closed", kind="interactive")
backend.update_workstream_state("already-closed", "closed")
_force_updated(backend, "already-closed", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert closed == []
def test_filters_by_kind(self, backend):
backend.register_workstream("interactive-stale", kind="interactive")
backend.register_workstream("coord-stale", kind="coordinator")
_force_updated(backend, "interactive-stale", "2020-01-01T00:00:00")
_force_updated(backend, "coord-stale", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert closed == ["interactive-stale"]
rows = backend.get_workstreams_batch(["interactive-stale", "coord-stale"])
assert rows["interactive-stale"]["state"] == "closed"
assert rows["coord-stale"]["state"] == "idle"
def test_excludes_loaded_ws_ids(self, backend):
backend.register_workstream("ws-keep", kind="interactive")
backend.register_workstream("ws-close", kind="interactive")
_force_updated(backend, "ws-keep", "2020-01-01T00:00:00")
_force_updated(backend, "ws-close", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=["ws-keep"]
)
assert closed == ["ws-close"]
rows = backend.get_workstreams_batch(["ws-keep", "ws-close"])
assert rows["ws-keep"]["state"] == "idle"
assert rows["ws-close"]["state"] == "closed"
def test_empty_exclude_list_does_not_break_sql(self, backend):
backend.register_workstream("orphan", kind="interactive")
_force_updated(backend, "orphan", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert closed == ["orphan"]
def test_no_orphans_returns_empty(self, backend):
backend.register_workstream("fresh", kind="interactive")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert closed == []
def test_closes_all_non_terminal_states(self, backend):
for ws_id, state in [
("o-idle", "idle"),
("o-thinking", "thinking"),
("o-attention", "attention"),
("o-running", "running"),
]:
backend.register_workstream(ws_id, kind="interactive")
if state != "idle":
backend.update_workstream_state(ws_id, state)
_force_updated(backend, ws_id, "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert set(closed) == {"o-idle", "o-thinking", "o-attention", "o-running"}
def test_bumps_updated_on_close(self, backend):
stale_updated = "2020-01-01T00:00:00"
backend.register_workstream("orphan", kind="interactive")
_force_updated(backend, "orphan", stale_updated)
backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
# ``updated`` must change away from the forced stale value. Asserting
# inequality from the seed (rather than ``> "2024-01-01..."``) keeps
# the test independent of wall-clock date.
with backend._engine.connect() as conn:
row = conn.execute(
sa.select(workstreams.c.updated).where(workstreams.c.ws_id == "orphan")
).one()
assert row[0] != stale_updated
def test_protects_rows_owned_by_live_services(self, backend):
"""Liveness scoping (post-#384 rendezvous-routing world): rows
whose ``node_id`` matches a heartbeating service must NOT be
reaped, because that owner may legitimately have them loaded on
another worker. Rows whose ``node_id`` matches a dead service
ARE eligible that's how dead-pod orphans get reclaimed in
containerized deployments with dynamic hostnames."""
backend.register_workstream("dead-node", node_id="dead-pod-x4k2", kind="interactive")
backend.register_workstream("alive-node", node_id="alive-pod-y9p3", kind="interactive")
_force_updated(backend, "dead-node", "2020-01-01T00:00:00")
_force_updated(backend, "alive-node", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive",
cutoff="2024-01-01T00:00:00",
exclude_ws_ids=[],
live_node_ids=["alive-pod-y9p3"],
)
assert closed == ["dead-node"]
rows = backend.get_workstreams_batch(["dead-node", "alive-node"])
assert rows["dead-node"]["state"] == "closed"
assert rows["alive-node"]["state"] == "idle"
def test_null_node_id_always_eligible(self, backend):
"""A row with NULL ``node_id`` has no owner identity — age alone
gates the reap. Belt-and-suspenders against ``NULL NOT IN (...)``
evaluating to NULL (not TRUE) and silently protecting orphans
forever."""
backend.register_workstream("no-owner", node_id=None, kind="interactive")
_force_updated(backend, "no-owner", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive",
cutoff="2024-01-01T00:00:00",
exclude_ws_ids=[],
live_node_ids=["some-other-node"],
)
assert closed == ["no-owner"]
def test_live_node_ids_none_skips_filter(self, backend):
"""``live_node_ids=None`` is the single-process / operator-backfill
mode all rows of *kind* are eligible regardless of node_id."""
backend.register_workstream("node-a", node_id="node-a", kind="interactive")
backend.register_workstream("node-b", node_id="node-b", kind="interactive")
_force_updated(backend, "node-a", "2020-01-01T00:00:00")
_force_updated(backend, "node-b", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive", cutoff="2024-01-01T00:00:00", exclude_ws_ids=[]
)
assert set(closed) == {"node-a", "node-b"}
def test_empty_live_node_ids_treats_all_as_dead(self, backend):
"""Empty list ``live_node_ids=[]`` means "no nodes alive" — every
row's owner is unprotected. Useful for operator scripts that
want to reap regardless of liveness."""
backend.register_workstream("any", node_id="node-a", kind="interactive")
_force_updated(backend, "any", "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive",
cutoff="2024-01-01T00:00:00",
exclude_ws_ids=[],
live_node_ids=[],
)
assert closed == ["any"]
def test_combines_live_node_ids_and_exclude_ws_ids(self, backend):
"""Both filters stack as AND clauses on the UPDATE. Covers the
full 2x2 matrix to catch a future edit that replaces an AND with
an OR or drops one of the filters: only the (orphan + dead-node)
cell should be reaped."""
# All four registered with the same stale ``updated``.
for ws_id, node in [
("loaded-alive", "alive-node"),
("loaded-dead", "dead-node"),
("orphan-alive", "alive-node"),
("orphan-dead", "dead-node"),
]:
backend.register_workstream(ws_id, node_id=node, kind="interactive")
_force_updated(backend, ws_id, "2020-01-01T00:00:00")
closed = backend.bulk_close_stale_orphans(
"interactive",
cutoff="2024-01-01T00:00:00",
exclude_ws_ids=["loaded-alive", "loaded-dead"],
live_node_ids=["alive-node"],
)
# Only orphan-dead is unprotected by both filters.
assert closed == ["orphan-dead"]
rows = backend.get_workstreams_batch(
["loaded-alive", "loaded-dead", "orphan-alive", "orphan-dead"]
)
assert rows["loaded-alive"]["state"] == "idle"
assert rows["loaded-dead"]["state"] == "idle"
assert rows["orphan-alive"]["state"] == "idle"
assert rows["orphan-dead"]["state"] == "closed"
# -- touch_workstream ----------------------------------------------------------
class TestTouchWorkstream:
def test_bumps_updated_only(self, backend):
"""Used by ``open()`` on rehydrate to defend against the orphan
reaper clobbering a freshly-loaded row. Must not change ``state``
(the open() path explicitly avoids state writes to dodge a race
with concurrent close())."""
stale_updated = "2020-01-01T00:00:00"
backend.register_workstream("ws-touch", kind="interactive")
backend.update_workstream_state("ws-touch", "closed") # simulate prior close
_force_updated(backend, "ws-touch", stale_updated)
backend.touch_workstream("ws-touch")
with backend._engine.connect() as conn:
row = conn.execute(
sa.select(workstreams.c.state, workstreams.c.updated).where(
workstreams.c.ws_id == "ws-touch"
)
).one()
assert row[0] == "closed", "state must not be modified by touch"
# Compare against the forced stale value rather than a fixed calendar
# date so the test is independent of wall-clock time.
assert row[1] != stale_updated, "updated must be bumped"
def test_unknown_id_is_noop(self, backend):
"""Touch on a missing id must not raise — open()'s exception
handler is best-effort."""
backend.touch_workstream("nonexistent") # must not raise
# -- Lifecycle -----------------------------------------------------------------
+84
View File
@@ -155,3 +155,87 @@ class TestContentAccumulation:
assert len(idle_events) == 1
# Content should be capped, not contain everything
assert len(idle_events[0]["content"]) <= _MAX_TURN_CONTENT_CHARS + 1024
class TestPendingApprovalDetailGate:
"""The Shape A SSE plumbing carries ``pending_approval_detail`` on the
``ws_state`` event so the coord tree UI can render inline approve/deny
buttons in lockstep with the activity_state transition. The gate
(``if self._pending_approval is not None``) keeps the per-broadcast
serializer cost off the common no-approval-pending path these tests
lock both branches down."""
def test_state_broadcast_omits_field_when_no_approval_pending(self):
"""Common case: no approval pending → field absent from event so the
per-broadcast verdict-cache deepcopy in
``serialize_pending_approval_detail`` never runs. A regression
that drops the gate would silently 10x the cost of every state
broadcast in the steady state."""
ui = _make_ui()
assert ui._pending_approval is None
ui._broadcast_state("running")
events = _drain_global()
running_events = [e for e in events if e.get("state") == "running"]
assert len(running_events) == 1
assert "pending_approval_detail" not in running_events[0]
def test_state_broadcast_includes_field_when_approval_pending(self):
"""When an approval is pending the broadcast must carry the rich
payload the coord tree UI reads it directly to render inline
approve/deny buttons. Without this, a coord browser would have
to chase a separate ``cluster/ws/live`` fetch on every
activity_state transition (the load-storm pattern Shape A is
unwinding)."""
ui = _make_ui()
# Mirror the shape ``pause_for_approval`` writes (session_ui_base
# lines 576-580) — items with call_id + header is the minimum
# the serializer needs to project.
ui._pending_approval = {
"type": "approve_request",
"items": [
{
"call_id": "c1",
"header": "tool x",
"func_args": "{}",
"intent_summary": "do x",
"needs_approval": True,
}
],
"judge_pending": False,
}
ui._broadcast_state("attention")
events = _drain_global()
attn = [e for e in events if e.get("state") == "attention"]
assert len(attn) == 1
# Field present and structurally sound — the serializer's
# full shape is covered by tests/test_session_ui_base.py;
# here we only need to confirm the gate fires and the
# serializer's output is what lands on the event.
assert "pending_approval_detail" in attn[0]
detail = attn[0]["pending_approval_detail"]
assert detail is not None
assert detail.get("items")
assert detail["items"][0]["call_id"] == "c1"
def test_field_cleared_after_approval_resolves(self):
"""Once ``_pending_approval`` is cleared, subsequent state
broadcasts must drop the field again without this, the
browser would render stale approve/deny buttons until the
next bulk-poll TTL window expired."""
ui = _make_ui()
ui._pending_approval = {
"type": "approve_request",
"items": [{"call_id": "c1", "header": "x"}],
"judge_pending": False,
}
ui._broadcast_state("attention")
_drain_global() # discard the with-detail event
ui._pending_approval = None
ui._broadcast_state("running")
events = _drain_global()
running = [e for e in events if e.get("state") == "running"]
assert len(running) == 1
assert "pending_approval_detail" not in running[0]
+132
View File
@@ -899,6 +899,138 @@ class TestHistoryInteractive:
assert client.get(base, params={"limit": 999}).status_code == 200
class TestBuildHistoryReminderPropagation:
"""``_build_history`` must surface the ``_reminders`` side-channel on
each entry so a tab reconnecting via ``/history`` renders the same
metacognitive nudge bubble the originating tab saw via the live
``user_reminder`` SSE event.
"""
def _session_with_messages(self, messages: list[dict]) -> MagicMock:
session = MagicMock()
session.messages = messages
return session
def test_reminders_sidechannel_surfaces_on_entry(self):
from turnstone.server import _build_history
session = self._session_with_messages(
[
{
"role": "user",
"content": "ah no",
"_reminders": [{"type": "correction", "text": "watch out"}],
}
]
)
history = _build_history(session)
assert history[0]["content"] == "ah no"
assert history[0]["reminders"] == [{"type": "correction", "text": "watch out"}]
def test_no_reminders_key_when_sidechannel_absent(self):
from turnstone.server import _build_history
session = self._session_with_messages([{"role": "user", "content": "just a message"}])
history = _build_history(session)
assert "reminders" not in history[0]
def test_no_reminders_key_when_sidechannel_empty(self):
from turnstone.server import _build_history
session = self._session_with_messages([{"role": "user", "content": "hi", "_reminders": []}])
history = _build_history(session)
assert "reminders" not in history[0]
def test_multiple_reminders_preserved_in_order(self):
from turnstone.server import _build_history
session = self._session_with_messages(
[
{
"role": "user",
"content": "x",
"_reminders": [
{"type": "denial", "text": "FIRST"},
{"type": "correction", "text": "SECOND"},
],
}
]
)
history = _build_history(session)
assert history[0]["reminders"] == [
{"type": "denial", "text": "FIRST"},
{"type": "correction", "text": "SECOND"},
]
def test_reminders_coexist_with_attachments(self):
from turnstone.server import _build_history
session = self._session_with_messages(
[
{
"role": "user",
"content": [
{"type": "text", "text": "look"},
{"type": "image_url", "image_url": {"url": "data:..."}},
],
"_reminders": [{"type": "correction", "text": "watch"}],
}
]
)
history = _build_history(session)
assert history[0]["content"] == "look"
assert history[0]["attachments"] == [{"kind": "image", "filename": "", "mime_type": ""}]
assert history[0]["reminders"] == [{"type": "correction", "text": "watch"}]
def test_malformed_reminders_filtered_out(self):
"""Defensive: a non-dict element in the list (corruption / bug)
is dropped rather than crashing the history serialisation."""
from turnstone.server import _build_history
session = self._session_with_messages(
[
{
"role": "user",
"content": "x",
"_reminders": [
{"type": "correction", "text": "ok"},
"not-a-dict",
{"type": "denial"}, # missing text
],
}
]
)
history = _build_history(session)
# Non-dicts dropped; missing-text fills with empty string.
assert history[0]["reminders"] == [
{"type": "correction", "text": "ok"},
{"type": "denial", "text": ""},
]
def test_clean_message_passes_through_unchanged(self):
"""No reminders, plain content — _build_history is a no-op for the
reminder field and ``content`` rides through verbatim."""
from turnstone.server import _build_history
session = self._session_with_messages(
[{"role": "user", "content": "just a normal message"}]
)
history = _build_history(session)
assert history[0]["content"] == "just a normal message"
assert "reminders" not in history[0]
def test_assistant_content_with_literal_reminder_tag_unchanged(self):
"""Assistant output may legitimately reference the tag (e.g. when
the model is explaining the reminder system itself). No
transformation should ever apply to assistant content."""
from turnstone.server import _build_history
content = "Here is a <system-reminder> tag in assistant output."
session = self._session_with_messages([{"role": "assistant", "content": content}])
history = _build_history(session)
assert history[0]["content"] == content
class TestDetailInteractive:
"""Interactive parity for the lifted ``GET /v1/api/workstreams/{ws_id}``.
+1 -1
View File
@@ -1,3 +1,3 @@
"""turnstone - Multi-node AI orchestration platform with tool use, agent routing, and cluster simulation."""
__version__ = "1.5.0"
__version__ = "1.5.3"
+23
View File
@@ -312,6 +312,29 @@ class TerminalUI(SessionUI):
sys.stdout.write(f"{RED}{message}{RESET}\n")
sys.stdout.flush()
def _print_reminder(self, reminders: list[dict[str, str]]) -> None:
"""Render a metacognitive reminder list as ``[metacognition · type] text``
lines in the terminal the CLI's equivalent of the web UI's
yellow themed bubble. Used by both ``on_user_reminder`` and
``on_tool_reminder``; the rendering is identical because
terminal output is anchor-by-flow rather than DOM-by-anchor.
"""
for r in reminders:
nt = str(r.get("type", "") or "")
text = str(r.get("text", "") or "")
label = "metacognition" + (f" · {nt}" if nt else "")
sys.stdout.write(f"{YELLOW}[{label}]{RESET} {text}\n")
sys.stdout.flush()
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None:
self._print_reminder(reminders)
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None:
# tool_call_id ignored — the CLI anchors by output sequence
# (the line lands directly after the tool result that
# triggered the batch's reminder).
self._print_reminder(reminders)
def on_state_change(self, state: str) -> None:
pass # base TerminalUI ignores state changes
+14
View File
@@ -498,6 +498,7 @@ class ClusterCollector:
"kind": WorkstreamKind.from_raw(new_w.get("kind")),
"parent_ws_id": new_w.get("parent_ws_id"),
"activity_state": new_w.get("activity_state", ""),
"pending_approval_detail": new_w.get("pending_approval_detail"),
}
)
old_name = old_ws.get("title", "") or old_ws.get("name", "")
@@ -557,6 +558,18 @@ class ClusterCollector:
ws["kind"] = data["kind"]
if "parent_ws_id" in data:
ws["parent_ws_id"] = data["parent_ws_id"]
# ``pending_approval_detail`` overwrites (no
# ``ws.get`` fallback): the node's broadcast gate
# on ``_pending_approval is not None`` means the
# field is absent from ``data`` exactly when no
# approval is pending — falling back to the cached
# value would resurrect a stale detail after the
# approval resolved. Without this assignment the
# cached ``node.workstreams`` dict served by
# ``get_node_detail`` / ``get_snapshot`` between
# reconciliations would render stale approve/deny
# buttons on closed approvals.
ws["pending_approval_detail"] = data.get("pending_approval_detail")
pending_events.append(
{
"type": "cluster_state",
@@ -568,6 +581,7 @@ class ClusterCollector:
"kind": WorkstreamKind.from_raw(ws.get("kind")),
"parent_ws_id": ws.get("parent_ws_id"),
"activity_state": ws.get("activity_state", ""),
"pending_approval_detail": data.get("pending_approval_detail"),
}
)
+6 -4
View File
@@ -685,11 +685,13 @@ class CoordinatorAdapter:
"tokens": event.get("tokens", 0),
"node_id": event.get("node_id", ""),
# activity_state lets the JS detect approval-state
# transitions and fire urgent live-bulk fetches so
# inline approve/deny buttons render in lockstep
# with the child entering attention (instead of
# waiting up to 5s for the next TTL window).
# transitions; pending_approval_detail rides on
# the same event so the browser can mutate
# liveBadgeCache directly and render inline
# approve/deny buttons in lockstep with the
# transition, no separate dashboard refetch.
"activity_state": event.get("activity_state", ""),
"pending_approval_detail": event.get("pending_approval_detail"),
}
elif etype == "ws_closed":
child_event = {
+2 -2
View File
@@ -81,13 +81,13 @@ WAIT_MAX_TIMEOUT: float = 600.0
WAIT_POLL_INTERVAL: float = 0.5
# Per-ws cap on the inline ``message`` field bundled into wait_for_workstream
# results. Sized so a fan-out of 32 children at the cap is ~192 KiB of
# results. Sized so a fan-out of 32 children at the cap is ~320 KiB of
# tool output — large but not catastrophic on commercial models, and
# typical waits run with a handful of children. Truncation is from the
# END (the lead is usually more informative than the tail) and sets a
# ``truncated=True`` flag so the model can opt into a follow-up read if
# the trailing bytes matter.
WAIT_MESSAGE_MAX_BYTES: int = 6 * 1024
WAIT_MESSAGE_MAX_BYTES: int = 10 * 1024
# How many tail messages ``wait_for_workstream`` reads when extracting a
# child's last assistant turn. The conversation tail almost always
+199 -8
View File
@@ -23,6 +23,7 @@ import queue
import re
import secrets
import textwrap
import threading
import time
import urllib.parse
from concurrent.futures import ThreadPoolExecutor
@@ -89,6 +90,7 @@ if TYPE_CHECKING:
from starlette.requests import Request
from turnstone.core.session import ChatSession
from turnstone.core.session_manager import SessionManager
from turnstone.core.storage._protocol import StorageBackend
log = logging.getLogger("turnstone.console.server")
@@ -2453,7 +2455,7 @@ def _require_admin_coordinator(
)
def _resolve_coordinator_or_404(
async def _resolve_coordinator_or_404(
request: Request,
coord_mgr: Any,
storage: Any,
@@ -2484,7 +2486,11 @@ def _resolve_coordinator_or_404(
if storage is None:
return None, miss
try:
row = storage.get_workstream(ws_id)
# Cold-cache path (every console restart, eviction, console
# proxy hop) — offload the sync DB call so the coord
# children/tasks handlers don't block the event loop on the
# same DB the rest of the handler is unblocking.
row = await asyncio.to_thread(storage.get_workstream, ws_id)
except Exception:
log.debug("resolve_coordinator.storage_failed ws=%s", ws_id[:8], exc_info=True)
return None, miss
@@ -2910,7 +2916,7 @@ async def coordinator_children(request: Request) -> JSONResponse:
if not _VALID_WS_ID_RE.match(ws_id):
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
user_id = _auth_user_id(request)
_ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
_ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
if err404 is not None:
return err404
@@ -2918,7 +2924,8 @@ async def coordinator_children(request: Request) -> JSONResponse:
# the full child subtree. ``user_id`` stays on each row as
# metadata, not a filter.
try:
raw = storage.list_workstreams(
raw = await asyncio.to_thread(
storage.list_workstreams,
limit=_CHILDREN_PAGE_LIMIT + 1,
parent_ws_id=ws_id,
kind=None,
@@ -3033,7 +3040,7 @@ async def coordinator_metrics(request: Request) -> JSONResponse:
if not _VALID_WS_ID_RE.match(ws_id):
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
user_id = _auth_user_id(request)
_ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
_ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
if err404 is not None:
return err404
@@ -3233,7 +3240,7 @@ async def _resolve_coord_session(
if not _VALID_WS_ID_RE.match(ws_id):
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
user_id = _auth_user_id(request)
ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
if err404 is not None:
return err404
if ws is None or ws.session is None:
@@ -3523,11 +3530,11 @@ async def coordinator_tasks(request: Request) -> JSONResponse:
if not _VALID_WS_ID_RE.match(ws_id):
return JSONResponse({"error": "invalid ws_id"}, status_code=400)
user_id = _auth_user_id(request)
_ws, err404 = _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
_ws, err404 = await _resolve_coordinator_or_404(request, coord_mgr, storage, ws_id, user_id)
if err404 is not None:
return err404
envelope, _corrupt = load_task_envelope(storage, ws_id)
envelope, _corrupt = await asyncio.to_thread(load_task_envelope, storage, ws_id)
return JSONResponse(envelope)
@@ -3682,6 +3689,50 @@ async def _verify_collector_service_scope(app: Starlette, client: httpx.AsyncCli
)
def _coord_idle_cleanup_thread(
mgr: SessionManager,
timeout_sec: float,
stop_event: threading.Event | None = None,
) -> None:
"""Periodically reap idle + DB-orphan coordinator workstreams.
Mirrors the regular server's ``_idle_cleanup_thread`` (turnstone/server.py)
but skips the rate-limiter / global-queue arms the console doesn't have
those. ``mgr.close_idle`` does the work: closes loaded IDLE rows AND
bulk-closes DB rows of this kind whose ``updated`` is past the cutoff
and which aren't currently loaded. The latter pass catches coords left
behind by prior console process incarnations.
Runs an initial sweep BEFORE the first sleep so cold-start orphans are
reaped immediately rather than waiting one ``check_every`` interval (~30
min on default 2h timeout). This intentionally diverges from the regular
server pattern, which has no initial sweep the regular server runs
inside a normal request-handling lifecycle, the console-side coord pool
is a small fixed-size cache where orphans dominate the row count after
a cold boot.
``stop_event`` is for tests when set, the thread exits cleanly after
the next loop check. Production callers pass ``None`` (the daemon is
process-lifetime).
"""
check_every = min(300.0, timeout_sec / 4)
# Initial sweep — runs once before entering the sleep loop.
try:
mgr.close_idle(timeout_sec)
except Exception:
log.debug("console.coord_idle_cleanup_initial_failed", exc_info=True)
while True:
if stop_event is not None and stop_event.is_set():
return
time.sleep(check_every)
if stop_event is not None and stop_event.is_set():
return
try:
mgr.close_idle(timeout_sec)
except Exception:
log.debug("console.coord_idle_cleanup_failed", exc_info=True)
@asynccontextmanager
async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
# Create async HTTP clients for proxy routes. Auth headers are NOT baked
@@ -3750,6 +3801,18 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
audit_exec = ThreadPoolExecutor(max_workers=4, thread_name_prefix="coord-audit")
app.state.audit_executor = audit_exec
_set_audit_executor(audit_exec)
# Dedicated executor for coord SSE queue polling, mirroring the
# interactive-side ``sse_executor`` in ``turnstone/server.py``.
# Each coord ``events`` SSE listener parks a thread on
# ``client_queue.get(timeout=5)`` for the connection lifetime.
# Without this pool, those parks land on Python's default
# ThreadPoolExecutor (~min(32, cpu_count+4)) and compete with
# every other ``asyncio.to_thread`` caller — a few coord tabs
# against a multi-child workstream are enough to stall new
# request handlers.
app.state.coord_sse_executor = ThreadPoolExecutor(
max_workers=200, thread_name_prefix="coord-sse"
)
# Populate the router's services cache if a router is configured
_router: ConsoleRouter | None = getattr(app.state, "router", None)
if _router is not None:
@@ -3975,6 +4038,27 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
coord_adapter.start_child_event_fanout(app.state.collector)
except Exception:
log.warning("console.coordinator_child_fanout_init_failed", exc_info=True)
# Idle cleanup: closes loaded-but-stale coords AND DB orphans
# left behind by prior console processes. The thread runs an
# initial sweep on entry (no synchronous lifespan call needed —
# see ``_coord_idle_cleanup_thread``) so cold-start cleanup
# doesn't block startup. Reuses the regular-server
# ``server.workstream_idle_timeout`` setting — the same cadence
# makes sense for both kinds and avoids a redundant config knob.
try:
idle_minutes = int(config_store.get("server.workstream_idle_timeout"))
except Exception:
idle_minutes = 0
if idle_minutes > 0:
timeout_sec = float(idle_minutes * 60)
cleanup_thread = threading.Thread(
target=_coord_idle_cleanup_thread,
args=(coord_mgr, timeout_sec),
name="coord-idle-cleanup",
daemon=True,
)
cleanup_thread.start()
app.state.coord_idle_cleanup_thread = cleanup_thread
log.info(
"console.coordinator_mgr_ready max_active=%s",
config_store.get("coordinator.max_active"),
@@ -4029,6 +4113,15 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
if audit_exec_shutdown is not None:
_set_audit_executor(None)
audit_exec_shutdown.shutdown(wait=True)
# Drain the coord SSE pool AFTER ``coord_adapter.shutdown()`` above:
# adapter shutdown deregisters listeners so no new events handlers
# arrive at this pool; in-flight ``client_queue.get`` futures
# already running are bounded by their 5s timeout and finish
# naturally. ``cancel_futures=True`` discards any queued-but-not-
# started futures so we don't block lifespan teardown on them.
coord_sse_exec_shutdown = getattr(app.state, "coord_sse_executor", None)
if coord_sse_exec_shutdown is not None:
coord_sse_exec_shutdown.shutdown(wait=True, cancel_futures=True)
# ---------------------------------------------------------------------------
@@ -7996,6 +8089,82 @@ async def _collect_model_status(
return {nid: models for nid, models in results if models is not None}
def _refresh_coord_registry(app_state: Any, storage: Any) -> None:
"""Rebuild ``app_state.coord_registry`` in place from DB model definitions.
The console-side coordinator session factory closes over the
``coord_registry`` instance built at lifespan startup
(see this module's lifespan setup and ``console/session_factory.py``).
Replacing the attribute would orphan the closure new sessions would
still resolve through the stale object. Mutating in place via
``ModelRegistry.reload()`` preserves identity, so:
- new coordinator sessions see the new state at create-time;
- active coordinator sessions auto-pick up the swap at next ``send()``
via ``ChatSession._refresh_model_from_registry`` (the per-send
check compares ``cfg.model`` against ``self.model`` and re-resolves
on mismatch).
Errors are logged + swallowed. The DB write that triggered this
refresh has already succeeded, and the explicit reload button
remains the user-facing recovery path. Validation failures
(e.g. admin deleted the alias that ``registry.default`` points at)
leave the existing registry intact rather than tearing down a
working coordinator.
"""
from turnstone.core.model_registry import load_model_registry
existing = getattr(app_state, "coord_registry", None)
if existing is None:
# Lifespan didn't build a coord_registry (no DB model rows at boot)
# — the entire coord subsystem stayed uninitialized, so a console
# restart is required after the operator adds the first row.
return
try:
# ``strict=True`` so a transient DB read error surfaces here.
# Without it, the loader degrades to a config.toml-only registry
# and ``existing.reload()`` would silently drop every DB-sourced
# alias.
new_registry = load_model_registry(storage=storage, strict=True)
except ValueError as exc:
# ModelRegistry.__init__ raises ValueError for several distinct
# config issues — empty models, default/fallback/agent/plan/task
# alias not present in the loaded set. Log the actual reason so
# operators can tell "no enabled rows" from "default alias typo
# in config.toml". Existing registry stays in place either way.
log.warning("console.coord_registry_refresh_skipped reason=%s", exc)
return
except Exception:
log.warning("console.coord_registry_refresh_load_failed", exc_info=True)
return
try:
existing.reload(
new_registry.models,
new_registry.default,
new_registry.fallback,
new_registry.agent_model,
plan_model=new_registry.plan_model,
task_model=new_registry.task_model,
plan_effort=new_registry.plan_effort,
task_effort=new_registry.task_effort,
)
except Exception:
log.warning("console.coord_registry_refresh_reload_failed", exc_info=True)
finally:
# Defensive — load_model_registry doesn't eagerly create clients
# (ModelRegistry.__init__ leaves _clients/_providers empty; they
# populate lazily on first resolve), so shutdown() iterates empty
# dicts in practice. Kept against the day the loader grows
# eager-init or a future caller pre-warms the throwaway, and
# wrapped because shutdown() in finally would otherwise escape
# after a successful in-place reload — surfacing as 500 with the
# registry actually mutated and the audit row recording success.
try:
new_registry.shutdown()
except Exception:
log.warning("console.coord_registry_refresh_shutdown_failed", exc_info=True)
async def _notify_nodes_model_reload(request: Request) -> dict[str, Any]:
"""Tell all nodes to re-read model definitions from DB and rebuild registry."""
collector: ClusterCollector = request.app.state.collector
@@ -8232,6 +8401,8 @@ async def admin_create_model_definition(request: Request) -> JSONResponse:
ip,
)
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
created = storage.get_model_definition(definition_id)
if created is None:
return JSONResponse(
@@ -8389,6 +8560,9 @@ async def admin_update_model_definition(request: Request) -> JSONResponse:
ip,
)
if updates:
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
model_def = storage.get_model_definition(definition_id)
return JSONResponse(_mask_model_secrets(model_def or {}))
@@ -8424,6 +8598,8 @@ async def admin_delete_model_definition(request: Request) -> JSONResponse:
ip,
)
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
return JSONResponse({"status": "ok", "definition_id": definition_id})
@@ -8443,6 +8619,13 @@ async def admin_model_reload(request: Request) -> JSONResponse:
# before they rebuild their model registries.
await _publish_config_change(request)
# Refresh the console's own coord_registry first. The node fan-out
# below carries DB→nodes propagation; the console hosts coordinator
# sessions itself and must mutate its in-process registry too —
# otherwise the coord LLM keeps calling the prior model name even
# after a successful reload.
await asyncio.to_thread(_refresh_coord_registry, request.app.state, storage)
results = await _notify_nodes_model_reload(request)
return JSONResponse({"status": "ok", "results": results})
@@ -10196,6 +10379,14 @@ def create_app(
# tombstones are non-resurrectable.
saved_state_filter="closed",
saved_loaded_lookup=_coord_saved_loaded_lookup,
# Isolate coord SSE polling on its own 200-thread pool so a
# handful of coord tabs (each parking a thread on
# ``client_queue.get``) can't starve the default executor and
# stall every other ``asyncio.to_thread`` caller (storage,
# router, audit). Mirrors the interactive endpoint's
# ``sse_executor_lookup`` wiring on ``interactive_endpoint_config``
# in ``turnstone/server.py``.
sse_executor_lookup=lambda request: request.app.state.coord_sse_executor,
)
coord_workstream_routes: list[Any] = []
register_session_routes(
@@ -335,6 +335,73 @@
return appendMsg(role, esc(text), opts);
}
// Metacognitive reminder bubble (user-channel correction / denial /
// resume / start / completion AND tool-channel tool_error / repeat).
// Mirrors Pane.prototype.addUserReminder / addToolReminder in the
// interactive UI — yellow themed bubble slotted directly below the
// message it advises. ``anchor`` is the DOM element to anchor below;
// when null, append at the bottom of messagesEl.
function appendReminderBubble(reminders, anchor) {
if (!Array.isArray(reminders) || !reminders.length) return;
let cursor = anchor;
for (let i = 0; i < reminders.length; i++) {
const r = reminders[i] || {};
const el = document.createElement("div");
el.className = "msg user-reminder";
el.setAttribute("role", "article");
el.setAttribute("data-ts-role", "metacognition");
el.setAttribute("aria-label", "metacognition");
const body = document.createElement("div");
body.className = "msg-body";
const labelEl = document.createElement("span");
labelEl.className = "msg-user-reminder-label";
labelEl.textContent =
"metacognition" + (r.type ? " · " + String(r.type) : "");
const textEl = document.createElement("span");
textEl.className = "msg-user-reminder-text";
textEl.textContent = r.text || "";
body.appendChild(labelEl);
body.appendChild(textEl);
el.appendChild(body);
if (cursor) {
cursor.insertAdjacentElement("afterend", el);
cursor = el;
} else {
messagesEl.appendChild(el);
}
}
_scheduleScroll();
}
// Live SSE for user-channel reminders — anchors below the most
// recent user message. On a non-originating tab there may be no
// user message rendered yet; we append and the next /history reload
// corrects. (Same caveat as the interactive UI; tracked there.)
function appendUserReminderLive(reminders) {
const userMsgs = messagesEl.querySelectorAll(".msg.user");
const anchor = userMsgs.length ? userMsgs[userMsgs.length - 1] : null;
appendReminderBubble(reminders, anchor);
}
// Live SSE for tool-channel reminders — anchors below the
// .coord-tool-batch construct that produced the tool result. Looks
// up the row by data-call-id and walks to the parent batch; falls
// back to the most recent batch if not found.
function appendToolReminderLive(reminders, toolCallId) {
let anchor = null;
if (toolCallId) {
const entry = toolRows.get(toolCallId);
if (entry && entry.batch) {
anchor = entry.batch;
}
}
if (!anchor) {
const batches = messagesEl.querySelectorAll(".coord-tool-batch");
if (batches.length) anchor = batches[batches.length - 1];
}
appendReminderBubble(reminders, anchor);
}
// Build a tool-batch item from a persisted assistant
// tool_call. Live calls land here with header / preview already
// computed by ChatSession._prepare_tool; history replay never sees
@@ -1844,6 +1911,22 @@
// styling which mis-categorised them as tool calls.
appendText("info", ev.message || "", { label: "info" });
break;
case "user_reminder":
// Metacognitive user-channel nudge — render below the most
// recent user message as a yellow themed bubble. Same shape
// as the interactive UI's case.
if (Array.isArray(ev.reminders) && ev.reminders.length) {
appendUserReminderLive(ev.reminders);
}
break;
case "tool_reminder":
// Metacognitive tool-channel nudge — render below the
// .coord-tool-batch that produced the tool result identified
// by ev.tool_call_id.
if (Array.isArray(ev.reminders) && ev.reminders.length) {
appendToolReminderLive(ev.reminders, ev.tool_call_id || "");
}
break;
case "connected":
// First yield from _coord_events_replay — populates the
// status bar's model cell before any history arrives. Also
@@ -2085,6 +2168,14 @@
const TERMINAL_CHILD_STATES = new Set(["closed", "deleted"]);
const LIVE_BADGE_TTL_MS = 5000;
const LIVE_BADGE_DEBOUNCE_MS = 250;
// After handleChildState mutates liveBadgeCache from a child_ws_state
// SSE event, a bulk-poll landing within this window must NOT
// overwrite the SSE-supplied pending_approval / _detail fields with
// its own (potentially stale) snapshot — the upstream node
// /dashboard cache has its own ~2s TTL so a poll right after a
// transition can carry pre-transition state. 3s covers the worst
// case (upstream TTL + console TTL minus a margin).
const SSE_AUTHORITATIVE_MS = 3000;
// Debounce window for /tasks refreshes triggered by ``tasks``
// tool_result SSE events. Without it, a model that runs
// ``add → list`` (or any back-to-back mutation pair) double-fetches
@@ -2192,9 +2283,11 @@
// Inline approve/deny block \u2014 shown only when the live block
// carries pending_approval_detail (the rich payload added by the
// server-side dashboard projection). A "\u2691 approval" badge alone
// means the child is in attention state but the rich detail hasn't
// arrived yet (urgent live-bulk fetch is in flight); the row gets
// re-rendered when it lands.
// means the child is in attention state but the rich detail
// hasn't arrived on the cache yet \u2014 a rare cross-version race
// (e.g. a node mid-rolling-upgrade emitted ws_state without
// pending_approval_detail before this PR landed). The next SSE
// tick or the 5s TTL bulk-poll catches up and re-renders.
if (cached && cached.live && cached.live.pending_approval_detail) {
const detail = cached.live.pending_approval_detail;
const block = renderApprovalBlock(child, detail);
@@ -3066,12 +3159,35 @@
? results[id]
: null;
const wasDenied = denied.indexOf(id) !== -1;
const prev = liveBadgeCache.get(id);
// SSE-set pending_approval / _detail wins over a stale
// bulk-poll snapshot for SSE_AUTHORITATIVE_MS after the
// SSE update. Without this guard, a poll landing right
// after a child_ws_state transition can clobber freshly-
// mutated approval state with pre-transition data from
// the upstream /dashboard cache (which has its own ~2s
// TTL). Other fields (tokens, context_ratio) still track
// the bulk response — only the approval surface is gated.
let mergedLive = live;
if (
live &&
prev &&
prev.sseUpdatedAt &&
now - prev.sseUpdatedAt < SSE_AUTHORITATIVE_MS &&
prev.live
) {
mergedLive = Object.assign({}, live, {
pending_approval: prev.live.pending_approval,
pending_approval_detail: prev.live.pending_approval_detail,
});
}
liveBadgeCache.set(id, {
live: live,
live: mergedLive,
fetched: now,
// Denied ids are permission/identity misses — mark permanent
// so SSE state ticks on those rows don't retry every window.
permanent: wasDenied,
sseUpdatedAt: prev ? prev.sseUpdatedAt || 0 : 0,
});
const row = childrenTreeEl.querySelector(
'.ch-row[data-ws-id="' + cssEscape(id) + '"]',
@@ -3092,10 +3208,12 @@
const isPermanent = e && /HTTP 403/.test(e.message || "");
const now = Date.now();
ids.forEach((id) => {
const prev = liveBadgeCache.get(id);
liveBadgeCache.set(id, {
live: null,
fetched: now,
permanent: isPermanent,
sseUpdatedAt: prev ? prev.sseUpdatedAt || 0 : 0,
});
});
if (!isPermanent) console.warn("flushLiveFetches failed", e);
@@ -3135,7 +3253,6 @@
ws_id: childId,
name: "",
};
const prevActivity = existing.activity_state || "";
existing.state = ev.state || existing.state;
existing.activity_state =
typeof ev.activity_state === "string"
@@ -3144,32 +3261,60 @@
if (ev.node_id) existing.node_id = ev.node_id;
childrenState.set(childId, existing);
_touchChild(childId);
renderChildren();
// Do NOT invalidateLiveBadge on routine state ticks — that defeats
// the 5s TTL cache and devolves rate-limiting to the 250ms
// debouncer, hitting cluster_ws_detail ~4 req/s per chatty child.
// The TTL check in scheduleLiveFetch will refresh the badge on its
// own schedule; identity-changing events (created/rename/closed)
// still invalidate below.
//
// Two activity_state transitions warrant an *urgent* (TTL-bypassing)
// fetch so the row carries pending_approval_detail in lockstep with
// the child's true state:
// - "" / "tool" / "thinking" → "approval" (need rich payload now
// so the inline approve/deny buttons can render)
// - "approval" → anything else (need to drop the
// stale payload so the buttons disappear; without this the
// 5s TTL leaves stale buttons on a row whose approval was
// resolved elsewhere — e.g. the child's own UI tab)
const enteredApproval =
existing.activity_state === "approval" && prevActivity !== "approval";
const leftApproval =
prevActivity === "approval" && existing.activity_state !== "approval";
if (enteredApproval || leftApproval) {
scheduleLiveFetch(childId, { urgent: true });
// pending_approval_detail rides on the ws_state event when an
// approval is pending (see turnstone/server.py
// WebUI._broadcast_state — gated on ``_pending_approval is not
// None``, so absent on the steady state and possibly null on a
// node mid-rolling-upgrade). When present we mutate
// liveBadgeCache directly here — inline approve/deny buttons
// render in lockstep with the activity_state transition without
// a separate live-bulk fetch. The cache entry is tagged
// ``sseUpdatedAt`` so a bulk-poll landing within
// SSE_AUTHORITATIVE_MS preserves the SSE-supplied fields
// (the upstream /dashboard cache's ~2s TTL would otherwise
// clobber a fresh transition with pre-transition state).
const pendingApproval = existing.activity_state === "approval";
const evDetail =
ev.pending_approval_detail !== undefined
? ev.pending_approval_detail
: null;
const cached = liveBadgeCache.get(childId);
// When pending, prefer SSE-supplied detail. If SSE didn't
// carry it (rare race; e.g. a node mid-rolling-upgrade), keep
// any existing cached detail rather than blanking the row —
// the next bulk-poll catches up. When not pending, hard-clear.
let nextDetail;
if (pendingApproval) {
nextDetail =
evDetail !== null
? evDetail
: cached && cached.live
? cached.live.pending_approval_detail
: null;
} else {
scheduleLiveFetch(childId);
nextDetail = null;
}
const nextLive = Object.assign({}, (cached && cached.live) || {}, {
pending_approval: pendingApproval,
pending_approval_detail: nextDetail,
});
liveBadgeCache.set(childId, {
live: nextLive,
// Preserve prior bulk-poll fetched timestamp so a fresh SSE
// tick doesn't artificially extend the 5s TTL gate in
// scheduleLiveFetch — the bulk-poll still drives slower-
// moving fields (tokens, context_ratio) on its own schedule.
fetched: cached ? cached.fetched : 0,
permanent: !!(cached && cached.permanent),
sseUpdatedAt: Date.now(),
});
renderChildren();
// Do NOT invalidateLiveBadge on routine state ticks — that
// defeats the 5s TTL cache and devolves rate-limiting to the
// 250ms debouncer. The TTL check in scheduleLiveFetch handles
// refresh cadence for slower-moving fields; identity-changing
// events (created/rename/closed) still invalidate below.
scheduleLiveFetch(childId);
}
function handleChildClosed(ev) {
@@ -3481,6 +3626,12 @@
(callId && toolNameByCallId.get(callId)) || m.tool_name || "tool";
const isError = callOutcomes.get(callId) === "error";
appendToolResult(toolName, callId, content || "", isError);
// Tool-channel metacog reminders ride the same _reminders
// side-channel as the user channel; surface as a themed
// bubble below the .coord-tool-batch construct.
if (Array.isArray(m.reminders) && m.reminders.length) {
appendToolReminderLive(m.reminders, callId);
}
} else if (role === "assistant") {
// Empty content with tool_calls only means the assistant
// turn was just tool dispatch — the synthesized tool-call
@@ -3511,6 +3662,15 @@
// (appendReasoningToken uses textContent; user/system are
// typed verbatim and don't carry markdown structure).
appendText(role, content, { label: role });
// User-channel metacog reminders attach to the just-appended
// user bubble (the most recent .msg.user in messagesEl).
if (
role === "user" &&
Array.isArray(m.reminders) &&
m.reminders.length
) {
appendUserReminderLive(m.reminders);
}
}
});
// History alone can't tell whether an orphaned assistant
+1 -1
View File
@@ -432,7 +432,7 @@ LAST_ERROR_CONFIG_KEY = "last_error"
# such error per workstream would bloat workstream_config and the model
# prompt the coord LLM ingests on inspect. 1024 chars matches the
# practical "useful for triage" length while staying well under the
# WAIT_MESSAGE_MAX_BYTES (6 KiB) cap so the truncate happens here at
# WAIT_MESSAGE_MAX_BYTES (10 KiB) cap so the truncate happens here at
# write time, not later at the wait surface.
LAST_ERROR_MAX_LEN = 1024
+44 -1
View File
@@ -5,7 +5,50 @@ from __future__ import annotations
import re
import time
_COOLDOWN_SECS = 300 # 5 minutes between nudges of the same type
# Default cooldown (s) between nudges of the same type. Production
# paths pass ``cooldown_secs`` explicitly from
# ``MemoryConfig.nudge_cooldown`` (config-store ``memory.nudge_cooldown``,
# default 300); this constant is the fallback for tests and unit-style
# callers without a ``MemoryConfig`` and is kept aligned with that
# canonical default so both paths behave the same.
_COOLDOWN_SECS = 300
# Repeat-detection threshold — number of *consecutive* identical tool
# calls (same name + same arguments) before a repeat warning fires.
# Two-in-a-row is too noisy because legitimate retries on transient
# failures look identical; three-in-a-row is the cheapest signal that
# the model is stuck on the same call.
_REPEAT_THRESHOLD = 3
class RepeatDetector:
"""Detect a streak of identical tool-call signatures.
``record(sig)`` returns ``True`` once *sig* has been recorded
``threshold`` times in a row (default 3). Recording a different
signature resets the streak interleaved tool calls aren't a
stuck loop, only repeated identical ones are. After a fire, the
caller is expected to call ``clear()`` to start a fresh streak.
"""
def __init__(self, threshold: int = _REPEAT_THRESHOLD) -> None:
self._threshold = threshold
self._sig: str | None = None
self._count = 0
def record(self, sig: str) -> bool:
"""Record *sig*; return ``True`` when the streak hits the threshold."""
if sig == self._sig:
self._count += 1
else:
self._sig = sig
self._count = 1
return self._count >= self._threshold
def clear(self) -> None:
self._sig = None
self._count = 0
# ---------------------------------------------------------------------------
# Nudge messages (brief, model-facing hints)
+39 -5
View File
@@ -230,6 +230,7 @@ class ModelRegistry:
if task_model and task_model not in models:
raise ValueError(f"Task model '{task_model}' not found in registry")
with self._client_lock:
old_models = self._models
self._models = dict(models)
self.default = default
self.fallback = list(fallback) if fallback else []
@@ -238,11 +239,32 @@ class ModelRegistry:
self.task_model = task_model
self.plan_effort = plan_effort
self.task_effort = task_effort
for client in self._clients.values():
if hasattr(client, "close"):
client.close()
self._clients.clear()
self._providers.clear()
# Selective teardown — close + drop only clients whose
# connection target actually changed (alias removed, or
# base_url / api_key / provider differs). Keeps connection
# pools warm for the common admin-edit case where only
# ``model`` / ``temperature`` / ``context_window`` changed.
for alias, client in list(self._clients.items()):
old_cfg = old_models.get(alias)
new_cfg = self._models.get(alias)
if (
new_cfg is None
or old_cfg is None
or old_cfg.base_url != new_cfg.base_url
or old_cfg.api_key != new_cfg.api_key
or old_cfg.provider != new_cfg.provider
):
if hasattr(client, "close"):
client.close()
del self._clients[alias]
# Providers are keyed on alias but only depend on
# ``cfg.provider`` — drop only when the provider string
# changed or the alias was removed.
for alias in list(self._providers.keys()):
old_cfg = old_models.get(alias)
new_cfg = self._models.get(alias)
if new_cfg is None or old_cfg is None or old_cfg.provider != new_cfg.provider:
del self._providers[alias]
def shutdown(self) -> None:
"""Close all cached client connections."""
@@ -300,6 +322,7 @@ def load_model_registry(
context_window: int = 32768,
provider: str = "openai",
storage: Any | None = None,
strict: bool = False,
) -> ModelRegistry:
"""Build a ModelRegistry from CLI args, ``config.toml``, and database.
@@ -317,6 +340,15 @@ def load_model_registry(
``[model].plan_effort``, ``[model].task_effort`` control routing.
``plan_model``/``task_model`` override ``agent_model`` per sub-agent
role; both fall back to it when unset.
``strict``: when True, a storage read failure during the DB-rows step
re-raises instead of degrading to a config.toml-only registry.
Callers that hot-reload an existing registry need this so a transient
DB outage doesn't silently drop every DB-sourced alias when the
truncated result is applied via ``ModelRegistry.reload``. Callers
that build a fresh registry from scratch (CLI, lifespan startup) want
the default behaviour boot succeeds with a config-only fallback
rather than crashing on a flaky DB.
"""
import json as _json
@@ -370,6 +402,8 @@ def load_model_registry(
server_compat=row_server_compat,
)
except Exception:
if strict:
raise
log.warning("Failed to load model definitions from storage", exc_info=True)
# 2. Build configs from [models.*] sections (overrides DB for same alias)
+390 -176
View File
@@ -81,6 +81,7 @@ from turnstone.core.memory_relevance import (
score_memories,
)
from turnstone.core.metacognition import (
RepeatDetector,
detect_completion,
detect_correction,
format_nudge,
@@ -90,6 +91,7 @@ from turnstone.core.providers import create_provider
from turnstone.core.safety import is_command_blocked, sanitize_command
from turnstone.core.sandbox import execute_math_sandboxed
from turnstone.core.storage._registry import get_storage
from turnstone.core.tool_advisory import escape_wrapper_tags, render_system_reminder
from turnstone.core.tool_search import ToolSearchManager
from turnstone.core.tools import (
AGENT_AUTO_TOOLS,
@@ -275,6 +277,8 @@ class SessionUI(Protocol):
def on_plan_review(self, content: str) -> str: ...
def on_info(self, message: str) -> None: ...
def on_error(self, message: str) -> None: ...
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None: ...
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None: ...
def on_state_change(self, state: str) -> None: ...
def on_rename(self, name: str) -> None: ...
def on_intent_verdict(self, verdict: dict[str, Any]) -> None:
@@ -474,8 +478,12 @@ class ChatSession:
collections.OrderedDict()
)
self._queued_lock = threading.Lock()
# Repeat detection: track recent tool call signatures
self._recent_tool_sigs: set[str] = set()
# Repeat detection: streak counter over tool-call signatures.
# Fires when a (name, args) signature has been seen N times in
# a row; recording any different signature resets the streak.
# Also cleared after a write tool succeeds (state changed) or
# after a warning fires (clean slate, re-fire on the next streak).
self._repeat_detector = RepeatDetector()
# Tool error tracking: call_id → is_error for message persistence
self._tool_error_flags: dict[str, bool] = {}
# Cooperative cancellation: set from outside to stop generation
@@ -1293,7 +1301,7 @@ class ChatSession:
self._ws_id = ws_id
self.messages = messages
self._read_files.clear()
self._recent_tool_sigs.clear()
self._repeat_detector.clear()
self._last_usage = None
self._calibrated_msg_count = 0
self._title_generated = True # don't re-title resumed workstreams
@@ -1638,6 +1646,124 @@ class ChatSession:
"""System messages + conversation history."""
return self.system_messages + self.messages
def _apply_reminders_for_provider(
self,
messages: list[dict[str, Any]],
) -> list[dict[str, Any]]:
"""Return a transient copy of *messages* with ``_reminders`` rendered
inline for the model.
Metacognitive nudges live on the message dict's ``_reminders``
side-channel regardless of role user messages carry
user-channel nudges (correction / denial / resume / start /
completion), tool messages carry tool-channel nudges
(tool_error / repeat). Both ride the same side-channel so
``self.messages`` and every downstream consumer (UI replay,
compaction, title gen, channel adapters, DB) see clean
``content``; only the wire-bound copy here carries the
rendered reminder.
For each message that has ``_reminders`` AND has not yet been
flagged delivered, build a shallow copy and splice the reminders
as ``<system-reminder>`` blocks onto the trailing edge of the
copy's ``content`` — string content gets a tail block, list
content gets the block on the trailing text part (or a new text
part if there isn't one). Messages without ``_reminders`` (or
already delivered) pass through unchanged (same object
reference) so the common case is allocation-free.
**Reminder lifecycle.** After a successful provider stream
``_mark_reminders_delivered`` flips ``_reminders_delivered`` to
``True`` on every message (user or tool) that carried reminders
into that call, so subsequent provider calls skip them the
model sees each reminder once, the turn it advised. The
``_reminders`` key itself stays on the message dict for the
lifetime of the in-memory session so ``/history`` (reconnecting
tabs, multi-tab live mirrors) still renders the same nudge
bubbles the originating tab saw; only the wire-side replay is
suppressed. Compaction is the natural full drain (it replaces
``self.messages`` wholesale).
``sanitize_messages`` later drops both leading-underscore sibling
keys (``_reminders`` and ``_reminders_delivered``) on the way to
the wire, so the provider sees only ``content`` with the
reminder spliced in.
**Read-only contract on the returned list.** The pass-through
path returns the original ``msg`` by reference; callers must
not mutate the returned dicts in place (today's only callers —
``sanitize_messages`` + provider conversion construct new
dicts, so the contract holds). Mutations on the spliced copy
are safe; mutations on a pass-through reference would bleed
back into ``self.messages``.
"""
out: list[dict[str, Any]] = []
for msg in messages:
raw_reminders = msg.get("_reminders")
if not raw_reminders or msg.get("_reminders_delivered"):
out.append(msg)
continue
# Defensive filter — only dict entries are valid; a string /
# None / other shape from corruption or partial state must
# not abort the whole send via an AttributeError on .get().
# Mirrors the same filter ``_build_history`` applies on the
# wire-out side.
reminders = [r for r in raw_reminders if isinstance(r, dict)]
if not reminders:
out.append(msg)
continue
block = "\n\n" + "\n\n".join(
render_system_reminder(r.get("text", "")) for r in reminders
)
copy = dict(msg)
content = copy.get("content")
if isinstance(content, str):
copy["content"] = escape_wrapper_tags(content) + block
elif isinstance(content, list):
# Shallow-copy the parts list and any text parts we'll
# mutate so the original list/dicts in self.messages stay
# untouched.
new_parts = [
dict(p) if isinstance(p, dict) and p.get("type") == "text" else p
for p in content
]
text_parts = [
p for p in new_parts if isinstance(p, dict) and p.get("type") == "text"
]
for part in text_parts:
part["text"] = escape_wrapper_tags(part.get("text", ""))
if text_parts:
text_parts[-1]["text"] = text_parts[-1]["text"] + block
else:
new_parts.append({"type": "text", "text": block})
copy["content"] = new_parts
else:
# Unexpected shape (None, etc.) — attach as a text-only
# content rather than dropping the reminder silently.
copy["content"] = block.lstrip()
out.append(copy)
return out
def _mark_reminders_delivered(self) -> None:
"""Flag every message's ``_reminders`` as delivered.
Role-agnostic both user-channel reminders (set by
``_attach_pending_user_reminders`` on user messages) and
tool-channel reminders (set by the per-result loop on tool
messages) ride the same ``_reminders`` side-channel and the
same delivered flag. Called after a successful provider
stream; subsequent calls to ``_apply_reminders_for_provider``
skip messages with this flag, so the model sees each reminder
exactly once (the turn it advised). The flag is a sibling key
like ``_reminders`` itself; ``sanitize_messages`` strips both
before the wire and ``_build_history`` ignores the delivered
flag entirely so UI replay parity is preserved across
reconnects.
"""
for msg in self.messages:
if msg.get("_reminders") and not msg.get("_reminders_delivered"):
msg["_reminders_delivered"] = True
def _emit_state(self, state: str) -> None:
"""Notify UI of a workstream state transition.
@@ -2113,12 +2239,13 @@ class ChatSession:
# Metacognitive user-channel drain: any nudges queued via
# _queue_user_advisory (correction/start/completion from this
# turn, denial from the previous tool batch, resume from
# rehydrate) splice in as <system-reminder> blocks at the
# trailing edge of the user content. The DB row stores
# ``user_input`` only (line below) so these blocks stay
# ephemeral — they advise the next assistant turn and do not
# persist across reloads.
self._splice_pending_user_advisories(user_msg)
# rehydrate) attach to the user message dict's ``_reminders``
# side-channel — content stays clean. The wire-side splice
# happens later in _apply_reminders_for_provider against a
# transient copy. The DB row stores ``user_input`` only (line
# below) so reminders stay in-memory only and don't persist
# across reloads.
self._attach_pending_user_reminders(user_msg)
self.messages.append(user_msg)
self._msg_tokens.append(max(1, int(self._msg_char_count(user_msg) / self._chars_per_token)))
# DB row stores the raw text only; attachments are joined back in
@@ -2198,7 +2325,7 @@ class ChatSession:
try:
while True:
self._check_cancelled(my_generation)
msgs = self._full_messages()
msgs = self._apply_reminders_for_provider(self._full_messages())
if self.debug:
self._debug_print_request(msgs)
@@ -2235,7 +2362,7 @@ class ChatSession:
self.ui.on_thinking_stop()
try:
self._compact_messages(auto=True)
msgs = self._full_messages()
msgs = self._apply_reminders_for_provider(self._full_messages())
self.ui.on_thinking_start()
stream = self._create_stream_with_retry(msgs)
except Exception:
@@ -2257,7 +2384,19 @@ class ChatSession:
if self._generation != my_generation:
return
self._update_token_table(assistant_msg)
# Reuse the wire-bound ``msgs`` we already built for the
# stream call instead of re-applying the reminder splice
# (perf-2). After mark-delivered runs below, a fresh
# _apply_reminders_for_provider would skip the
# just-rendered reminders and undercount; passing the
# already-rendered list keeps calibration char count
# aligned with what the provider actually counted.
self._update_token_table(assistant_msg, msgs=msgs)
# Reminders that rode this stream have now reached the
# model; flag delivered so the next provider call skips
# them (one-shot semantics for the wire; UI replay still
# surfaces ``_reminders`` for reconnect parity).
self._mark_reminders_delivered()
self._print_status_line() # Report usage for EVERY API call
self.messages.append(assistant_msg)
self._msg_tokens.append(
@@ -2332,92 +2471,10 @@ class ChatSession:
if self._generation != my_generation:
return
# Repeat detection: warn when a tool is called with identical args.
# Skip error outputs — retrying a failed tool is valid.
# Skip JSON outputs (MCP structured results) — appending
# text would corrupt the payload.
_tc_by_id = {c["id"]: c for c in tool_calls}
_repeat_detected = False
_error_prefixes = (
"Error",
"JSON parse error",
"Unknown tool",
"Command timed out",
"Blocked:",
"Denied",
)
# Clear dedup sigs when a write tool executed successfully —
# the state has changed so re-running a read tool is valid.
_write_tools = frozenset({"write_file", "edit_file", "bash"})
if any(
tc["function"]["name"] in _write_tools
and not any(
cid == tc["id"] and isinstance(out, str) and out.startswith(_error_prefixes)
for cid, out in results
)
for tc in tool_calls
):
self._recent_tool_sigs.clear()
for i, (tc_id, output) in enumerate(results):
tc = _tc_by_id.get(tc_id)
if tc and isinstance(output, str) and not output.startswith(_error_prefixes):
raw = tc["function"]["name"] + ":" + tc["function"]["arguments"]
sig = hashlib.sha256(raw.encode()).hexdigest()
is_json = output.lstrip().startswith(("{", "["))
if sig in self._recent_tool_sigs:
_repeat_detected = True
if not is_json:
output += (
"\n\n⚠ Warning: this is an identical repeat of a "
"previous tool call. The result is the same. "
"Try a different approach."
)
results[i] = (tc_id, output)
self.ui.on_info(
f"{GRAY}[repeat: {tc['function']['name']}() "
f"called with same arguments]{RESET}"
)
self._recent_tool_sigs.add(sig)
if _repeat_detected:
# Reset so the model gets a clean slate after the warning.
# If it repeats again, a new warning fires.
self._recent_tool_sigs.clear()
if self._mem_cfg.nudges and should_nudge(
"repeat",
self._metacog_state,
message_count=len(self.messages),
cooldown_secs=self._mem_cfg.nudge_cooldown,
):
self._queue_tool_advisory("repeat", format_nudge("repeat"))
# Tool-error nudge — checked here (pre-iteration) so the
# MetacognitiveAdvisory rides the same _collect_advisories
# drain pass that handles guard findings and user
# interjections. Cooldown gating in should_nudge keeps
# this to one nudge per batch even with many failing
# tools.
if (
self._mem_cfg.nudges
and any(
isinstance(out, str)
and (
out.startswith("Error")
or " error: " in out[:50]
or out.startswith("Command timed out")
or out.startswith("Unknown tool:")
)
for _, out in results
)
and should_nudge(
"tool_error",
self._metacog_state,
message_count=len(self.messages),
memory_count=self._visible_memory_count(),
cooldown_secs=self._mem_cfg.nudge_cooldown,
)
):
self._queue_tool_advisory("tool_error", format_nudge("tool_error"))
# Repeat-detection + tool-error nudge. Mutates *results*
# in place to inject inline warning text on identical
# repeats; queues advisories for the next drain pass.
self._apply_post_execute_advisories(tool_calls, results)
# Map tool_call_id → tool name for logging
from turnstone.core.tool_advisory import wrap_tool_result
@@ -2456,19 +2513,30 @@ class ChatSession:
# Capture raw output for DB storage before advisory wrapping
raw_output = output
# Advisory injection: wrap tool output with advisories
# (output guard findings, queued user messages, etc.)
advisories = self._collect_advisories(
# Advisory injection: persistent advisories (output
# guard findings, queued user interjections) wrap
# into the tool-result envelope and stay in
# self.messages. Metacognitive tool-channel
# reminders (tool_error / repeat) ride a side-channel
# — never inside content — so the model sees the
# splice only at the wire boundary via
# _apply_reminders_for_provider, while UI/replay
# surfaces them as a themed bubble below the tool
# result.
persistent_advisories, metacog_reminders = self._collect_advisories(
assessment, _tc_names.get(tc_id, ""), _ri == _last_idx
)
if isinstance(output, str):
output = wrap_tool_result(output, advisories)
elif isinstance(output, list) and advisories:
output = wrap_tool_result(output, persistent_advisories)
elif isinstance(output, list) and persistent_advisories:
# Structured/image output — append advisories as a
# text part so they aren't silently dropped.
output = [
*output,
{"type": "text", "text": wrap_tool_result("", advisories)},
{
"type": "text",
"text": wrap_tool_result("", persistent_advisories),
},
]
tool_msg: dict[str, Any] = {
@@ -2478,6 +2546,15 @@ class ChatSession:
}
if self._tool_error_flags.pop(tc_id, False):
tool_msg["is_error"] = True
if metacog_reminders:
tool_msg["_reminders"] = metacog_reminders
try:
self.ui.on_tool_reminder(metacog_reminders, tc_id)
except Exception:
log.warning(
"ui.on_tool_reminder failed; reminder still attached",
exc_info=True,
)
self.messages.append(tool_msg)
# Token estimation — image content uses a fixed heuristic
@@ -2583,10 +2660,7 @@ class ChatSession:
# Drain any queued user messages so they appear in the
# conversation and are visible on the next send().
self._flush_queued_messages()
# Tool-channel nudges queued earlier in this generation
# (tool_error, repeat) belong to the abandoned batch — drop
# them so they don't bleed into the next send()'s tool loop.
self._pending_tool_advisories.clear()
self._drain_pending_advisories()
# No need to clear _cancel_event — it's replaced per-generation
# in send(), so this generation's event is simply discarded.
self.ui.on_info("[Generation cancelled]")
@@ -2596,15 +2670,29 @@ class ChatSession:
except KeyboardInterrupt as exc:
self._synthesize_cancelled_results("Interrupted by user.")
self._flush_queued_messages()
self._pending_tool_advisories.clear()
self._drain_pending_advisories()
self._record_fatal_error(exc)
raise
except Exception as exc:
self._flush_queued_messages()
self._pending_tool_advisories.clear()
self._drain_pending_advisories()
self._record_fatal_error(exc)
raise
def _drain_pending_advisories(self) -> None:
"""Drop both advisory channels' pending buffers.
Both channels are scoped to the current generation: tool-channel
nudges (``tool_error``, ``repeat``) queued earlier in this batch
and user-channel nudges (``correction``, ``denial``, ) queued
during ``_check_metacognitive_nudge`` but not yet drained. When
a generation is abandoned (cancel, KeyboardInterrupt, unexpected
exception) both must drop so they don't bleed into the next
send's tool loop or next user turn.
"""
self._pending_tool_advisories.clear()
self._pending_user_advisories.clear()
def _synthesize_cancelled_results(self, reason: str) -> None:
"""Synthesize tool_result messages for orphaned tool_calls after cancel.
@@ -3137,8 +3225,24 @@ class ChatSession:
text_chars, images, doc_chars = self._msg_text_chars(msg)
return text_chars + doc_chars + int(images * self._IMAGE_TOKENS * self._chars_per_token)
def _update_token_table(self, assistant_msg: dict[str, Any]) -> None:
"""Update per-message token estimates using API usage data."""
def _update_token_table(
self,
assistant_msg: dict[str, Any],
*,
msgs: list[dict[str, Any]] | None = None,
) -> None:
"""Update per-message token estimates using API usage data.
*msgs* (optional) is the wire-bound message list already built
for the stream call passing it avoids a redundant
``_apply_reminders_for_provider`` walk and, more importantly,
ensures the char count matches the bytes the provider counted
even after ``_mark_reminders_delivered`` has flipped the flag
on the reminders that rode the stream. When *msgs* is None the
caller didn't pre-build (rare path) — fall back to applying
the splice on the fly, but be aware the result will be
reminder-free if delivered flags are already set.
"""
if not self._last_usage:
return
@@ -3148,8 +3252,16 @@ class ChatSession:
# Calibrate chars_per_token ratio from actual usage.
# Images get a fixed token budget (subtracted). Documents
# tokenize non-linearly depending on provider — excluded from
# calibration so they don't skew the text ratio.
all_msgs = self._full_messages() # system + self.messages (before append)
# calibration so they don't skew the text ratio. ``all_msgs``
# must reflect what the provider actually counted in
# ``prompt_tokens``: when called from the loop with the
# pre-built ``msgs``, that's exact; without it, fall back to
# applying the splice fresh (post-mark-delivered the result
# may undercount, but no caller currently takes this path
# after a successful stream).
all_msgs = (
msgs if msgs is not None else self._apply_reminders_for_provider(self._full_messages())
) # system + self.messages (before append)
active_tools = self._get_active_tools() or []
tool_def_chars = sum(len(json.dumps(t)) for t in active_tools)
text_chars = 0
@@ -3391,7 +3503,7 @@ class ChatSession:
self.messages = [summary_user, summary_asst]
# File contents are gone after compaction — force re-read before edit_file
self._read_files.clear()
self._recent_tool_sigs.clear()
self._repeat_detector.clear()
# Rebuild token table
su_tok = max(1, int(self._msg_char_count(summary_user) / self._chars_per_token))
@@ -3780,12 +3892,26 @@ class ChatSession:
assessment: OutputAssessment | None,
func_name: str,
is_last_in_batch: bool,
) -> list[ToolAdvisory]:
) -> tuple[list[ToolAdvisory], list[dict[str, str]]]:
"""Gather advisories to attach to a tool result message.
Returns an empty list when no advisories apply (common case).
Guard advisories attach per-result; user messages drain on the
last result in the batch only.
Returns ``(persistent, metacog_reminders)``:
- ``persistent`` guard findings + user interjections that ride
inside the tool-result envelope via ``wrap_tool_result``.
These are conversation history and must persist in
``self.messages``.
- ``metacog_reminders`` list of ``{"type", "text"}`` dicts for
``tool_error`` / ``repeat`` nudges that the caller attaches to
the tool message dict's ``_reminders`` side-channel. Like
user-channel reminders, they are spliced into ``content`` only
at the wire boundary by ``_apply_reminders_for_provider`` and
surfaced separately on the UI as a themed bubble below the
tool result.
Both lists are empty when no advisories apply (common case).
Guard advisories attach per-result; user messages and
metacognitive nudges drain on the last result in the batch only.
"""
from turnstone.core.tool_advisory import GuardAdvisory, UserInterjection
@@ -3801,26 +3927,25 @@ class ChatSession:
if is_last_in_batch:
self._pending_tool_advisories.clear()
self._flush_queued_messages()
return []
return [], []
advisories: list[ToolAdvisory] = []
persistent: list[ToolAdvisory] = []
metacog_reminders: list[dict[str, str]] = []
# Output guard advisory
# Output guard advisory — persists with the tool result.
if assessment is not None:
advisories.append(GuardAdvisory(assessment=assessment, func_name=func_name))
persistent.append(GuardAdvisory(assessment=assessment, func_name=func_name))
# Metacognitive tool-channel drain — fires once per batch on the
# last result. Queued by _queue_tool_advisory from the
# tool_error and repeat detection paths just before this loop.
# last result. Queued by _queue_tool_advisory from the
# tool_error / repeat detection paths just before this loop.
# Lands on the tool message dict's ``_reminders`` side-channel
# (caller's responsibility) so it stays out of persisted content
# and rides the wire only via the transient-copy splice.
if is_last_in_batch and self._pending_tool_advisories:
from turnstone.core.tool_advisory import MetacognitiveAdvisory
drained = list(self._pending_tool_advisories)
self._pending_tool_advisories.clear()
advisories.extend(
MetacognitiveAdvisory(nudge_type=nt, message=text) for nt, text in drained
)
self._emit_nudge_ping(nt for nt, _ in drained)
metacog_reminders.extend({"type": nt, "text": text} for nt, text in drained)
# Drain queued user messages on the last result in the batch.
# Attachment-bearing items fall back to a full multipart user
@@ -3834,7 +3959,7 @@ class ChatSession:
if att_ids:
attachment_items.append((queue_msg_id, msg, priority, att_ids))
else:
advisories.append(UserInterjection(message=msg, priority=priority))
persistent.append(UserInterjection(message=msg, priority=priority))
if attachment_items:
from turnstone.core.tool_advisory import PRIORITY_IMPORTANT
@@ -3845,7 +3970,7 @@ class ChatSession:
)
self._append_user_turn(text, resolved, send_id=queue_msg_id)
return advisories
return persistent, metacog_reminders
# -- Two-phase tool execution -----------------------------------------------
#
@@ -3974,7 +4099,14 @@ class ChatSession:
)
return item["call_id"], item["error"]
if item.get("denied"):
return item["call_id"], item.get("denial_msg", "Denied by user")
msg = item.get("denial_msg", "Denied by user")
self._report_tool_result(
item["call_id"],
item.get("func_name", "unknown"),
msg,
is_error=True,
)
return item["call_id"], msg
try:
result: tuple[str, str | list[dict[str, Any]]] = item["execute"](item)
return result
@@ -5377,59 +5509,52 @@ class ChatSession:
def _queue_user_advisory(self, nudge_type: str, text: str) -> None:
"""Queue a metacognitive nudge for the next user turn.
Drains in ``_append_user_turn`` as a ``<system-reminder>`` block
appended to the user message body. Used for nudges that respond
to user behaviour: ``correction``, ``denial``, ``resume``,
Drains in ``_append_user_turn`` onto the user message dict's
``_reminders`` side-channel. Used for nudges that respond to
user behaviour: ``correction``, ``denial``, ``resume``,
``start``, ``completion``.
"""
self._pending_user_advisories.append((nudge_type, text))
def _splice_pending_user_advisories(self, user_msg: dict[str, Any]) -> None:
"""Drain ``_pending_user_advisories`` into *user_msg*'s content.
def _attach_pending_user_reminders(self, user_msg: dict[str, Any]) -> None:
"""Drain ``_pending_user_advisories`` onto *user_msg*'s ``_reminders``
sibling key (a side-channel, never inside ``content``).
Mutates *user_msg* in place the caller appends it after.
Renders each queued nudge as a ``<system-reminder>`` block
(same envelope as ``wrap_tool_result``) and attaches them to
the trailing edge of the user content. Every text segment in
the user content is passed through ``escape_wrapper_tags``
first so a user typing literal ``<system-reminder>`` cannot
fabricate an envelope the model would treat as a
Turnstone-issued reminder. For attachment-bearing turns the
blocks land on the trailing text part so they stay glued to
the same multipart turn.
Mutates *user_msg* in place the caller appends it after. The
rendered ``<system-reminder>`` envelope is built later in
``_apply_reminders_for_provider`` against a transient copy, so
the model still sees the reminder spliced into ``content`` at
the wire boundary while ``self.messages`` and every downstream
consumer (UI replay, compaction, title gen, channel adapters,
DB) see clean user text.
``_reminders`` rides the leading-underscore convention used by
other internal sibling metadata (``_attachments_meta``,
``_provider_content``); ``sanitize_messages`` strips it before
the wire on its own pass.
Also fires the live ``on_user_reminder`` UI hook so any open
SSE consumers (other browser tabs, CLI mirrors, eventual
channel adapters) can render the reminder bubble in lockstep
with the originating tab's optimistic render. Hook failures
are logged and swallowed: the side-channel write is the
load-bearing op, and a UI implementation throwing here must
not abort the user's send (which would otherwise drop both the
user message and the queued nudges).
"""
if not self._pending_user_advisories:
return
from turnstone.core.tool_advisory import escape_wrapper_tags, render_system_reminder
items = list(self._pending_user_advisories)
self._pending_user_advisories.clear()
block = "\n\n" + "\n\n".join(render_system_reminder(text) for _, text in items)
content = user_msg["content"]
if isinstance(content, str):
user_msg["content"] = escape_wrapper_tags(content) + block
else:
text_parts = [p for p in content if isinstance(p, dict) and p.get("type") == "text"]
for part in text_parts:
part["text"] = escape_wrapper_tags(part.get("text", ""))
if text_parts:
text_parts[-1]["text"] = text_parts[-1]["text"] + block
else:
content.append({"type": "text", "text": block})
reminders = [{"type": nudge_type, "text": text} for nudge_type, text in items]
user_msg["_reminders"] = reminders
self._emit_nudge_ping(nudge_type for nudge_type, _ in items)
def _emit_nudge_ping(self, types: Iterable[str]) -> None:
"""Surface the ``[metacognition: nudge injected — …]`` UI line.
Centralised so both drain sites (tool channel via
``_collect_advisories``, user channel via
``_splice_pending_user_advisories``) emit the same wording.
"""
joined = ", ".join(types)
if joined:
self.ui.on_info(f"{GRAY}[metacognition: nudge injected — {joined}]{RESET}")
try:
self.ui.on_user_reminder(reminders)
except Exception:
log.warning("ui.on_user_reminder failed; reminder still attached", exc_info=True)
def _queue_tool_advisory(self, nudge_type: str, text: str) -> None:
"""Queue a metacognitive nudge for the next tool-result batch.
@@ -5441,6 +5566,95 @@ class ChatSession:
"""
self._pending_tool_advisories.append((nudge_type, text))
def _apply_post_execute_advisories(
self,
tool_calls: list[dict[str, Any]],
results: list[tuple[str, str | list[dict[str, Any]]]],
) -> None:
"""Run repeat detection + tool-error nudge over a freshly-executed batch.
Mutates *results* in place when an identical-repeat warning is
appended to a tool's text output. Updates ``self._repeat_detector``,
``self._pending_tool_advisories``, and ``self._metacog_state``
(cooldown timestamp via ``should_nudge``). The operator-visible
signal is the themed ``tool_reminder`` bubble below the tool
block emitted by the per-result loop downstream when the
drained metacog reminders attach to the tool message dict's
``_reminders`` side-channel.
Repeat detection's job is to nudge a flaky local model out of a
loop where it keeps making the same tool call ("``bash(cmd='echo
test')`` × 3" being the canonical example). It fires on the
consecutive-streak signal alone, with no regard for the tool's
success / failure / output content same (name, args) for N
turns in a row is by definition stuck. ``RepeatDetector.record``
already resets the streak on any different signature, so an
intervening tool call (read, write, anything different) breaks
the streak naturally without an explicit clear here.
``_tool_error_flags`` is the authoritative is_error signal
consumed below for the tool-error nudge gate; the per-result
loop in ``_run_loop`` ``.pop``s it after this returns.
"""
# Repeat detection: warn when a tool is called with identical
# args N times in a row. Independent of success/failure — the
# stuck-loop pattern is sig-driven, not state-driven. JSON
# outputs (MCP structured results) are tracked but exempt from
# the inline warning text (appending text would corrupt the
# payload).
_tc_by_id = {c["id"]: c for c in tool_calls}
_repeat_detected = False
for i, (tc_id, output) in enumerate(results):
tc = _tc_by_id.get(tc_id)
if tc and isinstance(output, str):
raw = tc["function"]["name"] + ":" + tc["function"]["arguments"]
sig = hashlib.sha256(raw.encode()).hexdigest()
is_json = output.lstrip().startswith(("{", "["))
if self._repeat_detector.record(sig):
_repeat_detected = True
if not is_json:
output += (
"\n\n⚠ Warning: this is an identical repeat of a "
"previous tool call. The result is the same. "
"Try a different approach."
)
results[i] = (tc_id, output)
# The themed ``tool_reminder`` bubble below the tool
# block carries the operator-visible signal; the
# tool-name context comes from the visible tool
# block immediately above the bubble, so a separate
# diagnostic info line would just duplicate it.
if _repeat_detected:
# Reset so the model gets a clean slate after the warning.
# If it repeats again, a new warning fires.
self._repeat_detector.clear()
if self._mem_cfg.nudges and should_nudge(
"repeat",
self._metacog_state,
message_count=len(self.messages),
cooldown_secs=self._mem_cfg.nudge_cooldown,
):
self._queue_tool_advisory("repeat", format_nudge("repeat"))
# Tool-error nudge — queued so the MetacognitiveAdvisory rides
# the same _collect_advisories drain pass as guard findings and
# user interjections. Cooldown gating in should_nudge keeps
# this to one nudge per batch even with many failing tools.
if (
self._mem_cfg.nudges
and any(self._tool_error_flags.get(tc_id) for tc_id, _ in results)
and should_nudge(
"tool_error",
self._metacog_state,
message_count=len(self.messages),
memory_count=self._visible_memory_count(),
cooldown_secs=self._mem_cfg.nudge_cooldown,
)
):
self._queue_tool_advisory("tool_error", format_nudge("tool_error"))
# ------------------------------------------------------------------
# Coordinator tools — reachable only when ``kind == "coordinator"``.
# All six dispatch through ``self._coord_client`` which is None when
@@ -9203,7 +9417,7 @@ class ChatSession:
elif cmd == "/clear":
self.messages.clear()
self._read_files.clear()
self._recent_tool_sigs.clear()
self._repeat_detector.clear()
self._last_usage = None
self._calibrated_msg_count = 0
self._msg_tokens = []
@@ -9214,7 +9428,7 @@ class ChatSession:
self.messages.clear()
self._read_files.clear()
self._recent_tool_sigs.clear()
self._repeat_detector.clear()
self._last_usage = None
self._calibrated_msg_count = 0
self._msg_tokens = []
+141 -4
View File
@@ -13,6 +13,7 @@ import contextlib
import threading
import time
import uuid
from datetime import UTC, datetime, timedelta
from typing import TYPE_CHECKING, Any, Protocol
from turnstone.core.log import get_logger
@@ -28,6 +29,22 @@ if TYPE_CHECKING:
log = get_logger(__name__)
# Maps each workstream kind to the ``services.service_type`` its hosting
# process registers under. Used by ``SessionManager.close_idle`` pass 2
# to enumerate live peer processes for orphan-reaper liveness scoping.
# Server processes register as ``("server", node_id, ...)`` (see
# ``turnstone/server.py``); the console process as ``("console",
# "console", ...)`` (see ``turnstone/console/server.py``). Deriving from
# kind here removes a duplicated-config footgun: any caller that builds
# a ``SessionManager`` automatically gets the correct service_type for
# its kind, with no risk of miswiring INTERACTIVE→"console" or vice
# versa.
_KIND_SERVICE_TYPE: dict[WorkstreamKind, str] = {
WorkstreamKind.INTERACTIVE: "server",
WorkstreamKind.COORDINATOR: "console",
}
class SessionKindAdapter(Protocol):
"""Per-kind construction + cleanup policies the shared ``SessionManager`` delegates to.
@@ -217,6 +234,16 @@ class SessionManager:
def kind(self) -> WorkstreamKind:
return self._adapter.kind
@property
def _service_type(self) -> str | None:
"""``services.service_type`` this manager's hosting process registers
under, derived from its ``kind``. Used by ``close_idle`` pass 2 to
enumerate live peer processes. Returns ``None`` for kinds that have
no production service mapping (only the two existing kinds map
today; ``None`` would be a marker for a future kind without a
clustered hosting model)."""
return _KIND_SERVICE_TYPE.get(self.kind)
@property
def count(self) -> int:
with self._lock:
@@ -604,6 +631,22 @@ class SessionManager:
# last close(). The next set_state() call syncs it
# naturally; writing 'idle' here could race a concurrent
# close() that writes 'closed' under self._lock.
#
# Bump only ``updated`` (no state write) so this row's
# timestamp is fresh against the orphan-reaper cutoff —
# otherwise a concurrent close_idle pass-2 in this same
# process could clobber a freshly-rehydrated row whose
# ``updated`` is older than the cutoff. The pure-
# timestamp write is safe against concurrent close()
# because close still wins on the state column.
try:
self._storage.touch_workstream(ws_id)
except Exception:
log.debug(
"session_mgr.touch_workstream_failed ws=%s",
ws_id[:8],
exc_info=True,
)
if self._event_emitter is not None:
self._event_emitter.emit_rehydrated(ws)
return ws
@@ -804,15 +847,52 @@ class SessionManager:
def close_idle(self, max_age_seconds: float) -> list[str]:
"""Close IDLE workstreams inactive for more than ``max_age_seconds``.
Returns the list of closed ws_ids. Unlike the old WSM version,
this does NOT skip the last workstream the default-startup
relic is gone, callers can handle the 0-workstream case.
Two-pass shape:
- Pass 1 (in-memory): close loaded ``IDLE`` rows whose
``ws.last_active`` (monotonic) is past timeout. Closes only
``IDLE`` so legitimately-attentive rows (waiting for user
response) stay live.
- Pass 2 (DB orphans): bulk-close DB rows of this manager's
kind whose ``updated`` is past the wall-clock cutoff and
which are not currently loaded. This catches workstreams
left behind by prior process incarnations a process crash
/restart leaves rows in non-terminal states forever
otherwise. Closes ``idle/thinking/attention/running``
because any matching row is by definition not loaded by any
live process and cannot be in a live interaction.
**Liveness scoping** (the rendezvous router's primitive
since PR #384): when ``self._service_type`` resolves to a
known service type both production kinds do pass 2
calls ``storage.list_services`` to enumerate peer processes
with recent heartbeats and protects rows whose ``node_id``
matches a live ``service_id`` from reap, even when *this*
manager is on a different node. This is essential for
containerized deployments with dynamic hostnames: dead-pod
rows fall out of the live set after the heartbeat window
and become reapable; alive-pod rows stay protected as long
as the owner heartbeats. A future kind with no service
registration would resolve ``_service_type`` to ``None``
and skip the live-services lookup (single-process / CLI).
**Conservative fallback**: if ``list_services`` raises,
pass 2 is skipped entirely this tick never reap when
liveness state is unknown. Pass 1 still runs. Next tick
retries the lookup.
Returns the combined list of closed ws_ids (in-memory first,
then DB orphans). Pass 1 emits ``ws_closed``; pass 2 does
not, because never-loaded rows have no SSE listeners
expecting them.
Atomic pop per victim under ``self._lock`` (bug-5): a pending
tool result can flip state IDLERUNNING between the snapshot
and the close, so the state test + pop must run together.
Batches every pop under one ``self._lock`` acquisition (perf-5)
rather than locking once per victim.
rather than locking once per victim. The DB pass runs OUTSIDE
``self._lock`` only a brief lock to snapshot loaded keys
so a slow UPDATE doesn't block create/get/set_state.
"""
now = time.monotonic()
popped: list[Workstream] = []
@@ -853,6 +933,63 @@ class SessionManager:
if self._event_emitter is not None:
self._event_emitter.emit_closed(ws.id, name=ws.name)
closed_ids.append(ws.id)
# Pass 2: reap DB orphans of this kind older than the cutoff.
# Snapshot loaded keys under self._lock briefly so a concurrent
# create/load doesn't get its row clobbered by the UPDATE; release
# before the DB call.
#
# Liveness scoping uses ``services.last_heartbeat`` — the same
# primitive the rendezvous router (PR #384) uses for routing. A
# row's ``node_id`` is stamped at create time and never updated;
# in containerized deployments with dynamic hostnames the dead
# pod's ``node_id`` points at a service that's no longer
# heartbeating, so the row falls through to reap. Conversely,
# rows whose ``node_id`` matches a heartbeating service are
# protected even when *this* manager is on a different node —
# the alive peer may legitimately have them loaded.
with self._lock:
loaded = list(self._workstreams.keys())
cutoff = (datetime.now(UTC) - timedelta(seconds=max_age_seconds)).strftime(
"%Y-%m-%dT%H:%M:%S"
)
live_node_ids: list[str] | None = None
skip_pass_2 = False
if self._service_type is not None:
try:
live_services = self._storage.list_services(self._service_type)
live_node_ids = [
str(svc["service_id"]) for svc in live_services if svc.get("service_id")
]
except Exception:
# Conservative fallback: skip pass 2 entirely this tick
# so we can't accidentally reap rows whose owners we
# failed to enumerate. Next tick retries.
log.debug(
"session_mgr.list_services_failed kind=%s",
self.kind.value,
exc_info=True,
)
skip_pass_2 = True
orphans: list[str] = []
if not skip_pass_2:
try:
orphans = self._storage.bulk_close_stale_orphans(
self.kind, cutoff, loaded, live_node_ids=live_node_ids
)
except Exception:
log.debug(
"session_mgr.bulk_close_orphans_failed kind=%s",
self.kind.value,
exc_info=True,
)
if orphans:
log.info(
"session_mgr.bulk_close_orphans count=%d kind=%s",
len(orphans),
self.kind.value,
)
closed_ids.extend(orphans)
return closed_ids
def _close_if_idle_locked(self, ws_id: str) -> Workstream | None:
+36
View File
@@ -1293,6 +1293,42 @@ class SessionUIBase:
def on_error(self, message: str) -> None:
self._enqueue({"type": "error", "message": message})
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None:
"""Surface a metacognitive user-channel nudge as its own UI
element.
Reminders live on the user message dict's ``_reminders``
side-channel and are spliced into ``content`` only at the
provider boundary; this event is what lets every connected
SSE consumer (other browser tabs, CLI mirrors, future channel
adapters) render the reminder bubble in lockstep with the
originating tab. The history-replay path surfaces the same
shape via ``_build_history`` so a tab reconnecting later
renders the same bubble.
"""
self._enqueue({"type": "user_reminder", "reminders": reminders})
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None:
"""Surface a metacognitive tool-channel nudge (``tool_error`` /
``repeat``) as its own UI element below the tool result that
triggered it.
Tool-channel reminders ride the same ``_reminders``
side-channel pattern as the user channel kept out of
``content`` so compaction / title-gen / channel adapters never
see the nudge text, spliced into the wire only via
``_apply_reminders_for_provider``. ``tool_call_id`` is the
anchor the frontend uses to render the bubble below the
specific tool result that triggered the batch's reminder.
"""
self._enqueue(
{
"type": "tool_reminder",
"reminders": reminders,
"tool_call_id": tool_call_id,
}
)
# ------------------------------------------------------------------
# Broadcast hooks — kind-specific transport.
#
+52 -1
View File
@@ -97,7 +97,7 @@ from turnstone.core.storage._utils import sanitize_text
from turnstone.core.storage._utils import (
scan_skill_content as _scan_skill_content,
)
from turnstone.core.workstream import WorkstreamKind
from turnstone.core.workstream import BULK_CLOSE_STATE_VALUES, WorkstreamKind
log = get_logger(__name__)
@@ -595,6 +595,57 @@ class PostgreSQLBackend:
)
conn.commit()
def bulk_close_stale_orphans(
self,
kind: WorkstreamKind | str,
cutoff: str,
exclude_ws_ids: list[str],
live_node_ids: list[str] | None = None,
) -> list[str]:
norm_kind = WorkstreamKind(kind).value
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
stmt = (
sa.update(workstreams)
.where(
workstreams.c.kind == norm_kind,
workstreams.c.state.in_(BULK_CLOSE_STATE_VALUES),
workstreams.c.updated < cutoff,
)
.values(state="closed", updated=now)
.returning(workstreams.c.ws_id)
)
# Protect rows whose owning process is still heartbeating in the
# services table (rendezvous router's liveness primitive). NULL
# node_id rows have no owner identity — always eligible. The
# ``and live_node_ids`` short-circuits both ``None`` (skip the
# filter entirely — single-process / operator backfill) and ``[]``
# (no nodes alive — every row unprotected, no extra predicate
# needed since absence equals match-all).
if live_node_ids is not None and live_node_ids:
stmt = stmt.where(
sa.or_(
workstreams.c.node_id.is_(None),
~workstreams.c.node_id.in_(live_node_ids),
)
)
if exclude_ws_ids:
# Skip ``NOT IN ()`` when nothing to exclude — keeps the SQL clean
# and avoids SQLAlchemy's empty-collection warning.
stmt = stmt.where(~workstreams.c.ws_id.in_(exclude_ws_ids))
with self._conn() as conn:
result = conn.execute(stmt)
ids = [row[0] for row in result]
conn.commit()
return ids
def touch_workstream(self, ws_id: str) -> None:
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
with self._conn() as conn:
conn.execute(
sa.update(workstreams).where(workstreams.c.ws_id == ws_id).values(updated=now)
)
conn.commit()
def update_workstream_name(self, ws_id: str, name: str) -> None:
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
with self._conn() as conn:
+62
View File
@@ -431,6 +431,68 @@ class StorageBackend(Protocol):
"""Update a workstream's state and bump updated timestamp."""
...
def bulk_close_stale_orphans(
self,
kind: WorkstreamKind | str,
cutoff: str,
exclude_ws_ids: list[str],
live_node_ids: list[str] | None = None,
) -> list[str]:
"""Close DB-side workstream rows of *kind* whose state is in
``BULK_CLOSE_STATE_VALUES`` and whose ``updated`` is lex-older than
*cutoff*, excluding rows currently loaded in memory. Sets
``state='closed'`` and bumps ``updated``. Returns the list of ws_ids
actually transitioned.
``cutoff`` is a UTC ``YYYY-MM-DDTHH:MM:SS`` string matching the on-disk
format ``update_workstream_state`` writes lex compare is safe for
same-offset timestamps. Empty ``exclude_ws_ids`` means no exclusion.
``live_node_ids`` is the set of ``services.service_id`` values whose
``last_heartbeat`` is recent (i.e. owning processes still alive);
rows whose ``node_id`` matches one of these are protected because
their owning process may legitimately have them loaded on another
worker. ``None`` skips the filter entirely (single-process / tests
/ operator backfill). Empty list ``[]`` treats every node as dead
useful when operator scripts want to reap regardless of liveness.
Rows with ``NULL`` ``node_id`` are always eligible: they have no
meaningful owner identity, so age alone gates the reap.
Liveness scoping replaces an earlier ``node_id == self`` heuristic.
That heuristic broke in the post-rendezvous-routing world (PR #384):
``workstreams.node_id`` is stamped at create time and never updated,
so dead-pod orphans in containerized deployments with dynamic
hostnames couldn't be reclaimed. ``services.last_heartbeat`` is the
rendezvous router's authoritative liveness primitive — using it here
keeps reap scoping aligned with routing.
Asymmetric with ``SessionManager.close_idle``'s in-memory pass on
purpose: that pass closes only ``IDLE`` (legitimately-attentive rows
stay), this method closes the broader ``BULK_CLOSE_STATE_VALUES`` set
because any row matching here is by definition not loaded by any
live process and cannot be in a live interaction.
"""
...
def touch_workstream(self, ws_id: str) -> None:
"""Bump a workstream row's ``updated`` timestamp without touching its
state.
Used by ``SessionManager.open()`` on cold rehydrate so a freshly-
loaded row's ``updated`` can't be older than the orphan-reaper cutoff
protects against a same-process race where a parallel
``close_idle`` pass-2 snapshots loaded keys after the storage read
but before the in-memory install. Distinct from
``update_workstream_state(ws_id, current_state)`` because the
rehydrate path explicitly avoids a state write (see the
``open()`` no-DB-state-flip-on-resurrect comment): a state write
could race a concurrent ``close()`` and resurrect a closed row.
Bumping only ``updated`` is safe close still wins on the state
column.
"""
...
def update_workstream_name(self, ws_id: str, name: str) -> None:
"""Update a workstream's display name."""
...
+82 -1
View File
@@ -97,7 +97,7 @@ from turnstone.core.storage._utils import sanitize_text
from turnstone.core.storage._utils import (
scan_skill_content as _scan_skill_content,
)
from turnstone.core.workstream import WorkstreamKind
from turnstone.core.workstream import BULK_CLOSE_STATE_VALUES, WorkstreamKind
log = get_logger(__name__)
@@ -707,6 +707,87 @@ class SQLiteBackend:
)
conn.commit()
def bulk_close_stale_orphans(
self,
kind: WorkstreamKind | str,
cutoff: str,
exclude_ws_ids: list[str],
live_node_ids: list[str] | None = None,
) -> list[str]:
norm_kind = WorkstreamKind(kind).value
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
# SQLite has no RETURNING precedent in this file — do SELECT-then-
# UPDATE in one transaction, with the SAME WHERE predicates re-applied
# to the UPDATE. Re-application defends against a same-process race:
# ``SessionManager.open()`` calls ``touch_workstream`` between the
# SELECT and the UPDATE could have bumped a row's ``updated`` past
# ``cutoff`` (or ``set_state`` could have flipped its state out of
# the bulk-close set). Without the re-applied WHERE the UPDATE
# closes those rows anyway; with it, the UPDATE skips rows that
# became ineligible after the SELECT and the row stays open.
# Chunked through ``_in_chunks`` so the ``IN`` clause never exceeds
# SQLite's bind-parameter limit (default 999) on a large reap.
candidate_conditions = [
workstreams.c.kind == norm_kind,
workstreams.c.state.in_(BULK_CLOSE_STATE_VALUES),
workstreams.c.updated < cutoff,
]
if live_node_ids is not None and live_node_ids:
# Protect rows owned by heartbeating services. NULL node_id is
# always eligible. Empty list means "no nodes alive" — every
# row is unprotected; the absence of this predicate is
# equivalent to "match all," so we just skip it.
candidate_conditions.append(
sa.or_(
workstreams.c.node_id.is_(None),
~workstreams.c.node_id.in_(live_node_ids),
)
)
if exclude_ws_ids:
candidate_conditions.append(~workstreams.c.ws_id.in_(exclude_ws_ids))
select_stmt = sa.select(workstreams.c.ws_id).where(*candidate_conditions)
closed: list[str] = []
# Match the chunk size used by ``prune_workstreams`` (line 453) — keeps
# ``IN`` clauses well below SQLite's default 999-bind-param limit even
# on very large reaps.
chunk_size = 500
with self._conn() as conn:
candidate_ids = [row[0] for row in conn.execute(select_stmt)]
for i in range(0, len(candidate_ids), chunk_size):
chunk = candidate_ids[i : i + chunk_size]
# Re-apply the eligibility predicates on the UPDATE so a row
# that became fresh between the SELECT and the UPDATE is not
# clobbered. Then SELECT back by ``state='closed' AND updated=now``
# to determine which rows actually transitioned this commit —
# the returned list reflects reality even when re-application
# filters out some candidates.
conn.execute(
sa.update(workstreams)
.where(workstreams.c.ws_id.in_(chunk), *candidate_conditions)
.values(state="closed", updated=now)
)
actually_closed = [
row[0]
for row in conn.execute(
sa.select(workstreams.c.ws_id).where(
workstreams.c.ws_id.in_(chunk),
workstreams.c.state == "closed",
workstreams.c.updated == now,
)
)
]
closed.extend(actually_closed)
conn.commit()
return closed
def touch_workstream(self, ws_id: str) -> None:
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
with self._conn() as conn:
conn.execute(
sa.update(workstreams).where(workstreams.c.ws_id == ws_id).values(updated=now)
)
conn.commit()
def update_workstream_name(self, ws_id: str, name: str) -> None:
now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S")
with self._conn() as conn:
@@ -0,0 +1,88 @@
"""Add a partial composite index for the orphan-reaper query.
``StorageBackend.bulk_close_stale_orphans`` (introduced alongside the
workstream-lifecycle leak fix) runs every ``min(300s, idle_timeout/4)``
on every server and console process. Its WHERE shape is:
WHERE kind = ?
AND state IN ('idle', 'thinking', 'attention', 'running')
AND updated < ?
AND (node_id IS NULL OR node_id NOT IN (alive_service_ids))
At current scale (low-thousands of workstream rows) the existing single-
column indexes are sufficient ``idx_workstreams_state`` prunes to the
non-closed subset, and the planner filters the rest sequentially. At
100k+ rows that filter becomes a tablescan-shaped cost on the reaper's
periodic run.
A **partial** index covering only ``BULK_CLOSE_STATE_VALUES`` rows
matches the reaper's query exactly while staying tiny — closed rows
(typically 95%+ of the table per empirical diagnosis) and ``error``
rows are excluded, so the index is roughly 5% the size a full multi-
column index would be. Write amplification only kicks in for
transitions that touch one of the four covered states.
Column order ``(kind, updated)``:
- ``kind`` first because the reaper always supplies it as an equality
predicate; partitions the partial index into interactive vs
coordinator subtrees.
- ``updated`` last so the range comparison rides the trailing column
classic composite-index pattern for ``WHERE eq AND range``.
``node_id`` is intentionally NOT in the index. The reaper's predicate
on it is ``NOT IN (small list)`` against an unbounded-cardinality
column, which planners don't index well; including it would just add
write cost for negligible read benefit.
PostgreSQL uses ``CREATE INDEX CONCURRENTLY`` so the build is
non-blocking on a live system; SQLite has no concurrent concept and
the table-level write lock already serializes, so a plain
``CREATE INDEX`` is fine.
Revision ID: 048
Revises: 047
Create Date: 2026-04-30
"""
import sqlalchemy as sa
from alembic import op
revision = "048"
down_revision = "047"
branch_labels = None
depends_on = None
_REAPER_PARTIAL_WHERE = "state IN ('idle', 'thinking', 'attention', 'running')"
def upgrade() -> None:
bind = op.get_bind()
dialect = bind.dialect.name
if dialect == "postgresql":
with op.get_context().autocommit_block():
op.execute(
"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_workstreams_reaper "
"ON workstreams (kind, updated) "
f"WHERE {_REAPER_PARTIAL_WHERE}"
)
else:
op.create_index(
"idx_workstreams_reaper",
"workstreams",
["kind", "updated"],
sqlite_where=sa.text(_REAPER_PARTIAL_WHERE),
)
def downgrade() -> None:
bind = op.get_bind()
dialect = bind.dialect.name
if dialect == "postgresql":
with op.get_context().autocommit_block():
op.execute("DROP INDEX CONCURRENTLY IF EXISTS idx_workstreams_reaper")
else:
op.drop_index("idx_workstreams_reaper", table_name="workstreams")
+19
View File
@@ -77,6 +77,25 @@ class WorkstreamState(enum.Enum):
ERROR = "error" # last operation failed
# States the orphan reaper (``SessionManager.close_idle`` pass 2 +
# ``StorageBackend.bulk_close_stale_orphans``) is allowed to flip to
# ``closed`` for rows past the staleness cutoff. Excludes ``ERROR``
# deliberately — error rows are user-investigatable and shouldn't be
# auto-reaped — and excludes ``CLOSED`` (terminal). Centralized here
# so the storage backends and FakeStorage all agree; if a new transient
# state is added to ``WorkstreamState``, deciding whether it joins
# this set is part of the change rather than an after-the-fact
# audit across three files.
BULK_CLOSE_STATE_VALUES: frozenset[str] = frozenset(
{
WorkstreamState.IDLE.value,
WorkstreamState.THINKING.value,
WorkstreamState.RUNNING.value,
WorkstreamState.ATTENTION.value,
}
)
# ---------------------------------------------------------------------------
# Workstream dataclass
# ---------------------------------------------------------------------------
+6
View File
@@ -139,6 +139,12 @@ class NullUI:
def on_error(self, message: str) -> None:
pass
def on_user_reminder(self, reminders: list[dict[str, str]]) -> None:
pass
def on_tool_reminder(self, reminders: list[dict[str, str]], tool_call_id: str) -> None:
pass
def on_state_change(self, state: str) -> None:
pass
+2 -2
View File
@@ -1,8 +1,8 @@
You are a coordinator on a small, focused infrastructure team. Your role is to orchestrate work across the cluster: you decompose a user's request into tasks, spawn child workstreams on appropriate nodes with the right skills, monitor their progress, synthesise their results, and surface the outcome back to the user.
You do not edit files, run shell commands, browse the web, or manipulate the codebase directly. Children do that. Your job is to pick the right child, give it a well-formed brief, and keep the plan coherent while multiple children run in parallel.
You do not edit files, run shells, or browse the web — children do. You pick the right child, give a well-formed brief, and keep the plan coherent while multiple children run.
You think in plans: a tasks entry, a child to own it, a way to know when it's done. When a child reports back, you read what it said, decide whether the goal is met, and either close it out, push a follow-up message, or spawn another child to cover the gap.
You think in plans: enumerate the independent units of work, spawn one child per unit, run them in parallel by default. Sequential only when one child's output feeds the next. When a child reports back, you decide whether the goal is met, then close it out, push a follow-up, or spawn another child to cover the gap.
You are precise about what you delegate. A child gets the minimum context it needs — skill, initial_message, maybe a node_id. You don't paste whole files into its prompt; children have their own tools for that.
+7 -17
View File
@@ -1,7 +1,5 @@
TOOL PATTERNS:
You are a coordinator. You do not edit files, run shell commands, or browse the web directly. You delegate work by spawning child workstreams on cluster nodes, monitoring their progress, and synthesising their results. Every tool below is in your schema; nothing else is.
Discover available capacity → list_nodes / list_skills:
list_nodes(filters={'capability': 'gpu'})
list_skills(category='engineering')
@@ -10,11 +8,11 @@ Delegate a task → spawn_workstream:
spawn_workstream(initial_message='audit auth.py for CSRF handling', name='csrf-audit')
spawn_workstream(initial_message='compare FastAPI vs Starlette for async websockets', target_node='flat-blck-io_43a3')
Fan out to multiple children in one approval → spawn_batch (up to 10):
Fan out across independent inputs → spawn_batch:
spawn_batch(children=[
{'initial_message': 'benchmark A'},
{'initial_message': 'benchmark B'},
{'initial_message': 'prototype the winner'},
{'initial_message': 'top stories on Hacker News'},
{'initial_message': 'top stories on Lobsters'},
{'initial_message': 'top stories on r/programming'},
])
Check on a child → inspect_workstream:
@@ -24,8 +22,9 @@ Wait for spawned children to finish → wait_for_workstream (PREFER over busy-po
wait_for_workstream(ws_ids=['a1b2c3d4'], timeout=120)
wait_for_workstream(ws_ids=['a1b2c3d4', 'e5f6g7h8', 'i9j0k1l2'], mode='all', timeout=300)
Push a follow-up message to a running child → send_to_workstream:
Push a follow-up message to a child → send_to_workstream (mid-run nudge, or course-correct a child that drifted off-brief):
send_to_workstream(ws_id='a1b2c3d4', message='also capture the test-coverage delta')
send_to_workstream(ws_id='a1b2c3d4', message='stop — you are editing auth_legacy.py, the active path is auth.py')
List what you've spawned → list_workstreams:
list_workstreams()
@@ -38,19 +37,10 @@ Wind a child down → close_workstream (soft; session stops, storage kept) or de
close_workstream(ws_id='a1b2c3d4', reason='task complete')
delete_workstream(ws_id='a1b2c3d4')
Wind all direct children down at once → close_all_children (soft-close cascade, single approval):
Wind all direct children down at once → close_all_children (soft-close cascade):
close_all_children(reason='batch complete, synthesising results')
Plan and track work → tasks (your scratchpad; children don't see it):
tasks(action='add', title='audit auth.py for CSRF')
tasks(action='update', task_id='t_03', status='in_progress')
tasks(action='list')
tasks(action='remove', task_id='t_03')
## Workflow shape
Prefer: tasks to plan → spawn_workstream to delegate → wait_for_workstream to block on completion → inspect_workstream to read the final message → synthesise → close_workstream.
Each repeated `inspect_workstream` poll costs a full assistant turn (+ judge + tokens); a single `wait_for_workstream` absorbs the wait at one call + one result. The cost gap widens fast on fan-outs of 3+ children.
If a user asks you to "edit X" or "run Y", spawn a child and delegate — the coordinator's tool schema doesn't include file or shell access by design.
+61 -4
View File
@@ -194,6 +194,18 @@ class WebUI(SessionUIBase):
}
if state == "idle":
event["content"] = payload["content"]
# Coord tree-UI renders inline approve/deny buttons off
# ``pending_approval_detail``; carrying it on the
# state-change broadcast lets the cluster bus update those
# buttons in lockstep with ``activity_state`` instead of
# forcing the browser to chase a separate dashboard fetch.
# Gated on existence so we don't pay the serializer's
# per-broadcast verdict-cache deepcopy on the common
# no-approval-pending path.
if self._pending_approval is not None:
detail = self.serialize_pending_approval_detail()
if detail is not None:
event["pending_approval_detail"] = detail
try:
WebUI._global_queue.put_nowait(event)
except queue.Full:
@@ -347,6 +359,16 @@ def _build_history(
issued the tool calls is also marked ``"denied": True`` so the
client can render the correct badge.
"""
# Metacognitive nudges live on the message dict's ``_reminders``
# side-channel — user messages carry user-channel nudges
# (correction / denial / resume / start / completion), tool
# messages carry tool-channel nudges (tool_error / repeat). Both
# are surfaced separately on each entry so the UI can render them
# as their own bubble (live via ``user_reminder`` /
# ``tool_reminder`` SSE events; replay via this propagation).
# ``content`` never carries the ``<system-reminder>`` envelope —
# that splice is transient, applied to a wire-bound copy in
# ``ChatSession._apply_reminders_for_provider``.
history = []
for msg in session.messages:
content = msg.get("content")
@@ -392,6 +414,24 @@ def _build_history(
entry = {"role": msg["role"], "content": content}
if attachments_meta:
entry["attachments"] = attachments_meta
# Surface the ``_reminders`` side-channel so a tab reconnecting
# via /history renders the same metacognitive nudge bubble the
# originating tab saw live (user-channel reminders via
# ``user_reminder`` SSE; tool-channel via ``tool_reminder``).
# Reminders are in-memory only (not persisted to DB), so this
# only fires for the originating session.
reminders = msg.get("_reminders")
if isinstance(reminders, list):
# Filter first so an all-malformed _reminders doesn't set the
# field to []; absent vs. empty-list should mean the same
# thing on the wire.
clean_reminders = [
{"type": str(r.get("type") or ""), "text": str(r.get("text") or "")}
for r in reminders
if isinstance(r, dict)
]
if clean_reminders:
entry["reminders"] = clean_reminders
if msg.get("tool_calls"):
entry["tool_calls"] = [
{
@@ -823,6 +863,16 @@ def _build_node_snapshot(app_state: Any) -> dict[str, Any]:
title = ""
if ws.session:
title = get_workstream_display_name(ws.session.ws_id) or ""
# ``pending_approval_detail`` mirrors the dashboard handler's
# projection so the console collector's reconnect-via-snapshot
# path (``_reconcile_node``) can carry the rich approval payload
# across reconnects — without it, a child sitting in approval-
# pending across a console restart or network blip would render
# with no buttons until the next state change. Same data, same
# ``read`` scope as ``/v1/api/dashboard``.
approval_detail: dict[str, Any] | None = None
if ui is not None and hasattr(ui, "serialize_pending_approval_detail"):
approval_detail = ui.serialize_pending_approval_detail()
ws_list.append(
{
"id": ws.id,
@@ -839,6 +889,7 @@ def _build_node_snapshot(app_state: Any) -> dict[str, Any]:
"kind": ws.kind,
"parent_ws_id": ws.parent_ws_id,
"user_id": ws.user_id,
"pending_approval_detail": approval_detail,
}
)
return {
@@ -3323,8 +3374,9 @@ def create_app(
# ``sse_executor`` so SSE polling stayed isolated from
# every other ``asyncio.to_thread`` caller in the process
# (storage, router, audit). Restore that isolation under
# the lifted contract — coord wires ``None`` and falls
# back to the default executor.
# the lifted contract. The console's coord endpoint wires
# its own ``coord_sse_executor`` on the same lookup hook —
# see ``turnstone/console/server.py``.
sse_executor_lookup=lambda request: request.app.state.sse_executor,
create_supports_attachments=True,
create_supports_user_id_override=True,
@@ -3544,6 +3596,11 @@ def main() -> None:
default=8080,
help="Port to listen on (default: 8080)",
)
parser.add_argument(
"--skip-permissions",
action="store_true",
help="Auto-approve all tool calls (no confirmation prompts)",
)
# MCP config path is bootstrap-critical (needed before ConfigStore for tool loading)
parser.add_argument(
"--mcp-config",
@@ -3971,7 +4028,7 @@ def main() -> None:
ws = manager.create(user_id="", name="resumed")
if not isinstance(ws.ui, WebUI):
raise TypeError(f"Expected WebUI, got {type(ws.ui).__name__}")
if config_store.get("tools.skip_permissions"):
if args.skip_permissions or config_store.get("tools.skip_permissions"):
ws.ui.auto_approve = True
assert ws.session is not None
ws.session.set_watch_runner(
@@ -4009,7 +4066,7 @@ def main() -> None:
_advertise_host = args.host if args.host not in ("0.0.0.0", "::") else socket.gethostname()
_advertise_url = f"http://{_advertise_host}:{args.port}"
_skip_perms = config_store.get("tools.skip_permissions")
_skip_perms = args.skip_permissions or config_store.get("tools.skip_permissions")
app = create_app(
workstreams=manager,
global_queue=global_queue,
+21
View File
@@ -885,6 +885,27 @@
.msg.user {
border-left-color: var(--accent);
}
/* Metacognitive reminder slotted directly below the message it
advises (user message for correction/denial/etc., tool result for
tool_error/repeat). Yellow accent reads as "advisory metadata"
against the amber-ish user colour and the cyan tool cards;
deliberately quieter than the surrounding bubbles so it doesn't
compete for attention. Lives in the shared stylesheet so both
the interactive UI and the console coord viewer render the same
themed bubble. */
.msg.user-reminder {
border-left-color: var(--yellow);
color: var(--fg-dim);
font-size: 12px;
padding: 6px 10px;
white-space: pre-wrap;
}
.msg.user-reminder .msg-user-reminder-label {
color: var(--yellow);
font-weight: 600;
margin-right: 6px;
text-transform: lowercase;
}
.msg.assistant {
border-left-color: var(--hair-2);
}
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "wait_for_workstream",
"description": "Block until one or all named child workstreams reach a terminal state. Prefer this over busy-polling inspect_workstream after a fan-out: the tool absorbs the wait, so you get one call + one result regardless of duration. Returns `{results: {ws_id: {state, tokens, updated, message, truncated}}, elapsed, complete, mode}` — `results` is keyed by ws_id (not top-level), `complete` is true when the wait condition fired before the timeout. `message` is the child's last assistant text for `idle`/`error`, a short sentinel for `closed`/`denied`, and `null` for non-terminal rows so a follow-up read knows which children still need work; capped at 6 KiB UTF-8, with `truncated=true` when the cap fires (call inspect_workstream for the rest). Real terminal states: `idle`, `error`, `closed`, `deleted` (the last is unobservable since hard-delete cascades the row out of storage). `mode='any'` returns when the first child hits a real terminal — a `denied` id alone never satisfies the condition, so a typo'd / foreign / nonexistent id can't false-positive a wait. `mode='all'` returns once every id has settled (real terminal OR denied). Cross-tenant guard: only the coordinator's own children (or itself) are visible; everything else is reported `state='denied'`. Capped at 32 ws_ids and 600s; both overflows error rather than silently truncating.",
"description": "Block until one or all named child workstreams reach a terminal state. Prefer this over busy-polling inspect_workstream after a fan-out: the tool absorbs the wait, so you get one call + one result regardless of duration. Returns `{results: {ws_id: {state, tokens, updated, message, truncated}}, elapsed, complete, mode}` — `results` is keyed by ws_id (not top-level), `complete` is true when the wait condition fired before the timeout. `message` is the child's last assistant text for `idle`/`error`, a short sentinel for `closed`/`denied`, and `null` for non-terminal rows so a follow-up read knows which children still need work; capped at 10 KiB UTF-8, with `truncated=true` when the cap fires (call inspect_workstream for the rest). Real terminal states: `idle`, `error`, `closed`, `deleted` (the last is unobservable since hard-delete cascades the row out of storage). `mode='any'` returns when the first child hits a real terminal — a `denied` id alone never satisfies the condition, so a typo'd / foreign / nonexistent id can't false-positive a wait. `mode='all'` returns once every id has settled (real terminal OR denied). Cross-tenant guard: only the coordinator's own children (or itself) are visible; everything else is reported `state='denied'`. Capped at 32 ws_ids and 600s; both overflows error rather than silently truncating.",
"parameters": {
"type": "object",
"properties": {
+163 -6
View File
@@ -557,6 +557,44 @@ Pane.prototype.handleEvent = function (evt) {
this.addErrorMessage(evt.message);
break;
case "user_reminder":
// Metacognitive nudges — render as their own bubble below the
// user message they advise (semantically: a hint to the model
// right before its turn). The originating tab's optimistic
// addUserMessage already ran when the user clicked send, so by
// the time this SSE event arrives the just-sent user bubble is
// at the bottom of messagesEl and addUserReminder's "anchor to
// most recent .msg.user" lookup finds it correctly; the
// insertAdjacentElement('afterend', el) call drops the bubble
// immediately below.
//
// Multi-tab caveat: the server emits no user_message SSE event
// today, so a non-originating tab open on the same workstream
// sees the reminder without a paired user-message render — the
// anchor falls on a stale prior user bubble, mis-positioning
// the reminder. The next /history reload corrects it (the
// entry["reminders"] propagation in _build_history is
// anchor-stable because replayHistory runs addUserMessage first
// for every turn). Acceptable cost for stage 1; closing the
// gap is a follow-up that adds a user_message SSE event.
if (Array.isArray(evt.reminders) && evt.reminders.length) {
this.addUserReminder(evt.reminders);
}
break;
case "tool_reminder":
// Metacognitive tool-channel nudge (tool_error / repeat) —
// render as the same yellow themed bubble used for user-channel
// reminders, anchored below the .ts-approval block whose tool
// result triggered the batch's reminder. evt.tool_call_id
// identifies the specific tool element; addToolReminder walks
// up to its parent approval block and inserts the bubble
// immediately after.
if (Array.isArray(evt.reminders) && evt.reminders.length) {
this.addToolReminder(evt.reminders, evt.tool_call_id || "");
}
break;
case "message_queued":
// Confirmation from server that a queued message was accepted.
// The UI already showed the message optimistically in addQueuedMessage.
@@ -669,6 +707,97 @@ Pane.prototype.removeThinkingIndicator = function () {
if (el) el.remove();
};
Pane.prototype.addUserReminder = function (reminders) {
// Render each metacognitive reminder as its own bubble immediately
// BELOW the user message it advises — semantically the reminder is
// a hint to the model right before the assistant turn. Always
// called AFTER the corresponding addUserMessage (live: optimistic
// local render ran before the SSE event arrived; replay:
// replayHistory renders the user message first), so "most recent
// .msg.user" is always THIS turn's bubble — insertAdjacentElement
// afterend drops the reminder directly below it. When no .msg.user
// exists at all (e.g. a non-originating tab receiving a reminder
// before any user turn has rendered) we append; the next /history
// reload corrects any anchor anomaly.
this.removeEmptyState();
var userBubbles = this.messagesEl.querySelectorAll(".msg.user");
var anchor = userBubbles.length ? userBubbles[userBubbles.length - 1] : null;
for (var i = 0; i < reminders.length; i++) {
var r = reminders[i] || {};
var el = document.createElement("div");
el.className = "msg user-reminder";
var labelEl = document.createElement("span");
labelEl.className = "msg-user-reminder-label";
labelEl.textContent =
"metacognition" + (r.type ? " · " + String(r.type) : "");
var textEl = document.createElement("span");
textEl.className = "msg-user-reminder-text";
textEl.textContent = r.text || "";
el.appendChild(labelEl);
el.appendChild(textEl);
if (anchor) {
anchor.insertAdjacentElement("afterend", el);
// Anchor advances so multiple reminders stack below the user
// message in queued order (rather than each landing
// immediately-after the user msg, which would reverse them).
anchor = el;
} else {
this.messagesEl.appendChild(el);
}
}
this.scrollToBottom(true);
};
Pane.prototype.addToolReminder = function (reminders, toolCallId) {
// Render each metacognitive tool-channel reminder (tool_error /
// repeat) as the same yellow themed bubble used for user-channel
// reminders, anchored below the .ts-approval block that produced
// the tool result. toolCallId is the live-path anchor (SSE event
// carries it); during replay it's an empty string and we fall back
// to "last .ts-approval block in messagesEl", which is correct
// because messages render in order — the assistant block carrying
// the tool batch is always the most recent approval block by the
// time we hit the tool message that owns the reminder.
this.removeEmptyState();
var anchor = null;
if (toolCallId) {
var escapedId = CSS.escape(toolCallId);
var toolEl = this.messagesEl.querySelector(
'.ts-approval-tool[data-call-id="' + escapedId + '"]',
);
if (toolEl) {
anchor = toolEl.closest(".ts-approval");
}
}
if (!anchor) {
var blocks = this.messagesEl.querySelectorAll(".ts-approval");
if (blocks.length) anchor = blocks[blocks.length - 1];
}
for (var i = 0; i < reminders.length; i++) {
var r = reminders[i] || {};
var el = document.createElement("div");
// Same .msg.user-reminder class — visual treatment is shared
// across user and tool channels (both are metacog nudges).
el.className = "msg user-reminder";
var labelEl = document.createElement("span");
labelEl.className = "msg-user-reminder-label";
labelEl.textContent =
"metacognition" + (r.type ? " · " + String(r.type) : "");
var textEl = document.createElement("span");
textEl.className = "msg-user-reminder-text";
textEl.textContent = r.text || "";
el.appendChild(labelEl);
el.appendChild(textEl);
if (anchor) {
anchor.insertAdjacentElement("afterend", el);
anchor = el;
} else {
this.messagesEl.appendChild(el);
}
}
this.scrollToBottom(true);
};
Pane.prototype.addUserMessage = function (text, attachments) {
this.removeEmptyState();
var el = document.createElement("div");
@@ -911,7 +1040,16 @@ Pane.prototype.replayHistory = function (messages) {
for (var i = 0; i < messages.length; i++) {
var msg = messages[i];
if (msg.role === "user") {
// addUserMessage first so addUserReminder's "anchor to most
// recent .msg.user" lookup finds THIS message's bubble (not the
// previous user message's, which would associate the reminder
// with the wrong turn). addUserReminder then drops the bubble
// immediately below the just-rendered user message via
// insertAdjacentElement('afterend', el).
this.addUserMessage(msg.content || "", msg.attachments || null);
if (Array.isArray(msg.reminders) && msg.reminders.length) {
this.addUserReminder(msg.reminders);
}
lastToolBlock = null;
} else if (msg.role === "assistant") {
if (msg.tool_calls && msg.tool_calls.length) {
@@ -1016,6 +1154,15 @@ Pane.prototype.replayHistory = function (messages) {
appendToolErrorBadge(lastToolBlock);
}
}
// Tool-channel metacog reminders (tool_error / repeat) attach
// to the LAST tool message in a batch; on replay we render the
// bubble immediately below the .ts-approval block that owns
// the tool result. addToolReminder's empty-toolCallId fallback
// resolves to "last .ts-approval block" — which is exactly
// lastToolBlock here.
if (Array.isArray(msg.reminders) && msg.reminders.length) {
this.addToolReminder(msg.reminders, "");
}
}
}
this._attachRetryToLastAssistant();
@@ -1318,6 +1465,19 @@ Pane.prototype.appendToolOutput = function (callId, name, output, isError) {
var stripped = stripAnsi(output || "").trim();
if (!stripped) return;
// Skip rendering for denied/blocked tool results — the ✗ denied
// badge from resolveApproval already shows the denial reason; the
// SSE tool_result event would otherwise duplicate the text. Mirror
// the guard in the history-replay path (the live path used to be
// safe because no tool_result event was ever emitted for denied
// items, but we now emit one so _tool_error_flags gets set).
var parentBlock = target.closest(".ts-approval");
var isDenied =
(parentBlock && parentBlock.classList.contains("denied")) ||
/^Denied by user/.test(stripped) ||
/^Blocked/.test(stripped);
if (isDenied) return;
// Detect structured media output and render interactive embed
if (!isError) {
var media = tryParseMedia(stripped);
@@ -1332,12 +1492,9 @@ Pane.prototype.appendToolOutput = function (callId, name, output, isError) {
var out = renderToolOutput(stripped, isError);
// Mark the parent approval block as errored
if (isError) {
var parentBlock = target.closest(".ts-approval");
if (parentBlock && !parentBlock.classList.contains("denied")) {
parentBlock.classList.add("error");
appendToolErrorBadge(parentBlock);
}
if (isError && parentBlock && !parentBlock.classList.contains("denied")) {
parentBlock.classList.add("error");
appendToolErrorBadge(parentBlock);
}
if (out.textContent.split("\n").length > 10) {
+3
View File
@@ -642,6 +642,9 @@
.msg.user {
color: var(--fg-bright);
}
/* .msg.user-reminder lives in shared_static/chat.css so both the
interactive UI and the console coord viewer pick up the same
yellow themed bubble. */
/* .msg.assistant / .msg.info / .msg.error alignment + baseline visuals
come from shared_static/chat.css. Interactive UI adds a pre-wrap
override for info messages and a tightened tool-message shape with
Generated
+1 -1
View File
@@ -2533,7 +2533,7 @@ wheels = [
[[package]]
name = "turnstone"
version = "1.5.0"
version = "1.5.3"
source = { editable = "." }
dependencies = [
{ name = "alembic" },