10 Commits

Author SHA1 Message Date
Patrick Buckley 2ff7c61051 feat(coordinator): idle nudges deliver only on the idle wake, and a queued interjection owns the seam
The idle nudges enqueued on the any channel, which every drain seam
serves — a deferred wake left them deliverable at the start of a real
user send or mid-turn at a tool batch, describing an idle moment that
no longer existed. They move to a new wake channel: wake-eligible,
invisible to USER_DRAIN, TOOL_DRAIN, and the quiet ride-along. A user
cancel drops pending wake entries rather than demoting them — the
quiet demote's whole value is later seam delivery, exactly what this
class may never have. Dropping a charged entry is the accepted
fail-closed cost; liveness surviving Stop means the next idle event
fires fresh, not that a queued entry re-wakes the workstream.

A queued user interjection owns the idle seam: at wake delivery, a
non-empty interjection queue drops the wake-channel entries and the
interjection runs as a genuine user send in their place — no wake
tag, so the caps reset as for any real send and the next genuine idle
re-derives both nudges over fresh reads. The check lives in the wake
worker (which owns the slot and can dispatch a full send), not the
watcher's state-transition thread, where skipping would strand the
message. Measured before building: send('') with queued messages
appends an empty user turn and delivers the interjection one
assistant turn late, so the handoff pops first and sends the popped
text — one rendering shared with the flush seams.

