fix(agent-status): a cancel never hides live work - #22476
brennanb2025 wants to merge 16 commits into
Conversation
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
One gap in the changed flow: the delayed-hook suppression in server-status-update.ts was not extended to the new non-terminal cancel row, so a late Claude tool hook erases the cancellation and flips the row back to a live working turn. Details below.
Reviewed changes
- Cancel is now a lead-only fact:
foldAgentLeadStatusdrops itsinterruptedinput and themonitoring && !interruptedguard; the shell/cron/subagent a turn left running holds the row open until its own inventory omits it or the session ends. - Claude hook lane:
normalizeClaudeEventno longer retires shell/cron latches on interrupt, derivesturnCompletedAtviaisAgentStatusHeldOpenByChildWork+outcome !== 'cancellation', andbuildClaudeStatusPayloadderivesinterrupted/turnCompletedAtfrom the lead record. - Server inference:
inferInterruptnow admits a Claude cancel whenpayload.lead.state === 'working', folds it throughmarkClaudeLeadTurnInterrupted(which folds child work and returns the row), refuses at an idle prompt held open by child work, and keeps the evidence guard for rows withoutleadand all other providers. - Grok: every turn end (
stop/stop_failure/stop_cancelled) reports remaining finite tasks / an active stop hook as monitoring. - Structured lane: drops the hardcoded
interrupted: false; docs updated. - Tests/fixtures: captured 2.1.280 PTY fixtures replayed through server ingress, new inference-guard cases, and expanded parity/fold/background-task pins. I ran the three server suites (31 tests) plus the fold/parity suites — all green.
⚠️ A delayed tool hook still resurrects a cancelled lead that child work is holding open
server-status-update.ts suppresses a late post-Ctrl+C tool/working hook only when the previous row is state: 'done' with interrupted === true. This PR deliberately makes a cancel-with-child-work row state: 'working' (monitoring or working) with interrupted absent and the verdict on lead.outcome, so that guard no longer matches and the delayed hook is accepted — the cancellation is dropped and the row flips back to a live lead turn. I reproduced it with a throwaway probe: after an inferred cancel the row is working / workingMode: monitoring / lead { state: 'done', outcome: 'cancellation' }; a delayed same-prompt PostToolUse then advances receivedAt and republishes the row as live working work.
Technical details
# Delayed-hook suppression does not cover the new non-terminal cancel row
## Affected sites
- `src/main/agent-hooks/server/server-status-update.ts:158-186` — both suppression blocks require `previous.payload.state === 'done' && previous.payload.interrupted === true`. Unchanged by this PR, but its input shape changed.
- `src/main/agent-hooks/server/server-status-inference.ts:90-122` — the inferred cancel row is `state: 'working'` whenever child work survives, and `interrupted: true` is emitted only when `state === 'done'`.
- `src/shared/agent-hook-listener/providers/claude-events.ts:84-91, 280-287` — a non-turn-boundary lead event writes the lead record as `state: 'working'` and `continueAgentLeadStatus` drops `outcome`, so the verdict is erased by the accepted late hook.
- `src/shared/agent-hook-listener/providers/claude-status-build.ts:52` — `interrupted` is derived from the lead record and is clamped off non-done payloads by the normalizer.
## Required outcome
A delayed same-turn tool-progress hook (`isToolProgressWorkingAfterInterrupt`, i.e. `PreToolUse`/`PostToolUse`/`PostToolUseFailure` for claude/codex) arriving after an inferred cancel must not replace a cancelled-lead row with a fresh working turn, even when a shell/cron/subagent holds the row open and the row itself carries no `interrupted` flag.
## Suggested approach
Extend the two guard predicates (`server-status-update.ts:159-166` and `:170-180`) so the "previous is a cancelled turn" test also accepts the new shape, e.g. `(previous.payload.state === 'done' && previous.payload.interrupted === true) || (previous.payload.state === 'working' && previous.payload.lead?.state === 'done' && previous.payload.lead?.outcome === 'cancellation')`, keeping the existing agent-type/prompt/time-window conditions. Add a regression case alongside `server-interrupt-inference-resurrection.test.ts:90` that ingests a Claude row with `lead` + `claudeRunningNonAgentTask: true`, infers the cancel, then sends the late `PostToolUse` and asserts the row stays monitoring with the verdict intact.
## Open questions for the human (optional)
Reachability depends on Ctrl+C emitting a delayed tool hook for the killed foreground tool. The captured fixtures were recorded with Esc (`hooks_before_next_typed_prompt: []`), while the guard's comment and `isToolProgressWorkingAfterInterrupt`'s claude/codex-only scope assert Ctrl+C does emit one. If Ctrl+C genuinely fires no hook, this is latent rather than live; the fix is cheap either way.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
5371958 to
2055768
Compare
3798c11 to
2b24918
Compare
2055768 to
26e9e2d
Compare
After the user cancels a turn, a background shell, scheduled check or subagent that is still running keeps reading as it truly is in both lanes. The fold no longer takes a verdict input; the cancellation survives only as lead.outcome, restated as the row's interrupted flag on a settled row for readers that predate lead.
26e9e2d to
7b1c388
Compare
There was a problem hiding this comment.
ℹ️ No new issues in the delta since the last review — it is a rebase onto
main, not new code.
Reviewed changes
- Retargeted the base from the stack branch
brennanb2025/lead-status-pr-atomainand rebased the single commit directly onto#22452's squashed merge (8352752e54). - Reconciled the inferred-cancel row with main's fallback:
mainAgent: folded?.mainAgent ?? (payload.mainAgent?.state === 'done' ? payload.mainAgent : { state: 'done', outcome: 'cancellation', … }). Unreachable for the Claude fold path (admission requirespayload.mainAgent.state === 'working'); it restores main's behavior for non-Claude and no-mainAgentrows. - Adopted main's optional
mainAgentreturn fromclaudeMainAgentStatusForPayloadinmarkClaudeLeadTurnInterrupted. - Pulled in main's Grok verdict carry-forward (
previousMainAgent/mainAgentOutcome) and a doc reword; the PR-authored semantics are unchanged.
The before/after policy and all test/fixture content are byte-identical to the previously reviewed revision apart from those reconciliation lines.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe shared status fold no longer uses the main agent's cancellation verdict to determine whether child work remains active. Claude hook handling retains task and cron evidence, and inferred Ctrl+C cancellation folds the main-agent state with child-work evidence. Grok and structured status paths also use the shared fold. Tests and captured fixtures cover cancelled turns with running shells, crons, and subagents. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Child work can remain visible as monitoring after cancellation without reopening the cancelled turn. No actionable merge-blocking risk remains in the reviewed changes. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 40.91% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 26 files. (1 skipped: 1 unsupported.) 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: c245b0f2-5a38-46ec-8f19-777b33e1b9e0
📒 Files selected for processing (20)
docs/reference/agent-status-store.mdsrc/main/agent-hooks/ended-process-reconciliation.test.tssrc/main/agent-hooks/server-claude-cancel-captures.test.tssrc/main/agent-hooks/server-interrupt-inference-guards.test.tssrc/main/agent-hooks/server/server-status-inference.tssrc/shared/__fixtures__/claude-cancel-shell-hooks.jsonlsrc/shared/__fixtures__/claude-cancel-shell-hooks.meta.jsonsrc/shared/__fixtures__/claude-cancel-subagent-hooks.jsonlsrc/shared/__fixtures__/claude-cancel-subagent-hooks.meta.jsonsrc/shared/agent-hook-listener/providers/claude-events.tssrc/shared/agent-hook-listener/providers/claude-lifecycle-events.tssrc/shared/agent-hook-listener/providers/claude-roster-state.tssrc/shared/agent-hook-listener/providers/claude-status-build.tssrc/shared/agent-hook-listener/providers/grok-events.tssrc/shared/agent-lead-status-fold.test.tssrc/shared/agent-lead-status-fold.tssrc/shared/claude-background-task-status.test.tssrc/shared/main-agent-status-parity.test.tssrc/shared/structured-agent-session-agent-status.test.tssrc/shared/structured-agent-session-agent-status.ts
💤 Files with no reviewable changes (1)
- src/shared/structured-agent-session-agent-status.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| const folded = cancelledMainAgentFolds | ||
| ? markClaudeLeadTurnInterrupted(this.state, existing.paneKey, payload) | ||
| : undefined |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP -C4 "mainAgent" src/main/agent-hooks --type=ts -g '!**/*.test.ts' | rg -n -C2 "osc|OSC|title|ingestRemote"
rg -nP -C3 "claudeLeadStateByPaneKey\.(set|get)" src --type=ts -g '!**/*.test.ts'Repository: stablyai/orca
Length of output: 8412
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- changed file diff ---'
git diff --no-ext-diff --unified=35 80f5aae0f9f96babe221950280359a19bcfda9bf 7b1c38893d80a99749b99d53dc3d979a0b3f2e1b -- src/main/agent-hooks/server/server-status-inference.ts
printf '%s\n' '--- current inference source ---'
sed -n '1,220p' src/main/agent-hooks/server/server-status-inference.ts
printf '%s\n' '--- roster helper definitions ---'
sed -n '1,190p' src/shared/agent-hook-listener/providers/claude-roster-state.ts
sed -n '280,370p' src/shared/agent-hook-listener/providers/claude-roster-state.ts
printf '%s\n' '--- cached payload and lifecycle consumers ---'
sed -n '1,75p' src/shared/agent-hook-listener/providers/claude-status-build.ts
sed -n '1,165p' src/shared/agent-hook-listener/providers/claude-lifecycle-events.ts
printf '%s\n' '--- relevant ingestion caller context ---'
rg -n -C8 "inferInterrupt|markClaudeLeadTurnInterrupted|buildClaudeCachedLeadStatusPayload" src/main src/shared --type=ts -g '!**/*.test.ts'Repository: stablyai/orca
Length of output: 42298
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- interruption helper ---'
sed -n '190,235p' src/shared/agent-hook-listener/providers/claude-roster-state.ts
printf '%s\n' '--- cached lifecycle payload ---'
sed -n '110,175p' src/shared/agent-hook-listener/providers/claude-lifecycle-events.ts
printf '%s\n' '--- lifecycle status resolution ---'
sed -n '320,370p' src/shared/agent-hook-listener/providers/claude-roster-state.ts
printf '%s\n' '--- terminal ingestion around OSC ---'
sed -n '100,175p' src/main/agent-hooks/server/server-ingest-terminal.ts
printf '%s\n' '--- payload/mainAgent construction references ---'
rg -n -C6 "mainAgent:" src/shared/agent-hook-listener/providers src/main/agent-hooks/server --type=ts -g '!**/*.test.ts'Repository: stablyai/orca
Length of output: 19804
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- resolveClaudePaneStatus definition ---'
rg -n -C5 "function resolveClaudePaneStatus|export function resolveClaudePaneStatus|const resolveClaudePaneStatus" src/shared/agent-hook-listener/providers/claude-roster-state.ts
sed -n '320,390p' src/shared/agent-hook-listener/providers/claude-roster-state.ts
printf '%s\n' '--- legacy mainAgent compatibility ---'
sed -n '80,135p' src/main/agent-hooks/server/server-persistence-validation.ts
printf '%s\n' '--- Claude status builder ---'
sed -n '1,70p' src/shared/agent-hook-listener/providers/claude-status-build.tsRepository: stablyai/orca
Length of output: 7476
🏁 Script executed:
sed -n '172,215p' src/shared/agent-hook-listener/providers/claude-roster-state.tsRepository: stablyai/orca
Length of output: 2301
Keep the Claude lead record synchronized for rows without mainAgent.
When a Claude row has no mainAgent and no child work is evidenced, inferInterrupt publishes done with interrupted: true but does not call markClaudeLeadTurnInterrupted. An existing working lead record can remain stale. A later child lifecycle event reads that record and can publish the pane as working again.
Call the helper for every inferred Claude interrupt. Use its folded result only when the input row contains mainAgent.
Suggested fix
- const folded = cancelledMainAgentFolds
- ? markClaudeLeadTurnInterrupted(this.state, existing.paneKey, payload)
- : undefined
+ const claudeFold =
+ agentType === 'claude'
+ ? markClaudeLeadTurnInterrupted(this.state, existing.paneKey, payload)
+ : undefined
+ const folded = cancelledMainAgentFolds ? claudeFold : undefined📝 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.
| const folded = cancelledMainAgentFolds | |
| ? markClaudeLeadTurnInterrupted(this.state, existing.paneKey, payload) | |
| : undefined | |
| const claudeFold = | |
| agentType === 'claude' | |
| ? markClaudeLeadTurnInterrupted(this.state, existing.paneKey, payload) | |
| : undefined | |
| const folded = cancelledMainAgentFolds ? claudeFold : undefined |
…path A Grok turn cancelled while a task ran now reads monitoring, and the idle_prompt backstop that later settles it restated done without the row's `interrupted` flag, so notification readers announced the cancelled turn as a clean finish. Derive `interrupted` from the main agent's outcome, as the Claude builder already does. The inferred Claude cancel now folds through the host's local main agent record, which a relayed pane never refreshes, so a second cancel on an SSH pane inherited the first cancel's clock. The caller admits only a working main agent, so the cancel always starts a new done clock.
There was a problem hiding this comment.
ℹ️ No new issues in the delta since the last review — the two behavior fixes check out.
Reviewed changes
- Grok keeps the cancel verdict through the idle backstop:
normalizeGrokEventnow derivesinterruptedfrommainAgent.outcome === 'cancellation'instead of the current event's outcome, so theidle_promptthat settles a task a cancelled turn left running still readsinterrupted. The normalizer still clamps the flag todonerows, so a monitoring row does not publish it. The captured task-completion settle is a freshuser_prompt_submit task-completed-<id>+stopturn, which clears the verdict as a new turn. - Relayed Claude cancel restarts its clock:
markClaudeLeadTurnInterruptedpasses an explicitstateStartedAt: Date.now(), so a second cancel on a relayed pane starts a fresh done clock rather than inheriting the host's local record from the first cancel (the relay never refreshes that record). The caller admits the fold only whenpayload.mainAgent.state === 'working', so this changes the relay case only. - Tests: a Grok completion test pinning the idle-backstop verdict, and an inference-guard test pinning the relayed second cancel's
stateStartedAt. Both are meaningful — each fails against the un-fixed code.
I re-ran the touched suites (agent-hook-listener-grok-completion, server-interrupt-inference-guards, server-claude-cancel-captures, main-agent-status-parity, agent-lead-status-fold) — 83 tests, all green.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…rt can seed it An inferred Ctrl+C cancel beside a working subagent publishes a row held open by child work, but the synthesized event dropped the row's paired claudeRunningNonAgentTask fact because mainAgent changed. Hydration seeds a settled main agent only when that fact says no shell ran, so after a restart the child's drain left the row working with no mainAgent. Carry the fact forward: a cancel does not change what the shell inventory said.
There was a problem hiding this comment.
ℹ️ No new issues in this delta. One note: the delayed-tool-hook suppression gap in
server-status-update.tsthat the earlier review flagged (root cause in unchanged pre-existing code) is still open, so this is not an approval.
Reviewed changes
- Kept the shell fact on an inferred cancel:
server-status-inference.tsnow copiesexisting.claudeRunningNonAgentTaskonto the row it synthesizes for an inferred Ctrl+C cancel, so hydration can still seed the settled main agent after a restart and a child's later drain can settle the row. - Added a restart round-trip test for that path in
server-last-status-main-agent-fact.test.ts. It fails without the source change (the row staysworkingwith the main agent unseeded) and passes with it.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Do not settle monitoring on idle_prompt. · grok-events.ts:216
src/shared/agent-hook-listener/providers/grok-events.ts:216
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not settle monitoring on
idle_prompt.
Notification/idle_promptsetsleadStatetodone, and line 216 supplies no child-work liveness for that event. The fold therefore clears monitoring even when the precedingStopCancelledreported a running shell. Grok documentsNotificationas an agent notification, not as task completion. Retain task liveness until a task update or session boundary, and update the test to expect monitoring for this sequence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: f26cee21-fe52-4641-9bfe-40d5985a4f4d
📒 Files selected for processing (6)
src/main/agent-hooks/server-interrupt-inference-guards.test.tssrc/main/agent-hooks/server-last-status-main-agent-fact.test.tssrc/main/agent-hooks/server/server-status-inference.tssrc/shared/agent-hook-listener-grok-completion.test.tssrc/shared/agent-hook-listener/providers/claude-roster-state.tssrc/shared/agent-hook-listener/providers/grok-events.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.
…thing Every row that publishes the main agent fact now admits an inferred cancel only while that main agent is working. Grok's Ctrl+C at the idle prompt leaves its background task running, so settling the monitoring row to done hid live work. Rows without the fact keep the evidence guard, and Codex keeps it too because its synthesized row is a plain done.
… desktop's records The inferred Claude cancel read and wrote the desktop's own listener records for every pane. For an SSH pane those records are not the relay's: hydration seeds them from the saved row and nothing reaps them, so a subagent that finished on the remote after a desktop restart kept a cancelled row spinning with nothing running. A local pane still records the verdict on its listener and folds its own roster; a relayed pane folds only the child work its row carries. The relayed-pane parameter and forced clock the shared record path grew for this are gone.
…n or the provider's own A relay never learns of the cancel the desktop infers from Ctrl+C, so its next child hook or reconnect replay restated the main agent as working and flipped the row back. The late-hook suppression that guarded this keyed on a done row flagged interrupted, which a cancel held open by a shell or subagent no longer is; it also dropped Grok's own stop_cancelled when the inference won the settle race, hiding the task that hook reported. The suppression is replaced by a latch derived from the row: its main agent reads cancelled (or, from an older host, a done row flagged interrupted). A settled incoming main agent, another prompt, an explicit prompt or a session start releases it. Child and replayed events keep the latched main agent and are re-folded with their own child evidence; late main agent work is held as before, and Codex keeps its record re-mark.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Hold a cancel verdict in the store: new
server-cancel-verdict-latch.tsreplaces the two delayed-hook suppression blocks inserver-status-update.ts. "Latched" is derived from the row (mainAgent.outcome === 'cancellation', or a legacystate: done+interrupted), never stored, and dies on a settled incomingmainAgent, aSessionStart, a prompt change, or a non-tool-progress working event outside the window. Child-attributed/replayed events carrying child work are re-folded under the latched main agent. This is the shape the earlier review reproduced, and it now holds. - Fold a relayed pane's cancel from its row:
server-status-inference.tssplits local vs relayed. A local Claude pane records the verdict on its listener and folds its own roster; a relayed pane folds only the child work its row carries, via the newserver-row-child-work-fold.ts.markClaudeLeadTurnInterrupteddrops therowSubagentsparameter and the forced clock the shared path had grown for the relay. - A Ctrl+C at an idle main agent's prompt cancels nothing: the inference guard now switches on
payload.mainAgent.state === 'working'for every provider that publishes the fact; Codex keeps the child-work guard, and rows withoutmainAgentkeep the evidence guard. - Tests: new
server-relayed-claude-cancel.test.tsandserver-grok-cancel.test.ts; the capture loader is extracted toclaude-cancel-capture.test-fixture.ts; a Codex evidence-guard pin and two revised inference-validation cases.
I re-ran the touched suites (108 + 66 tests) green, and confirmed the earlier concern is fixed on the local lane too: a throwaway probe (shell Stop → new turn → Ctrl+C → late same-prompt PostToolUse) left the row working/monitoring with mainAgent { done, cancellation } byte-identical.
ℹ️ Nitpicks
- The local lane has no committed regression for the latched
mainAgent+isToolProgressWorkingAfterInterrupthold (the exact prior reproduction): the latch is pinned via the relayedisReplay/child-event paths and the captures replay. Since the captures show Ctrl+C emitting no hook, this is latent rather than live — a case besideserver-interrupt-inference-resurrection.test.tswould be nice-to-have when convenient, not required.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
The task notification Claude starts when background work ends is a real turn, but it keeps the cached prompt and carries no explicit prompt, so within 15 s of a cancel the latch held its prompt submission and every tool event after it: the turn read as monitoring under a cancelled main agent until its Stop. The captured shell cancel has exactly this: the notification lands 0.17 s after the cancel key.
…n agent The cancel verdict latch lets any settled mainAgent through, so a late root Stop after an inferred Codex cancel now applies where the old same-prompt window held it. It restates the cancellation on mainAgent but, unlike Claude and Grok rows, carried no interrupted flag, so mobile, the dashboard and notification text read the cancelled turn as finished. Codex rows (local and relayed) now derive the flag from the main agent record, like the other providers that publish one.
…the store's cancel-verdict hold
There was a problem hiding this comment.
ℹ️ No critical issues — two minor observations inline.
Reviewed changes
- A prompt submission ends the cancel verdict latch:
resolveCancelVerdictLatchnow ends the latch on a root, non-replayUserPromptSubmitthrough the newopensNewTurn, so the task notification Claude starts when background work ends is treated as a real new turn instead of being held under the cancelled main agent; the capture test now pins that transition. - Codex rows derive
interruptedfrom the main agent: newcodexMainAgentTurnInterruptedand its use inbuildCodexStatusPayloadandreconcileRemoteCodexState; a late root Stop after an inferred Codex cancel now carriesinterrupted: truefor readers that predatemainAgent, with new local and relayed assertions inserver-main-agent-turn-verdicts.test.ts.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| interrupted: | ||
| effectiveState === 'done' && codexMainAgentTurnInterrupted(lead) ? true : undefined, |
There was a problem hiding this comment.
interrupted is recomputed from the local lead unconditionally here, which drops an incoming interrupted: true from a relay host that predates mainAgent: with no mainAgent in its payload, the seeded lead carries no outcome, so the flag becomes undefined. I reproduced this through ingestRemote — a relayed Codex Stop with state: 'done', interrupted: true and no mainAgent lands as interrupted: undefined, so mobile/dashboard read the cancelled turn as a clean finish when the desktop's own inference did not record the cancel (a host-side interrupt). Consider falling back to payload.interrupted when effectiveState === 'done' and the lead carries no outcome.
Technical details
# Relayed Codex `interrupted` is dropped for hosts without `mainAgent`
## Affected sites
- `src/shared/agent-hook-listener/providers/codex-state.ts:223-224` — `reconcileRemoteCodexState` overrides `interrupted` from the local lead, discarding `payload.interrupted`.
- `src/shared/agent-hook-listener/providers/codex-state.ts:96-130` — `seedCodexStateFromSnapshot` seeds the lead from `payload.mainAgent` when present, else infers state only; an old host supplies neither outcome.
- `src/main/agent-hooks/server/server-status-update.ts:86-101` — every relayed Codex event with a `hookEventName` routes through this reconciliation.
## Required outcome
A relayed Codex `done` row that the host published as `interrupted: true` still reads `interrupted` on the desktop when the local lead has no `outcome` to derive it from.
## Suggested approach (optional)
`effectiveState === 'done' && (codexMainAgentTurnInterrupted(lead) || payload.interrupted === true)`.| function opensNewTurn(event: AgentHookEventPayload): boolean { | ||
| return ( | ||
| event.hookEventName === 'SessionStart' || | ||
| (event.hookEventName === 'UserPromptSubmit' && | ||
| event.toolAgentId === undefined && | ||
| event.isReplay !== true) | ||
| ) | ||
| } |
There was a problem hiding this comment.
opensNewTurn treats every root, non-replay UserPromptSubmit as a new turn, but the codebase already classifies a family of harness-injected prompts (isKnownHarnessInjectedUserTurnText) that keep the cached prompt and are not the user's ask. That breadth is what makes the task notification work, but if any other injected form (an interruption notice, a teammate message) lands within the 15 s window it now ends the latch and publishes a live working row under the stale prompt. Is the broad predicate intended, or should it match only the injected shape this change targets?
Technical details
# `opensNewTurn` ignores the harness-injected classification
## Affected sites
- `src/main/agent-hooks/server/server-cancel-verdict-latch.ts:62-69` — predicate checks only `hookEventName`, `toolAgentId`, `isReplay`; never `hasExplicitPrompt` or `isKnownHarnessInjectedUserTurnText`.
- `src/shared/agent-hook-listener/prompt-fields.ts:103-106` — harness-injected turns keep the cached prompt, so the latch's `prompt` mismatch guard does not catch them.
- `src/shared/harness-injected-user-turns.ts:43-51` — injected prefixes include interruption and teammate notices.
## Required outcome
Confirm that a root `UserPromptSubmit` which is not a genuine new turn cannot arrive after a cancel; if it can, the latch should keep holding it.
## Open questions for the human (optional)
The captured Claude 2.1.280 cancels show no hook at all, so no post-cancel injected `UserPromptSubmit` is evidenced; is that still true for the other providers that emit `UserPromptSubmit` (kimi, muse, codex, devin, droid)?
Review status: ready for merge reviewHead: Review loops (5). Each loop was a fresh reviewer with the same brief; loop 5 came back clean.
Readiness checklist. PASS at the start and at the end. The only findings were stale text, now fixed: the PR body, and one paragraph in Electron QA (isolated dev build of
Screenshots are in the PR body. Validation: Not covered / follow-ups:
|
…tus-pr-c # Conflicts: # docs/reference/agent-status-store.md # src/shared/agent-hook-listener/providers/claude-events.ts # src/shared/agent-hook-listener/providers/claude-lifecycle-events.ts # src/shared/agent-hook-listener/providers/claude-roster-state.ts # src/shared/agent-hook-listener/providers/codex-state.ts # src/shared/agent-hook-listener/providers/grok-events.ts # src/shared/agent-lead-status-fold.ts # src/shared/main-agent-status-parity.test.ts
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the merge of origin/main (85642d0) into the PR branch, the only change since the prior pullfrog review at 8e90d142b9. The branch's cancel policy survives the merge intact, and main's #22475 Codex-child-work refactor is adopted cleanly.
- Adopted main's shared child-work refactor:
foldAgentLeadStatusgains main'swaiting/blockedearly-returns and theAgentChildWorkLivenesswaitingarm;mainAgentTurnInterruptedconsolidates the Claude and Codex helpers intoagent-lead-status-fold.ts;codexRosterChildWorkLiveness+resolveCodexPaneStatusreplacecodexRosterEffectiveState;grok-events.tsclassifies a listed subagent as agent work (working) and a shell / stop hook asmonitoring. - Preserved the cancel policy through the merge: no
interruptedfold input, theinferInterruptguard (mainAgent.state === 'working', Codex child-evidence guard, no-mainAgentfallback), the store-side cancel-verdict latch, the relayed row-child-work fold (now passinghasWaitingChildWork: false), andhasConfirmedDoneGatewithout the interrupt discount. - Reconciled docs and the parity table to main's structure while keeping the cancel text; the Grok completion test now pins a background subagent surviving a cancel.
Two minor threads from the prior review remain open and unchanged by this merge, so approval is withheld on those alone: the opensNewTurn breadth question in server-cancel-verdict-latch.ts and the relayed Codex interrupted fallback in codex-state.ts. Neither is a blocker; they carry over as noted on the PR.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Correction to my earlier status comment (#22476 (comment)): it claimed a Ctrl+C at an idle prompt cancels nothing. That is not true of the CLIs themselves. What this PR guarantees is that Orca refuses an inferred cancel at an idle prompt; the PR body and Live validation at 239992f (real CLIs in Orca terminal panes):
The three remaining issues — Claude's idle-prompt stop of its subagents, the refused Codex cancel with a live subagent, and Grok cancel state driven by Grok's own hooks instead of keypress inference — are each being fixed in a separate stacked PR on this branch. |
There was a problem hiding this comment.
✅ No new issues in this delta.
Reviewed changes
This run re-reviewed the only commit since the prior pullfrog review at bff608c870 (397d22fb7e): a behavior fix in the relayed row-child fold plus its regression test.
- A relayed pane's cancel now folds a
waitingchild, not justworking:foldMainAgentWithRowChildWorkderives its child-work liveness fromagentChildWorkLivenessover the row's subagent snapshots and shell fact, the same helpercodexRosterChildWorkLivenessuses. A relayed subagent inwaitingnow holds the rowwaitingwhere the previousstate === 'working'test ignored it, and any live (non-idle/non-done) child state —blocked,unverifiable— now counts as agent work. - A new test pins a waiting relayed child surviving a cancel:
server-relayed-claude-cancel.test.tsingests a relayed Claude row with awaitingsubagent, infers Ctrl+C, and asserts the row stayswaitingwithmainAgent: { done, cancellation }. It fails against the old fold, which settled the row todone.
The touched suites (server-relayed-claude-cancel, server-interrupt-inference-guards, server-main-agent-turn-verdicts, main-agent-status-parity, agent-status-child-work-liveness) pass — 73 tests.
ℹ️ Nitpicks
docs/reference/agent-status-store.md:241still reads "Only the Codex hook lane feeds that input today". With this commit the relayed row-child fold also feedschildWorkLiveness: 'waiting'from a child's own state, so that sentence could name the relayed lane too.
The two prior minor threads (opensNewTurn breadth in server-cancel-verdict-latch.ts:62-69; the relayed Codex interrupted fallback in codex-state.ts:223-224) remain open and unchanged by this commit, so approval is withheld on those alone.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…n card adopts a relayed main agent The permission-card hold took the incoming event's mainAgent before the cancel hold ran, so on an SSH pane a child's next tool under a sticky card restated the relay's stale working main agent and dropped the cancellation the desktop had inferred.
…les as stopped, not completed
There was a problem hiding this comment.
✅ No new issues in this delta.
Reviewed changes
This run re-reviewed the only commit since the prior pullfrog review at 397d22fb7e (d3eb919805): an ordering fix in the hook-status ingress plus its regression test.
- The cancel hold resolves before a child's permission card adopts a relayed main agent:
server-status-update.tsnow runsresolveCancelVerdictLatchon the attached payload beforeshouldKeepClaudePermissionVisible/withHeldChildWaitMainAgent. That hold adopts the event'smainAgent, and on a relayed pane the relay's record still says the main agent is working, so the hold previously overwrote the desktop's cancel with the stale restatement. With the latch first, a child-attributed event carrying child work is refolded under the latched cancelled main agent, and the hold then keeps that verdict. - A new test pins the child card staying up over a denied request's next tool:
server-relayed-claude-cancel.test.tsposts a childPermissionRequest, then a differenttool_use_idon the same child, and asserts the row stayswaitingwithmainAgent: { done, cancellation }. I verified it fails against the old ordering (mainAgent.statereverts toworking).
The touched suites (server-relayed-claude-cancel, server-claude-cancel-captures, server-interrupt-inference-guards, server-grok-cancel) pass — 34 tests.
The two prior minor threads (opensNewTurn breadth in server-cancel-verdict-latch.ts:62-69; the relayed Codex interrupted fallback in codex-state.ts) remain open and unchanged by this commit, so approval is withheld on those alone.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues in this delta.
Reviewed changes
This run re-reviewed the only commit since the prior pullfrog review at d3eb919805 (0c5e474d9d): a test-only addition.
- A regression test pins that a cancelled turn's drained row settles as stopped, not completed:
server-claude-cancel-captures.test.tsreplays the subagent capture up to the cancel, posts the child's post-cancel tool, then stops both children and asserts the row settlesdonewithinterrupted: trueandmainAgent: { done, cancellation }, and thatturnCompletedAtis never set. The assertions are exact, so the test would fail if the settle path stamped a completion or dropped the verdict.
The full captures suite passes (6 tests).
The two prior minor threads (opensNewTurn breadth in server-cancel-verdict-latch.ts:62-69; the relayed Codex interrupted fallback in codex-state.ts) remain open and unchanged, so approval is withheld on those alone.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…ok and a teammate's idle A relay that restarts after a desktop-inferred cancel has lost its prompt cache, so the child's next hook arrived with an empty prompt, read as a new turn, and replaced the cancelled main agent with none; the row then stayed working after every child stopped. A child's empty prompt is now unknown, not another turn; a non-empty different one still releases, since it is the listener's newer prompt. TeammateIdle names its child by teammate_name and carries no agent id, so the latch treated it as the main agent's and let the late-hook window apply it after 15 s, reviving the cancelled turn. It is now re-folded as child work.
There was a problem hiding this comment.
✅ No new issues in this delta.
Reviewed changes
This run re-reviewed the only commit since the prior pullfrog review at 0c5e474d9d (130f162af0): the cancel-verdict latch now survives a restarted relay and a teammate's idle.
- A restarted relay's empty-prompt child hook no longer releases the cancel: new
isChildAttributed(toolAgentId !== undefinedorTeammateIdle) andrestatesAnotherPrompt; a child-attributed event whose prompt is''(a restarted relay has lost its prompt cache) is treated as unknown rather than another turn, so the latch holds and the child is re-folded under the cancelled main agent. TeammateIdlecounts as child-attributed: the refold gate now acceptsisChildAttributed(so a teammate idle, which names its child byteammate_nameand carries noagent_id, is re-folded under the latched main agent) and its empty prompt no longer restates another turn.- Tests:
server-relayed-claude-cancel.test.tsgainscreateRelay/restartRelayhelpers plus three cases — a restarted relay's first (child) hook keeping the cancel, a replayed prompt the desktop missed releasing it, and a teammate idle past the late-hook window still settlingdone+interrupted.
The touched suite passes (10 tests). I also verified the empty-prompt child path does not regress the row label: refoldUnderLatchedMainAgent carries the incoming empty prompt, but the pre-fix path applied the same empty prompt after releasing the latch, so the observable prompt is unchanged.
The two prior minor threads (opensNewTurn breadth in server-cancel-verdict-latch.ts:62-69; the relayed Codex interrupted fallback in codex-state.ts:223-224) remain open and unchanged by this commit, so approval is withheld on those alone.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Review status: ready for review at
|
This branch has not been deployed
1 participant
ELI5
When you cancel an agent's turn with Ctrl+C, a shell or subagent it started in the background keeps running. Orca used to react to the cancel in ways that hid that work: either the row read "done" while the shell was still going, or it kept showing the cancelled turn as "Working" because the cancel was refused. Now every surface (sidebar,
worktree ps, mobile, dashboard) shows what is really happening: "Monitoring background tasks" for a shell or scheduled check, and "working" for a subagent, until that work reports its own end. Orca also no longer infers a turn cancel from a Ctrl+C at an idle prompt, so that keypress alone does not change the row. The keypress is not always harmless in the CLI itself — see Live validation below.What Changed
Before and after, as the user experiences it
stop_cancelledorstop_failurethat lists running tasks. Before: "done". After: the fold treats it like a plainstop, so at the hook level such a payload reads monitoring. Live, this does not yet hold end to end: Orca still infers a Grok cancel from the keypress itself — Esc never cancels a Grok turn (Grok's own user guide), and a mid-turn Ctrl+C first opens Grok's "Stop them?" dialog — so the row is marked done at the keypress and stays done after Grok's own cancel hook while the work runs, as on main. A stacked follow-up PR drives Grok cancel state from Grok's own hooks (see Known limitations).interruptedwhen it settles (Claude, Codex and Grok alike). When the background work ends, the agent's own follow-up turn (Claude's task notification) runs and announces normally.The mechanism
foldAgentLeadStatus) no longer takes a verdict input. A cancel is a fact about the main agent, carried asmainAgent.outcome: 'cancellation'(added in feat(agent-status): publish the main agent's own state beside the combined row state #22452), never about the work the turn left running. That work leaves the row only when its own inventory omits it or the session ends. Every site that used the verdict to retire work is gone: the shell gate's interrupt parameter, the cron gate, the deletes in the inferred-cancel path, the delete block on a turn boundary, the child-lifecycle done-gate's discount, and Grok's "only a plain stop reports what it left running".interruptedflag keeps flowing for the readers that ship today (mobile's state dot, the dashboard row, pane hibernation, notification dispatch). Each provider now derives it from its main agent record instead of each caller restating it: the Claude status builder, the Codex builder and relay reconcile, and Grok's normalizer. The Claude turn stamp (turnCompletedAt) is derived there too, and a cancelled turn earns none.mainAgentit is admitted only whenmainAgent.stateisworking; Codex also keeps the child-evidence guard (its synthesized row is a plain done, which would retire children its combine keeps working); a row from a host too old to publishmainAgentkeeps only that guard. The cancelled row is the fold of the cancelled main agent with the child work its owner can see: the local listener's roster for a local pane, the row's own subagents and shell fact for a relayed pane (server-row-child-work-fold.ts), whose provider records live on the remote host.server-cancel-verdict-latch.ts). This replaces the old late-hook suppression, which only covered rows alreadydone+interrupted. The hold is read off the row (mainAgent.outcome), not stored beside it, and ends on a new turn (a main agent prompt submission, a changed prompt, a session start) or the provider's own settledmainAgent. Child and replayed events under the hold keep the cancelled main agent and are re-folded with their own child evidence, so live work still shows.Why
The cancel policy was decided with the main-agent status work (a cancel never hides live work): if the shell is still running, the user should see that something is still running. Recorded evidence against Claude Code 2.1.280 settled the precondition for a mid-turn cancel: it fires no hook, kills only the foreground tool, leaves background shells and subagents running, and every later Stop inventory matched the process table, so for that case there is no stale inventory to distrust and the hiding code had nothing left to protect against. Grok 1.0.41's idle prompt likewise leaves its work running. One measured exception: a single Ctrl+C at Claude's idle prompt makes Claude itself stop its background subagents (shells survive), so an inventory recorded before that keypress does go stale — a stacked follow-up PR reflects Claude's own stop (see Known limitations). Orca does not own the agent process in a terminal pane, so it cannot stop that work on the user's behalf; showing it is the honest state.
Alternatives considered:
Linked Issue
Follow-up to #22452 and #22295; no separate issue.
Visual Proof
Isolated dev Orca build of this branch (hidden window, throwaway profile, folder workspace), Claude Code 2.1.280 and Grok 1.0.41 in terminal panes, keys sent through the real terminal input path.
After, Claude: turn 1 starts a 600 s sleep in the background; turn 2 runs a 120 s foreground sleep. Mid-turn, then right after Ctrl+C (row reads "Monitoring background task…", store row
working / monitoring,mainAgent: { done, cancellation }, the 600 s sleep still alive):A second Ctrl+C at the idle prompt changes nothing; killing the background sleep lets Claude's own task-notification turn run and the row settles to done:
After, Grok: a background task holds the row in monitoring; Ctrl+C at the idle prompt leaves it monitoring (store row
working / monitoring, nointerrupted), and the task is still alive:Before and after from the first round, against #22452 (Claude, same scenario):
Before (this branch's base, #22452): after Ctrl+C the sidebar row keeps the cancelled turn's spinner and prompt, and the store row is
state: working, mainAgent: { state: working }with no cancellation recorded.After: the sidebar row reads "Monitoring background task…" and the store row is
state: working, workingMode: monitoring, mainAgent: { state: done, outcome: cancellation }.Testing
pnpm tcexit 0;pnpm exec oxlint src/main/agent-hooks src/shared/agent-hook-listenerexit 0;pnpm run check:code-quality:changed0 findings.pnpm test src/main/agent-hooks src/shared/agent-hook-listener src/relayplus the fold, background-task, parity, structured-status and Grok completion suites: 308 files, 3148 passed. All ofsrc/shared: 12110 passed. Renderer store and hooks: 4502 passed.src/shared/__fixtures__/claude-cancel-shell-hooks.jsonlandclaude-cancel-subagent-hooks.jsonl(with.meta.jsonsidecars) are hook payloads recorded from Claude Code 2.1.280 over a real PTY, merged in time order with the driver's cancel and kill markers, scrubbed with the repo's transcript scanner.server-claude-cancel-captures.test.tsreplays them through the server's HTTP ingress and cancel inference;server-relayed-claude-cancel.test.tsreplays them through a real relay-side listener into the desktop (restart with seeded subagents, a child hook after the cancel, a child permission prompt, reconnect replay);server-grok-cancel.test.tscovers Grok's idle-prompt Ctrl+C and its ownstop_cancelledtrailing the inferred cancel.state,workingMode,interrupted,mainAgent) is unchanged.Live validation (real CLIs in Orca terminal panes, isolated dev build at 239992f; Claude Code 2.1.280, Codex 0.156.1, Grok 1.0.41):
Claude, Ctrl+C or Esc mid-turn: background shells and subagents keep running and the row keeps showing them, with the cancellation recorded on the main agent — the headline claim holds.
Claude, a single Ctrl+C at the idle prompt: Claude itself stops its background subagents ("All background agents stopped"); shells survive. Orca refuses that cancel and keeps showing the stopped subagent as working — stacked follow-up PR.
Claude, rapid double Ctrl+C with background work running: Claude asks "Exit and stop tasks / Move to background and exit / Stay"; exiting removes the row (correct).
Codex, Ctrl+C or Esc mid-turn: background work keeps running and the row keeps showing it — holds.
Codex, a single Ctrl+C at the idle prompt: Codex quits and kills its work; the row is removed (correct).
Codex, Ctrl+C mid-turn with a live subagent: the cancel is refused (as on main), so the row stays "working" even after all the work has ended — stacked follow-up PR.
Grok: Orca infers a cancel from keypresses Grok does not honor (Esc never cancels a Grok turn; a mid-turn Ctrl+C opens Grok's "Stop them?" dialog), marks the row done at the keypress, and the row stays done after Grok's own cancel hook while work runs. Also on main — stacked follow-up PR.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Author: @BrennanKB5
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)