Skip to content

fix(agent-status): end a Codex turn on Codex's own cancel, decided by turn id - #22727

Closed
brennanb2025 wants to merge 30 commits into
mainfrom
brennanb2025/codex-cancel-with-subagent
Closed

brennanb2025 wants to merge 30 commits into
mainfrom
brennanb2025/codex-cancel-with-subagent

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 26 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2385 $\color{#cf222e}{\Huge{\mathbf{−}}}$​232 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2153
Prod 37 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​762 $\color{#cf222e}{\Huge{\mathbf{−}}}$​228 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​534

ELI5

You ask Codex to do some work, and it starts a helper (a subagent). Midway you press Ctrl+C or Esc to cancel. Codex stops its own turn but lets the helper finish. Orca's status for that pane used to stay stuck on "working" for the main agent for up to 30 minutes. Now Orca listens to Codex's own "the user cancelled" signal, so the main agent reads cancelled right away, the pane keeps showing the helper while it runs, and it settles once the helper finishes. If Codex genuinely starts working again, the pane shows working again. The helper also stays on the pane if you send another prompt and that turn finishes first.

What Changed

Before: a Codex pane cancelled mid-turn while a subagent ran kept reading mainAgent: working until the 30-minute staleness cutoff. Orca guessed the cancel from the key the user pressed, and a guard refused that guess whenever a subagent was live. A second rule (the cancel "latch") then held later hooks back after a guessed cancel, so a wrong guess could also hide real work that followed.

After: the main agent reads cancelled the moment Codex reports the cancel. The row stays working while the subagent runs and settles to done and interrupted on the subagent's own SubagentStop, even if a later turn ends first. A cancel with nothing left running settles at once. A key that cancels nothing (a Ctrl+C that only clears a draft, an Esc that only closes a popup, the shared-server chooser's "Run in background" or a dismissed chooser) changes nothing. Any hook for a new Codex turn, including one Codex starts without a prompt, reads working again.

Mechanism, in five parts:

  1. Codex's own report. Codex (0.150+) fires an Interrupt hook for the main agent on every real cancel, and Orca never subscribed to it. Every installer that writes Orca's managed Codex hooks (managed home, real home, SSH remote, WSL, Windows .cmd) now registers it. Codex runs Interrupt with at most 3 seconds and warns about more, so the entry declares 3; Codex hashes the clamped timeout, so Orca's trust hash computes it the same way, otherwise the hook reads as modified and never fires.
  2. Turn identity decides, not time. The listener reads Interrupt as the main agent's done with a cancellation outcome, folded with the live subagent roster exactly like a normal end of turn; unlike Stop, it never drops the roster. The Codex main agent record now carries Codex's turn_id, which Codex puts on every main-agent hook except SessionStart. A turn ends once: after Codex aborts it, or once its rollout records it ended. After that, a later fact for that turn (a hook the cancel overtook, a Stop racing the Interrupt) changes nothing, a late hook for any turn the rollout already ended changes nothing, and a hook for any other turn is Codex working again. A Stop alone is not final, because a Stop hook that blocks makes Codex continue the same turn under the same id; a later hook for that turn reads working again. A main-agent hook with no turn_id (SessionStart, or a Codex build that omits it) belongs to the turn the rollout shows open, and a running record with no id adopts the rollout's latest turn once the rollout shows it open or started since the last read. SessionStart with source compact fires mid-turn in the same session and rollout, so it no longer drops the roster, the rollout state or the turn id; only a real session start (startup, resume, clear, fork) does, and a changed rollout path resets the rollout state on its own. This replaces, for Codex, the latch's time window and tool-progress hold and the old "late Stop keeps the verdict" rule.
  3. A backstop that reads Codex's own record. Codex writes turn_aborted (or task_complete) to its rollout file right after it runs the Interrupt (or Stop) hook, whether or not the hook was delivered, and it kills a slow Interrupt hook at 3 seconds (1 of 21 cancels in the capture). Every Codex event first catches up on the pane's rollout. Behind that, a Codex rollout watch, owned by the host that runs Codex (main for a local pane, the relay for an SSH one), reads the rollout once a second while the main agent's turn is open (by its own record or the rollout's) or any subagent runs, and publishes the row rebuilt from what it reads. It replaced a timer that re-ran the pane's last hook body: that re-ran the body's side effects (a replayed SessionStart wiped the roster and re-read the rollout from the start every second), stamped every result as a prompt boundary, and stopped whenever anything else replaced the row, so a lost Interrupt with no later hook stayed lost. The watch's result carries no hook name or prompt, so it restates the row rather than starting a turn, and its life follows the pane's records, not the row object. The rollout's turns are tracked by id: the open turn, the latest one started, and the last 32 ended turns with how each ended, so two markers in one read no longer lose the first. Every turn_aborted is a cancellation, whatever its reason (interrupted, replaced, review_ended, budget_limited, or none), because Codex's own app-server reports every aborted turn as interrupted and its TUI says so ("the turn was stopped"). A lost Interrupt or Stop self-corrects within a second, with or without subagents, and Codex builds without the Interrupt hook are covered too. Hook and rollout write through the same turn-id-keyed transition, so they converge instead of racing.
  4. No more guessing from keys. Codex is taken out of keypress inference in the renderer and in the server's re-check, the same way Escape was earlier taken out for agents whose Escape also navigates. Claude, Grok and every other agent keep keypress inference exactly as before.
  5. A subagent leaves the row only by its own records. Before, the main agent's Stop dropped every subagent the pane did not also track through Codex's newer rollout path (a rule added when Codex 0.144 could skip a child's Stop hook), so a new turn that ended while a cancelled turn's subagent still ran read done for about a minute, then flipped back to working on the subagent's next hook (seen live on 0.157.1). Codex fires no SubagentStop for a subagent it aborts, which is the case that rule really covered. Now every subagent in the roster, whether a hook or the parent rollout announced it, is also read from its own rollout (the path its own hooks name in transcript_path, else the file named by its thread id in the folder of the day it started), so it leaves on its own SubagentStop, when its rollout records its turn's end, when its rollout stays unreadable for a minute, or when a new session starts in the pane. The main agent's Stop never drops it, on the desktop or the relay; a restored subagent is settled the same way instead of on the next Stop. A subagent's hook on a pane with no record of the main agent (its records were cleared, or an SSH relay restarted) no longer invents a working main agent that nothing would ever end: the subagents alone drive the row, and the row carries no mainAgent.

For SSH, the relay runs on the machine that runs Codex, so it runs the same rollout watch on the remote rollout and applies the turn rule to the raw hooks. The desktop now mirrors the relay's main-agent fact instead of re-deriving it from event names, and for the relay's rollout observations (source codex with no hook name) its whole roster too, so a subagent the relay saw finish in the rollout is not revived by the desktop's copy; that copy only fills a child event that a restarted relay publishes without one.

After an Orca restart, a Codex row that was not settled is re-read from the rollout its saved row names (providerSession.transcriptPath, already persisted), and the watch publishes what it finds on its first tick, so a turn that ended while Orca was down settles instead of reading working until the next hook.

Removed: the earlier version of this PR folded the keypress guess with the subagent roster, re-based the guess on the main agent's turn, and dropped a held-open guess when the CLI exited. All three existed only to make the Codex guess work and are reverted, as is the capture recorded without Interrupt registered, whose premise ("Codex sends no hook") was only true of Orca's old hook list.

Why

Guessing from keys cannot be right for Codex: whether a Ctrl+C or Esc cancelled anything depends on state only the TUI holds (a draft in the composer, an open popup, which chooser option the user picked). A guess plus a latch to protect it produced both failures we saw: a real cancel refused, and real later work held back. Using only the hook would leave a lost Interrupt (Codex kills it at 3 seconds under load) stranding the row. Using only the rollout would add a second of latency to every cancel and nothing for the common case. Hook first, with the rollout re-deriving the truth behind it, gives an immediate answer that cannot be stranded, and turn ids make both sources agree without any timing window. Subagents follow the same rule for the same reason: their own records say when they end, and the main agent's turn ending says nothing about them. The backstop reads current records instead of replaying the last hook because a replay can only ever restate a stale event: it re-runs that event's side effects and inherits its identity, and it has to die when the row changes to avoid overwriting newer truth, which is exactly when a lost cancel needs it.

Linked Issue

No tracking issue; reported directly.

Visual Proof

Live Electron QA screenshots are pending; QA will add them to this PR.

Testing

  • Capture first: real Codex TUIs (0.156.1 embedded, 0.157.0 shared background server) driven over a PTY with real keystrokes and Interrupt registered and trusted, 10 runs, committed as src/shared/__fixtures__/codex-interrupt-hooks.jsonl with a sidecar (user name, session, turn, agent and tool-call ids replaced by same-length placeholders). It shows Interrupt on every real cancel (Ctrl+C, Esc, chooser "Cancel task" and "Exit", a cancelled approval prompt), never on a draft clear, popup close, dismissed chooser, "Run in background", natural completion or idle Ctrl+C; only for the main agent; and no main-agent hook after it.
  • server-codex-interrupt-captures.test.ts replays that capture through the server's real HTTP ingress: seven busy-subagent cancels read cancelled at once, stay working through every later subagent hook, and settle done and interrupted on SubagentStop, and where the next turn completes, read it as completed; six cancels with nothing running settle at once; six keys that cancel nothing leave the turn working, and where that turn later ends with Stop, it reads completed; the one lost Interrupt in the capture settles from a rollout turn_aborted within one rollout read, and on the very next subagent hook; a lost Stop settles from task_complete; another turn's end in the rollout is ignored.
  • server-codex-interrupt-captures.test.ts also replays the "Run in background" run: the turn reads working with its prompt, its Stop leaves the still-running subagent holding the row, and its SubagentStop settles it; and a cancel followed by a new turn that ends before the cancelled turn's subagent keeps that subagent holding the row. A scratch replay of the same captures against main reproduced the later Stop dropping the subagent, as seen live on 0.157.1.
  • agent-hook-listener-codex-main-agent.test.ts: a subagent's hooks on a pane with no main-agent record leave the row done once the subagent stops, with no mainAgent, on the execution host and in the desktop's copy of a relayed row; server-codex-rollout-watch.test.ts: a subagent whose later turn comes on a different day than it started is read from the rollout its hook names and leaves when that rollout records its abort.
  • agent-hook-listener-codex-turn-identity.test.ts: same-turn restatements keep the cancel, another turn id reads working with or without a prompt, a turn a blocking Stop hook continued reads working again, a late fact cannot overturn a turn the rollout records complete, a late hook for a turn the rollout ended while the next one runs changes nothing, Interrupt keeps the roster, the rollout settles the current turn from a child's event, a turn's end is read when the next turn's start lands in the same read, an abort for any reason (or none) reads as the cancel, hooks without a turn id adopt the open turn (and a turn that started and ended within one read), a mid-turn compaction keeps the roster and the turn id (also when the rollout shows no open turn), and the watch arms only while a turn is open and a rollout is known, publishing a row with no hook name or explicit prompt.
  • server-codex-rollout-watch.test.ts: a lost Interrupt settles after an OSC repaint replaced the row object; a hook-announced subagent that gets no SubagentStop leaves once its own rollout records its abort; a subagent whose start is more than one 1 MiB read back survives the watch (a replayed SessionStart used to drop it); a turn that ended while Orca was down settles on restart, and one still open stays working and settles when it ends; relay to desktop, the cancel and a subagent the rollout finished reach the desktop, and a restarted relay's child event does not revive either.
  • Relay: agent-hook-server-codex-subagent-transcript.test.ts forwards the cancel the relay reads from the remote rollout when Interrupt is lost, with no hook name or explicit prompt; server-main-agent-turn-verdicts.test.ts drives the relay's real listener into the desktop and shows a late same-turn hook cannot revive the cancel, plus a new turn right after a cancel reads working.
  • Installers and trust: Interrupt is written with timeout 3 and trusted by the hash Codex computes (recomputed independently in the test from Codex's canonical form); the hash clamps a declared 10 to 3 and an absent timeout to 1; managed, WSL and SSH installer tests include it.
  • Renderer: Codex Ctrl+C and Esc never call inferInterrupt; Claude and other agents are unchanged. Parity table gains Codex arms driven by the real Interrupt hook.
  • Ablation, each on the committed tree and restored after: not registering Interrupt, dropping the hash clamp, not normalizing Interrupt, reading it as a completion, letting it clear the roster, removing the turn-id guard, removing the rollout settle, letting a Codex key infer again, ignoring the relay's main-agent fact, and keeping Codex under the latch each redden the tests that pin them. For the rollout watch: the previous commit's production code fails all five server-codex-rollout-watch.test.ts cases; a watch that never arms, keeping only the last turn marker, reading only some abort reasons as a cancel, removing turn-id adoption (or only its same-read case), not attributing an id-less hook to the open turn, applying a late hook for another ended turn, wiping or dropping the turn id on compaction, skipping the rollout read or the publishing tick on hydrate, a relay that never arms the watch, and a desktop that does not mirror (or merges instead of replacing) the relay's roster each redden the tests that pin them. For the subagent rules: restoring the old abort-reason rule reddens all four non-interrupted abort cases; a main-agent Stop that drops the roster reddens both new capture stories, the normalization, parity and restart tests; not reading a hook-announced subagent's own rollout, or not counting its turn_aborted as an end, reddens the abort and restart tests; keying the watch on already-tracked subagents reddens the late-subagent watch test; and a subagent's event that assumes a working main agent (on the host, or in the desktop's copy of a relayed row) and ignoring the rollout path a subagent's hook names each redden the tests that pin them.
  • pnpm tc, oxlint on changed files, and check:code-quality:changed pass. Vitest over src/shared, src/main/agent-hooks, src/main/codex, src/relay, src/main/native-chat, src/main/ipc, src/main/codex-accounts, src/main/runtime and the terminal-pane renderer passes, apart from structured-agent-session-lease-renewer.test.ts, which also fails intermittently alone on a temp-directory cleanup race in code this PR does not touch. On the latest head, pnpm tc, oxlint on changed files and check:code-quality:changed pass, and Vitest over src/shared, src/main/agent-hooks, src/main/ipc, src/main/runtime, src/main/codex, src/relay, src/preload, the terminal-pane renderer, the renderer's IPC events, store slices and src/renderer/src/lib passes.

Platforms: unit and capture replay on macOS. Windows .cmd and WSL installer paths are covered by unit tests only; live Electron QA is pending.

  • 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

  • Mixed versions, SSH: the desktop only talks to a relay built from its own bundle (the handshake refuses a different bundle hash), and the WSL relay is installed per bundle version, so an old relay never feeds a new desktop or the reverse. If a user downgrades, an old relay that receives an Interrupt hook ignores it as an unknown event, which is the behavior before this PR.
  • Mixed versions, paired remote runtime: the host owns hooks and the status store, and clients only render its rows. A new host publishes the Codex cancel on mainAgent.outcome and interrupted, fields old clients already read. A new client paired with an old host sees exactly what that host publishes today; keypress inference never reached a paired host, since the web client stubs it and the desktop sends it to its own local server.
  • Wire: no new opcodes and no new row fields. Codex rows carry mainAgent.outcome: 'cancellation' from Codex's own report, a value the field already had. The relay's rollout observation is an ordinary row with source: 'codex' and no hook name, which the desktop reads as a full restatement; the desktop and relay always come from one bundle.
  • Older Codex: Codex's hooks.json parser rejects unknown top-level keys but not unknown event names, so a build without Interrupt ignores the entry and relies on the rollout backstop instead. I checked this in current Codex source, not against a pre-0.150 binary.
  • Restart: the turn id is not persisted. On restart a Codex row that was not settled re-reads its saved rollout once (up to its last 1 MiB, the same bound every first read has), which recovers the turn id and any end recorded while Orca was down. A row saved as settled is not re-read, so a stale same-turn hook arriving right after a restart could still revive a cancelled turn; the capture shows Codex sends no main-agent hook after Interrupt, so this needs a delivery reorder across a restart.
  • Cost: the watch runs only while a Codex turn is open or rollout-tracked subagents run. Each tick is one stat of the parent rollout plus one per tracked subagent, and reads only bytes appended since the last read; it keeps no per-pane state beyond the existing rollout cursor, and the ended-turn window is capped at 32 ids per pane. It shares the existing coalesced one-timer scheduler and stops when the pane closes or when the turn and subagents end.
  • Docs: docs/reference/agent-status-store.md now describes Codex's own cancel report, the turn-id rule, the rollout watch, compaction and restart, and no longer says Codex sends no hook.
  • Separate follow-ups, not in this PR: a hook-durability change for all agents (a hook killed or lost in transit), and [Bug]: Codex 0.157 shared app-server daemon runs hooks with the first worker's env — concurrent workers are misattributed, and releasing one kills the others #22873, where Codex's shared background server attributes hooks to the pane that started it, which affects every Codex hook, not just Interrupt.

Out of scope

What a Codex row shows after the Codex TUI exits while Codex 0.157's shared background server keeps working ("Run in background", or "Exit" with a subagent still running) is not changed here. Today the row disappears for about 70 seconds, comes back from the server's later hooks with an empty prompt, and after "Exit" can stick at working over an idle shell. That is pre-existing on main and belongs with the shared-server work around #22873, because deciding it needs the host to know whether Codex's work can outlive the TUI. #23590 tracks it. This PR only removes one piece of that story: a subagent's late hook on a pane with no main-agent record no longer invents a working main agent.

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

  • A Codex mid-turn cancel now folds with its live children — inferInterrupt drops the agentType === 'codex' && childWorkEvidenced refusal, so a Codex row whose mainAgent is working is cancelled even while a subagent runs. The inferred row is built from markCodexLeadTurnInterrupted's fold rather than a hardcoded done.
  • markCodexLeadTurnInterrupted returns its fold — it now mirrors markClaudeLeadTurnInterrupted, returning { state, workingMode?, mainAgent? } derived from resolveCodexPaneStatus + codexMainAgentStatusForPayload. The existing latch caller in server-status-update.ts ignores the return, so that path is unchanged.
  • Relayed Codex rows are seeded before the fold — seedCodexStateFromSnapshot runs first, so a relayed row that never carried a hookEventName keeps the children its own snapshot evidences.
  • Captured Codex 0.156.1 evidence — new codex-cancel-bg-subagent-hooks.jsonl + meta sidecar and a new replay test through the real /hook/codex HTTP ingress (plus a relayed-pane variant).
  • Test and doc updates — parity table gains a Codex "interrupted with a live subagent" arm, the stale refusal test is rewritten to assert the fold, remote-hook-service-installers.test.ts now iterates CODEX_EVENTS (broadening coverage to SubagentStart/SubagentStop), and the capture fixture is renamed to agent-cancel-capture.test-fixture.

I ran pnpm tc:node, oxlint on the changed files, and the four touched/new test files (59 passing). The old-host guard (payload.mainAgent absent ⇒ keep child evidence), the child-held idle-prompt refusal (mainAgent.state !== 'working'), and the interrupted-on-done clamping are all preserved.

ℹ️ The live subagent-hook delivery path is not proven end to end

The fold depends on Codex's SubagentStart/SubagentStop reaching Orca's roster. The PR's own notes record that in a real-home dev instance the real subagent hooks did not surface on the row while a synthetic SubagentStart to the same ingress did, and the setup-specific cause could not be identified retrospectively. If that delivery gap is real in production, the cancel still settles the row correctly to done, but it loses the "keep showing the helper while it runs" behavior this PR adds. This is not introduced by the diff and does not block merge; it is worth a follow-up issue rather than relying on the capture plus replay tests alone.

Technical details
# Codex subagent hook delivery is asserted only by synthetic ingress

## Affected sites
- `src/main/agent-hooks/server-codex-cancel-captures.test.ts` — drives `SubagentStart`/`SubagentStop` via `postHookEvent(... '/hook/codex')`; this proves server handling, not that the real managed Codex home delivers those events.
- `src/shared/__fixtures__/codex-cancel-bg-subagent-hooks.jsonl` — proves the CLI emits the hooks, not that they reach Orca in a real home.
- `src/main/codex/codex-hook-definition.ts:17` — `CODEX_EVENTS` includes `SubagentStart`/`SubagentStop`, so subscription is present in source; the failure is setup/trust/runtime-home specific.

## Required outcome
- Track the delivery gap (trust entries / runtime home / endpoint env) so the roster-based hold is verified live, or document that the settled-`done` fallback is the accepted behavior when child hooks are absent.

## Open questions for the human
- Should a follow-up issue track the real-home subagent-hook delivery failure, and does it need a live transcript capture as evidence?

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

@brennanb2025
brennanb2025 force-pushed the brennanb2025/codex-cancel-with-subagent branch from 65f3bfd to 940b49e Compare September 25, 2026 04:28
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/lead-status-pr-c to main September 25, 2026 04:29
@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.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Codex hook configuration now includes the Interrupt event with an event-specific timeout and trust identity. The listener records cancellation and completion by turn ID, and it can settle an open turn from matching rollout lifecycle records. Codex keypresses no longer trigger inferred cancellation. Server and renderer status handling, tests, and reference documentation were updated to reflect these rules.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 0f466

A delayed Codex hook can incorrectly mark an active turn as done. Fix turn ordering before merging, and clarify the blocked-Stop documentation.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 0f466

The change should make cancellations visible sooner, but a late event from an earlier turn may replace the status of a newer turn. That could show an agent as stopped while it is still working. No new privilege escalation has been established.

Retained concerns

  • Medium · reliability · inferred: A delayed earlier-turn event can replace a newer turn’s lead status: the new guard rejects only same-turn terminal restatements, while root Interrupts publish cancellation and remote child events can supply main-agent state without turn identity. Whether delivery ordering prevents this remains unverified.
Security review details

Security Blast Radius

  • inferred — The demonstrated effect of a stale turn update is incorrect pane activity and cancellation status, potentially visible through relay-backed status. The examined path does not establish command execution, credential access, or an authorization bypass.

Trust Boundaries and Controls

  • observed — Parent-rollout reconciliation reads a hook-supplied absolute .jsonl path. The examined reader has no allowed-root check, but transcript reading existed before this change; the evidence does not establish that this PR newly exposes that file-read capability.

Resilience and Maintainability Implications

  • inferred — Status correctness under repetition is supported for an ended turn with the same ID, but correctness under out-of-order delivery across turns depends on an unverified producer or relay ordering guarantee.

Hardening Proposals

  • proposed — Preserve or independently verify current-turn ownership before applying a differing-ID hook or relayed main-agent fact; exercise delayed prior-turn events after a new turn starts.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 69 functions across 51 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 identifies the primary change: using Codex's own cancellation signal and turn ID to end Codex turns.
Description check ✅ Passed The description is comprehensive and follows the template. It explains the user impact, implementation, rationale, testing, compatibility, and scope. It records that the issue link is unavailable, vis…
Full details: Docstring Coverage

Explanation

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

✨ 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.

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Use the cached Codex roster for inferred-row child snapshots. · server-status-inference.ts:112-128

src/main/agent-hooks/server/server-status-inference.ts:112-128
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the cached Codex roster for inferred-row child snapshots.

When a no-hook Codex update omits subagents or reports a cached child as done, seedCodexStateFromSnapshot preserves the existing roster. markCodexLeadTurnInterrupted can therefore fold the cancellation to working. The inferred row then copies the raw payload.subagents, which can persist state: 'working' with a cancelled mainAgent but no working-child snapshot.

Use the cached roster when it exists. Use the incoming snapshots only when no roster exists.

Suggested fix
 import { foldMainAgentWithRowChildWork } from './server-row-child-work-fold'
+import { codexRosterToSnapshots } from '../../../shared/codex-subagent-roster'
 
 ...
     const state = recordFolded?.state ?? rowFolded?.stateName ?? 'done'
     const workingMode = recordFolded?.workingMode ?? rowFolded?.workingMode
+    const subagents =
+      agentType === 'codex' && this.state.codexSubagentRosterByPaneKey.has(existing.paneKey)
+        ? codexRosterToSnapshots(this.state.codexSubagentRosterByPaneKey.get(existing.paneKey))
+        : payload.subagents
     const inferred = this.applyNormalizedStatus({
 ...
-        ...(payload.subagents ? { subagents: payload.subagents } : {}),
+        ...(subagents ? { subagents } : {}),

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 8a7ca1ea-923d-4d16-8d20-b5e4ae921ccd

📥 Commits

Reviewing files that changed from the base of the PR and between 940b49e and c7edc31.

📒 Files selected for processing (9)
  • docs/reference/agent-status-store.md
  • src/main/agent-hooks/server-codex-cancel-captures.test.ts
  • src/main/agent-hooks/server-interrupt-inference-turn-baseline.test.ts
  • src/main/agent-hooks/server/server-status-identity.ts
  • src/main/agent-hooks/server/server-status-inference.ts
  • src/renderer/src/components/terminal-pane/agent-interrupt-inference-turn-baseline.test.ts
  • src/renderer/src/components/terminal-pane/agent-interrupt-inference.ts
  • src/shared/agent-interrupt-intent.ts
  • src/shared/agent-interrupt-turn-baseline.ts
💤 Files with no reviewable changes (1)
  • src/main/agent-hooks/server/server-status-identity.ts

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

@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

The newest commit (c7edc31) baselines an inferred interrupt on the main agent's own turn rather than the row's write time, closing the settle-window race where a busy subagent's hook silently dropped a Ctrl+C.

  • Judges the keypress's turn, not the row write — new shared isAgentInterruptTurnCurrent (src/shared/agent-interrupt-turn-baseline.ts) compares agent type and prompt, then for a row publishing mainAgent requires mainAgent.state === 'working' and the same mainAgent.stateStartedAt. A child hook or same-turn restatement re-stamps the row but not the turn, so it no longer voids the cancel; a Stop, a wait, or a new prompt still wins.
  • One predicate across renderer and server — the renderer's settle-window check, its acknowledged-keypress check (previously an object-identity comparison), and the server's re-check now call the same predicate, replacing the row-time comparisons rather than sitting beside them.
  • New optional wire field — baselineMainAgentStateStartedAt on the renderer-to-main agentInterrupt request; absent or non-number falls back to the exact row-write-time match (older renderer/host), and a row without mainAgent keeps the previous behavior.
  • Tests — new renderer and server suites pin each keep-vs-drop case (subagent hook, same-turn restatement vs. Stop, new prompt, new turn with the same prompt, permission wait, malformed field), and the captured-hook replay adds the settle-window race through the real HTTP ingress.

I traced mainAgent.stateStartedAt provenance on both sides (continueMainAgentStatus, setCodexMainAgentTurnState, seedCodexStateFromSnapshot, reconcileRemoteCodexState, hydration, OSC preservation) and could not construct a case where the two diverge for the same turn and refuse a valid cancel, nor a cross-turn inference wrongly admitted. The fallback arms reproduce the previous exact comparison. New/changed test files pass locally (20 tests across 3 files).

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

@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

The newest commit (55f434e) closes the last gap in the codex-cancel follow-up: an inferred cancel that live child work holds working is now dropped when the agent CLI exits.

  • Held-open inferred cancel is recognized on CLI exit — dropCommandFinishedStatusIfSameTurn (src/renderer/src/components/terminal-pane/pty-connection/interrupt-input-intent.ts:89) only matched a just-flushed cancel as a done + interrupted row, so a Codex cancel that live children keep working fell through and the pane stayed working until the 30-minute stale cutoff after the CLI and its subagent were gone. The new arm entry.mainAgent?.state === 'working' && current.mainAgent?.outcome === 'cancellation' matches the pre-flush snapshot (entry is read before flushPendingInterruptInference at pane-agent-identity.ts:230), still gated on allowInferredInterrupt and same prompt/agentType. A subsequent child hook re-stamping the row does not break the match, since the check reads the main agent's verdict rather than the row write time.
  • Regression test — pty-connection-command-finished-cleanup.test.ts drives the real connectPanePty path, admits Ctrl+C with a live subagent holding the row, then delivers OSC 133;D before the settle window and asserts the row is dropped. Ablating only the new arm reddens exactly this test (1 failed / 16 passed); restoring it passes (17/17). Confirmed locally.

The parked-terminal-command-status.ts port needs no change — it is deliberately inference-free because parked panes receive no keypresses.

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

…n instead of refusing it

Captured from Codex CLI 0.156.1 (codex-cancel-bg-subagent-hooks.jsonl): a mid-turn
Ctrl+C or Esc paints Interrupted but fires no hook, and the turn's subagent and
background terminals keep running, so only Orca's inferred cancel can settle the
main agent. The old child-evidence guard refused that inference, leaving
mainAgent working forever. The inferred cancel now folds through Codex's own
combine (mark the lead record, resolve against the roster), for local and
relayed panes alike; a relayed row that never reconciled into main's cache is
seeded from its own subagents first. The subagent's SubagentStop still fires
after an interrupted turn and is what settles the row to done + interrupted with
no completion stamp. Background terminals emit no evidence at start or end
(captured), so they never hold a Codex row open.
…rn, not the row write time

A subagent hook that landed in the 500 ms Ctrl+C settle window re-stamped the row, so the
renderer and server dropped the inferred cancel and the main agent read working forever.
For rows that publish mainAgent, one shared predicate now compares the main agent's turn
(agent type, prompt, mainAgent working with the same stateStartedAt); rows without it and
requests from an older renderer keep the exact row-write-time match.
When the agent CLI exits (command finished) while a Ctrl+C inference is
still pending, the renderer flushes it and drops the row if the row is
still that turn or the cancel it just inferred. It only recognised the
inferred cancel as a done+interrupted row, so a cancel that live child
work holds `working` (now admitted for Codex) was kept, leaving the
pane working until the stale cutoff after the CLI and its subagent were
gone. Recognise the main agent's cancellation verdict too.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/codex-cancel-with-subagent branch from 55f434e to 19c3015 Compare September 28, 2026 00:51
…ent's turn, not the row write time"

This reverts commit aaaf9ce.
…st hash Codex uses

Codex (0.150+) fires an Interrupt hook for the main agent when the user
cancels its turn, and Orca never subscribed to it. Every installer that writes
Orca's managed Codex hooks (managed home, real home, SSH remote, WSL) now
registers it. Codex runs Interrupt with at most 3 seconds and warns about a
larger value, so the managed entry declares 3. Codex hashes the timeout it will
run, clamped, so the trust hash and trust signature compute Interrupt's timeout
the same way; without that the hook reads as modified and never fires.
… turn id

A Codex turn cancelled while a subagent ran kept its main agent working for
30 minutes. Orca guessed the cancel from the keypress, and for Codex no key is
proof of one: Ctrl+C with a draft only clears it, Esc with a popup open only
closes it, and in shared-server mode Ctrl+C opens a chooser whose "Run in
background" cancels nothing. A cancel-verdict latch then held later real work.

Codex now reports its own cancel. The listener reads the Interrupt hook as the
main agent's `done` with a `cancellation` outcome for that turn, folded with
the subagent roster it leaves running, so the row stays working until the last
SubagentStop settles it done and interrupted. The Codex main agent record
carries Codex's turn id: a later fact for the ended turn restates it, and any
root hook for another turn is Codex working again. That replaces, for Codex,
the latch's time-window and tool-progress holds and the late-Stop restatement.

Codex writes turn_aborted (or task_complete) to its rollout before it runs the
Interrupt (or Stop) hook, and kills a slow Interrupt at 3s. So every Codex event
first catches the pane up on its parent rollout, and the existing one-second
rollout poll is armed while the main agent's turn is open, settling a lost
hook within a tick. Both paths write through the same record transition, so
they converge. The execution host owns this: a relay reads its own rollout,
and main mirrors the relay's main agent fact.

Codex is taken out of keypress inference in the renderer and in the server's
re-check. The Codex fold of an inferred cancel and its baseline and exit-drop
follow-ups are gone. The real-TUI capture with Interrupt registered replaces
the capture recorded without it, and drives the replay tests.
@brennanb2025 brennanb2025 changed the title fix(agent-status): fold a Codex mid-turn cancel with its live children instead of refusing it fix(agent-status): end a Codex turn on Codex's own cancel, decided by turn id Sep 28, 2026
… read working again

A Stop hook that returns a block makes Codex continue the same turn with the
same turn id, so treating any done record as the end of its turn left the row
done while Codex kept working. A turn is now over only once Codex aborted it
or its rollout records it complete; a Stop alone can be reopened by a later
root hook for the same turn.

Also corrects comments that said Codex writes turn_aborted/task_complete
before running the Interrupt/Stop hook: it writes them right after.

@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

This run re-reviewed the four commits pushed since the prior pullfrog review (55f434e): two reverts and the pivot to Codex's own cancel report.

  • Reverted the keypress-inference follow-ups — 8a0b3448/ca6829ee undo the mainAgent.stateStartedAt settle-window baseline and the drop-on-CLI-exit arm; src/shared/agent-interrupt-turn-baseline.ts and both turn-baseline suites are deleted.
  • Registered Codex's Interrupt hook in every installer — CODEX_EVENTS gains Interrupt, and each installer declares a per-event timeout (getCodexManagedHookTimeoutSec: interrupt = 3, otherwise MANAGED_HOOK_TIMEOUT_SECONDS) so the written timeout matches the clamped value Codex hashes for the [hooks.state] trust entry.
  • Made a Codex turn end once, decided by turn_id — the lead record carries Codex's turn_id and setCodexMainAgentTurnState ignores any later fact naming an already-ended turn; codexOutcomeRestatedByStop is removed.
  • Taught the listener to settle a Codex turn from the parent rollout — new codex-rollout-turn-lifecycle.ts, mainTurn on the transcript state, and catchUpOnCodexParentRollout recover a lost Interrupt (3 s cap, captured in run-d-0.157.0) either on the next hook or via a 1 s poll now driven by codexRolloutCanSettleMainAgent; transcriptPollUpdate also fires on mainAgent change.
  • Disabled Codex keypress inference — isInconclusiveInterruptIntent treats every Codex key as inconclusive (renderer skip + server re-check), the cancel-verdict latch exempts Codex, and the store docs are updated to match.

I read the full diff, traced the turn-id/rollout settle paths on both the local and relayed lanes, and confirmed the Codex trust-hash/timeout contract against the Codex source (default 1, cap 3, no matcher, hash over the normalized timeout). Ran the new/changed suites (44 + 81 passing) and pnpm tc:node, all green.

ℹ️ The PR description still documents the reverted mechanism

The prose (ELI5 / What Changed / Why / Visual Proof) and several Notes still describe Orca inferring the cancel from the Ctrl+C/Esc keystroke and baselining it on mainAgent.stateStartedAt, and name markCodexLeadTurnInterrupted — all of which the newest commits removed. The reference doc (docs/reference/agent-status-store.md) is current, so this only misleads a human reading the PR itself; worth a pass before merge.

Technical details
# PR body describes the pre-pivot design

## Affected sites
- PR #22727 body — "What Changed", "Why", and "Notes" describe keypress inference, the 500 ms settle-window baseline, and `markCodexLeadTurnInterrupted`.
- `docs/reference/agent-status-store.md` — already updated to the `Interrupt`-hook + rollout-read-back design, so the two disagree.

## Required outcome
- The body should describe: Codex reports its own cancel via the managed `Interrupt` hook; `turn_id` decides whether a fact restates an ended turn or opens a new one; the parent rollout (and the 1 s poll) is the backstop for a lost hook.

## Open questions for the human
- None blocking; purely a description refresh.

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

@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: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: e9549b60-9667-47ec-ba2f-7d4d227d2558

📥 Commits

Reviewing files that changed from the base of the PR and between f09e7db and 0f46695.

📒 Files selected for processing (6)
  • docs/reference/agent-status-store.md
  • src/relay/agent-hook-server-codex-subagent-transcript.test.ts
  • src/shared/agent-hook-listener-codex-turn-identity.test.ts
  • src/shared/agent-hook-listener/providers/codex-state.ts
  • src/shared/agent-hook-listener/transcript-poll-policy.ts
  • src/shared/agent-interrupt-intent.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/relay/agent-hook-server-codex-subagent-transcript.test.ts
  • src/shared/agent-hook-listener/transcript-poll-policy.ts
  • src/shared/agent-interrupt-intent.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Comment thread docs/reference/agent-status-store.md Resolved

@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

This run re-reviewed the two commits pushed since the prior pullfrog review (f09e7dbf): 592ab6c030 and 0f46695cde.

  • A Stop-completed Codex turn is final only once the rollout records it — new codexTurnIsOver gates the same-turn restatement. After a normal Stop, a later root hook for the same turn_id reads working again (Codex continued the turn because a Stop hook blocked), while an aborted turn (outcome: 'cancellation') or a rollout-recorded task_complete still resists later same-turn facts.
  • The rollout arm of codexTurnIsOver — a done record with no cancellation outcome stays final only when the parent rollout's mainTurn names the same turn and is completed.
  • Corrected the doc and comment ordering claim — docs/reference/agent-status-store.md and the inline comments now say Codex writes its rollout end marker right after it runs the Interrupt/Stop hook, rather than before.
  • Extended the turn-identity suite — a blocking-Stop continuation reads working then cancels on Interrupt, and a rollout-recorded completion resists a later same-turn PostToolUse/Interrupt.

I read the incremental delta and the surrounding codex-state.ts settle paths on both the local and relayed lanes, traced Stop → continuation → Interrupt and the rollout-completion sequences, and ran the focused suites (agent-hook-listener-codex-turn-identity, server-codex-interrupt-captures, server-main-agent-turn-verdicts: 39 passing). Reverting only the Stop-finality condition reddens exactly the new continuation test (1 failed / 9 passed), so it gates the change.

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

…s Interrupt hook

Codex runs the Interrupt hook first and writes TurnAborted to the rollout after it
returns or is killed at its 3s cap. The lost-Interrupt capture test still carried
the reversed ordering that the docs and PR body already dropped.
…ng the last hook

The Codex backstop re-ran the pane's last hook body every second. That re-ran its
side effects (a replayed SessionStart wiped the roster and re-read the rollout from
the start), published old hook identity (a prompt boundary on every tick), and died
whenever the row object changed, so a lost Interrupt with no later hook stayed lost.

- A shared rollout watch, owned by the execution host (main and the relay), reads the
  parent rollout and publishes the row rebuilt from current records, with no hook name
  or explicit prompt. It is armed while the main agent's turn is open (its own record
  or the rollout's) or rollout-tracked children run, and follows the pane's records,
  not row identity. Muse keeps its replay poll.
- The rollout's turns are tracked by id: the open turn plus a bounded window of ended
  turns with their end. Only turn_aborted with reason interrupted is a cancellation.
  A hook for an ended turn changes nothing; a record with no turn id adopts the open
  turn.
- SessionStart(compact) no longer drops the roster, rollout state or turn id.
- Hydrate reads a restored Codex row's saved rollout once, so a turn that ended while
  Orca was down settles on the first tick.
…han one rollout read back

A restart's rollout read covers at most the last 1 MiB, so a long turn's
task_started can precede it. The read then saw only the turn's end, never
learned it as the latest turn, and the restored row stayed working.

@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 in this delta — e295ac70dc is a correct, test-backed fix. Approval is held only by the still-open prior inline suggestion on codex-state.ts:88-90, which this commit does not touch.

Reviewed changes

This run re-reviewed the single commit pushed since the prior pullfrog review (5ef9f736c6).

  • Seed latestTurnId from a rollout end marker — recordCodexRolloutTurn now does turns.latestTurnId ??= turnId when recording an end (src/shared/codex-rollout-turn-lifecycle.ts:89-90). When a restart's first read begins past a long turn's task_started (reads are bounded to the last 1 MiB), the end marker previously left latestTurnId undefined, so adoptable stayed false and the restored working row never adopted the ended turn — stranding it until the next root hook. With the seed, adoption attaches the turn id and recordedEnd settles the record to done/cancellation.
  • Regression test — settles a turn whose start is more than a read back once Orca starts again appends >1 MiB of filler between task_started and turn_aborted, then restarts Orca and asserts the row settles. Falsifiable: removing line 90 reddens exactly this test (state: 'working' instead of done); restored and green.

I traced the adoption/latestBefore interaction: the ??= can only seed latestTurnId on the first marker seen, and a start marker sets it unconditionally, so an end marker for an older turn cannot clobber a newer latestTurnId. Ran server-codex-rollout-watch (6) and agent-hook-listener-codex-turn-identity (19) — 25 passing.

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

…reports

Codex's app-server reports every aborted turn as interrupted whatever its
TurnAbortReason, and its TUI says the turn was stopped. Read any turn_aborted
(interrupted, replaced, review_ended, budget_limited, or no reason) as the same
cancellation an Interrupt hook produces, and stop parsing the reason.
…, not the main agent's Stop

The main agent's Stop dropped every subagent the pane did not track through the
parent rollout, so a new turn that ended while a cancelled turn's subagent still
ran read done for about a minute, then flipped back to working on the
subagent's next hook. What that rule really covered is a subagent Codex aborts,
which fires no SubagentStop.

Every subagent in the roster, whether a hook or the parent rollout announced it,
is now also read from its own rollout (named by its thread id, the hook's
agent_id). It leaves on its own SubagentStop, when that rollout records its
turn's end (completed or aborted), when the rollout stays unreadable for a
minute, or when the session ends. The watch keeps reading while any subagent
runs. A restored subagent is settled the same way instead of on the next Stop.
…ows working

Codex's turn and subagents run in its app-server, and Codex 0.157's shared
background server keeps running them after the TUI exits ("Run in background",
or "Exit" with a subagent still running), still posting hooks to the pane. The
command-finished cleanup dropped the row and wiped the pane's Codex records once
the shell was back, so the row vanished while the work ran quietly, came back
from the next hook with no main agent or prompt, and after "Exit" stuck at
working over an idle shell.

The cleanup, for mounted and parked panes, now leaves a Codex row that still
shows work and names its rollout in place and only clears the CLI's launch
record; the host's rollout reads end the row. An embedded Codex that exits
aborts its open turns and subagents, which their rollouts record.

@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 subagent- and TUI-exit commits look sound, but 342c83a697 now reports every Codex-aborted turn as a user cancellation. Codex itself distinguishes abort reasons (replaced and review_ended are errors, not interrupts), so those turns would now read interrupted. Details inline.

Reviewed changes

This run re-reviewed the four commits pushed since the prior pullfrog review (e295ac70dc).

  • Every Codex turn_aborted now reads as a cancel — the rollout lifecycle drops the TurnAbortReason and setCodexMainAgentTurnState sets outcome: 'cancellation' for any abort, whatever its reason.
  • A Codex subagent leaves the row by its own records — the root Stop no longer drops the roster; every roster child, hook- or parent-rollout-announced, is read from its own rollout, and the watch arms on roster size.
  • A Codex TUI exit no longer ends a row that still shows work — new shared cliExitEndsAgentRow gates the mounted and parked command-finished drops so a working Codex row naming a rollout is kept and settled by the host's rollout reads.
  • Tests — capture replays for a subagent outliving its parent turn, a child retired from its own rollout abort, hydration retiring a restored child, and TUI-exit retention across mounted and SSH panes.

ℹ️ Nitpicks

  • Child rollouts are resolved purely from the hook's agent_id (the -${threadId}.jsonl suffix); the new tests build the filename from that same id, so they encode the assumption rather than prove it. A real child-rollout capture would pin the match.

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

const settled: Pick<CodexLeadTurnState, 'state' | 'outcome'> = recordedEnd
? {
state: 'done',
outcome: recordedEnd === 'interrupted' ? 'cancellation' : 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.

Every turn_aborted now publishes mainAgent.outcome: 'cancellation' and interrupted: true. Codex's own core maps replaced and review_ended aborts to AgentStatus::Errored (not an interruption) and renders budget_limited as a budget stop, so this labels non-user-cancel ends as user cancels. Keep the reason (or at least stop treating replaced/review_ended as cancellation).

Technical details
# Every `turn_aborted` is published as a user cancellation

## Affected sites
- `src/shared/agent-hook-listener/providers/codex-state.ts:89` — `outcome: recordedEnd === 'interrupted' ? 'cancellation' : undefined`; since `CodexRolloutTurnEnd` is now only `'completed' | 'interrupted'`, every abort sets `cancellation`.
- `src/shared/codex-rollout-turn-lifecycle.ts:22-24,42-49` — `decodeCodexRolloutTurnLifecycle` maps any `turn_aborted` to `'interrupted'`, and the type drops the reason.
- `docs/reference/agent-status-store.md:241-244` — states the (lossy) justification.
- `src/shared/agent-hook-listener-codex-turn-identity.test.ts:157-172` — locks the behavior in for `replaced`, `review_ended`, `budget_limited` and reasonless aborts.

## Evidence from openai/codex
- `codex-rs/core/src/agent/status.rs:12-19` maps `Interrupted | BudgetLimited` → `AgentStatus::Interrupted`, and `Replaced | ReviewEnded` → `AgentStatus::Errored(...)`.
  https://raw.githubusercontent.com/openai/codex/main/codex-rs/core/src/agent/status.rs
- `codex-rs/tui/src/chatwidget/input_restore.rs:311-337` renders `BudgetLimited` as "Goal budget reached - the turn was stopped." and only `Interrupted` as "Conversation interrupted".
  https://raw.githubusercontent.com/openai/codex/main/codex-rs/tui/src/chatwidget/input_restore.rs
- `codex-rs/app-server/src/bespoke_event_handling.rs:1533-1560` does collapse every `TurnAborted` to `TurnStatus::Interrupted`, but only by discarding `reason` — a lossy projection, not evidence that each abort was a user cancel.

## Required outcome
- A `turn_aborted` with `reason: 'replaced'` or `'review_ended'` must not set `mainAgent.outcome: 'cancellation'` / `interrupted: true`. `budget_limited` is defensible either way, but Codex's TUI treats it as a budget stop, not an interrupt.

## Suggested approach
- Keep the reason (or a boolean "user cancel" flag) on the decoded lifecycle and gate the `cancellation` outcome on it, restoring the pre-`342c83a` distinction.

…ackground server runs its work

The TUI-exit cleanup kept any Codex row that still showed work and named its
rollout. A Codex that runs its session in the TUI itself (embedded: Codex before
the shared server, Windows panes, or a TUI that could not use it) takes that work
with it when killed mid-turn and writes no end marker, so its row stayed working
until the staleness cutoff, the watch kept reading its rollout every second, and
every later shell command in the pane re-kept it.

The execution host now reports where a Codex row's work runs: sessionRunner
'background-server' while the shared server of the rollout's Codex home is
running, read from the pid record that server keeps under
<CODEX_HOME>/app-server-daemon (the relay reads it for SSH panes). The renderer
keeps a row past its CLI only for that fact and retires every other row as
before. If the server itself dies, its record stays but its pid is gone, and
the watch ends the main agent's turn as cancelled and drops its subagents.
A turn the main agent already ended as cancelled no longer keeps the watch
reading an open rollout turn.
A child's hook on a pane with no record of the main agent built a row that
assumed the main agent was working, and nothing ever ended that assumption: after
the child's SubagentStop the row stayed working over an idle pane. It is reachable
once the pane's records are gone while a child still runs, for example after an
SSH relay restart, where main also inferred and kept a working main agent from
the relay's child-driven row.

With no record of the main agent, the children alone drive the row, on the
execution host and in main's copy of a relayed row, and the row carries no
mainAgent (unknown) instead of a made-up one.
…name

Since every roster child is read from its own rollout, a hook-announced child's
file was looked up by thread id in the parent's day folder and the folder of the
day the child re-entered the roster. Codex files a rollout under the day it
started, so a child spawned on one day whose later turn came on another missed
both folders: it was never seen to end and was dropped as unreadable after a
minute while still working, then re-added by its next hook.

A child's own hooks carry its rollout path (transcript_path), so the child is
read there; the folder lookup remains for children only the parent rollout
announces.

@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 critical issues — one minor suggestion inline. Approval is held by the still-open prior inline concern on the Codex abort-reason handling, which this delta does not touch.

Reviewed changes

This run re-reviewed the four commits pushed since the prior pullfrog review (57f14f3e2a): the host-reported background-server retention, the child-only row, the child-rollout path, and the docs.

  • Gated the TUI-exit retention on the host-reported background server — new src/shared/codex-background-server.ts reads the Codex app-server daemon pid record at <CODEX_HOME>/app-server-daemon/{daemon,app-server}.pid; the execution host publishes sessionRunner: 'background-server' on a working Codex row while that home's server is alive, and cliExitEndsAgentRow keeps (instead of drops) the row.
  • Ended the server's work once its pid is gone — endCodexWorkOfStoppedServer cancels the main agent's open turn and clears its roster when the reported server is gone, since a killed server writes no end marker.
  • Stopped a child event from inventing a main agent — buildCodexChildDrivenStatusPayload and reconcileRemoteCodexState (inferMainAgent: agentId === undefined) let the children alone drive the row and publish no mainAgent when the host has no root record.
  • Read a child from the rollout its own hooks name — recordCodexChildRollout records the transcript_path a child's hook carries, so a child whose rollout the date-folder lookup misses is still read and retired from its own records.
  • Updated the docs and comments — the TUI-exit rule reads as host-reported and the background-server limits are documented.

I verified the pid-record path and JSON shape against the upstream openai/codex app-server-daemon source (<CODEX_HOME>/app-server-daemon/daemon.pid, legacy app-server.pid, camelCase pid), and the committed capture fixture confirms a child hook's agent_id is the thread id in its rollout filename.

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

…odex's background server runs its work"

This reverts commit e5ecdd5.
…lives the main agent's turn

The revert of the TUI-exit change took these host tests with it, but they pin
that the main agent's Stop leaves a running subagent holding the row, which the
subagent-records change still owns: both fail if Stop clears the roster again.

@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 in this delta — it removes the background-server retention feature and restores the host tests that pinned roster-holding.

Reviewed changes

This run re-reviewed the four commits since the prior pullfrog review (33140e93): three self-reverts and the test commit that reinstates coverage the reverts dropped.

  • Reverted the TUI-exit retention feature — d6306852/234963f2/b7cd07b5 remove sessionRunner: 'background-server' from the wire and the store, cliExitEndsAgentRow, src/shared/agent-row-cli-exit.ts, agent-session-runner.ts, codex-background-server.ts, server-codex-background-server.test.ts, pty-connection-codex-tui-exit.test.ts, and the endCodexWorkOfStoppedServer watch hook. A repo-wide search finds no leftover references and the docs no longer mention the background server, so the Codex keypress-inference and rollout pieces read cleanly on their own.
  • Reinstated the roster-holding host tests — 0953102f adds an a subagent outlives the main agent's turn suite to server-codex-interrupt-captures.test.ts: the run-d3bg ("Run in background") and run-esc captures assert that a main-agent Stop leaves a running child holding the row and that a later turn completing first does not inherit the cancelled turn's verdict. Both assert exact mainAgent/subagent/interrupted values, so a Stop that cleared the roster would fail them; the file passes 25/25 on this head.

Approval is withheld only because two prior review threads remain open on codex-state.ts (the different-turn guard and the abort-reason handling); neither is touched by this delta.

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

…d row

A row dropped while its Codex turn was open (the CLI exited, or the user
dismissed it) keeps only its resume identity. On the next Orca start the
restart read armed the rollout watch for it, and the watch published the
row rebuilt from the rollout as a live one, so the dropped row came back
(or, with no end recorded, the watch read the rollout every second for as
long as the pane lived). The watch now only restates a live Codex row.

@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 in this delta — the one commit is a correct, test-backed guard. Approval remains held by the two prior open threads on codex-state.ts, which this commit does not touch.

Reviewed changes

This run re-reviewed the single commit pushed since the prior pullfrog review (0953102f).

  • A dropped Codex row is no longer revived by the rollout watch — new liveCodexRow treats a providerSessionOnly remnant as absent, so codexRolloutNeedsWatch does not arm on it and observeCodexRollout publishes nothing for a row a drop reduced to its resume identity.
  • Regression test — does not bring back a row that was dropped while its turn was open drops the row mid-turn, appends turn_aborted, restarts Orca, and asserts the snapshot stays the providerSessionOnly/working remnant. Ablating the guard would publish a settled done row, so it gates.

I traced the drop path (dropStatusEntry → toRetainedProviderSessionRow) and hydration's arm-then-tick path: with the guard, the tick's observeCodexRollout returns nothing and sync clears the watch, so the remnant is preserved as intended. The pre-existing settles from the rollout once Orca starts again case (a non-dropped row) still arms and settles.

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing this older combined implementation. #24332 already uses Codex’s Interrupt hook; create an issue for the remaining delayed-event and missed-report cases before closing.

The remaining problem is tracked in #27674.

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