External events are not idle nudges: any-channel entries still arm
the wake alone, and with an interjection waiting they ride the
genuine turn's drain seam — both deliver, only the idle nudges drop.
A failed wake send drops wake entries alongside user ones; externals
requeue quiet as before.
2026-07-29 22:11:38 -07:00
Patrick Buckley ab7d56e0ba feat(bash): opt-in background shells with delta output reader and kill tool (#817)
Restore 'start a dev server, use it in a later call' as an explicit opt-in
after #816 made bash reap its whole process group on return. The surface
mirrors the dominant coding-agent convention: bash(run_in_background=true)
returns a bash_N handle immediately; bash_output(id, filter?) returns only
output produced since the previous read plus status and exit code;
kill_shell(id) terminates the shell's whole process group.

- Per-session BackgroundShellRegistry: capped rolling line buffer with
  drop-oldest gap accounting, exit-order record pruning, owner scoping for
  task_agents (shells reaped when the agent finishes), liveness-guarded
  group kills (a stale pgid is never signalled), budgeted teardown joins.
- Exit notices ride a shared external-event rail (sanitize, soft cap,
  channel 'any', idle wake) now common to watch fires; a new 'quiet'
  NudgeQueue channel lets a user cancel defer pending notices without
  letting them re-wake the stopped workstream, and failed wake delivery
  re-queues external notices seq- and predicate-intact without re-arming
  the wake gate.
- The bash_output filter runs in a killable subprocess: sre holds the GIL
  for an entire search, so no in-process timeout can bound a hostile
  pattern. Scrubbed child env, pinned UTF-8 pipes, honest timeout-vs-
  helper-failure error taxonomy, per-line match window with explicit
  clipping notes; a failed filter never consumes the delta.
- run_in_background rides the bash intent-judge projection; bash_output is
  exempt from the repeat warning but still recorded so interleaved polls
  keep breaking other tools' streaks; all bash boolean args share one
  lenient coercion dialect.
- Shells survive generation cancel and die with the workstream: every
  teardown path funnels through ChatSession.close(); CLI exit and the
  server lifespan now close every loaded session, signal-first and
  Ctrl-C-safe, so nothing detached outlives a graceful shutdown.
2026-07-10 15:42:33 -07:00
Patrick Buckley 98d4be8ffe fix(watch): deliver terminal fires instead of dropping them silently
WatchRunner._poll_watch committed active=False to the row BEFORE
calling _dispatch_result for a terminal fire, and the dispatch closure
registered by ChatSession.set_watch_runner enqueued each reminder with
a valid_until=is_watch_active predicate that re-read the row at drain
time. Since the runner already flipped active to 0, the predicate
returned False for every dispatched fire and NudgeQueue.drain silently
dropped the entry — the model never saw a watch result. Then a
subsequent action=cancel call hit list_watches_for_ws (filters
active==1), the now-inactive row was invisible, and the cancel
returned 'Watch "X" not found.' regardless of whether the watch had
actually run.

Reorder _poll_watch to dispatch before the row write, drop the
valid_until predicate from the watch closure (its only effect was the
bug above), and add a _terminal_dispatched guard on the runner so a
transient storage failure between dispatch and row-write doesn't
re-fire the reminder on the next tick. Add WatchRunner.forget_terminal_dispatched
and call it from the cancel path so an out-of-band deactivate (next_poll='')
doesn't leak the watch_id from the runner's pending-retry set indefinitely.

Cancel-by-name now routes through a new find_watch_by_name storage
method that ignores the active filter and prefers active rows over
newer-inactive same-name siblings. The session.py cancel branch
distinguishes 'already completed (auto-cancelled)' from 'not found'
so the model can tell apart 'this watch ran and finished' from
'no such watch.' Consolidate the two byte-identical _escape_like
/ _escape_ilike helpers in the storage backends into a single
turnstone.core.storage._utils.escape_like and apply it to the new
find_watch_by_name LIKE pattern so a model-supplied watch name
containing % or _ can't redirect a cancel to a sibling watch.

NudgeQueue.drain previously dropped predicate-failed entries without
logging anything, which is what hid this bug for so long. Drain now
emits nudge_queue.predicate_dropped: info for reason=predicate_false
(the normal lifecycle case — idle_children when every active child
finished between enqueue and drain), warning with exc_info for
reason=predicate_raised (a misbehaving predicate).

Tests: new test_poll_watch_terminal_fire_survives_drain (parametrized
stop_on_fired + max_polls_reached) drives the real WatchRunner._poll_watch
against a real tmp_db row and confirmed to fail against pristine main.
test_poll_watch_retry_deactivate_after_update_watch_failure exercises
the _terminal_dispatched retry-deactivate branch end to end.
test_cancel_clears_pending_terminal_dispatched_entry covers the cancel-
path leak case. test_find_by_name_prefers_active_over_newer_inactive
catches the ordering regression. test_find_by_name_treats_percent_as_literal
+ test_find_by_name_treats_underscore_as_literal pin the LIKE escape.
2026-05-14 15:13:00 -07:00
Patrick Buckley 30b7e4dd24 refactor(metacog): widen NudgeQueue._Entry with optional metadata field
Producers (today only watch_triggered) can now attach a metadata dict
to a queued nudge so the rendered reminder dict on the user/tool side
carries fields beyond {type, text}.  Wire shape stays additive: the
SSE event picks up the optional fields when present, and producers
without metadata leave it None.

* _Entry grows from 4 fields to 5 — metadata: dict[str, Any] | None.
* enqueue accepts metadata=... as a kwarg.
* drain returns list[tuple[str, str, dict | None]] (was 2-tuples).
* pending stays narrow at (type, text) for legacy callers; new
  pending_with_metadata projects the third slot for tests that need
  to assert producer-specific fields.
* Three drain consumers in session.py — _collect_advisories,
  _attach_pending_user_reminders, deliver_wake_nudge_from_queue —
  unpack the new 3-tuple shape and merge metadata into each
  reminder dict.
* on_user_reminder / on_tool_reminder protocol signatures widen
  from list[dict[str, str]] to list[dict[str, Any]] across
  ChatSession.UI, SessionUIBase, CLI, eval harness.

Plan reference: docs/design/watch-card-ux.md §4 Step 6 + Step 8 _Entry
subset (Commit 2).
2026-05-06 23:34:29 -07:00
Patrick Buckley c757c22f55 fix(metacog): atomic cap-and-drop helper for soft-cap producers
Closes PR #484 review findings (Copilot): the soft-cap pattern in
``ChatSession.set_watch_runner``'s dispatch closure was a non-atomic
two-call pair (``count_by_type`` then ``drop_oldest_by_type``) with
two separate lock acquisitions.  A concurrent drain on the worker
thread (``USER_DRAIN`` / ``TOOL_DRAIN`` consuming ``"watch_triggered"``
entries via the ``"any"`` channel) could slip between the two calls,
making the drop a no-op.  The dispatch closure also discarded
``drop_oldest_by_type``'s return value and unconditionally logged
``dropped_oldest=True``, so a no-op drop got reported as a successful
drop.

* New ``NudgeQueue.cap_at_or_drop_oldest(nudge_type, max_depth,
  channel=None) -> bool`` does the count+drop in a single critical
  section.  Returns the actual outcome.

* Dispatch closure (``session.py:1410-1416``) now calls the helper and
  uses its return value to gate the WARNING log line, so the log is
  accurate when a drop did NOT happen.

* ``drop_oldest_by_type``'s docstring no longer overstates the
  per-call lock as covering a count+drop pair — it points readers
  to ``cap_at_or_drop_oldest`` for that contract.

7 new tests in ``TestCapAtOrDropOldest`` cover: below-cap no-op,
at-cap drop-oldest, above-cap drop-only-one (per-call), channel
filter, other-type isolation, ``max_depth <= 0`` defensive no-op,
no-match.

5708 non-live tests pass; ruff + mypy clean.

The github-code-quality bot finding ("Statement has no effect" on
``_protocol.py:939``'s ``...`` body) is a false positive — every
Protocol method in ``_protocol.py`` uses ``...`` as its body, which
is the canonical Python Protocol pattern.  Replacing with ``pass``
would diverge from the file's existing style.  No code change.
2026-05-06 16:16:57 -07:00
Patrick Buckley e5e6e13307 fix(metacog): NudgeQueue.count_by_type primitive + channel-aligned soft cap
Closes review findings perf-2, q-3, bug-3.

The watch dispatch closure's soft-cap pre-check materialised the whole
queue snapshot via ``pending(channel="any")`` only to throw away the
text and count the type — wasteful at typical drain depths (cap-50 +
mixed producers means a 50-tuple allocation per fire just to read a
length).  The other half of the cap pair (``drop_oldest_by_type``)
walked the *whole* queue regardless of channel, so a future producer
that enqueued ``"watch_triggered"`` on a different channel could be
dropped by the watch cap, and vice versa — silently surprising once
that producer existed.

Adds ``NudgeQueue.count_by_type(nudge_type, channel=None) -> int`` that
walks ``_items`` once under the queue lock without materialising
tuples; extends ``drop_oldest_by_type`` to take an optional ``channel``
filter so both halves can agree on the entry set being capped.  The
watch dispatch closure now passes ``channel="any"`` to both —
consistent with where the closure enqueues — so a future channel split
can't bleed across producers.

Adds ``TestCountByType`` mirroring the existing ``TestDropOldestByType``
shape, plus a ``test_drop_oldest_by_type_channel_filter`` case pinning
the new optional argument's behaviour.
2026-05-06 16:16:57 -07:00
Patrick Buckley 74f1958e47 feat(metacog): NudgeQueue.drop_oldest_by_type helper for soft-cap producers
Adds an atomic drop-oldest-by-type operation to NudgeQueue used by
producers that need a per-type soft cap on their own queue depth.
The watch dispatcher (next commit in this stack) is the first user:
when "watch_triggered" saturates, the dispatch closure drops its
oldest entry under the queue lock so the count snapshot and drop
can't interleave with a concurrent enqueue from the same producer.

Implements watch-switchover plan section 3.1 — the producer-side soft
cap takes the place of the deleted _watch_pending maxsize=20 bound.
Other producers (idle_children, advisories) have natural rate limiters
already, so the helper is opt-in per producer rather than a global cap
in enqueue itself.
2026-05-06 16:16:57 -07:00
Patrick Buckley 908e67fe4f feat(metacog): coord idle-children nudge — observer + valid_until predicates
Adds the first concrete consumer of the wake trigger: when a coordinator
goes IDLE while interactive children are still running, a
``CoordinatorIdleObserver`` enqueues an ``idle_children`` nudge that the
``IdleNudgeWatcher`` then dispatches as a synthetic empty-user-turn
``send``.  The model receives a system-reminder body listing the active
children (capped at 6 inline + 32 in the suggested ``wait_for_workstream``
call) and a nudge to block on them rather than reply prematurely.

Observer gates (in order): coord-only filter, skip if last assistant
turn used ``wait_for_workstream``, per-(ws, nudge_type) hard cap (3)
that resets only on non-wake leave-IDLE, active-children query,
``should_nudge`` cooldown.  Console lifespan registers the observer
BEFORE the watcher so subscriber-fire order has the observer
enqueueing first on the same IDLE event.

Adds an opt-in ``valid_until`` predicate on ``NudgeQueue.enqueue``
(R9 from the design risk register) — drain re-checks the predicate
outside the queue lock; falsy / raising drops the entry without
delivering it.  ``deliver_wake_nudge_from_queue`` now drains inline
before synthesizing the empty user turn so a stale predicate-drop
doesn't leave the wake send with empty content; ``_attach_pending_user_reminders``
consumes the pre-drained reminders via ``_wake_drained_reminders``.

The observer's ``valid_until`` uses ``count_workstreams_by_state``
(boolean check, no row fetch) instead of full ``list_workstreams``,
keeping the chat-loop user-attach path off the heavy query.

User-controlled child workstream names are sanitized
(``_sanitize_child_name``) before interpolation so a name like
``</thinking>...`` can't steer the model's reasoning channels through
the rendered body — the wire-boundary ``escape_wrapper_tags`` only
covers ``<system-reminder>`` / ``<tool_output>`` envelopes.
2026-05-06 12:02:27 -07:00
Patrick Buckley f0e7fea549 feat(metacog): wake trigger — IdleNudgeWatcher + ChatSession.deliver_wake_nudge_from_queue
Adds the third metacog channel: an out-of-band wake that converts a
workstream's IDLE transition into a synthetic empty-user-turn ``send``
when the session has any-channel nudges queued.  The ``IdleNudgeWatcher``
subscribes to ``SessionManager.subscribe_to_state``; on IDLE it dispatches
via ``session_worker.send`` with a no-op ``enqueue`` callback so a
busy-worker race silently drops without spawning a competing worker.

Wake-source-tag plumbing on ``ChatSession`` short-circuits metacog
detection on the synthetic empty input, suppresses queue producers
during the wake's own tool dispatch, and stamps ``_source = "system_nudge"``
on the synthetic user-message for audit / replay distinction.  The tag
is saved / restored across ``_dispatch_pending_watch`` so watch chains
recursing off the wake are processed as normal user turns rather than
inheriting the wake's guards.

Generic ``install_idle_nudge_watcher`` / ``shutdown_idle_nudge_watchers``
helpers wire the watcher into both the interactive and coord lifespans
via a single ``app.state`` registry so both surfaces share the same
teardown contract.

Foundation for PR 3 (CoordinatorIdleObserver + idle_children formatter)
and PR 4 (watch dispatcher switchover).
2026-05-06 12:02:27 -07:00
Patrick Buckley 94b3720916 refactor(metacog): unify advisory channels into pull-model NudgeQueue
Replaces the dual `_pending_user_advisories` / `_pending_tool_advisories`
list pair with a single channel-tagged `NudgeQueue` per session.
Producers tag entries with a channel ("user", "tool", or "any");
consumers drain by channel filter at their existing seams. Foundation
for the wake trigger (PR 2) and coordinator idle-children nudge (PR 3).

Existing nudges (start, correction, completion, denial, resume,
tool_error, repeat) keep their wire shape and drain timing — zero
behavior change. Cancel paths now `clear()` the unified queue.
2026-05-06 12:02:27 -07:00