[hussainph]

Problem

A delegated subagent that leaves a shell_start process running (e.g. a dev server) keeps it after its turn completes: the child's attachment stays open, and shells only die with the attachment. When the process later exits, crashes, or a person kills it from the Island, the shell notice is steered into the answered child and opens a new turn. agents.ts re-arms the delegation on any turn.started of an unwatched subagent, so that reactive turn's end delivers a second, indistinguishable "completed its task" notice to the parent. Meanwhile the child may edit the shared tree.

Fix

• Shells end at the VC-754 boundary. In PiBinding#observe, a subagent turn that completed now calls the shell port's dispose() next to the headless-tab cleanup. That is BackgroundShellHost.disposeSession, the door the attachment's end already uses: it kills every shell the child owns (SIGTERM to the group, then SIGKILL), drops pending exit notices, and forgets the entries, so no exit or match notice follows (VC-495's "a Session that has ended is told nothing"). The port stays usable, so a turn a person resumes the child with can start shells, and those are retired at that turn's end too. As with the tab cleanup, a failure is logged and never costs the delegate its answer, and the attachment's end still disposes.
• Re-arm only for resumptions. Delegations.rearm on an answered delegation reads the child's projection. If the wake's turn is still the latest and its recorded origin is a host notice (watch-, subagent-, browser-, shell-, worktree-notice), nothing is armed and no :resume:…:answer command is sent.
• CONTEXT.md (Subagent, Background shell) states both rules.

Decisions

