Repository navigation
Conversation
Adds src/agent_turn.py (vendored into ag2-sparrow): the 0400 turn file at state/turns/<task_id>_g<gen>_<ulid>.json, its .closed fence, the journal reader for dispatch_intent/dispatch_outcome records, and classify_turn. A plain-text answer may be sent only when the turn has no bound intent, no unbound intent since it began, and core-api confirms zero operations.
…gate on results Behind REMOTE_TASK_LIFECYCLE_V2 (default off) the bridge sends one lease-holder worker_id and protocol_version 2 on ack, records the execution_generation the ack reports, beats each live attempt on POST /v1/tasks/heartbeat, and puts the generation and worker_id on every result. A fenced attempt (task_cancelled, stale_execution_generation) stops the core through a CANCEL_INSTRUCTION task and is cancel-acked; its late result is settled, never posted. Every five minutes GET /v1/tasks/active is reconciled with the in-flight set. With REMOTE_TASK_TURN_CONTEXT too, each attempt's turn file is written on ack and closed when its result settles, and the result is gated on the turn's journal: skipped when the task closed terminally through MCP, posted as waiting_input + no_send while suspended, and posted with no_send when the same text was already sent as a room message. A result 409 that ends the task, or a 400 waiting_input_without_blocker, settles the item instead of quarantining it. A re-served task carrying `wake` is left unacked rather than closed as a redelivery.
…arison apart from the delivery reconcile
…ask names The coverage gate measures tests/**/*.test.py in process: the package test moves to tests/sparrow-task-attempts.test.py, the bridge hooks get an in-process suite, and the vendored agent_turn.py copy is omitted like the other src/ copies. Two cancels in one millisecond no longer share a control-task name, and a recorded acked_at of 0 is read as itself.
…er turn context With REMOTE_TASK_TURN_CONTEXT, a task re-served with `wake` after its question was answered runs once per blocker_episode_id: it is queued again under the same id with each answer appended to the task's line as quoted room data, acked at the same generation into a new turn, and its result rides its own outbox item `<id>.wake-<episode>`. The earlier attempt's archived copies no longer settle it. Each task file also carries one writer-owned line telling the agent to ask a question that blocks the task with room.question.ask on its exact task_id.
… unrun wake once per episode
…erminal nor suspended
…urn-gate # Conflicts: # packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py
…re, a corrupt journal is unreadable Under REMOTE_TASK_LIFECYCLE_V2 a result written before its task's ack named the lease waits for that ack (at most 120 s), and records the proxy could not bind but that name the task count as the attempt's turn from publication on, so neither the lease fields nor the journal gate can be skipped by a fast core. Every fencing error_code on either ack path (404 or 409) fences the attempt, retires its pending ack and, for a published task, stops the core, except a stale older lease whose newer serve is pending here. A journal line that does not parse makes the journal unreadable instead of absent; an unterminated last line younger than 30 s defers the result. A served task carrying `wake` is left unacked whether or not this node saw its first attempt. A late ack for a task no longer queued opens no turn.
…pt to settle and always runs with its answer Merges feat/agent-activity-turn-gate. A served `wake` is decided before any local shortcut: run with its answer under turn context, or left unacked (turn context off, ack_first, an episode already run, or no usable episode), whether or not this node saw the first attempt. A wake served while an earlier attempt of the same id is still settling here is left unacked until it is served again, under a lock the settle also holds, so that settle can never retire the wake. Finishing an attempt checks it is still the one the result belonged to, and opening a wake starts a fresh turn.
…edelivery keeps the journal boundary On a broker that has ever named a lease, a result whose ack never answered keeps waiting until the ack lands, the attempt is fenced, or the server closes the task. On a broker never seen to name one, the 120 s fallback is gated on the journal records that name the task, so an early send, terminal action or blocker still rules. A task served again before its ack keeps its first publication time, so its earlier unbound room actions stay inside the turn's journal window.
…on every settle branch The normal, skip-marker, dedup and orphan-recovery branches archive a result and then retire its id (in-flight removal, room and media sidecars). Each now runs that whole transition under the wake handoff lock and marks the id retiring, so a wake served in that window, on another thread or this one, waits to be served again instead of being published and then losing its tracking.
…-wake # Conflicts: # packages/ag2-sparrow/ag2_sparrow/task_attempts.py
…r reconcile from its first publication A result held because its ack never named the lease on a broker that names leases is now released by the reconcile too: once the complete active listing omits the task past the grace, the attempt is fenced, the core stopped, and the result settled without a post.
…nd a settle never archives an open wake The no-live-result recovery branch now judges disposal inside the same locked retirement step as its settle, so a wake admitted after the judgement waits to be served again. A settle also re-checks inside the lock: while a wake for the id is open here and its own result is not delivered, nothing is archived or retired, whatever the caller judged before it took the lock.
… every held task beats A task_cancelled in a per-task beat no longer fences the attempt. The core gets one CANCEL_INSTRUCTION task; once it has handled it, a result it wrote for the task is posted normally with no cancel-ack, and with no result the stop is confirmed on cancel-ack and a later result is settled. A task_cancelled on the ack still means the task is not started and is cancel-acked at once. The per-task beat now runs on its own 60 s cadence ahead of the client heartbeat's backoff, so a Stop reaches every task the bridge holds for as long as it holds it.
…-wake # Conflicts: # tests/gateway-agent-activity-hooks.test.py
… stopped, not no_action_needed While a Stop is pending, a core result that is no_send or empty confirms the Stop with a cancel-ack and is settled locally; it is never posted as no_action_needed. A failed cancel-ack holds the result for the next pass. A result with a body is still posted (a finished reply is never dropped).
…lier send does not silence the reply - read_turn_records tested `turn_id in line` with turn_id None (an attempt with no turn), so one stale torn line raised TypeError on every delivery pass. - An earlier generation's room.message.send counts as already sent only when its outcome is succeeded, dispatched_unknown or not yet written; a failed send delivered nothing, so the new generation's reply is posted. - Smaller: the wake-unacked doc line names its flag; write_if_absent fsyncs the directory and names its temp `.tmp`; a busy journal lock costs one wait per close pass; a core reading moving but dead or abnormal counts as idle for the stall; docstrings state the torn-before-ids and deferred-close behaviour.
…-wake # Conflicts: # docs/remote-gateway-protocol.md
…led failed ask does not park The ask rule's DM route now ends the task with a [no-send] result and a one-line note. An ask whose reconciled outcome is known (failed) no longer parks the task as waiting_input; only an ask whose outcome is still unknown does.
…turn A T turn that ends without a result (waiting on a background subagent) leaves a Stop-hook thinking row, not a done row, and a later unbound turn in the pane could then take the Escape. Now only a working or processing row as T's newest lets the Stop interrupt; any other kind means T's turn already ended.
|
Item (a) is fixed in 81f2661 (head 81f2661, which also carries #5885, #5883's A/B fix, and #5941 with its census re-anchor). The Stop now sends only when T's newest session row is Your probe is |
sonichi
left a comment
There was a problem hiding this comment.
Approving at 81f2661 (agent-assisted, Blue for Chi's seat). MID_TURN_KINDS = ("working", "processing") closes the gap. Probes:
- T's newest row is a Stop-hook
thinkingrow, with a cron turn in the pane: no Escape, fresh or 25 minutes later. - A
donerow: no Escape. - T's newest row is
working/processingwith T mid-turn: exactly one Escape.
stop-interrupt (38) passes locally with a stand-in tmux whose log stayed empty. A hand-appended mid-turn thinking row now also blocks the Escape, which errs on the safe side. CI was still running when I approved.
An ask whose outcome is a failure, in the journal or from reconciliation, means no question exists, so the result posts failed with no_send instead of the note as the completed reply. An ask whose outcome is still unknown keeps posting waiting_input.
keweichen
left a comment
There was a problem hiding this comment.
Changes requested at current exact head 9061731bd797553bdff10a421c59fef72420590b — not ready to merge. The requested 81f26619… snapshot moved during review; this verdict covers the current merge head.
The thinking-row fix is valid. The 38-case Stop suite passes, and its old 532521f6… control sends Escape/fails where current HEAD refuses.
Blocker 1 — canonical tmux socket precedence is still wrong. At src/stop_interrupt.py:196-199, _socket() returns legacy SUTANDO_TMUX_SOCK before calling the canonical config helper. With both variables present:
SUTANDO_TMUX_SOCKET=/canonical.sock
SUTANDO_TMUX_SOCK=/legacy.sock
resolved=/legacy.sock
helper_calls=[]
Both launchers use SUTANDO_TMUX_SOCKET. The test covers legacy alone, not the adjacent both-set case. Make canonical precedence match the launchers/config helper and add that control. Worst case after opt-in is Escape sent to the wrong tmux server or turn.
Blocker 2 — no current-head post-restart Stop witness. The live build named in the body predates the critical stale-row fix and this head. Please run a restarted current-head Stop round trip that proves exactly one Escape reaches task T and none reaches a later/unbound turn.
The cumulative stack also inherits #5885’s confirmed post-delivery/pre-archive wake-mapping defect at 489745f7…. CI was still running with no completed failures at review time; CLA passes.
…ves flight finish() forgot the wake item as soon as the wake's result was confirmed, before the caller archived the still-live result. A restart (or a failed archive) then re-offered that result through the first attempt's delivered item, which quarantined it to undelivered. The item is now kept in relay-wake-items until the startup sweep sees the task out of flight, and a re-offer through a settled wake's own item is recognised as delivered rather than re-sent.
…legacy name _socket() read the legacy SUTANDO_TMUX_SOCK first. The launchers and sutando-config.sh tmux-socket use SUTANDO_TMUX_SOCKET, so it now wins when both are set, then the legacy name, then the config helper.
|
Fixed in 3ad8244 (head 3ad8244, which also merges #5885's wake fix). |
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
Re-review at 3ad8244 (delta from 9061731: 3ad8244 plus the merge of #5885's 0e70ff5, reviewed on #5885). Code: resolved. Verdict: not approving yet, only because the REVIEW.md lesson 15 live witness is still missing; no code blockers from me.
Verified:
src/stop_interrupt.py:194-201_socket()now readsSUTANDO_TMUX_SOCKET, then legacySUTANDO_TMUX_SOCK, thensutando-config.sh tmux-socket. That matcheswatch-tasks-stream.sh:1001and the helper (scripts/sutando-config.sh:516), which also reads the ambient canonical name.- The both-set control in
tests/stop-interrupt.test.py:334-339fails on 9061731'sstop_interrupt.py('/explicit.sock' != '/canonical.sock') and passes at head (38/38). sparrow-task-attempts passes too. - The rest of the earlier fixes (MID_TURN_KINDS, done/result refusal) is unchanged in this delta.
- CI: 39 pass, 3 skipped. Mergeable. Off by default behind
SUTANDO_STOP_INTERRUPT.
To approve: a restarted current-head Stop round trip showing exactly one Escape reaches task T and none reaches a later or unbound turn (keweichen's blocker 2). Worst case after opt-in without it is an Escape into the wrong turn.
(agent-assisted, Sutando for bassilkhilo-ag2)
keweichen
left a comment
There was a problem hiding this comment.
Changes requested at current exact head 3ad8244165d5ca68e50c2091bcff2e0986770b98 — not ready to merge.
The requested socket fix is correct: _socket() now prefers canonical SUTANDO_TMUX_SOCKET over legacy SUTANDO_TMUX_SOCK; the both-set control resolves the legacy socket on the parent and /canonical.sock here. tests/stop-interrupt.test.py passes 38/38.
Blocking — a late Stop can Escape an unrelated successor turn. src/stop_interrupt.py:213-232 snapshots the latest task row and result before capture-pane, then sends Escape without revalidating either. On this exact head I made task T finish while the capture call was in flight and returned a pane already busy on a cron successor. The result was:
sent=True escape_calls=1 result_exists=True
decision=Stop: interrupted the running turn (the pane is mid-turn on this task)
The same happens when a newer non-mid-turn thinking row lands during capture. This violates the feature's core safety claim and can abort another task or destroy its draft. Revalidate the task-bound row/result after capture and immediately before claiming/sending (or use a stronger task-bound fence), with a deterministic state-change-during-capture regression.
Inherited blocker. This head contains #5885's finish() transition, which clears/saves the active attempt before separately saving seen-episode and retained-wake records and ignores all three write results. Injected write failures respectively republish a delivered wake, allow an episode to rerun, or quarantine the live result. See my current-head review on #5885.
Blocking live gate. The posted Stop witnesses are from earlier engines and predate the stale-row/socket changes. There is no restarted 3ad82441 witness proving one Escape reaches T and none reaches a finished/successor/unbound turn; the inherited #5885 current-head witness is also absent.
Focused cumulative suites passed (Stop 38, activity-bus 43, agent-activity 48, gateway hooks 67, task-attempts 41), and all current GitHub checks including CLA are green. The blockers above are runtime safety issues not exercised by those happy-path checks.
finish() ignored each write. It now saves the wake's episode as seen, then its outbox item, and only then removes the attempt; any failed write returns False with the attempt kept, so the identity is never lost to a half-done finish.
…e the key interrupt() decided from a snapshot taken before capture-pane. A row or result written while the pane was read (T finishing, a successor turn starting) now aborts the Escape before the at-most-once claim, so a late Stop cannot land in the turn that followed.
|
@cla-assistant check |
|
Fixed in 56b9a29 (head 56b9a29, which also merges #5885). |
sonichi
left a comment
There was a problem hiding this comment.
Re-approving at 56b9a29 (agent-assisted, Blue for Chi's seat). The newest-row snapshot and T's result are re-read right before the Escape, and any change since the pane was read cancels the key. That narrows the read-then-send window. stop-interrupt passes locally with a stand-in tmux.
…-turn on that task [AgentActivity] (#6089) * feat(watcher): a Stop interrupts the core's turn when its pane is mid-turn on that task [AgentActivity] Rebuilt on main from #5913, behind SUTANDO_STOP_INTERRUPT (off by default). The stalled attempt's Stop instruction is a debt written with its failure, on the transition API. * fix(watcher): a Stop reaches a task parked on its first prompt, before it wrote any row The newest session row at any age decided which task the session was on, so a stale row of an earlier task blocked the Escape. When that row closed its turn and the task is the only one in the inbox, a live dialog is the task's. * fix(watcher): one session-rows module; the stalled Stop rides the switch; the changelog cites #6089 * docs(gateway): a stalled attempt's Stop is cooperative; no Escape follows it * feat(agent-activity): the Stop interrupt lives in the skill and reads the turn from Claude Code's own turn hooks * fix(agent-activity): the Escape goes through the one key sender's fenced-keys mode * test(agent-activity): cover the Stop interrupt's card expiry, fallbacks and the bus's stop request * fix(agent-activity): a Stop reads worker-seat paths, never keys a no-task turn, and rules on ready results only * fix(worker-pool): a stop instruction follows its task to the worker that holds it * fix(worker-pool): the Stop interrupt switch reaches each worker's watcher, and a change replaces it * docs(agent-activity): a worker-held stop is routed to its worker; the stop module's wording and imports * fix(gateway): a Stop's log lines name the task's turn, not the core, since a worker may hold it * fix(agent-activity): only a trusted stop source stops a task; real control ids are ruled; no stale switch; no tool input kept * fix(gateway): a served task quoting a stop is told it is no control; the hook's ruling notes go to a log, not stderr * fix(agent-activity): the runtime API's task.cancel is a trusted stop * fix(agent-activity): only the runtime API's task.cancel is a stop; untrusted numbers read through one finite() helper * test: the finite() helper on huge, non-finite and non-number values * test(agent-activity): the off/on stop test waits for both detached emits before counting
Stacked on #5885 (which is stacked on #5883), and also contains #5941, which carries the lane-id (
~) fixes this layer used to duplicate. Merge order: #5941 and #5883 → #5885 → this. The diff againstmainincludes both parents; this PR's own change isa8e2903db..aec46dfba(10 commits). The two newest:5bd1526be(a Stop answers the terminal prompt a parked turn waits on: a live dialog or an open TUI card makes the task current at any row age, and the card is expired) and31cd2d893(a task failed as stalled gets the same Escape). After #5885 lands, I'll merge main into this PR and rerun its full checks.Setting: off by default
Everything this PR adds to a cancel is behind
SUTANDO_STOP_INTERRUPT, off unless it reads as1/true/yes/on(the same values assrc/skill_config.py).src/task-emit.shmarks the cancelled task CANCELLED on arrival, as on main (activity_bus.py transition CANCELLED --reason "cancel requested"). No Escape, no confirm poller, nostop requestedrow.stop_interrupt.pyitself also exits at once when off.stop requestedat once, and CANCELLED only once the stop is confirmed.It is read from the environment. The
agent-activityskill'smanifest.jsondeclares it in itsconfigblock (default empty = off), so the launcher's manifest-config export applies a manifest value when the env leaves it unset. Both launchers (src/agent/claude/cli/start-cli.sh,src/agent/codex/cli/start-cli.sh) forward it to the task-watcher session.To turn it on in the desktop release that ships the backend stack: set
SUTANDO_STOP_INTERRUPT=1in the environment the app starts the engine launcher (start-cli.sh) with. The launcher passes it to both the core session and the watcher session. Do not edit the manifest in the engine checkout: the engine updater would treat that as local work.Always on, as fixes: the
~in a lane task id is kept by the cancel-target pattern and the activity hook (below), so the CANCELLED-on-arrival path marks the right task. The tmux socket now comes fromscripts/sutando-config.sh tmux-socket(orSUTANDO_TMUX_SOCK), not a hardcoded/tmppath.Tests (
tests/stop-interrupt.test.py, 31): off for unset,"",0,no,offand junk gives exactly the main-line CANCELLED call; on for1,true,YES,Onhands the task tostop_interrupt.py; the CLI does nothing when off; the socket comes from the helper, an explicit socket wins, and no socket means no key.Why
A Stop is cooperative (O5b). The bridge's per-task beat hears it within ~15 s and queues one
CANCEL_INSTRUCTION, but the core is a single Claude Code session that only reads the next queued task after the current turn ends. A turn blocked in a long tool call (asleep 200in the live rig) therefore stops only after that tool finishes: the cancel-ack came at +209 s for a Stop pressed at +10 s.What
When the watcher announces a
CANCEL_INSTRUCTION, it now hands the named task tosrc/stop_interrupt.py, which sends one Escape to this watcher's own pane (the same as the owner pressing Esc in Claude Code) only when all of these hold:state/agent-activity.jsonlnames exactly that task and is under 30 min old. Delivery rows (TASK_STATUS) and this module's own owner rows are ignored;deliveries/<worker>/folder, and a worker's watcher acts only on a task in its own inbox;esc to interrupt, so a turn is running.Otherwise the Stop stays cooperative. The Escape goes through the existing guarded
src/tmux-pane-keys.shsend, and the pane isSUTANDO_TMUX_PANEor the session's lowest window, the same rule as the watcher's wake helper. There is at most one Escape per task, using a marker understate/stop-interrupts/. Every decision (sent, or skipped with its reason) is logged on the watcher's stderr and as an ownernoticerow, for exampleStop: interrupted the running turn (the pane is mid-turn on this task)orStop: left the turn to finish (the session is on task-…, not task-…). The interrupt is advisory: any failure leaves the Stop cooperative, and the call is fire-and-forget from the emit, so it can't delay or fail it. Nothing changes in the ag2-sparrow bridge.Bug fixed on the way. The cancel-target pattern in
src/task-emit.shandactivity_bus.CANCEL_REdid not admit~, so a gateway instance'stask-<instance>~task-<id>was cut totask-<instance>. That marked the wrong task CANCELLED, and would have pointed the interrupt at a non-existent task. Both now keep the whole id.Evidence
Before (parent
9b241f374, i.e. #5885) and after, with this PR's tests:The 15
stop-interruptcases cover:=sutando-core:0, plus an owner row in the task's room;SUTANDO_TMUX_PANE;_run;activity_cancel_targethands the full~id tostop_interrupt.py.Related suites: 19/20 tests touching task-emit or cancel instructions pass.
watch-tasks-stream-shutdown-emit.test.shfails identically on the parent. ruff andscripts/review-checks.share clean, and diff coverage of the new module is 99% locally.Live, desktop test build .14 (engine
81d0c7397), 2026-10-30: a task parked on a terminal permission dialog insutando-core, then Stop from the room. Dismiss returned 200cancel_requested; the bridge loggedStop requested; the core got itsCANCEL_INSTRUCTION. The owner row read "Stop: interrupted the running turn (a dialog in the pane holds this task; 1 terminal prompt card(s) expired)" about 1 s after Stop, the TUI requirement card went toexpiredat the same time, and the dialog was gone 15.2 s after Stop. The ledger reacheddead/cancelled_by_user/cancel_confirmedabout 30 s after Stop. Nothing was posted to the room.Live finding on the .2 DMG (engine
5cf3c1f8a): the interrupt did not fire, so the hook was fixed (0b9200679)Mark pressed Stop at 23:31:30Z on
task-ag2space_dlocal~task-9c65da61713b5c635f(sleep 90). The CANCEL_INSTRUCTION arrived at 23:31:41Z, and stop_interrupt loggedStop: left the turn to finish (the session is on no task, not task-ag2space_dlocal~task-9c65da61713b5c635f). The only rows for that task were the bus's TASK_STATUS projections; the session's hooks had written none.The cause is the same
~gap as the cancel parser.skills/agent-activity/hooks/activity-hook.py'sTASK_ID(task-…[\w-]*) stops at~, sotasks/task-<instance>~task-<id>.txtnever matched. A lane task was never bound to the session and got noprocessingorworkingrows, so the owner saw none either. The pattern now admits~. The safety rule is unchanged: Escape is sent only when the newest session-hook row names exactly the stopped task.python3 tests/agent-activity.test.py:tests/stop-interrupt.test.pyadds the live row sequence: with only the TASK_STATUS rows there is no key; with the hook'sprocessingandworkingrows for the task there is exactly one Escape.