Skip to content

fix(agent-status): reflect Claude's idle-prompt Ctrl+C killing background subagents - #22722

Open
brennanb2025 wants to merge 8 commits into
mainfrom
brennanb2025/claude-idle-ctrl-c-subagents
Open

brennanb2025 wants to merge 8 commits into
mainfrom
brennanb2025/claude-idle-ctrl-c-subagents

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​580 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​580
Prod 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​208 $\color{#cf222e}{\Huge{\mathbf{−}}}$​8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​200

ELI5

In a Claude Code terminal you can have background helpers running: a background shell (some long command) and a background agent (a helper AI working on a side task). Pressing Ctrl+C once while Claude sits idle at its prompt (or using "stop all agents") makes Claude kill every background agent at once. Background shells keep running, and Claude tells nobody: no hook fires.

Orca's sidebar didn't know this, so it kept showing the killed agent as "working" until the next turn ended. Claude does write one line about it in its own session log. Orca now watches that log, only while an agent is running, and when the line appears it removes the killed agents from the row. A surviving shell keeps the row on "Monitoring background tasks", and the row turns "done" only when nothing is left.

What Changed

Before: a Claude pane with a background shell and a background agent; the main turn has finished, so the row reads working because of the agent. One Ctrl+C at the idle prompt kills the agent right away (measured on Claude Code 2.1.280), but Orca kept showing it as working until the next Stop hook listed what was really left.

After: within about a second of the kill, the killed agents leave the row. A teammate-style agent is shown as idle instead of removed, which is what its own stop event would do. Shells and scheduled checks stay, and the main agent's result, clock and last reply are unchanged: the kill ends no turn, so no cancellation and no "interrupted" flag are invented. This works the same for local panes and for SSH and WSL panes.

Mechanism: when Claude really kills every running background agent, it appends one line with no agent ids to its session transcript: {"type":"system","subtype":"agents_killed","timestamp":...}. Orca's shared hook listener already reads transcripts for other agents. It runs on whichever machine owns the session: the desktop for a local pane, the SSH relay or the WSL guest relay for a remote one. That listener now watches a Claude pane's transcript, but only while the pane has a working agent child and a transcript path it can open locally:

  • The watch starts at the file's current end, so older lines never count. --resume keeps old lines in the same file, and "Move to background and exit" forks into a new file that copies the old line.
  • It reads only the bytes added since the last check (capped at 1 MiB), on the same 1-second transcript-poll timer the Codex and Muse lanes already use in both the desktop and relay listeners.
  • It stops when the pane has no working agent child, when the file can't be read, or when the pane closes. An idle session costs nothing.
  • A new agents_killed line retires the children that started by the line's timestamp, with SubagentStop semantics. A child that started after the kill survives.
  • The result is published through the existing status store as that child's own SubagentStop row. Because the row is tied to a child, the desktop keeps a Ctrl+C cancel it inferred earlier for an SSH pane: the relay never learned of that cancel.
  • There is no new wire message, field or reader-side rule.

The keypress inference this PR first used is deleted completely. A mid-turn Ctrl+C is still inferred as a cancel, unchanged from #22476.

The trigger was established on real Claude Code 2.1.280 captures (committed as src/shared/__fixtures__/claude-idle-ctrl-c-*-hooks.jsonl, plus raw runs recorded outside the repo):

  • The line is written only when all running background agents were really killed: an idle Ctrl+C with or without an unsent draft, a Ctrl+C with the footer agent list focused, or ctrl+x ctrl+k ("stop all agents"). It is appended about 110–190 ms after the keypress.
  • It is not written for:
    • a mid-turn Ctrl+C or Esc (agents survive);
    • stopping a single agent (that already starts a new turn with hooks);
    • a Ctrl+C that /tasks, / or /model swallowed;
    • a shell-only session;
    • an agent that finished on its own.

Older hosts: an SSH or WSL relay from before this change has no watcher. On those hosts a killed agent stays on the row until the next Stop, which is the same as before this PR.

After an Orca restart: Orca restores a settled main agent only when its row says no background shell was running, because it cannot prove a shell survived the restart. So if Orca restarts while a local Claude pane has both a background shell and a background agent, a kill that happens before the main agent's next hook is not reflected until Claude sends its next hook, the same as before this PR.

Windows limit: the Windows desktop listener watches only drive-letter paths. If a WSL pane's hooks reach the Windows listener directly instead of the guest relay (mirrored networking or WSL1, while the guest relay isn't running), its transcript path is a guest Linux path. Orca does not watch it, because a synchronous read across the WSL file share can stall the main process. The guest relay watches such sessions natively when it runs. Otherwise the behavior is as on an older host.