• Deny-list of host-notice origins, not an allow-list of user/parent session. This is the conservative reading. Every origin that is not a host notice keeps the VC-457 re-arm: unknown/null (e.g. two pending commands at turn start), automation, relaunch-recovery, scheduled-resume and supervision. So a real resumption is never silenced by an origin we failed to attribute.
• Only answered delegations are filtered. An unanswered one (its notice never landed) still reports on any turn: the parent was never told, so the report is not a duplicate. This also leaves VC-799's (VC-799: Hold a Session's kickoff through a failed first attach and deliver it on Retry #926) held-task path untouched, since it re-arms with answered: false under a parent session origin.
• The origin read fails open. A projection read that throws is reported and re-arms, as before. A turn superseded before the read (latestTurnId ≠ the wake's turn) also re-arms.
• Steered messages count as resumptions (VC-809 review). A non-host-notice message.submit that joins a running turn re-arms on that turn, so a lost report is never the failure mode; at worst there is a duplicate.
• Shells are killed rather than only muted. The ticket offered "end, or at minimum suppress notices". Ending them also releases the ports and CPU that nobody can reach. Board and Ticket Sessions' shells are unchanged (asserted).

Review (supervisor review at 1ad6eef, landed in 9669aa9 and a test-placement follow-up)

• SHOULD 1, required: done. A turn's origin is recorded only from the command pending at turn.started. So a person's message, or the parent's session_send, that joins a settled child's host-notice turn kept that notice's origin, and the parent was never told. That is worse than the old duplicate. Now agents.ts also wakes on a child's command.recorded message.submit. Delegations.rearmJoined reads the command's origin with commandOriginOf, from the slim field or the provenance attribution:
  • It returns for a host-notice origin, for an unanswered delegation (its turn's start already re-arms), and when the child is idle (the turn that message opens re-arms at its own start).
  • Otherwise it watches the running turn under resumedNoticeIds(op, turnId). A racing rearm for the same turn spends the same ids, so the parent hears once.
  • An unreadable projection is reported. Queued follow-ups are not message.submit commands until they are released, so every mid-turn message.submit is a steer that joins the turn.
  • New unit cases: a parent steer joining a shell-notice turn gives exactly one report (a second joining message and a late rearm add nothing); host-notice and idle cases arm nothing; unanswered is left alone; unreadable projection; commandOriginOf.
• SHOULD 2: done, in the shell tools' descriptions rather than the prompt. The prompt bundle does not record whether the shell tools are present, and the descriptions are what a subagent reads at the call. shell_start now says: "A Subagent Session's shells end silently when its turn completes, so a subagent reads what it needs or waits inside the turn." shell_kill's "readable until the attachment ends" now adds the subagent turn-end case. These descriptions are role-neutral, so the provider tool array changes once for every Session.
• SHOULD 3: done. The integration test takes each group leader's pid from shell_start's own result ((pid N), known at the spawn) instead of a pid file the shell writes, which could race the turn's end. It also asserts that the project parent's group is still alive.
• Doc nits: done. I fixed the docs rather than narrowing the kill to running shells, so a completed subagent turn forgets every shell it owns, running or exited, and publishes their removal. Updated: the AttachmentShellPort doc ("forgets only the shells it killed" was wrong), background-shell-host.ts's WHAT IT REMEMBERS / WHAT ENDS IT and the disposeSession doc, CONTEXT.md's Background shell (exited shells' output is forgotten at turn end for subagents), and CONTEXT.md's Subagent entry (a steered message still reports).
• Optional Record<reason, boolean>: done. IS_HOST_NOTICE covers every volli reason, so a new reason fails to compile until it is classified.
• Optional foreground-cancel wording: declined. No execute call can be running at a completed turn's boundary, so disposeSession's "attachment ended" cancel message cannot reach a model from this path.
• Scope gap, noted: failed or interrupted subagent turns still keep their shells, following VC-754's rule that only a completed delegate retires its leftovers. A shell left by an interrupted child can still reopen it, but that turn's origin is a host notice, so it no longer re-arms the parent.
• Fakes typed as Delegations (agent-tool-door.test.ts, facade.test.ts) gained rearmJoined. facade.test.ts also routes a steered message.submit wake to rearmJoined with its origin, and filters out other commands. That keeps agents.ts at the protected 100% branch coverage, which run 39256414893 flagged. Its ./delegate-session mock now spreads the real module, for commandOriginOf.
• Local: delegate-session.test.ts 59/59, delegate-browser-cleanup.integration.test.ts 4/4, agent-runtime shell-tools.test.ts 24/24, facade.test.ts 19/19, each run once on one worker through /tmp/volli-serial.sh; vp check is clean on all touched files.

Overlap

Stacked on #929 (VC-778 crash-recovery fixture fix, test-only): its head 637c7d1 is merged into this branch with an additive merge commit, on the supervising session's direction, instead of a further rerun. Merge #929 first; this PR's own diff is otherwise unchanged.

git merge-tree is clean against origin/main and the heads of #909, #919, #920, #921, #922, #926 (VC-799, which also edits delegate-session.ts and its test) and #930. The first cut collided textually with #926 because both inserted a helper at the same line, so the helper was moved below answerOf. Likewise, the review round's new test blocks sit before the VC-457 rearm block rather than where #926 adds its own. No stacking on #926 needed. #651 and #873 also touch CONTEXT.md, but both already conflict with main on their own.

Tests

Each new test was run on one worker through the machine's serial runner.

• delegate-session.test.ts (53 tests; 5 fail on main, all pass with the fix):
  • Host-notice turns (shell, browser, watch, worktree) in a settled child arm nothing and send no :resume: command.
  • A person's, the parent's and an unknown origin still re-arm.
  • A superseded turn still re-arms.
  • An unreadable projection re-arms and reports.
  • An unanswered delegation still re-arms on a shell-notice turn.
• delegate-browser-cleanup.integration.test.ts: new VC-809 case, a real BackgroundShellHost through the real runtime and Pi adapter (fails on main at "no live shells after the first turn"):
  • The child's shell_start process group is gone after its turn completes and the attachment stays open.
  • The project parent's shell keeps running.
  • A person's resume starts a shell without error, and it is retired at that turn's end.
  • No shell notice fires, and the child has exactly 2 turns.
• vp check on the touched files.

CI notes

• First run, on 2d3ce02, failed for two reasons:

  • A type error in the new fixture's surface typing, fixed in the next commit and confirmed with one tsc --noEmit -p packages/host-core.
  • crash-recovery.integration.test.ts › reconciles an accepted command with a different persisted timestamp… read rejected. That is the known flake VC-778; the test involves no subagent or shell. The fix push re-ran CI; no manual rerun was used.

• Second run, on a9382cb: everything green except Test (packages), which failed on the same VC-778 file with expected { closed: 1, errors: [ … ] } to deeply equal { closed: 0, errors: [] }. Main's own latest CI (run 39138198050, 7170501) fails this file with the same assertion, and the root cause is a test-only 50 ms bound (fix pending in VC-778: Bound only deliberately stalled stages in the crash-recovery fixture #929). Rerun disclosed: gh run rerun 39245944393 --failed, once, for the known flake VC-778.

• That rerun cleared VC-778, and every check except Test (packages) is green on a9382cb. Test (packages) then failed only on worktree/watch.test.ts › pollRetention — terminal PRs are not re-polled (VC-836) › 500 Done tickets … cost 10 gh calls a cycle, which timed out (12.3 s against a 5 s limit). That is the filed flake VC-856; the worktree watcher has nothing to do with this change. VC-856 is not on this session's pre-approved rerun list, so no further rerun was made without approval.

• Then, on the supervising session's direction, VC-778: Bound only deliberately stalled stages in the crash-recovery fixture #929 (head 637c7d1) was merged into this branch instead of rerunning again; the new run is on the merge commit.

• Review round: run 39256414893 (60c3e4a) failed only the protected branch-coverage gate on agents.ts. After the coverage commit, run 39258089500 (4edbfed) is all green on the first run, with no rerun.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 90539039-9a85-4bdc-90e0-8e64fca378b9

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}