mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-24 12:54:48 -06:00
perf/webui-transcript-windowing
87 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Patrick Buckley
|
c6e5794125 |
fix(compaction): recover from context overflow on resume across providers
A session created under the openai-compatible provider and resumed under the anthropic-compatible provider (same vLLM model) failed with an opaque InternalError instead of recovering. Root cause: vLLM returns a context-window overflow as HTTP 400 BadRequestError on /v1/chat/completions but HTTP 500 InternalServerError on /v1/messages, and the rehydrated resume payload overflowed the window. The 500 was retried four times then surfaced as a bare class name. - Detect overflow by message text, not exception class (_is_ctx_overflow), shared across the fatal-error formatter, both stream-retry gates, the send-loop recovery, the chunker, and the task_agent loop. Overflow is non-retryable (deterministic; no backoff). Phrasing is overflow-specific so a token-quota rate-limit isn't misclassified. - Proactive pre-send compaction (Layer A): when already over the hard ceiling, compact once before the first stream so a resume that arrives over-window (or follows a switch to a smaller-context model, with no prior compaction) doesn't go out blind. Generation-guarded end to end so an orphaned or superseded send can never swap the live generation's history. - Binary-subdivision chunker: an over-window summary batch is split in half and the partials merged (~log2(N) calls, not one per block); a lone over-window block is truncated progressively down to a floor before bailing irreducible. - Cooperative cancellation honored through compaction; send() consumes its own generation's cancel signal on exit, so a stale cancel can't block a later idle /compact and a live cancel is never disarmed. - _format_backend_error surfaces "Context window exceeded ..." instead of an opaque InternalServerError, and only for unrecognized classes. - retry/rewind, the continuation hint, and title generation all exclude the synthetic [Conversation summary] turn so they can't target the label. - task_agent salvages a sub-agent's partial work on any terminal error (not only overflow), re-raising only when there is nothing to salvage. |
||
|
Patrick Buckley
|
7f1329d3b0 |
fix(memory): atomic single-statement upsert for memory save/update (#735)
* fix(memory): atomic single-statement upsert for memory save/update save_structured_memory used "try INSERT -> catch IntegrityError -> SELECT + UPDATE". On PostgreSQL a model saving the same key twice in a turn logged a uq_smem_name_scope violation on the failing INSERT, and the pattern threw + caught an exception on every update. Replace it with one statement: a new StorageBackend.upsert_structured_memory on both backends emitting INSERT ... ON CONFLICT (name, scope, scope_id) DO UPDATE ... RETURNING. It returns (row, was_update) -- the full saved row and whether an existing row was updated -- like Django's update_or_create; was_update is the supplied (fresh) memory_id differing from the returned id. save_structured_memory is a thin wrapper over it. description / mem_type of None mean "leave unset": the column default applies on insert and the stored value is kept on conflict; an explicit value (including "" / "general") overwrites -- so clearing a description or setting type back to "general" now persists, where the prior "if mem_type != 'general'" / "if description" semantics silently dropped it. The memory tool and the memories HTTP endpoint pass None for omitted fields and read effective type/scope from the returned row; the HTTP endpoint returns that row directly (one query, no follow-up SELECT). Removes the now-unused update_structured_memory primitive and its dead STRUCTURED_MEMORY_MUTABLE constant. Adds cross-backend storage tests and a session tool-path test (preserve-on-omit / overwrite-on-explicit), run on PostgreSQL via --storage-backend -- the save-over-existing path was previously SQLite-only. * docs(memory): clarify upsert was_update precondition Lead the upsert_structured_memory docstring with the behavioral contract (callers MUST supply a fresh unique memory_id) rather than the internal id-comparison mechanism, so a future caller can't reuse an existing id and silently get was_update=False on a real update. |
||
|
Patrick Buckley
|
de60127c45 |
fix(memory): don't recompose system prefix on memory write
Injected memories ride in the cached system block, so calling _init_system_messages() on every memory save/update rebuilt the prompt prefix and busted the provider prompt cache (a full system + history re-write) -- for a memory the model already holds via the tool result. memory(save) now only invalidates the per-turn search cache, so an in-turn memory(search)/(list) still reflects the write; the new memory folds into the prefix at the next natural recompose or the next session. Also drop the redundant _init_system_messages() in the /reason handler: reasoning effort rides in request kwargs (output_config / thinking), not the composed prompt, so it recomposed to byte-identical output. Add a chain-level test through the real _exec_memory -> no-recompose path (asserts prefix unchanged, search cache invalidated, next recompose folds the memory in). The prior memory tests either drove _init_system_messages directly or patched it out, so this path was uncovered. |
||
|
Patrick Buckley
|
8dd356b7e6 |
fix(task-agent): keep sub-tool steps nested + preserve denial reasons
Address the Copilot review on #732 plus a task-agent sub-tool nesting race surfaced alongside it. Nesting (web UI): - A sub-tool step whose task_agent row hasn't painted yet (the 4-wide tool pool's ordering window) buffers and nests when the row lands, instead of escaping to a top-level row that looks main-harness-issued. - A row that never paints (id-correlation mismatch / aborted agent) escapes its buffered steps back to a visible top-level paint after a grace window, so steps are never buffered invisibly or leaked. - The nested card survives the parent row's pending->resolved rebuild; a call_id reused across turns builds a fresh card rather than stealing the prior agent's steps. - tool_info routes through the same nesting path (no duplicate top-level row); a namespaced sub-tool result no longer grafts onto an unrelated top-level row. Denial reasons (backend): - Preserve the specific denial reason a gate already stamped (operator feedback, or the matched policy pattern; web and CLI contracts) instead of clobbering it with a flat "Denied by user" -- in both the sub-agent and the main tool loop. Verified with the livepass task_agent harness (race + orphan-escape scenarios, headless) and unit tests. |
||
|
Patrick Buckley
|
77cb76c006 |
feat(task-agent): recall sub-trajectory + per-agent read isolation
Final chunk of the task_agent modernization: rebuild a finished task
agent's card from /history (reload / reopen while the workstream is in
memory) and isolate each sub-agent's file-read tracking.
Recall: _project_agent_steps projects a sub-agent's trajectory into step
items (FIFO-per-call_id pairing via _iter_agent_tool_results, shared with
_cancel_ledger; output/arguments/count capped); _stash_agent_trajectory
keeps them on the UI in an LRU-bounded store; make_history_handler
attaches them as agent_steps to each task_agent tool_call, and
replayHistory/_replayAgentCard rebuild the collapsed card. In-memory only
(durable persistence deferred); a cold/evicted entry renders the flat
parent row ("not retained"), never a fabricated 0-step card.
Read isolation: _read_files (the blind-overwrite guard's memory) is now
per-sub-agent via the _active_read_files contextvar -- _exec_task copies
the parent's set on spawn and merges the agent's reads back on
completion, so a sibling in the 4-wide pool can't suppress another
agent's guard.
Also: _exec_task now self-reports the task_agent tool_result on every
path (the parent loop only reports error/denied results centrally) --
without it the live card never completed and a failed task recorded
is_error=False in the canonical trajectory. is_error flows from
_tool_error_flags to the recalled step; on_info suppression is per-thread
so a parallel sibling tool's progress isn't dropped.
|
||
|
Patrick Buckley
|
ca7958329a |
feat(task-agent): nest sub-tool steps in an expandable card
Route a task agent's sub-tool events (tool_pending / approve_request, tagged with parent_call_id) into a collapsible card under the task_agent row, replacing the blue on_info turn-legs. - conversation.js / interactive.js: buildAgentCardBody + _routeAgentItems / _ensureAgentCard nest steps by parent_call_id. Collapsed by default (a task agent can run 100+ steps and the parent fans out many in parallel); the label carries the live count + state. Auto-expand when a nested approval is pending so the blocking prompt can't hide behind the toggle. - session.py / session_ui_base.py: on_agent_step paints auto-tool step rows; namespace child call_ids by parent so the 4-wide task pool can't collide on local sequential ids (call_0); suppress sub-agent on_info on the web pane (no call_id to nest by — the card carries steps + result). - cli.py: on_agent_step prints a dim step leg (no card on the CLI, which keeps its on_info). - livepass.py: task-agent card harness driving the real InteractivePane. |
||
|
Patrick Buckley
|
65eaacb341 |
feat(task-agent): Turn-IR sub-harness + parent-tagged step events
Rebuild the task_agent sub-harness on the canonical Turn trajectory (build list[Turn], lower via dicts_from_turns at the wire boundary) instead of hand-rolled OpenAI dicts; the cancel-ledger helpers read Turns. Tag each sub-tool's events with parent_call_id via a lock-guarded child registry stamped centrally in SessionUIBase._enqueue, so a later UI can nest a task agent's steps under its card. Getattr-guarded on the session side so CLI/eval/test UIs are unaffected. Behaviour-preserving (same wire shape, same cancellation semantics); the parent tag is wire-invisible and unconsumed until the frontend card lands. |
||
|
Patrick Buckley
|
b1542ad62d |
fix(title): reliable titles on thinking models; defer utility temperature
Auto-title generation and manual refresh stopped producing titles on reasoning models (the cluster serves qwen3.6). The title call capped max_tokens at 200, so the model's think pass consumed the whole budget and content came back empty (finish_reason=length) -> the title was skipped. Both paths share _generate_title, so both broke. Title path: - Raise the title completion to 2048 tokens so reasoning finishes and the title text actually lands. - Recover the title from content (never reasoning): reuse the canonical _strip_reasoning (handles <think>/<reasoning>, paired or unclosed) plus a backstop for the opener-absent </think> shape some templates emit, take the first non-empty line, and peel a "Title:" label and wrapping markdown/quote decoration. Internal punctuation is preserved. Cap at 80 to match the manual-alias bound. Temperature: - _utility_completion no longer hard-codes a temperature; it defaults to the session/registry value the main turn uses. Title (was 0.7/0.3), web-fetch extraction (was 0.2), and compaction all defer. Hard-coding a constant fought thinking/no-temp models and silently overrode an explicit [models.*] temperature; the provider still gates temperature per model. Tests: title sanitization across think/reasoning variants, truncation, and a trailing-prose case; utility-completion temperature deferral + explicit override. |
||
|
Patrick Buckley
|
a318265946 |
fix(fence): bracket trust-fence markers instead of angle-bracket XML
Swap the trust-fence marker shape from <tag_nonce>...</tag_nonce> to [start tag_nonce]...[end tag_nonce] for both the operator fold (system-reminder) and the output-guard judge (tool_output). Angle-bracket markup pushed some local models out of distribution and toward emitting their own turn-structure tokens: chat templates built around rigid <...>-style structural tokens derail once a few folded reminders accumulate. The start/end keywords carry no slash (no </ or [/ closing-tag shape) and read as ordinary text. Single-source the shape in fence.py (_OPEN_KW/_CLOSE_KW + detection_pattern) so wrap, neutralize, the forgery/leak detector, and both trust declarations track one definition. The nonce still rides both boundaries (unforgeable close); the leak-vs-forgery split and the forge-in / break-out defang are preserved. The fold is wire-only, so there is no migration; the legacy persisted-envelope readers keep the old shape. Add regression tests pinning each trust declaration to fence.wrap's emission so a future keyword change fails loudly instead of silently desyncing the anchors. |
||
|
Patrick Buckley
|
2169559d6e |
feat(projects): governed project containers — memory scope, grouping, manage UI (#724)
* feat(projects): governed project containers — memory scope, grouping, manage UI
A workstream can attach to a project: a first-class, shareable resource
container that owns a `project` memory scope, groups conversations, and is
managed from the console.
Storage / migration 062: projects + project_members tables, workstreams.
project_id, and the memory type default project→general; grants
project.{create,read,write,delete} (admin-default).
Recall + writes: project memory is recalled iff the workstream is attached AND
the user has access (owner ∨ member ∨ public-for-read), resolved once at session
construction; coordinators recall it too. New saves default to the project when
attached + writable; the save and delete paths are write-gated; deleting a
project purges its scoped memory; archived projects aren't recalled.
Access = RBAC capability ∧ per-project ACL (auth.resolve_project_access, a
single-fetch resolver); visibility changes, member management, and delete are
owner-only.
API: project CRUD routes on both the server and console; project_id threaded
through workstream creation, spawn inheritance, the cluster-create proxy, the
dashboard / snapshot / coordinator row builders, and the collector deltas.
UI: a project picker with an inline "+ New project" creator in every creation
box (console launcher + standalone dialog + dashboard); group-by-project in the
rail; a project badge in the composer and on dashboard rows; a console manage
tab (list + create/edit + members shelves). The admin Memories view gains
coordinator/project scope filters and human scope labels (name, not hex). The
memory tool schema documents the project scope and the attach-aware default.
* fix(projects): client refresh hardening, creator race guard, SDK project_id
Addresses PR #724 review feedback plus two bugs found while validating it.
- projects.js refreshProjects: a non-OK status (e.g. 403 when the caller
lacks project.read) or a network/parse error no longer blanks the cache
or masquerades as "no projects" -- the prior cache is preserved, the
failure is recorded (new projectsError()) and warned. Honors the
long-standing "a transient error can't blank the rail" docstring.
- projects.js _fp: the fingerprint separators were raw control bytes,
which made git treat the whole file as binary (no reviewable diff).
Rewritten as escape sequences instead of raw bytes -- behavior is
byte-identical at runtime.
- project_creator.js: createProject() could reject unhandled (authFetch
throws on network/401; r.json() throws on a non-JSON body), leaving the
widget stuck busy/disabled. Added a .catch, plus a generation guard so a
create whose widget was cancelled/reopened mid-flight drops its result
instead of selecting a project the user backed out of.
- types.ts: add project_id to CreateWorkstreamRequest / WorkstreamInfo /
DashboardWorkstream to match the server schemas (was SDK-invisible).
- test_project_api.py: move side-effecting HTTP calls out of asserts so
the requests run even under python -O.
* fix(projects): JSON.stringify the cache fingerprint, drop control-byte separators
_fp joined fields/rows on raw NUL/SOH bytes, which made projects.js read as binary to git. Replace with a collision-proof, escape-free JSON.stringify encoding -- same change-detection semantics, zero embedded control characters.
|
||
|
Patrick Buckley
|
1860d14a65 |
fix(coordinator): persist + eagerly generate workstream titles
Coordinator workstream LLM titles were written to workstreams.title but never read back, and were rarely generated in the first place: - Read path: the dashboard's `_coordinator_rows` builder hardcoded title="" and used the synthetic `ws.name`, so a generated title (or a user alias) reverted to `ws-xxxx` on every refresh. Interactive rows resolve via get_workstream_display_name, so the gap was coord-only. - Write path: the auto-title trigger only fired on a tool-call-free assistant turn, which coordinators (near-constant tool use) seldom reach — so the title almost never generated. Read path: - Project `title` + `alias` in list_workstreams (appended after user_id so existing positional fallbacks stay valid). `_coordinator_rows` resolves the display name (alias > title > name) for both lanes — live names via the bulk get_workstream_display_names (exact ids, no row cap), persisted rows from their own _mapping. - Seed the console pseudo-node fan-out with the resolved display name so a rehydrated coordinator shows its title in the live tree immediately (one bulk lookup instead of an N+1 over mgr.list_all()). Write path: - Fire auto-title right after the user turn is recorded in send(), gated on a real (non-wake, non-empty) user message, instead of waiting for the terminal tool-call-free turn. Applies to interactive + coordinator. - Snapshot self.messages in _generate_title since it can now run concurrently with the streaming turn. |
||
|
Patrick Buckley
|
30b590fb25 |
feat(memory): durable per-user coordinator scope + anonymous-coordinator guard
The coordinator memory scope was keyed by the session's ws_id, so every new coordinator session started with an empty namespace and its rows were orphaned on close — coordinator memory never actually persisted. Re-key the scope to the coordinator's creator user_id: one durable orchestration namespace per user, shared by all of that user's coordinator sessions (concurrent ones included; upsert-by-name is the collision rule). The child-containment threat model is unchanged: the gate is session KIND — children are always interactive and share the parent's user_id, so _validate_scope rejects them before scope resolution, and the REST memories API still rejects the coordinator scope outright. The implicit visibility lane now also fails closed on an empty scope_id to match the explicit search/list lanes (the storage helpers treat a falsy scope_id as 'no scope_id filter', which would have read every user's rows). Anonymous coordinators are no longer constructible: ChatSession refuses kind=COORDINATOR with an empty user_id at the constructor — the single choke point covering create, rehydration of legacy rows (surfaced by the open handler as a 503 with remediation text), and any future host — and the console no longer masks an empty uid as a phantom 'system' principal when minting coordinator JWTs, per CoordinatorTokenManager's documented 'sub = the real creator user_id' contract. Migration 061 carries existing coordinator rows across: rows whose owning workstream is gone or ownerless are deleted (unreachable under user keying), same-name collisions within a user keep the newest updated row (memory_id tiebreak), and survivors re-key to the owner's user_id. |
||
|
Patrick Buckley
|
ee799f67de |
fix(memory): touch access metadata on composition and tool reads
The touch_structured_memories facade and both storage backends were implemented but had zero call sites, so access_count never moved and last_accessed never advanced past write time on any deployment. Wire two touch points: - proactive composition touches the injected top-k (post-rerank) set, deduped per turn since _init_system_messages recomposes many times within a single turn; - the memory tool's search and get reads touch their returned rows, counted per call. save/delete/list do not touch. Touches are best-effort through the facade, which already swallows storage errors, so a failed touch never breaks composition or a tool call. |
||
|
Patrick Buckley
|
8f415f9c68 |
docs(judge): say llm_fallback, not 'heuristic fallback', for cancelled items
Review feedback: the cancel-path docstrings described undone items as degrading to 'heuristic fallback verdicts', but the emitted and persisted tier is llm_fallback (heuristic content relabeled). Aligned all eight occurrences — including the pre-existing _deliver_fallbacks docstring — so docs, logs, and audit rows use one vocabulary. |
||
|
Patrick Buckley
|
fb282fab73 |
fix(judge): honor the cancel_on_approval=False run-to-completion contract
judge.cancel_on_approval=False (the default) promises the daemon evaluates every tool call to completion so all verdicts are available for later review. Two sites conspired to break that: the approval gate's finally set the cancel event unconditionally the moment a decision landed, and _evaluate_single's poll loop honors the event regardless of config — so every item the sequential judge hadn't reached degraded to a heuristic llm_fallback row. On a 22-call parallel batch, approving after the third verdict silently downgraded the other 19; the elaborate late-verdict machinery in on_intent_verdict was effectively dead code. Make the event a pure abort signal whose firing policy lives with the caller: the gate fires it only when cancel_on_approval is enabled, while generation supersede (next batch) and close() keep firing it unconditionally, bounding a stale daemon to one batch of real work. _run_judge drops its own config second-guessing — a fired event always fast-forwards the remainder to fallbacks (every call still gets exactly one verdict), and the fallback reason no longer claims 'user approval' for supersede/close aborts. |
||
|
Patrick Buckley
|
effdb8f365 |
fix(judge): persist superseded late verdicts for the audit trail
ChatSession._on_verdict guards on judge-generation identity so a stale verdict can't ride a reused call_id into the Smart-Approvals cache — but it dropped those verdicts entirely, before persistence. Every ruling the sequential judge delivered after the next turn began left intent_verdicts claiming the judge never answered. Route superseded verdicts to a new persist-only hook (SessionUIBase.on_superseded_intent_verdict): the row lands with user_decision="superseded" while every live surface stays untouched (no SSE, no replay cache, no pending-decision park). The hook is duck-typed; display-only UIs (CLI/eval) don't define it and keep the plain drop. upsert_intent_verdict already excludes user_decision from its on-conflict SET, so a superseded fallback upgrading its heuristic row in place cannot clobber a decision already stamped there. |
||
|
Patrick Buckley
|
4927efe942 |
refactor: pre-push review — content-addressed upload buffer, GC dedup, security hardening
- Buffer (attachment_buffer.py): content-address staged bytes once and track the
per-(ws_id,user_id) references to them, so identical bytes staged from two tabs
dedupe to one copy yet neither scope's send can drop the other's pending upload
(the prior hash-only key let one overwrite the other). Single lock; add a public
clear() that replaces test reaches into the private store.
- GC: lift the byte-identical _release_attachment_refs out of both backends into one
dialect-agnostic storage/_utils.release_attachment_refs with a portable searched-
CASE single-query decrement (was one UPDATE per id in a Python loop).
- _format_messages_for_summary: mark by-reference vision results
({type:image, attachment_id}) as [image], not just inline image_url.
- Security: escape_like() the attachment_referenced_in_ws LIKE needle on both
backends; secrets.compare_digest for the output-guard operator-fence leak check.
|
||
|
Patrick Buckley
|
164f74dead |
feat(operator-context): deliver structured per-kind meta to the UI
Operator-context system turns (watch results, output-guard findings, idle children, user interjections) carried their kind (_source) and a flattened text content, but the structured per-kind fields were dropped at every persist/deliver boundary — so the UI rendered every kind as one generic operator bubble and the structured watch-result card was lost. Wire the structured meta through as the single source of truth: - Storage: new conversations.meta JSON column (migration 060); threaded through save_message/save_messages_bulk (facade + protocol + both backends) and rehydrated in reconstruct_turns onto Turn.meta.extra["source_meta"]. - Canonical: make_system_turn carries meta as one _source_meta dict; turn_from_dict/turn_to_dict bridge it to/from Turn.meta.extra. - Live + history: widen on_system_turn(content, source, meta) across all impls + the SSE payload; surface _source_meta -> meta in the /history projection. SDK HistoryEvent docs note the field. - Producers derive both the model-facing content text AND the card from one meta dict, so they cannot drift: render_output_guard_text, build_watch_ reminder carrying output, idle_children and user_interjection metadata. - Frontend: addSystemContext / renderSystemTurn dispatch by source to the watch-result, guard-finding, idle-children, and queued-message cards in both the interactive and coordinator panes; every untrusted field renders via textContent. The meta is a leading-underscore key, stripped before the wire (sanitize_ messages and the native mid-conversation path copy only role+content), so the per-provider wire payloads stay byte-identical. Additive column, no backfill: operator turns predating it reload as plain text bubbles. |
||
|
Patrick Buckley
|
964a390e5e |
feat(attachments): resolve AttachmentRef at the translator; drop RawContentBlock
The by-reference content lane now materializes at the provider translator (the
C layer), not in the session. Each create_streaming / create_completion takes
a resolve_attachments callback and runs materialize_attachments() up front,
expanding {type:kind, attachment_id} placeholders to inline data-URI / document
parts by a content-addressed point-lookup the session hands down
(_resolve_attachments). _full_messages emits placeholders; the dict bridge
carries only placeholders.
RawContentBlock is removed — ContentBlock = TextBlock | AttachmentRef. A
resolved inline part is terminal (the wire payload / display output) and never
re-enters the canonical path, so turn_from_dict drops a stray inline image_url
rather than carrying bytes. resolve_attachment_parts / materialize_attachments
operate on the dict projection. Tool vision output rides by reference too
(_tool_content_by_reference): the turn carries placeholders, the bytes persist
content-addressed. The per-turn token estimate counts a by-ref image as one
fixed image budget; the document char budget lands at send (on resolution).
Wire harness byte-identical (the multipart fixture is a placeholder + a matching
resolver); full non-live suite green (7136).
|
||
|
Patrick Buckley
|
dc88060b79 |
refactor(core): session.messages is the canonical Turn trajectory
ChatSession.messages flips from list[dict] to list[Turn] — the in-memory canonical trajectory. Reads migrate to typed fields (turn.role, turn.text, turn.tool_calls); appends and assignments go through turn_from_dict / turns_from_dicts; the fork bulk-save and retry's multipart check read via turn_to_dict. _full_messages lowers Turns→dicts at the wire boundary — the fold/repair and provider translators still consume dicts until the next slice. The token-accounting helpers accept a dict or a Turn. Non-session consumers migrate too: coordinator_idle_observer and eval to typed fields (mypy-enumerated), and server's last-assistant extractor via turn_to_dict (an Any-typed call site mypy could not flag). An all-text multipart content list (the unreadable-attachment placeholder path) now round-trips faithfully through the adapter (single text block → str, multiple → list). Tests that inspected session.messages as dicts read it through the dicts_from_turns / turn_to_dict bridge; those that built it pass dicts through turns_from_dicts / turn_from_dict. Byte-identical wire harness; full non-live suite green (7130). |
||
|
Patrick Buckley
|
f3c96e6493 |
feat(skills): make skill hints first-class system turns; drop escape_wrapper_tags
_skill_hint spliced its guidance into the tool result as a bare <system-reminder>
block — but the operator declaration now tells the model to treat bare markers
as untrusted, silently demoting the hint. Make the hint first-class instead:
- _skill_hint returns the tool result verbatim and queues the guidance via
_queue_tool_advisory("skill_hint", ...); _collect_advisories drains it into a
{role:system, _source:"skill_hint"} turn after the clean result — folded in
the trusted nonce fence for non-native models, inline for native. (Queuing
no-ops mid-wake, like the other tool-channel advisories.)
- skill_hint added to SYSTEM_TURN_SOURCES (an advisory-producer source).
- escape_wrapper_tags removed outright: it was the last consumer, and its job
(defang a marker next to the bare block) is now covered at fold time by
_neutralize_host. The result message rides through verbatim. This also
collapses the two-escaping-mechanism confusion the review flagged.
Tests assert the clean result + the queued/drained hint, plus wake suppression.
|
||
|
Patrick Buckley
|
8513016503 |
refactor(storage): drop the dead _reminders column instead of carrying it
Operator context moved to first-class system turns, leaving _reminders written by nothing and read by nothing. Nulling it (the prior 060 step) left a writable dead column — a foot-gun inviting accidental reuse. Drop it outright and remove every reference in one shot so there is no half-alive state: - migration 060: replace the wholesale null with batch_alter_table drop_column (per migration 027); downgrade re-adds the empty column to match the 059 schema (the envelope un-wrap stays irreversible). - _schema.py: remove the column. - _sqlite / _postgresql: drop the reminders save param, the INSERT/bulk values, and both SELECT columns. - reconstruct_messages: the row tuple is now 8/9-tuple (event_id shifts from index 9 to 8); _utils + the _row test helper updated. - _protocol / memory save_message: drop the reminders param + docstrings. - tests: replace the reminders-roundtrip tests with a _source-only file and a 060 drop-column assertion; remove the obsolete legacy-reminders wire test. No production caller passed reminders=, and the SELECT no longer reads the column, so an un-migrated DB simply ignores any residual values. |
||
|
Patrick Buckley
|
99ba82e8ec |
fix(session): operator-turn wire correctness — framing, empty turns, leading system
Phase-2 follow-ups to the mid-conversation-system consolidation: - user_interjection framing (known #2): a queued message that drains mid-turn is re-framed via render_user_interjection ("The user sent … User message: …") so the user's words keep USER authority, not operator authority — the regression mattered most on the native path, where the turn enters as a real role=system message. Empty/whitespace interjections (e.g. a bare "!!!") are dropped (bug-2). - empty-content user turns dropped at the wire boundary after the fold (known #3): the wake pipeline's synthetic empty send("") leaves an empty user turn on the native path (the nudge stays inline); an empty user message is invalid on every provider. The drop runs after the fold so the fold-path wake turn, which the nudge fills, survives. - leading-system guard (_anthropic): a turn that converts to nothing no longer lets a system message become messages[0] (the API requires messages[0]=user). Newly reachable now that the empty-turn drop can expose it on a fresh-session native wake. - refresh stale .msg.watch-result comments (the card was removed) to describe the current operator-bubble rendering. |
||
|
Patrick Buckley
|
c6b2288302 |
feat(session): consolidate operator-context into first-class system turns
Replace the two operator-context hacks (the <tool_output>/<system-reminder> content envelope and the transient _reminders side-channel) with one persistent {role: system, _source} trajectory turn. Adds supports_mid_conversation_system (claude-opus-4-8): native models take the turn inline; all others fold it into the preceding turn as a nonce-delimited <system-reminder> block declared in the system prompt as the sole trusted marker. Producers (advisories, metacog nudges, user interjections, idle/watch) emit system turns; the envelope/_reminders machinery, escaping round-trip, replay parser, and reminder SSE events are removed. Eager 060 migration un-wraps legacy envelopes. Net -1662 lines.
Known follow-ups from review (unfixed here): (1) the 060 un-wrap heuristic can irreversibly mis-rewrite bare tool rows that resemble the envelope, so do not run the migration until it is tightened; (2) user_interjection turns lost the user-framing/priority preamble (a regression, and a native-path authority-framing concern); (3) native-path wake nudge can emit empty user content.
|
||
|
Patrick Buckley
|
24d75a690b |
fix(review): address review findings on the rerank/memory stack
- rerank_config.py: the runtime instruction fallback had a dead tail (`get_rerank_instruction() or str(cs.get(...))` -- the cs.get term can only return the registry default ""), via a stored_keys() branch that also diverged from the calibrate CLI / endpoint. Collapse to the sibling idiom (`cs.get(...) or get_rerank_instruction()`) so the instruction used at calibration time matches the one used at runtime. Correct the module docstring: ChatSession is the sole caller (the CLI/endpoint share only the instruction precedence, not this function). - session.py: the deferred first-turn memory recompose was gated on the flag alone, so a synthetic wake send (empty user content -> flag stays False) re-ran the full compose on every wake before the first real turn. Gate on a non-empty query too, so wakes don't re-pay it and the real turn still fires exactly once (+ test). Accepted as-is: the __init__ compose (kept so system_messages/_agent_system_messages are valid for early readers; one cheap extra compose per fresh session) and the orphaned tools.rerank_* config rows (inert -- no read path, never listed or redacted; a purge migration would collide with the 060 in flight on another branch). |
||
|
Patrick Buckley
|
47d0b6c3c6 |
fix(memory): defer system-message compose to the first user turn so memory selection has a query
Proactive memory selection scores candidates against the recent-user-message query (extract_recent_context), but a fresh session composes the system prefix once in __init__ while self.messages is still empty. That empty query takes the no-context path: _select_memory_candidates returns recency order and score_memories returns memories[:k] verbatim -- the 5 most recently UPDATED memories, with BM25 and the reranker never invoked. send() never recomposes, so those recency-only memories are what the model sees for the whole session (until an unrelated event -- skill / MCP / model refresh / resume / memory write / command -- happens to rebuild the prefix). Net effect: the injected memories are unrelated to the actual question. Fix: defer the memory-bearing compose to the first real user turn. - Track _system_composed_with_context, set once extract_recent_context is non-empty in _init_system_messages. - send() recomposes once, right after _append_user_turn, while the flag is still False -- so the opening turn's memory block is selected (and reranked) against the real message. The flag then stays True, so the prefix is composed once and stays cache-stable exactly as before (no per-turn prompt-cache churn). This is the targeted fix; per-turn memory refresh (so later topic shifts also re-rank) is the larger tail-injection redesign tracked on another branch. Two adjacent gaps are left as-is for now: the reranker/BM25 only see content[:200], and build_memory_context flat-truncates each memory to 500 chars (max_content is the save cap, not an injection budget). Tests: flag is False on a fresh session and after a whitespace-only wake turn, flips True on a real query; send() runs the deferred recompose with the user message in the query. |
||
|
Patrick Buckley
|
110d44b07e |
refactor(tools): remove man, math, and plan_agent built-in tools
`man` and `math` duplicated capabilities already reachable through `bash`; `plan_agent` is better expressed as a `task_agent` running a planning skill, and carried a large amount of special-case machinery (plan-review gate, refinement loop, per-kind model routing). Removing all three shrinks the tool surface and cuts per-call token cost. Also removed, as dead-once-the-tools-are-gone: - the `math` sandbox executor (`turnstone.core.sandbox`) and its `[sandbox]` extra; the eval analyst now runs bash-only - the read-only `AGENT_TOOLS` sub-agent tool set and the `agent` tool-metadata key (`task_agent`/`TASK_AGENT_TOOLS` retained) - the plan-review protocol end to end: the `on_plan_review` UI hook, `resolve_plan`, `POST /v1/api/plan` + `POST /v1/api/route/plan`, the `plan_review`/`plan_resolved` SSE events, and their Python SDK / TypeScript SDK / OpenAPI / frontend / Discord+Slack bindings - the `model.plan_alias` / `model.plan_effort` settings and the registry `plan_model` / `plan_effort` routing fields TOOLS 31->28, TASK_AGENT_TOOLS 13->11; COORDINATOR_TOOLS unchanged. BREAKING CHANGE: removes the `man`, `math`, `plan_agent` tools, the plan-review SSE/HTTP/SDK surface, and the plan_* model-routing settings from the experimental 1.6 line. |
||
|
Patrick Buckley
|
948e413f66 |
feat(judge): add Smart Approvals (auto-approve trusted judge verdicts)
Opt-in judge.smart_approvals (default off): when the intent-validation LLM judge returns a high-confidence "approve" verdict, the tool batch is approved automatically with no operator prompt. review/deny recommendations, low confidence, judge errors (llm_fallback), and a deterministic heuristic deny/critical finding all still require a human. Requires judge.enabled. - Batch-atomic: a parallel tool batch auto-approves only if every call qualifies; one non-qualifying call holds the whole batch for a human. - Gate: tier==llm + recommendation==approve + confidence >= judge.confidence_threshold (default raised 0.7 -> 0.95), with a floor that never clears an explicit heuristic deny/critical verdict. - approve_tools waits for the async LLM verdicts, finalises the audit trail (AutoApproveReason.smart_approval), and re-emits verdicts after the card so the live chip updates; the auto-approved row renders the LLM verdict rather than the cautious heuristic carry-over. - judge: always deliver exactly one verdict per call (fallback on error); reject non-finite confidence so NaN can't clear the bar. - Drop verdicts from a superseded judge generation so a reused call_id from a prior turn's still-running daemon can't satisfy the gate's wait. Config plumbed through the server/console/CLI builders and the live _judge_cfg; admin Judge tab renders the toggle. Docs + example config updated. ~35 tests covering the gate matrix, batch-atomicity, the heuristic floor, audit stamping, the streaming re-emit, NaN/duplicate-id defenses, and the cross-turn generation guard. |
||
|
Patrick Buckley
|
bdc1f35f94 |
fix(usage): correct dashboard totals + record auxiliary LLM token spend
The Usage dashboard summary cards read the oldest day bucket (`summary.breakdown[0]`) instead of the window SUM, so every headline (total/prompt/completion/tool-calls/cache) showed a single day's value — e.g. 30-day tool-calls reading lower than 7-day. Read `.summary[0]` and collapse the redundant two-request fetch into one (the response already carried both `summary` and `breakdown`). Only the main streaming loop (`on_status`) recorded `usage_events`. Auxiliary non-streaming calls — title generation, conversation compaction, web-fetch summarization, and plan/task sub-agents — bypassed that path and were never counted, undercounting real consumption by a large factor for agent-heavy workstreams. Add an `on_aux_usage` UI hook (storage row via a shared `_write_usage_row` helper with `on_status`; `WebUI` override feeds Prometheus) and route `_utility_completion` and sub-agent turns through it, attributed to the agent's own model. Judge token spend remains uncounted — deferred to a follow-up. |
||
|
Patrick Buckley
|
30d670338e |
feat(judge): merge output-guard heuristic + LLM judge, annotate findings
Surface the output-guard LLM judge on the inline finding chip and merge it with the regex heuristic instead of one stage winning outright. Merge rule (issue #560, "show, annotated"): - risk_level = max(heuristic, llm); flags = union. The judge can escalate but never lower a heuristic positive — it evaluates adversarial tool output, so defeating it must not erase a deterministic regex finding. Credentials stay heuristic-only and are always redacted. - The judge's own verdict rides along as a dissent-aware annotation (judge_risk / confidence / reasoning / judge_model) on the chip in both the interactive and coordinator UIs, live and on reconnect. One shared merge_guard_display_payload drives both paths so they cannot drift. - The model is shown the merged risk + flags but never the judge's "benign" verdict — a fooled judge must not talk the model out of caution. Fixes a reconnect bug: a judge that ran but failed wrote a risk="none" row that won the replay dedup and hid the heuristic finding (it showed live but vanished on refresh). Failed judges now persist under tier="llm_error", excluded from the display merge; the max-merge also floors the displayed risk at the heuristic level so the chip never vanishes. Also adds a regression test confirming the LLM judge runs on every tool output, not just heuristic-flagged ones. Tests: merge unit tests, storage-backed replay regression, live/replay wire-shape parity, SDK-event drift guard. ruff + mypy clean. |
||
|
Patrick Buckley
|
ee8dc7c1c3 |
refactor(history): project the /history wire shape server-side
Collapse the three hand-synced "raw storage -> render shape" projections into one server-side projection. The projection previously lived in a test-only `_build_history` (SSE-era reference impl), a client-side JS normaliser (`history_normalize.js`, the transitional bridge), and coord's inline `init()` handling -- drifting silently with no parity test. Add `project_history_messages` to `history_decoration.py` and run it as the final step of the `make_history_handler` pipeline (load_messages -> decorate -> extract_reasoning -> project), so `GET /history` emits the canonical render shape directly: flat tool_calls (with verdict / output_assessment), top-level source / reminders / attachments, collapsed multipart content, derived denied / is_error / pending, reasoning, and advisories. Interactive `replayHistory` now consumes the payload verbatim. Close two gaps the JS bridge deferred: - list-content <tool_output> advisory extraction (decorate handles only string content; the projection extracts list-carrier advisories, then joins remaining text parts to the string the renderers require); - orphan->pending marks ONLY the last orphan tool-call turn, so a mid-conversation cancelled tool still renders instead of vanishing. Delete `history_normalize.js` (+ its <script> tag and node test) and the test-only `_build_history` (+ orphaned imports); retarget its direct tests onto the projection helpers. Update the WorkstreamHistoryResponse description and the Web UI Resilience architecture note to the projected shape. Coord's `init()` still reads the raw side-channels; migrating it to the projected shape is the next commit, browser-verified separately. Refs #549. |
||
|
Patrick Buckley
|
3233719856 |
feat(judge): output_guard LLM stage with capability gate (#560 mitigation #1)
Adds a second, LLM-driven stage to the output guard so domain-camouflaged prompt-injection payloads that the regex stage misses (arXiv:2605.22001 — Llama 3.1 8B evades the existing regex set on ~90% of camouflaged prompts) get caught before the tool output lands in the assistant's context. ## Surface * New `OutputGuardJudge` in `turnstone/core/output_guard_judge.py` — synchronous, single-shot LLM call. Inlines the alias-resolution + client-config + JSON-parsing helpers (copied verbatim from `IntentJudge` at `judge.py:917-969` / `1604-1659`) rather than going through a shared module — when `IntentJudge` lifts its own helpers, both copies move together. * JSON-in-content verdict with a 3-strategy parser (direct / markdown fence / balanced braces). `IntentJudge` ships a 4th regex-field fallback; OutputGuardJudge deliberately doesn't, because strategy-4 hits on broken LLM output can extract a "verdict" from the model's reasoning quote that lands in storage looking identical to a clean strategy-1 result. Failure of all three returns `error="unparseable_verdict"` and the heuristic stage stands. * `OutputJudgeVerdict` is a frozen dataclass with: `risk_level` (none/low/medium/high — normalises `critical`→`high` and `info[rmational]`→`low` for IntentJudge-echo safety), `flags: tuple[str, ...]`, `reasoning`, `confidence: float` (0.0-1.0, parsed + clamped from the LLM's self-report; pass-through to audit, no threshold gating), `judge_model`, `latency_ms`, `error`. * Real wall-clock timeout via `ThreadPoolExecutor.shutdown(wait=False, cancel_futures=True)` on the timeout/cancel path — `with ... as ex:` would block return until the worker drained. 1s `cancel_event` poll mirrors `IntentJudge._run_judge` at `judge.py:1117-1118`. * HTTP client lazy-init + reuse for the judge instance's lifetime. Session-side model swap drops the entire judge, dropping the client with it. * Untrusted tool output wrapped in per-call random-nonced `<tool_output_NONCE>...</tool_output_NONCE>` fence. Closing-tag substrings in the raw text are case-insensitively backslash-escaped first (`</tool_output` → `<\/tool_output`) so an attacker can't break out even if they guess the nonce. System prompt classifies the fenced region as UNTRUSTED DATA so directives inside are evaluated as content, not obeyed. * Judge user prompt carries the heuristic verdict (risk + flags + annotations), the tool description (looked up from the session's tools registry), and the tool args (truncated to 500 chars, also classified UNTRUSTED in the system prompt since they may be caller-supplied). Lets the judge defer to the regex on credential leaks and focus on injection signals the regex set misses; also enables output-vs-request plausibility reasoning. ## Session integration * `_evaluate_output(call_id, output, func_name, *, tool_args="")` — heuristic always runs; LLM stage runs when `judge.output_guard_llm` is enabled. When the LLM produces a usable verdict and the heuristic didn't detect credentials, the LLM verdict is acted on; otherwise the heuristic stands. * Credential redaction is a regex-only signal. When `heuristic. sanitized` is non-None, the heuristic owns the acted assessment regardless of what the LLM said — an LLM asked about prompt- injection can correctly label a credential-bearing output as "none" risk for injection, but the secret still needs redaction. * `_batch_evaluate_outputs` runs the per-tool guard concurrently (4-worker pool) when LLM is enabled and there are ≥2 string outputs — collapses N×LLM-latency to ⌈N/4⌉×latency on the common 5-20 tool-calls-per-turn turn. * Per-session `TokenBucket(rate=1.0, burst=60)` caps adversarial LLM-fan-out cost at 60 calls/min/session. * Pre-truncation: the per-tool loop truncates output before the judge sees it, so the judge evaluates exactly what enters the assistant's context (no wasted tokens on text that won't land). * Both heuristic and LLM tier rows persisted to `output_assessments` when the LLM ran (audit completeness); heuristic-only rows skip when matched-clean to keep the table focused. ## Storage Migration 057 extends `output_assessments` with five LLM-tier columns: `tier` (`heuristic` / `llm`, backfilled to `heuristic`), `reasoning`, `judge_model`, `latency_ms`, `confidence`. Tie-break on `(created DESC, tier='llm' first)` so downstream consumers see the acted verdict first when the two rows tie at second resolution. `StorageBackend.record_output_assessment` + sqlite/pg implementations + `SessionUIBase.record_output_assessment` + `SessionUI` protocol + the test stub overrides (cli, eval, 9 test files) all take the new LLM-tier kwargs. ## Config surface Three new judge.* settings in `settings_registry`: * `judge.output_guard_llm` (bool, default False) — capability gate. Default off; operators opt in once a small/fast model is pointed at `output_guard_model`. * `judge.output_guard_model` (str, default "") — alias for the LLM stage. Empty inherits the session model (same fallback shape as `judge.model`). * `judge.output_guard_llm_timeout` (float, default 30.0, min 1.0) — wall-clock budget per call. Both `server.py` and `console/session_factory.py` wire these into the `JudgeConfig` they hand to `ChatSession`. ## Notes * No backwards-compatibility shims — the LLM stage is purely additive. * No reasoning/threshold gating on confidence; it rides as an audit-only signal per maintainer direction. Surface it in the `on_output_warning` dict so live UI / cluster broadcast can sort flagged outputs by judge certainty. * Tests: 392 lines of judge-only coverage (`test_output_guard_judge. py`) + 629 lines of session-integration coverage in `test_session. py`, plus the storage and stub-shape updates. |
||
|
Patrick Buckley
|
bf14fc8b45 |
fix(output_guard): harden against domain-camouflaged injection (#560) (#573)
* fix(output_guard): harden against domain-camouflaged injection (#560) Three layered mitigations against the camouflage attack class described in arXiv:2605.22001 (Pai, May 2026), which demonstrates 90.3% evasion on Llama 3.1 8B and 44.4% on Gemini 2.0 Flash against pattern-based detectors: - Sub-agent synthesis is now scanned by output_guard at the sub-agent boundary in _run_agent, in addition to the existing scan at the parent's tool-result loop. Covers all four return paths (clean exit, truncation, context-limit recovery, turn-limit forced synthesis), closing the cross-workstream summary laundering surface. - Adds pair-of-signals camouflage detection: imperative recommendation phrase combined with either an authority frame ("consistent with our risk framework") or a caps action verb (SELL/BUY/TRANSFER/...). New flag camouflaged_injection at medium risk; deliberately partial — the paper's augmented-detector approach recovers only ~10% on Llama-class models, so this is duct-tape pending a semantic-evaluator follow-up. - Bumps output_guard's wall-clock budget default from 5s to 30s and exposes it as judge.output_guard_budget_seconds in ConfigStore, so the expanded regex set has headroom on large tool outputs. * Fix test_budget_kwarg_is_honored to exercise deadline logic path The test previously passed an empty string which short-circuited evaluate_output() before budget_seconds was used. Now uses a non-empty input and monkeypatches time.monotonic() to deterministically verify the deadline path is exercised. |
||
|
Patrick Buckley
|
471d48abd9 |
feat(skills): unify skill + list_skills into dual-kind action-multiplexed tool
Replaces the legacy `skill` (load + search) and `list_skills` tools with a single `skills(action=...)` tool serving both interactive and coordinator sessions. Stacks on the model.skills.write permission introduced in PR 1. Tool surface - `find`: filter by category/tag/risk_level/enabled_only/limit with optional BM25 query ranking; auto-approved on both kinds; kind-scoped at the storage filter (interactive sees interactive+any, coord sees coordinator+any). - `get`: fetch a single skill including content; cross-kind misses collapse to "not found" so a model can't enumerate the other surface by name-probing. - `load`: activate a skill in the current session (interactive-only; coord sessions get an explicit hint pointing at spawn_workstream). - `create`/`update`/`enable`/`disable`: require approval AND model.skills.write; permission re-checked at exec time to catch a revocation between approval and write. - No `delete` — hard-delete stays admin-UI exclusive; tool description documents the soft-delete-via-disable pattern. Defenses on the write surface - Approval cards surface projected risk_level (scanner re-run against the proposed final state) and warn explicitly when allowed_tools + auto_approve combine (auto-fire-on-load consequence is spelled out, not just shown as raw field values). - Toggle preview surfaces existing risk_level + allowed_tools count so re-enabling a critical-tier skill is never a one-click bypass. - Update path now re-fetches the row at exec to catch a readonly flip between approval and write, filters updates back to the runtime-only set if so, refuses if no fields survive. - Update path rejects empty content (hollow-out via emptying bypassed the soft-delete-via-disable invariant), non-list tags, and empty category — failures are loud rather than silent. - Permission denials audit `skill.write_denied` with actor_source=model so probing the permission state leaves a trail. Audit failures log at error (not warning) — a successful write without a row is the exact gap the trail exists to surface. - `_skill_hint` routes both message and system_reminder through escape_wrapper_tags so caller-controlled values can't close the <system-reminder> envelope and let the model fabricate directives in its own future context. Shared validation - `parse_skill_session_config` lifted from console/server.py to turnstone/core/skill_field_validation.py; both the HTTP admin path and the model-tool path consume it. Single source of truth so field rules can't drift between layers. - `SKILL_RUNTIME_CONFIG_FIELDS` lifted similarly (was duplicated as _SKILL_RUNTIME_CONFIG_FIELDS in server.py and _SKILLS_READONLY_FIELDS on ChatSession). - `notify_on_complete` validator now accepts list input from the JSON schema's `array` type — previously rejected because str() of a list yields Python repr that json.loads then refuses. Performance - Update prepare skips the projected-risk scan when neither content nor allowed_tools is changing (storage re-scans on write authoritatively). Metadata-only updates no longer pay the ~25 regex-pass scan cost. Cleanup - CoordinatorClient.list_skills deleted (-91 lines); model-tool path talks to storage directly via list_skills_filtered. - Roles admin UI gains a Model section exposing model.skills.write. - tests/test_load_skill.py renamed to tests/test_skills_tool.py and rewritten for the new tool — 48 tests covering registration, prepare dispatch, permission gating (including TOCTOU-revoked exec deny), audit actor_source on create + disable + permission-denied probe, BM25 ranking, invalid-kind branches, audit-failure swallow, and <system-reminder> envelope injection resistance. |
||
|
Patrick Buckley
|
b4299f8888 |
fix(task_agent): address Copilot feedback on skill parameter
- Put ``skill`` back in the access-denial list in the tool
description with a clarification — TASK_AGENT_TOOLS does not
include the skill tool, so sub-agents cannot switch personas
mid-task. Removing the disclaimer entirely created an ambiguity
the LLM could misread.
- Minimize the skill_data carried on the approval item dict to
``name`` / ``content`` / ``risk_level`` only. ``get_skill_by_name``
returns the full ~30-column prompt_templates row including
``scan_report``, ``installed_by``, ``source_url`` — none of those
flow through ``_exec_task`` / ``_evaluate_intent``, and they
shouldn't ride along any future audit serializer that reads the
approval item shape.
- Regression test for ``skill=""``, whitespace-only, and ``\t\n``
values — pins the documented "empty value is acceptable" contract
at the ``(args.get("skill") or "").strip()`` chokepoint.
|
||
|
Patrick Buckley
|
7d58df0d22 |
feat(task_agent): add optional skill parameter for per-call personas
The task_agent tool now accepts an optional ``skill=<name>`` argument that loads the named skill's content as the sub-agent's persona, substituting the hardcoded "# Task Agent" identity statement. The operating-guidance numbered list (one-shot, tool-use over narration, no follow-up questions) is layered on top of every persona and always applies — those are sub-agent semantics that a persona should ride on top of, not replace. Validation lives in ``_prepare_task`` so the approval surface tells the operator what they're consenting to: the validated skill dict (including content) rides on the item dict from prepare to exec to defeat TOCTOU between consent and execution. An unknown skill returns a clean error item with a hint pointing at ``skill(action='search')``; a disabled skill returns a distinct error so the LLM's recovery path can tell "not found" from "quarantined", mirroring the enabled gate that ``_exec_skill(action='load')`` and skill-search already apply. High and critical skills now surface their risk tier on the approval header (``, risk: critical``) and emit a ``task_agent.high_risk_skill`` warning — same signal ``_load_skills`` emits for session-level skills, so the operator sees the same flag whether the skill is loaded session-wide or per-call. ``_exec_task`` emits a ``task_agent.skill_invoked`` info log on the skill branch for forensic traceability — the approval row captures the choice at consent time, this log captures it at exec time so post-incident search doesn't have to cross-walk approval and exec tables. The ``_evaluate_intent`` func_args projection now includes the skill name — without it, heuristic ``arg_pattern`` rules targeting a risky persona name on ``task_agent`` silently no-op and the audit row loses the choice. Mirrors the long-standing ``spawn_workstream`` projection. |
||
|
Patrick Buckley
|
ee163c0ae4 |
fix(session): prevent LLM bypass of per-role plan/task model overrides
The LLM was passing ``task_agent(model="default")`` (and the same for plan_agent) and routing to whichever backend the auto-created ``default`` alias was attached to at boot — flatspark in the verified case (ws_id 7dde674) — silently bypassing the operator-configured ``model.task_alias`` / ``model.plan_alias`` (gh200). Root fix: - ``load_model_registry`` only synthesises the back-compat ``default`` alias when neither DB nor ``[models.*]`` populate the registry. The shim was only ever meant for single-CLI-model setups; with a multi- model DB it became a phantom routing target aliasing ``LLM_BASE_URL``. - ``_render_agent_tool_descriptions`` filters ``default`` out of the LLM-visible alias list. The English reading of "default" trips the model into picking it explicitly even when the description tells it to omit ``model=`` for the per-role default. Defense-in-depth at the validator chokepoint (``_validate_agent_model_override``): explicit rejection of ``alias == "default"`` (post-strip) with corrective guidance; ``default`` filtered out of the unknown-alias retry list so an LLM probing with a bogus alias can't enumerate it back; the no-alternatives wording is distinguished from the no-registry-configured wording. The render path also always rewrites tool descriptions instead of returning early on filter-empty, so a reload that drops the registry to only ``default`` clears stale alias names left over from a prior render. |
||
|
Patrick Buckley
|
6bdc6cf0bd |
feat(audit): emit memory tool save/update/delete events
Previously only the admin-console DELETE route emitted memory.delete audit rows, so a long-running session whose memory was deleted via the admin UI had no log trail showing what happened — masking out-of-band deletes as apparent tool bugs. The save branch now stamps memory.save (new row) or memory.update (upsert); the delete branch does a lookup-then-delete-by-id pair so the audit can record the resolved memory_id and type. All emissions are best-effort: failures log at debug and swallow so an audit hiccup never breaks the tool call itself. Reads (get/search/list) remain un-audited. |
||
|
Patrick Buckley
|
29b850919f |
feat(sse): refresh-resume for mid-stream page reloads
Refreshing a coordinator or interactive workstream pane while the LLM is mid-stream now restores the partial assistant text + reasoning immediately and flips the composer back to stop-mode, instead of showing nothing until the response completes. Per-turn inflight buffers (`_ws_inflight_content`, `_ws_inflight_reasoning`, `_ws_inflight_seq`) on `SessionUIBase` are kept separate from the existing multi-turn `_ws_turn_content` buffer that drives the dashboard's IDLE-piggyback payload. New `on_turn_start` (top of send-loop, defensive) and `on_turn_committed` (right after `messages.append(assistant_msg)`, primary) lifecycle hooks reset inflight at turn boundaries. The seq counter is monotonic across turns so a long-lived subscriber's `snap_seq` cutoff stays valid for the lifetime of the connection — resetting per-turn would silently drop turn N+1's first M tokens (M = whatever was streamed pre-snapshot in turn N). `snapshot_and_consume_state_payload` also drains inflight at idle/error so cancel and exception paths don't leak stale text. New `register_listener_with_in_progress_snapshot` atomically registers a listener and snapshots the inflight buffers; `make_events_handler` emits a `state_change` event (so the JS busy machine flips to stop-mode) followed by a one-shot `in_progress_snapshot` after the kind-specific replay, then strips the internal `_seq` field from yielded live events while filtering against `snap_seq`. A per-listener shallow `dict` copy in the live drain prevents the multi-tab race where one listener's `del event["_seq"]` would corrupt another listener's filter view. `_synthesize_cancelled_results` now emits synthetic `on_tool_result` events for each cancelled tool so live coord tabs can drop the newly-additive `coord-tool-batch--running` indicator cleanly. The indicator now coexists with `--auto`/`--approved` (applied on `tool_info` and `approval_resolved` approved; removed when every row in the batch has a result), making live tool execution visually parallel to the replay-time orphan rendering. Frontend handlers in `app.js` (interactive) and `coordinator.js` (coord) absorb EventSource auto-reconnect re-replays via a length-based prefix check on the in-progress buffer. New `InProgressSnapshotEvent` + `StateChangeEvent` dataclasses in the Python and TypeScript SDKs with type guards. `_MAX_TURN_CONTENT_CHARS` lifted 256 KiB → 512 KiB (single constant for both buffers — headroom for current commercial models). Regression tests cover race-free composition under concurrent writers, seq-filter dedup invariants, the cross-turn seq monotonic invariant, idle/error inflight drain, synthesized `on_tool_result` on cancel (including UI-hook failure isolation), and the multi-listener shared-dict invariant. |
||
|
Patrick Buckley
|
c2cb6a7ea5 |
fix(replay): apply PR #488 review findings
Four Copilot findings on
|
||
|
Patrick Buckley
|
eca4bb79e4 |
fix(replay): seam 1 splice + storage symmetry for queued user messages
Reverses the seam-2-only design from the prior commits on this branch.
Queued user messages arriving DURING a tool batch (Seam 1) splice into
the last tool result's envelope as ``UserInterjection`` advisories via
``wrap_tool_result``. Messages arriving BETWEEN turns (Seam 2) drain
as a single trailing user row via ``_flush_queued_messages`` with
``user_feedback`` (operator text alongside an approval, e.g. "y, use
full path") folded in as a prefix. Cancel/exception drains (Seam 3)
keep the existing ``_flush_queued_messages()`` call unchanged.
Why all three seams:
* Strict-template providers (Mistral, Llama via vLLM with stock chat
templates) reject role-alternation violations. A literal ``user``
row mid-tool-batch breaks ``assistant(tool_calls) → tool → ... →
assistant``; back-to-back ``user → user`` rows on the wire also fail.
* The seam-2-only design produced back-to-back ``user`` whenever
``user_feedback`` and queued items both fired — bug-1 from the round-1
review. Folding ``user_feedback`` as a prefix to the queue-drain
collapses the two into one row.
* During-batch arrivals couldn't ride seam 2 — the splice was the only
way to deliver same-turn without violating role alternation.
Storage symmetry:
Tool DB rows now store the wrapped ``output`` (envelope + advisories)
unconditionally — ``self.messages[i]['content']`` and
``conversations.content`` match exactly. List-typed output (image /
structured MCP results) uses ``wrap_tool_result(raw_joined_text,
advisories)`` at save time so the persisted string is anchored on
``<tool_output>\n`` for the replay parser. ``TOOL_RESULT_STORAGE_CAP``
is removed entirely; tools are responsible for bounding their own
output, storage faithfully represents in-memory. Removing the cap
also simplifies the parser — no truncated-envelope edge case.
Replay extraction:
``decorate_history_messages`` (REST ``/history``) and ``_build_history``
(SSE replay, resume, rewind, retry, post-load, rename re-replay) both
call the public ``extract_advisories_from_tool_envelope`` helper to
pull the envelope back into structured ``advisories`` for JS replay.
Both string content and list-typed content (image+queued-message
combo) covered. JS renders extracted advisories as normal user
bubbles after the tool block via the shared ``replayAdvisoriesAfterTool``
helper in ``shared_static/utils.js``.
Wrapper-tag escape and provider splice:
``escape_wrapper_tags`` now encodes pre-existing ``&`` first using an
``&`` sentinel so tool output containing literal entity strings
(documentation viewers, code analyzers, web scrapers returning entity-
encoded markup) round-trips correctly. Both encode and decode helpers
short-circuit on absence of ``<`` / ``&``.
``_apply_reminders_for_provider`` detects already-wrapped content
(string body and list text-part) by ``startswith("<tool_output>\n")``
and skips re-escape so existing envelopes survive intact when a tool
message also carries ``_reminders`` (the queued-message + tool-error
co-occurrence case is now common).
``decorate_history_messages`` runs in ``asyncio.to_thread`` to keep
MB-scale string work off the event loop.
Other cleanup:
* ``_collect_advisories`` delegates the queue drain to a named helper
``_drain_queued_messages_to_advisories`` so the swap-and-clear pattern
lives next to ``_flush_queued_messages``'s identical pattern and the
side-effect is documented at the call site.
* Preamble strings + body marker for ``UserInterjection`` round-trip
detection moved to module-level constants in ``tool_advisory.py``;
imported by ``history_decoration.py`` so a producer-side rephrase
can't silently desync the parser.
* ``_send_with_mocks`` ctxmgr extracted in ``test_session.py`` — the
six new send-driven tests share an 8-deep ``patch.object`` block.
* ``replayAdvisoriesAfterTool`` shared helper in
``shared_static/utils.js``; ``app.js`` and ``coordinator.js`` both
invoke it.
* Dead truncation-pill CSS removed (``.tool-output-truncated`` and
``.coord-tool-truncated``); the JS that added these elements went
away with ``TOOL_RESULT_STORAGE_CAP``.
* Tautological tests (``TestBuildHistoryAdvisoryPropagation``)
replaced with production-realistic round-trip tests built from
``wrap_tool_result(...)`` envelopes — REST and SSE-replay surfaces
pinned to the same wire shape; full DB round-trip pinned end-to-end.
Negative-tested:
* Reverting the prefix-merge in ``_flush_queued_messages`` produces
back-to-back ``user`` rows, breaking
``test_user_feedback_and_queued_coexistence_single_row_with_prefix``.
* Reverting the ``extract_advisories_from_tool_envelope`` call in
``_build_history``'s tool branch leaves the envelope verbatim in
wire content, breaking the round-trip tests.
* Reverting the wrapper-detection in ``_apply_reminders_for_provider``
entity-encodes the existing envelope's literal tags, breaking both
the string-content and list-content envelope-preservation tests.
* Reverting the ``wrap_tool_result(raw_text, advisories)`` projection
at the DB save site produces a string starting with the original
raw text, breaking
``test_tool_db_row_round_trips_list_output_with_advisories``.
Tests: 5918 passed, 3 deselected. Lint + format + mypy clean on
touched files.
|
||
|
Patrick Buckley
|
a032e71ff3 |
fix(replay): apply review findings q-2 through q-7
Round-1 ``/review`` apply-pass. Drops stale ``UserInterjection`` references from comments and docstrings that no longer describe the post-PR drain shape, asserts the two-stream invariant in the new queued-message persistence test, and pins the ``content.trim()`` + ``renderAssistantToolBatch`` invariants on coord-side so a future refactor can't silently regress the Qwen3 phantom-card fix or the chronological-order render fix. Deferred: * **bug-1** (back-to-back ``user`` row when ``user_feedback`` from the approval-prompt UI callback coexists with a queued-message drain). Reachable on strict OpenAI-compatible local templates (Anthropic and Anthropic-via-merge-consecutive collapse fine; vLLM-hosted Mistral / Llama enforcing role alternation can reject). The pre-PR splice guarded against this case by riding queued items inside the tool result envelope; that guard is what motivated the original UserInterjection design, so the fix lane needs a deliberate decision rather than a quick patch. Sleeping on it. * **q-1** (delete dead ``UserInterjection`` class + tests). Held for the bug-1 decision — if the chosen fix is to resume the splice for the ``user_feedback``+queue coexistence case, the advisory shape stays load-bearing. Class now carries a docstring note marking it retained-pending-decision so a passing reader doesn't grep for producers and assume it's actually dead. Apply-pass content: * ``q-2``: drop "queued user interjections" from the persistent- advisory parenthetical in ``send``'s tool-result loop comment; rewrite to point at ``_flush_queued_messages`` for the queue path. * ``q-3``: ``__init__`` channel-routing comment loses "and ``UserInterjection``" — only ``GuardAdvisory`` remains. * ``q-4``: ``_queue_tool_advisory`` docstring + the tool-error nudge comment lose the user-interjection mentions; the docstring also now describes the side-channel + ``_apply_reminders_for_provider`` splice path (the actual mechanism). * ``q-5``: ``AttachmentsNotQueueableError`` docstring rewritten to describe the post-PR ``_flush_queued_messages`` flow — the single-combined-turn ``\n\n``-join shape can't carry image / file blocks, and per-item separate user turns would expand the strict- template role-ordering surface that the post-batch drain already balances. * ``q-6``: the new ``test_queued_message_persists_as_user_row_after_tool_batch`` in ``test_session.py`` now asserts ``stream_idx == 2`` so a future regression where the post-batch flush runs but the send-loop short- circuits before the next iteration surfaces in CI rather than manual repro. * ``q-7``: ``test_coordinator_page.py`` gets two new string-grep pins mirroring the existing ``test_app_js.py`` shape — ``content.trim()`` on coord's assistant-replay branch and ``renderAssistantToolBatch`` for the hoisted helper that orders content card before tool batch. ## Test plan - [x] ``ruff check`` clean - [x] ``mypy turnstone/`` clean (189 source files) - [x] Affected test surface (``test_session.py`` + ``test_tool_advisory.py`` + ``test_app_js.py`` + ``test_coordinator_page.py``) — 240 passed |
||
|
Patrick Buckley
|
c11692b327 |
fix(replay): coord render order + blank assistant cards + queued message persistence
Three independent rehydrate / replay regressions reported on long multi-turn conversations after the pull-model wake stack landed. **1. coord history replay rendered tool_calls above the assistant narration that announced them.** In ``coordinator.js``'s loadHistory loop, the ``role === "assistant"`` ``tool_calls`` branch sat above the role switch — every assistant turn with both narration AND tool dispatch produced ``[tool batch][content card]`` in the DOM, even though chronological order is content first. On a parallel fan-out (e.g. four ``close_workstream`` calls in one turn) operators saw the assistant text "Let me close them out and summarize" with NO tool batch between it and the next assistant message — the four-row batch had been rendered above the announcing text and was scrolled out of view. Hoisted the ``tool_calls`` synthesis into a local ``renderAssistantToolBatch(m)``, called from inside the assistant branch AFTER the content card. Live SSE order (text → dispatch → results) now matches replay order. **2. Whitespace-only assistant content rendered as a blank card on replay.** Models with vLLM's ``--reasoning-parser`` (Qwen3 in production) strip ``<think>…</think>`` and emit only the trailing ``"\n\n"`` as ``content`` before a tool call. ``content_parts = ["\n\n"]`` saves ``content = "\n\n"`` to the conversations row. Live the user only sees ``.msg.reasoning`` (the thinking content) — the empty ``.msg.assistant`` card lives next to it but reads as a thin divider. On rehydrate the reasoning bubble is gone (not persisted) and the empty assistant card is the only thing left, surfacing as "blank cards where the assistant message was." Both UIs now check ``content && content.trim()`` before rendering the body — whitespace-only content skips the card entirely instead of showing a phantom row. Live render unchanged. **3. Queued user messages disappeared on reconnect.** PR #474 routed queued user messages into the tool-result envelope via ``UserInterjection`` advisories — same-turn delivery, but no persisted user row. On page reload / cross-tab replay the optimistic ``.msg-queued`` bubble vanished: there was no DB row to rehydrate it. Dropped the ``UserInterjection`` splice in ``_collect_advisories``; the queue drains through ``_flush_queued_messages`` AFTER the tool batch completes instead. Sequence becomes ``assistant(tool_calls) → tool … tool → user(drained)``, which is valid for Mistral and Anthropic strict role validators (the only forbidden shape was user injected mid-batch BEFORE the tool result, which this still avoids). Persists a real user row → bubble survives reconnect, and stays in the session's wire-side context window on the next turn. ## Test plan - [x] ``ruff check`` clean - [x] ``mypy turnstone/`` clean (189 source files) - [x] ``pytest -m "not live"`` — 5798 passed, 3 deselected - [x] Updated ``test_collect_advisories_does_not_drain_queued_messages`` (was pinning the old UserInterjection shape) - [x] Added ``test_queued_message_persists_as_user_row_after_tool_batch`` (drives ``send`` end-to-end with a queued message arriving during the tool batch; asserts the user row lands in self.messages AND hits ``save_message``) - [x] Updated ``test_replay_history_renders_content_before_tool_block`` to tolerate the new ``msg.content && msg.content.trim()`` guard - [ ] Live browser pass on coord (close_workstream parallel fan-out rehydrates with the 4-row batch BETWEEN the announcing assistant text and the summary) and interactive (Qwen3 ``"\n\n"`` rows no longer paint blank cards on reload; queued bubble survives a tab refresh) |
||
|
Patrick Buckley
|
b120ee2fd7 |
fix(session): trim tombstone refs + WHAT-narration in apply-pass comments
Closes round-2 review findings q-1 (minor), q-3 (nit), q-4 (nit), q-5 (nit). * **q-1:** Drop the ``post-migration 050`` clause from the fork-block comment — the apply-pass relocated rather than removed the tombstone-style temporal reference round-1 q-2 was supposed to fix. The bulk-row dict shape and ``_encode_reminders`` are self-explanatory; the WHY is pinned by ``test_fork_preserves_source_and_reminders``. * **q-3:** Replace ``DOES persist now`` framing on the wake-row save comment with a present-tense invariant. The ``now`` implies the reader knows the prior state, same family as the temporal tombstones. * **q-4:** Trim the 12-line WHAT-narration block above the resume-time ``_reminders_delivered = True`` loop to two lines stating the WHY only. The new regression test pins the contract. * **q-5:** Reframe ``test_fork_preserves_source_and_reminders`` docstring as a forward-looking invariant; drop the ``Dropping them was the original bug`` and ``post-migration 050`` fix-narration. Project convention: invariant statements, present tense; don't reference the current task / fix / migration number. |
||
|
Patrick Buckley
|
7e35050b68 |
fix(metacog): cleanup batch — share watch-key constant, sanitize metadata, drop tombstones
Closes round-1 review findings q-2 (minor), q-5 (minor), q-6 (nit), q-7
(nit), sec-1 (nit), perf-4 (nit).
* **q-5:** Export ``_WATCH_REMINDER_OPTIONAL_KEYS`` from
``turnstone/core/watch.py`` and import in the dispatch closure
(session.py) and the replay filter (server.py:_build_history). The
three-place duplication of the literal tuple
``("watch_name", "command", "poll_count", "max_polls", "is_final")``
is gone; future field adds touch one constant.
* **sec-1:** Run ``sanitize_payload`` over string-typed metadata fields
(``watch_name`` / ``command``) before they enter the queue. Today's
consumers all use ``textContent``, but the asymmetry — sanitised
``text`` alongside unsanitised metadata — would survive forever in
DB rows and resurface if a future consumer used a non-textContent
sink (aria-label, copy-to-clipboard, markdown render).
* **q-7:** Drop the per-iteration ``isinstance(reminder, dict)`` from
the dispatch closure's metadata comprehension. By the time the
block runs, ``text = reminder.get("text", "") if isinstance(...)``
+ the ``if not sanitized: return`` guard above already established
``reminder`` is a non-empty dict.
* **q-2:** Strip tombstone-style references — "post-#482", "post-#484",
"Step 7 of the watch-card UX plan", "Post-Step-7 dispatch surface",
and the brittle line-anchor "session.py:2685-2686" — across
``session.py``, ``test_session.py``, ``test_watch.py``,
``test_watch_dispatch.py``, ``test_watch_integration.py``. Comment
intent preserved; historical anchors gone.
* **q-6:** Drop the ``del source`` line in ``cli.py``'s
``on_user_reminder``; the parallel ``on_tool_reminder`` ignores
``tool_call_id`` without ``del`` and the comment alone is enough.
* **perf-4:** Document the SQLite ``render_as_batch=True`` recreate
cost in migration 050's docstring — first deployment after upgrade
copies the conversations table twice (one per ``add_column``).
PostgreSQL is unaffected.
5734 non-live tests pass; ruff + mypy clean.
|
||
|
Patrick Buckley
|
91e7f2daca |
fix(session): preserve _source/_reminders on fork + cap persisted reminder text
Closes round-1 review findings bug-2 (major), perf-1 (minor), perf-6 (nit).
* **bug-2:** ``ChatSession.resume(..., fork=True)``'s bulk-row builder
silently dropped the ``_source`` and ``_reminders`` side-channel
data the source workstream had persisted via ``_append_user_turn``.
Both backends' ``save_messages_bulk`` already accept these keys
(the columns exist post-migration 050) — the bulk builder just
didn't supply them. The fork's resumed transcript would then look
like the assistant turn answered out of nowhere: every wake marker
and every reminder bubble that survived to disk on the source got
dropped on the fork. New regression test
``test_fork_preserves_source_and_reminders`` pins the contract.
* **perf-6:** Extracts ``_encode_reminders(reminders) -> str | None``
near ``_apply_reminders_for_provider`` so the user-turn save path,
the tool-turn save path, and the new fork bulk builder share one
encoder. Eliminates the drift risk between three near-identical
``json.dumps(..., separators=(",", ":")) if X else None`` patterns.
* **perf-1:** The new helper clamps each entry's ``text`` field at
``REMINDER_TEXT_STORAGE_CAP = 8192`` characters before encoding so
a single rogue producer (a watch streaming unbounded shell output,
a corruption-class steering payload) can't blow the conversations
row width or the FTS5 index. The in-memory side-channel keeps the
full body — only the persisted JSON is clamped. Mirrors
``TOOL_RESULT_STORAGE_CAP`` on tool result rows.
5734 non-live tests pass; ruff + mypy clean.
|
||
|
Patrick Buckley
|
f1466ca7e3 |
fix(session): flag persisted reminders delivered on resume
Persisted ``_reminders`` survive ``load_messages`` but the in-memory ``_reminders_delivered`` flag does not (it's session-scoped — set by ``_mark_reminders_delivered`` after each successful provider stream, never persisted alongside the JSON column). Without a re-splice guard at resume time, ``_apply_reminders_for_provider`` would walk every loaded message, see ``_reminders`` set + the flag falsy, and splice every historical ``<system-reminder>`` envelope onto the wire on the very next user turn — leaking each reminder a second time, the turn after it had already advised. Mirror the post-stream hook in ``resume()``: every loaded message that carries reminders has already been delivered (it survived to disk), so flag it accordingly so ``_apply_reminders_for_provider`` short-circuits on the pass-through path. Test pins the contract end-to-end — stage a workstream with a persisted reminder, resume into a fresh session, append a live user turn, run the wire transform, and assert the historical reminder body does NOT land in the rendered output. |
||
|
Patrick Buckley
|
6ae6877acc |
feat(ui): structured watch-result card + system-nudge marker on replay
User-visible slice of the watch-card UX workstream — combines the
replay-path widening, both frontend renderers, the CSS, and the
cross-cutting Python tests.
server._build_history widens the reminder filter from {type, text} to
project on a known set of optional fields (watch_name, command,
poll_count, max_polls, is_final) and surfaces _source as
entry["source"] when set. The known-key filter narrows the blast
radius if a future producer accidentally stuffs sensitive fields
into the dict.
SessionUIBase.on_user_reminder takes a new source: str | None kwarg
that rides on the SSE event when set. _attach_pending_user_reminders
forwards user_msg["_source"] so non-originating tabs see the wake's
"system_nudge" tag and render the thin marker. Protocol + cli + eval
implementations widen accordingly.
Frontend (coordinator.js + app.js — touched in lockstep per project
memory's "logic that lands in BOTH UIs must touch both files"):
* Branch on r.type === "watch_triggered" for a structured
.msg.watch-result card with header / $ command / <pre> body /
poll N/M [· final] footer.
* New addSystemNudgeMarker (interactive) + appendSystemNudgeMarker
(coord) renders a thin .msg.user.system-nudge anchor for
wake-driven reminders, both live (source === "system_nudge" on the
SSE event) and replay (msg.source === "system_nudge").
* Default .msg.user-reminder rendering preserved for every other
metacog nudge type.
CSS (shared_static/chat.css):
* New .msg.watch-result rules — full-width treatment, cyan accent,
monospace body with word-break: break-word for mobile.
* New .msg.user.system-nudge rule — thin yellow marker.
* Bonus newline-collapse fix: .msg.user-reminder .msg-body now sets
white-space: pre-wrap so multi-line shell output / bulleted lists
stay readable inside the advisory bubble.
Plan reference: docs/design/watch-card-ux.md §4 Steps 9-12 + bonus
CSS §11 (Commit 4).
|
||
|
Patrick Buckley
|
f64c3e7b10 |
feat(storage): persist _source + _reminders side-channels on conversations
Adds two TEXT-NULL columns to the conversations table so multi-tab / multi-device replay sees the same metacognitive bubble shape the originating tab saw live. Until now, reminders lived only on the in-memory ChatSession.messages dict, and the wake-driven empty user turn was not persisted at all (skip at session.py:2685-2686) — a second tab connecting via /history saw the assistant turn with no preceding wake context, and missed every other tab's reminder bubbles besides. Single Alembic revision 050 (head was 049) adds: * conversations._source — today only "system_nudge" for wake rows * conversations._reminders — JSON-encoded reminder list Both backends (sqlite + postgresql) thread the columns through save_message / save_messages_bulk / load_messages. reconstruct_messages unpacks the row tuple as 9 elements (was 7), JSON-decoding _reminders on the user AND tool branches with the same contextlib.suppress guard the existing provider_data / tool_calls decode uses. Tool-row reminders ride the same column so tool_error / repeat replay shape matches user-channel parity. session.py:2685-2686 wake-row persist skip is dropped; _append_user_turn JSON-encodes user_msg["_reminders"] and passes both source + reminders to save_message. The tool-message save site at session.py:3014-3020 mirrors with metacog_reminders. Plan reference: docs/design/watch-card-ux.md §4 Steps 1-5 (Commit 1). |
||
|
Patrick Buckley
|
94ed79d488 |
feat(metacog): switchover — watches enqueue onto NudgeQueue not _watch_pending
Replaces the bespoke _make_watch_dispatch / _watch_pending /
_dispatch_pending_watch / _MAX_WATCH_CHAIN machinery with a single
NudgeQueue.enqueue("watch_triggered", ...) call inside
ChatSession.set_watch_runner. Watch results now drain at the same
<system-reminder> envelope seams as every other metacog nudge
(USER_DRAIN, TOOL_DRAIN, IdleNudgeWatcher IDLE wake) — no separate
worker-spawn, no recursive watch chain, no per-session queue.Queue.
The dispatch closure built inside set_watch_runner carries:
- producer-side sanitize_payload over the whole formatted message
before enqueue, so steering-vector / control-char shell output
can't tamper with the envelope at interpolation time
- a soft cap of 50 entries on per-session "watch_triggered" depth
via the new NudgeQueue.drop_oldest_by_type, replacing the prior
_watch_pending maxsize=20 + _MAX_WATCH_CHAIN=5 bounds; drop policy
is drop-oldest (latest output most useful), logged at WARNING
- a valid_until predicate that re-checks
storage.get_watch(watch_id)["active"] at drain time so a cancelled
watch's last splat doesn't ride out a future wake
Behavioural delta documented in the plan section 3.4: N back-to-back
watch fires now drain into ONE assistant turn responding to all N
(via the envelope splice) instead of N separate send turns. This is
intentional — fewer model invocations for noisy watches, and uniform
with the rest of the metacog pull-model surface introduced by #482.
Implements watch-switchover plan steps 5-8. Server-side simplifications
let the previously-load-bearing _make_watch_dispatch (47 lines), its
session_worker.send import, and the chat-loop _dispatch_pending_watch
seam at the no-tools IDLE branch all disappear. The obsolete
tests/test_watch_dispatch.py and the wake-tag test in test_session.py
(both pinning contracts that no longer exist) are removed; the
NudgeQueue-based replacement plus an integration test land in the
following commit.
|