Why

  • Keypress guess (this PR's first version), rejected. A Ctrl+C that Claude's /tasks panel swallows kills nothing, and we observed this live. Guessing from the keypress would have hidden an agent that was still running. The desktop also never learns of a keypress's effect on an SSH or WSL host, so the remote relay kept restating the dead agent.
  • Transcript line, chosen. The line is Claude's own record of exactly the kill. Reading it on the machine that runs the session puts the fact where the status store's rule says it belongs: the execution host owns agent status.
  • Always-on transcript polling, rejected. It would cost every idle session. The watch is armed only while an agent child is working, and it reuses the existing poll timer and incremental reader rather than adding a scheduler.

Linked Issue

No standalone issue. This was found during live cancel QA of #22476 (Claude, idle Ctrl+C left a killed subagent showing as working).

Fixes: the stale "working (subagent)" row after Claude kills its background agents at the idle prompt.

Visual Proof

Live run on an isolated Orca dev instance with real Claude Code 2.1.280 in a terminal pane: a background shell (sleep 600) and a background agent, main turn finished.

Before: the row is working, with the background agent listed as a child.

before

After ONE Ctrl+C at the idle prompt: Claude prints "All background agents stopped", the agent child leaves the row within about a second, and the surviving shell keeps the row on "Monitoring background tasks".

after

Ctrl+C while Claude's /tasks detail panel is open: Claude swallows the key and kills nothing, and Orca correctly keeps the agent as working (this is the case the removed keypress guess got wrong).

tasks panel

Agent only, no shell: after the kill the row settles to done.

agent only

Over SSH (throwaway Linux container as the SSH host): the transcript exists only on the remote host, and appending the agents_killed line there retires the child on the desktop row, so the watch ran on the remote host. Before and after:

ssh before

ssh after

Testing

  • Captured hook payloads replayed through the server's real HTTP ingress, on the capture's own clock, with the captured agents_killed line appended to a temp transcript (src/main/agent-hooks/server-claude-idle-ctrl-c-captures.test.ts). Cases:

    • Retire and keep monitoring the shell, with the Stop's turn stamp kept so no second completion notification fires.
    • Settle to done when no shell is left.
    • The unsent-draft case.
    • A local mid-turn cancel followed by a kill.
    • A line already in the file when the watch arms.
    • A line stamped before a child's start.
    • A fork into a new file that copies the old line.
    • A line written while the watch is off.
    • Pane close.
    • The shell-only and already-finished cases never arm.
  • Real RelayAgentHookServer tests (server-relayed-claude-cancel.test.ts):

    • SSH retirement, and a reconnect replay does not bring the child back.
    • A teammate-style child is parked idle.
    • The desktop's inferred mid-turn cancel is kept through the relay's retirement.
    • A desktop restart does not re-add the killed child.
  • Unit tests for the path rule, arming and disarming (a relayed row is never watched), the waiting-child case, and the roster function.

  • Each safeguard was removed on its own, and the removal made a test fail for the right reason:

    • Removing the Claude watch: all 13 retirement stories fail.
    • Dropping the child attribution: the relayed mid-turn cancel test fails because the desktop holds the row.
    • Starting the watch at offset 0: the old-line, fork and written-while-off tests retire a child.
    • Dropping the timestamp check: a child that started after the kill is retired.
    • Tying the poll to the hook that armed it instead of the current row: the local mid-turn-cancel test never retires.
    • Keeping the reading position after the watch stops: a stale line retires a later child.
  • pnpm tc, oxlint on the touched directories, and pnpm run check:code-quality:changed pass. The full src/main/agent-hooks, src/shared/agent-hook-listener and src/relay suites pass; two unrelated timing flakes (src/relay/subprocess.test.ts, managed-hook-script-refresh.test.ts) passed when re-run.

  • Live Electron QA with real Claude Code 2.1.280 on macOS, all passing: idle Ctrl+C with a shell and an agent (monitoring), agent only (done), unsent draft, shell only (unchanged), /tasks panel open (nothing retired), Ctrl+C inside one agent's view (only that agent leaves, through Claude's own hooks), the next turn (the killed agent does not come back), mid-turn Ctrl+C (agent survives and stays listed), and "stop all agents". Retirement showed 0.7–1.4 s after the kill, with no notification for the killed agent.

  • SSH QA over a real SSH connection to a throwaway Linux container: Claude cannot run there without copying credentials in, so its captured hook payloads were replayed through the real installed hook command in a real remote Orca terminal, and the captured agents_killed line was appended to the remote transcript. Retirement, reconnect replay (the child does not come back), a stray line with no live child, and a line already in the file before the child started all behaved as described. Windows and WSL were not run live.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Author: @BrennanKB5

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Wire compatibility: nothing new on the wire. A current relay publishes a child SubagentStop row that older desktops already understand; an older relay publishes nothing new (see Older hosts).
  • Codex and Grok idle-Ctrl+C behavior is a different problem and out of scope: Codex quits outright.
  • docs/reference/agent-status-store.md (already tracked) is updated to describe the watch.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@brennanb2025
brennanb2025 marked this pull request as ready for review September 24, 2026 23:42

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Idle-prompt Ctrl+C inference for Claude — a new branch in server-status-inference.ts inferInterrupt admits the keypress when the main agent is settled and the row still has working agent children, retiring exactly those children (one-shots leave, teammate-shaped ids park idle) instead of refusing the cancel.
  • Local vs relayed retirement — local panes retire the listener roster (markClaudeBackgroundAgentsStopped); relayed panes retire the row's own snapshots and re-fold, since the provider records live on the relay.
  • No verdict fabricated — the settled main agent's mainAgent verdict and clock pass through untouched; interrupted is only restated if already present, and shells/crons are never retired.
  • Captured-evidence tests + fixtures — four scrubbed real-PTY captures (claude-idle-ctrl-c-*) replayed through the server's HTTP ingress cover retire-and-monitor, settles-to-done, the unsent-draft double-press, and the two no-ops; relayed coverage plus roster helper unit tests.
  • Docs — docs/reference/agent-status-store.md updated to describe the mirrored behavior.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…ound subagents

Measured on Claude Code 2.1.280 (fixtures claude-idle-ctrl-c-*): ONE Ctrl+C at
the idle prompt kills every background agent immediately, paints 'All
background agents stopped', and fires no hook; background shells and crons
survive, and only the next turn's Stop inventory reflects the kill. Orca's
inferred-cancel gate refused the keypress outright on a settled mainAgent,
so the dead subagent kept the row 'working' until the session ended.

The inference now mirrors exactly what the CLI does: on a Claude row whose
mainAgent is settled, a Ctrl+C retires the row's agent children (listener
roster locally, row snapshots for a relayed pane; teammate-shaped ids park
idle) and touches nothing else - no verdict, no shell/cron retirement. The
row keeps monitoring a surviving shell and settles to done when nothing is
left. A child the CLI did not kill is re-added by its own next hook, and
every later inventory re-derives the truth.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/claude-idle-ctrl-c-subagents branch from c138184 to f7f7e36 Compare September 25, 2026 04:27
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/lead-status-pr-c to main September 25, 2026 04:27

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Captured unsent-draft evidence — added the claude-idle-ctrl-c-draft-hooks jsonl plus meta sidecar, a real 2.1.280 PTY capture showing that with unsent text at the prompt, one Ctrl+C still kills the background agent (time.sleep(240) dies, sleep 600 survives) and a second leaves the shell untouched, with no hook on either press.
  • Draft regression test — new an unsent draft before idle-prompt Ctrl+C (captured) case replays hooks 0–10 through the server's HTTP ingress, asserts the first inference retires the agent into monitoring with no fabricated interrupted, and the second keypress is a no-op.
  • Fixture loader fields — CapturedCancel gained optional draft_present/ps_before/ps_after; a small parseCapturedBackgroundTask narrowing helper replaced an inline Reflect.get filter in the existing settle-to-done test.

This commit is test-only — the prod inference path is unchanged from the previously approved revision, and the new case passes (46 tests green across the two suites run).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

Claude transcript polling now detects newly appended system/agents_killed records for eligible local panes with working agent children. It retires children by timestamp and publishes a SubagentStop update when lead status is available. Server and relay polling support Claude, and pane cleanup removes its transcript cursor. New captures and tests cover local and relayed cases, including shell-only work, draft cancellation, natural completion, and pane closure.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 8ca1e

Idle Ctrl+C agent kills in Claude are now detected from the session transcript, and the row updates to show surviving shell work or done. In one narrow case, a pane with no recorded main-agent state, the killed agent can stay visible on the row and reappear after a restart. The change is mergeable, but this small follow-up should ideally be fixed first.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 8ca1e

Background-task status can now change based on a local activity record. The update is scoped to a session, but the record is not visibly tied to that session’s identity, so someone able to alter it could make a working child appear stopped. No broader execution authority is established.

Retained concerns

  • Medium · security · inferred: A party able to append a plausible kill record to the watched transcript can cause working children to be retired without a session- or writer-identity check on that record. This is a conditional status-integrity concern, not evidence of unauthenticated remote access or unauthorized tool execution.
Security review details

Security Blast Radius

  • inferred — The demonstrated effect is integrity of child-status and pending-wait state for a watched pane, including status forwarded from a relay host. The evidence does not show file-content exfiltration, cross-pane mutation, or gained execution privileges.

Security Findings and Attack Paths

  • inferred — If an actor can append an accepted record to the file being watched, the record can make a working child appear stopped and clear its pending wait. The supplied Security assessment retains no verified finding, and the actor’s access to a production transcript has not been established.

Trust Boundaries and Controls

  • observed — The relay hook endpoint checks a token, while transcript polling separately requires a local Claude row with working children and reads only records appended after arming. These checks constrain exposure but do not authenticate the transcript writer or the identity of an individual kill record.

Resilience and Maintainability Implications

  • inferred — Discarding a cursor after a transient read error, or skipping an oversized unread prefix, can preserve stale status until another authoritative update. This limits the new recovery mechanism; it does not by itself show a worse outcome than the former absence of kill detection.

Hardening Proposals

  • proposed — Bind the watched file and accepted lifecycle records to the active session where a trustworthy identity is available; preserve or reconcile the unread cursor position after transient failures and bounded-read skips.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 15 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description is detailed and covers the change, rationale, behavior, testing, visual proof, compatibility, and checklist. However, the required Linked Issue reference is missing; it states there is… Add a valid linked issue reference in the Linked Issue section, such as Fixes #12345``. If no issue exists, create or link the required tracking issue before merging.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: updating agent status when Claude kills background subagents after an idle-prompt Ctrl+C.
Full details: Docstring Coverage

Explanation

Docstring coverage is 48.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 15 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description is detailed and covers the change, rationale, behavior, testing, visual proof, compatibility, and checklist. However, the required Linked Issue reference is missing; it states there is no standalone issue and does not provide a valid issue number.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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

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

…retires agents

The synthesized row rebuilt the payload from a field whitelist, so the
keypress also erased the main agent's last reply and tool from the row.
Carry the existing payload forward and replace only child-derived fields.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Payload carry-forward on idle Ctrl+C retirement — applyClaudeIdleBackgroundAgentStop no longer rebuilds the synthesized row from a field whitelist; it spreads the prior payload and recomputes only the six child-derived fields (state, workingMode, subagents, interrupted, turnCompletedAt, mainAgent), so the settled turn's lastAssistantMessage, toolName, and toolInput survive the keypress.
  • Reply-preservation assertions — the local and relayed idle-Ctrl+C captures now pin lastAssistantMessage: 'STARTED' (plus toolName: 'Agent' locally) on the row both before and after the keypress, which would fail against the old whitelist.

The fix is correctly scoped: the six excluded fields are exactly those the retirement recomputes, and interrupted/turnCompletedAt/mainAgent are still re-added under their original conditions, so the settled verdict and clock pass through unchanged. Ran the three affected suites (58 passed) and pnpm tc (clean).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…t's agents_killed record

Claude kills every background agent on an idle-prompt Ctrl+C (and on "stop all
agents") without firing a hook; its only trace is an id-less
system/agents_killed line in the session transcript. Replace the keypress
inference with that fact: the shared hook listener, on whichever host runs the
session (desktop for local panes, the SSH/WSL relay for remote ones), arms a
bounded transcript watch at the file's end while the pane has working agent
children, and on a new agents_killed line retires the children that started
by then with SubagentStop semantics. The result is published as a
child-attributed SubagentStop row through the existing store, so the desktop's
cancel latch re-folds it on SSH panes. Both existing transcript-poll schedulers
carry the watch; no new wire message.

The keypress path (server inference branch, idle-agent-stop provider and the
snapshot retirement) is deleted.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The rework from keypress inference to the transcript watch is sound, and the captured evidence is strong. One regression to fix before merge.

Reviewed changes

  • Replaced keypress inference with a session-transcript watch — the earlier ctrl-c-while-idle branch and its roster mutation are gone; the id-less system/agents_killed line Claude appends to its own transcript is now the sole trigger.
  • Added the watcher provider (claude-agents-killed-transcript.ts) — syncClaudeAgentsKilledWatch arms at the file's end only while the pane has a working agent child and no connectionId, re-arms on path change, and pollClaudeAgentsKilled retires children that started by the line's timestamp and publishes one SubagentStop row with no fabricated verdict.
  • Factored the transcript-poll policy into shared helpers (transcriptPollAnchor, hookTranscriptPollUpdate) used by both the desktop server and the relay scheduler, and taught both scheduleTranscriptPoll paths to accept claude.
  • Added stopWorkingClaudeSubagentsStartedBy with SubagentStop semantics (one-shot leaves, teammate-shaped parks idle, later starters survive) and createJsonlCursorAtEnd.
  • Replaced the fixtures with four captured Claude 2.1.280 transcripts (bg-agent, unsent-draft, shell-only, already-finished) plus relay and unit coverage; docs/reference/agent-status-store.md updated.

This is a large behavioral rewrite of previously approved code, so the findings below are against the new mechanism.

ℹ️ A working child restored from a snapshot never arms the watch

The watch only starts when a hook reaches scheduleTranscriptPoll. After an Orca or relay restart, seedClaudeSubagentRosterFromSnapshots restores a working child into the roster, but hydration does not arm a watch, and the only other arm sites are the HTTP ingress handlers and the poll's own reschedule. So a pane that comes back with a running background agent, then gets an idle Ctrl+C before any further hook, writes agents_killed with nobody reading it — the stale child persists until the next Stop's inventory, which is the pre-PR behavior.

Technical details
# Restored working child does not arm the watch

## Affected sites
- `src/main/agent-hooks/server/server-hydration.ts:126-133` — seeds the roster/lead turn from persisted status, no scheduling.
- `src/shared/agent-hook-listener/providers/claude-roster-state.ts:210` — `seedClaudeSubagentRosterFromSnapshots` populates working rows.
- Only `scheduleTranscriptPoll` callers: `server-lifecycle.ts:135` (HTTP ingress), `agent-hook-server.ts:287` (relay HTTP ingress), and the reschedule inside `runTranscriptPoll`.

## Required outcome
- Either accept this explicitly (it degrades to the documented older-host behavior), or arm the watch once for hydrated panes that have a working Claude child and a readable local transcript path.

## Open questions for the human
- Is restart coverage intentionally out of scope? The PR body describes the older-host degradation but not the post-restart case.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

return pollClaudeAgentsKilled(state, anchor)
}
const polled = normalizeHookPayload(state, source, body, env)
return polled && transcriptPollUpdate(source, anchor, polled)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polled && transcriptPollUpdate(...) returns undefined, not the documented null, when normalizeHookPayload fails — so the update === null early return in both runTranscriptPolls (desktop and relay) is dead, and Codex/Muse transcript polls now reschedule on every tick instead of stopping. That drops the if (!normalized) return guard the pre-refactor code had.

Technical details
# `hookTranscriptPollUpdate` never signals stop-on-failed-normalization

## Affected sites
- `src/shared/agent-hook-listener/transcript-poll-policy.ts:54-55` — `return polled && transcriptPollUpdate(...)`; when `polled` is `undefined` this yields `undefined`.
- `src/main/agent-hooks/server/server-status-retries.ts:78` — `if (update === null) return` is unreachable.
- `src/relay/agent-hook-result-retry-scheduler.ts:115` — same unreachable branch.

## Required outcome
- A failed re-normalization must stop the poll (or the `null` contract and its two branches must be removed). The `undefined` result must keep meaning "transcript added nothing, keep polling".

## Suggested approach
- `return polled ? transcriptPollUpdate(source, anchor, polled) : null`

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c5c49785-837a-4a71-ac5e-93ed3c91441a

📥 Commits

Reviewing files that changed from the base of the PR and between 04c59d6 and 8ca1e1a.

📒 Files selected for processing (13)
  • docs/reference/agent-status-store.md
  • src/main/agent-hooks/server-claude-idle-ctrl-c-captures.test.ts
  • src/main/agent-hooks/server-relayed-claude-cancel.test.ts
  • src/main/agent-hooks/server/server-state.ts
  • src/main/agent-hooks/server/server-status-retries.ts
  • src/relay/agent-hook-result-retry-scheduler.ts
  • src/shared/agent-hook-listener/listener-state.ts
  • src/shared/agent-hook-listener/providers/claude-agents-killed-transcript.test.ts
  • src/shared/agent-hook-listener/providers/claude-agents-killed-transcript.ts
  • src/shared/agent-hook-listener/transcript-poll-policy.ts
  • src/shared/claude-subagent-roster.test.ts
  • src/shared/claude-subagent-roster.ts
  • src/shared/codex-rollout-jsonl-cursor.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Comment on lines +96 to +108
const retired = stopWorkingClaudeSubagentsStartedBy(roster, killedAt)
if (retired.length === 0) {
return undefined
}
clearClaudePendingWaitForAgent(state, paneKey, (agentId) => retired.includes(agentId))
if (roster.size === 0) {
state.claudeSubagentRosterByPaneKey.delete(paneKey)
}
// Why: null without a main agent record — the kill ends children, it never wakes a parent.
const payload = buildClaudeCachedLeadStatusPayload(state, 'SubagentStop', paneKey, {})
if (!payload) {
return undefined
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- changed file diff ---'
git diff --unified=30 f9356d491ab535022e0c6381d0941bd9d7bf4eff 8ca1e1a1f20fba3cf2f92d81d047867718f49181 -- src/shared/agent-hook-listener/providers/claude-agents-killed-transcript.ts
printf '%s\n' '--- changed file source ---'
cat -n src/shared/agent-hook-listener/providers/claude-agents-killed-transcript.ts
printf '%s\n' '--- payload builder definitions/usages ---'
rg -n -C 12 'buildClaudeCachedLeadStatusPayload|endedChildWork' src
printf '%s\n' '--- cited tests ---'
rg -n -C 20 'pollClaudeAgentsKilled|agents_killed|endedChildWork' src/shared/agent-hook-listener --glob '*test*' --glob '*spec*'
printf '%s\n' '--- hydration and persistence symbols ---'
rg -n -C 10 'claudeLeadStateByPaneKey|claudeSubagentRosterByPaneKey|mainAgent|persist.*Claude|Claude.*persist|hydrate' src/shared

Repository: stablyai/orca

Length of output: 45653


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions

Length of output: 18343


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- lifecycle builder locations ---'
rg -n 'export function buildClaudeCachedLeadStatusPayload|function buildClaudeCachedLeadStatusPayload|endedChildWork|seedClaudeStateFromSnapshot|seed.*Claude.*Snapshot' src/shared/agent-hook-listener/providers src/shared --glob '*.ts'
printf '%s\n' '--- lifecycle builder ---'
sed -n '1,240p' src/shared/agent-hook-listener/providers/claude-lifecycle-events.ts
printf '%s\n' '--- transcript listener tests ---'
sed -n '1,240p' src/shared/agent-hook-listener-claude-agents-killed-transcript.test.ts
printf '%s\n' '--- Claude roster hydration/persistence references ---'
rg -n -C 8 'claudeLeadStateByPaneKey|claudeSubagentRosterByPaneKey|mainAgent' src/shared/agent-hook-listener src/main --glob '*.ts' | head -n 1200

Repository: stablyai/orca

Length of output: 10790


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- matching test files ---'
git ls-files | rg 'claude.*(killed|transcript)|agent-hook-listener.*claude|claude.*agent-hook'
printf '%s\n' '--- killed-transcript test references ---'
rg -n -C 18 'pollClaudeAgentsKilled|agents_killed|expects.*undefined|undefined.*payload' src --glob '*.test.ts'
printf '%s\n' '--- Claude snapshot seed definition and callers ---'
sed -n '180,280p' src/shared/agent-hook-listener/providers/claude-roster-state.ts
rg -n -C 18 'seedClaudeSubagentRosterFromSnapshots|seedClaude.*Lead|mainAgent.*done|state\.claudeLeadStateByPaneKey' src --glob '*.ts' | head -n 1600
printf '%s\n' '--- payload publication/persistence call sites ---'
rg -n -C 15 'buildClaudeStatusPayload|normalizeClaudeSubagentLifecycleEvent|pollClaudeAgentsKilled|agentStatusChanged|persist.*status|status.*persist' src/shared src/main --glob '*.ts' | head -n 1800

Repository: stablyai/orca

Length of output: 45667


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- exact transcript listener test ---'
cat -n src/shared/agent-hook-listener/providers/claude-agents-killed-transcript.test.ts
printf '%s\n' '--- roster snapshot seeding ---'
cat -n src/shared/agent-hook-listener/providers/claude-roster-state.ts | sed -n '180,280p'
printf '%s\n' '--- relevant seed/set locations ---'
rg -l 'seedClaudeSubagentRosterFromSnapshots|claudeLeadStateByPaneKey\.set' src/main src/shared --glob '*.ts' | while read -r f; do
  echo "--- $f"
  rg -n -C 12 'seedClaudeSubagentRosterFromSnapshots|claudeLeadStateByPaneKey\.set' "$f"
done
printf '%s\n' '--- poll caller ---'
rg -n -C 25 'pollClaudeAgentsKilled' src/main src/shared --glob '*.ts'

Repository: stablyai/orca

Length of output: 41984


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- transcript poll update path ---'
rg -n -C 25 'hookTranscriptPollUpdate|shouldPollHookTranscript|transcriptPoll' src/main/agent-hooks src/shared/agent-hook-listener --glob '*.ts'
printf '%s\n' '--- status mutation and persistence path ---'
rg -n -C 20 'ingestHook|ingest.*Status|lastStatusByPaneKey|scheduleStatusPersist|notifyStatusChangeListeners' src/main/agent-hooks/server --glob '*.ts' | head -n 1400

Repository: stablyai/orca

Length of output: 43181


Publish the roster transition when no lead state exists.

When the kill retires the last child, buildClaudeCachedLeadStatusPayload returns null without lead evidence. The poll then returns undefined, disarms the transcript watch because no working child remains, and leaves the last published row unchanged. A persisted working row can therefore retain the killed child and seed it again after restart.

Pass endedChildWork: true so the builder publishes the roster as unconfirmed working without marking the lead complete.

🐛 Suggested fix
-  // Why: null without a main agent record — the kill ends children, it never wakes a parent.
-  const payload = buildClaudeCachedLeadStatusPayload(state, 'SubagentStop', paneKey, {})
+  // Why: the kill ends children and never wakes a parent; without a main agent record the
+  // roster change still has to reach the row, as unconfirmed working (never a completion).
+  const payload = buildClaudeCachedLeadStatusPayload(state, 'SubagentStop', paneKey, {}, {
+    endedChildWork: true
+  })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const retired = stopWorkingClaudeSubagentsStartedBy(roster, killedAt)
if (retired.length === 0) {
return undefined
}
clearClaudePendingWaitForAgent(state, paneKey, (agentId) => retired.includes(agentId))
if (roster.size === 0) {
state.claudeSubagentRosterByPaneKey.delete(paneKey)
}
// Why: null without a main agent record — the kill ends children, it never wakes a parent.
const payload = buildClaudeCachedLeadStatusPayload(state, 'SubagentStop', paneKey, {})
if (!payload) {
return undefined
}
const retired = stopWorkingClaudeSubagentsStartedBy(roster, killedAt)
if (retired.length === 0) {
return undefined
}
clearClaudePendingWaitForAgent(state, paneKey, (agentId) => retired.includes(agentId))
if (roster.size === 0) {
state.claudeSubagentRosterByPaneKey.delete(paneKey)
}
// Why: the kill ends children and never wakes a parent; without a main agent record the
// roster change still has to reach the row, as unconfirmed working (never a completion).
const payload = buildClaudeCachedLeadStatusPayload(state, 'SubagentStop', paneKey, {}, {
endedChildWork: true
})
if (!payload) {
return undefined
}

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: ready for review

Head: 8ca1e1a1f2 · CI green (24 pass, 12 skipped, 0 failing)

What changed during this review

  • The first version inferred the kill from the Ctrl+C keypress. Review found two real problems with that: on SSH/WSL panes the remote host never learned of it, so a later update from that host brought the killed agent back; and a Ctrl+C that Claude swallows (for example with its /tasks panel open) killed nothing, yet Orca still removed a running agent. Live QA reproduced the second one: the row read "done" for ~97 s while the agent was still running.
  • The PR was rebuilt on Claude's own record of the kill: the system/agents_killed line it appends to the session transcript. The hook listener on the machine that runs the session (desktop for local panes, the SSH/WSL relay otherwise) watches for it only while the pane has a working agent child, and publishes the retirement through the existing status store. The keypress path is deleted.
  • Before building on that record, a live capture on Claude Code 2.1.280 confirmed it is written only when every running background agent was really killed. It is not written on a mid-turn Ctrl+C or Esc, a single-agent stop (from the agent panel, /tasks, or Ctrl+C inside one agent's view), a Ctrl+C swallowed by /tasks, / or /model, a shell-only session, a natural finish, or either exit-dialog option. It lands in the file ~110–190 ms after the keypress.
  • An earlier fix from this review is kept in the new design: the row's last reply and tool are no longer wiped when agents are retired.

Review loops

  • Loop 1 (keypress version): fixed the wiped last reply; raised the SSH/WSL issue.
  • Architecture review: recommended switching to the transcript record; the owner approved it, gated on the capture above.
  • Loop 2 (transcript version): clean, no code changes. Codex/Muse transcript polling logic is unchanged; there is no new wire message and no reader-side rule; each safeguard was removed on its own and a test failed for the right reason.
  • Readiness checklist at the start and at the end: no blocking items.

Live QA (screenshots are in the PR body)

  • macOS, real Claude Code 2.1.280, isolated Orca dev instance: all scenarios pass.
    • Idle Ctrl+C with a shell and an agent: the row reads "Monitoring background tasks".
    • Agent only: the row settles to done.
    • Unsent draft, and shell only: both behave correctly.
    • /tasks panel open: nothing is retired.
    • Ctrl+C inside one agent's view: only that agent leaves.
    • Next turn: the killed agent does not come back.
    • Mid-turn Ctrl+C: the agent survives and stays listed.
    • "Stop all agents": the agents are retired.
    • Idle sessions: zero polling.
    • Retirement shows 0.7–1.4 s after the kill, with no notification for a killed agent.
  • SSH: a real SSH connection to a throwaway Linux container. Claude can't be signed in there without copying credentials, so its captured hook events were replayed through the real installed hook command in a real remote Orca terminal, and the captured line was appended to the remote transcript. Retirement, a reconnect replay, a stray line and an old line all behaved correctly.
  • Not run live: Windows and WSL (covered by unit tests only).
  • Known limits (stated in the PR body):
    • An SSH/WSL host running an older version behaves as it did before (the killed agent stays until the next Stop).
    • A WSL pane whose hooks reach the Windows desktop directly is not watched.
    • After an Orca restart with a background shell running, a kill before Claude's next hook isn't shown until that hook.

Seen during QA, not caused by this PR

  • Turns that Claude starts on its own after a background task finishes send the completion notification request twice with the same id, 13–19 ms apart. The notification burst cooldown should drop the second one; this was not confirmed on screen.
  • A tool hook from Claude's internal helper agent can add a "working" child with no type or description that never gets a stop event.
  • Over SSH, if Claude exits without its session-end hook and the connection then reconnects, the remote host replays a row the desktop had already dropped.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant