feat(agent-status): combine Codex child work through the shared main-agent status fold - #22475
Conversation
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the Codex hook lane's private child-combine is deleted and Codex now folds through the shared foldAgentLeadStatus, which gains a waiting-child input; the full 16-file diff at a897f06602 was reviewed.
- Shared fold gains a
waitingchild arm —agentChildWorkLivenessranks a child inwaiting/blockedaboveworking/monitoring, andfoldAgentLeadStatusreturnswaitingfor a working or settled lead unless the lead is itself asking. - Codex combine deleted, fold adopted —
codexRosterEffectiveStateis removed;resolveCodexPaneStatusfeeds the roster through the shared rule on all three publish paths (root events, child-driven events, relayed rows). - Cancellation carried into a late
Stop—codexCarriedTurnOutcomepreserves Orca's inferred cancel where Codex's own Stop carries no verdict, mirroring the Claude lane. - Claude lane insulated —
hasWaitingChildWork: false, with the wait still expressed on the displaced lead record (waitingAgentId/stateBeforeWait).
Verified: Codex's published { state, workingMode, lead } is unchanged for every reachable input (the only delta is the intended lead.outcome: 'cancellation' on a late Stop); the new waiting arm is unreachable for the structured, Claude hook, and Grok lanes' current producers; no wire change. 71 scoped tests pass and pnpm tc exits 0. Two non-blocking notes: a foreign/other-version host emitting a structured live task in waiting/blocked would now flip the row to waiting, and the hasWaitingChildWork: false hardcode relies on TrackedClaudeSubagent.state staying working|idle.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
a897f06 to
b376568
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the delta since Pullfrog's prior review at a897f06 (a rebase onto PR A's mainAgent naming plus shared-helper consolidation) at b376568.
- Rebased onto the renamed base — the PR now builds on PR A's
mainAgentterminology; the parity and Codex test files,main-agent-turn-state.ts, and docs follow. - Interrupted helper consolidated into the fold — the base's Claude-local
claudeMainAgentTurnInterruptedis deleted in favor of the sharedmainAgentTurnInterruptedinagent-lead-status-fold.ts(now typed offAgentMainAgentStatus). Semantics are unchanged (outcome === 'cancellation'). - Codex parity assertion simplified — the Codex lane now refolds solely through
refold(row.mainAgent, codexRosterChildWorkLiveness(roster)); the now-unreachablecodexMainAgentStateblocked-root guard was dropped.
Verified locally: pnpm tc exits 0 and the six touched suites pass (71 tests). The waiting-child arm, Codex fold and cancellation carry are behaviorally unchanged from the previously reviewed commit.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
3798c11 to
2b24918
Compare
b376568 to
4d76d65
Compare
…agent status fold
4d76d65 to
f0486e5
Compare
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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 (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe shared status fold now accounts for child work waiting on a human. Blocked child work remains live but does not set the combined row to waiting. Claude and Codex hook-provider paths use shared interruption detection, and Codex pane status is resolved from main-agent state and child-work liveness. Tests cover these behaviors across provider lanes and structured sessions. Documentation describes the combining rule and the remaining named differences. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The changed status paths have no established merge-blocking issue. The Codex child-event fallback preserves a recorded root state. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 15 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
…ade stale A child failure reported in place as `blocked` now pins the row `waiting`, not `working`; and no relay ever sent an unfolded `working` beside a waiting child.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the delta since Pullfrog's prior review at b376568 (a rebase onto the newer base 80f5aae0f9, plus a helper consolidation), and the full 16-file diff at f0486e5f.
- Adopted the base's
mainAgentsemantics for the Claude wait — the base now publishes the pre-wait state (stateBeforeWait) asmainAgentinstead ofwaiting, so the parity table's two child-wait Claude rows expectworking/donebeside the waiting row, the test'sclaudeChildWorkLivenesshelper deriveshasWaitingChildWorkfromwaitingAgentId, and the docs drop the Claude divergence note. - Dropped the redundant cancellation helper — the feature's
codexCarriedTurnOutcomeis deleted; both Codex publish paths now use the base'scodexOutcomeRestatedByStop. Codexfailureis unreachable (codexLeadStateForHookEventhandles noStopFailure), so this is behavior-preserving for every reachable input. - The waiting-child arm, Codex fold adoption, and relay re-derivation are unchanged from the previously reviewed commit.
Verified locally: the six touched suites pass (71 tests). No new issues.
ℹ️ The PR description's Claude divergence note is now stale
The "Two things a reviewer should know" point #2 still says Claude's published mainAgent.state reads waiting while the main agent is really working or done, and that the parity table pins that divergence. The code and docs now say the opposite — claudeMainAgentStatusForPayload publishes stateBeforeWait, so Claude's mainAgent matches Codex and only the cancelled-turn-with-watch-loop divergence remains. Worth updating the description so a reviewer is not sent looking for a divergence that no longer exists.
Technical details
# Stale PR description: Claude mainAgent no longer reads `waiting`
## Affected sites
- PR description, "Two things a reviewer should know" point #2 — claims Claude's `mainAgent.state` reads `waiting` and that the parity table pins it as a known divergence.
- `src/shared/agent-hook-listener/providers/claude-roster-state.ts:151` — `claudeMainAgentStatusForPayload` publishes `stateBeforeWait` when `waitingAgentId` is set.
- `docs/reference/agent-status-store.md:240` — "publishes the displaced state as `mainAgent`, so its rows match Codex."
## Required outcome
- The description's Claude note reflects the current behavior: the displaced wait is the child-work fact, Claude's published `mainAgent` matches Codex, and only the cancelled-turn divergence remains.DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
A child's `blocked` state means its task failed (the only producer maps a failed background task to it, and the background-task view labels it "failed"), not that a human must act. Folding it into the waiting arm would surface a failed child as needs-you. It stays live work, as before this series.
… row wait A child's blocked state means it failed; only its waiting state feeds the waiting arm. Two fold comments, a test describe and two parity story names still called the waiting child blocked.
There was a problem hiding this comment.
ℹ️ No critical issues — a couple of doc/description alignment nits for the narrowed waiting arm.
Reviewed changes — the delta since Pullfrog's prior review at f0486e5f (commits 2db5562 and 5fecbe2).
- Child-waiting arm narrowed to
waiting—isWaitingChildWorkno longer treats a child'sblockedstate as a human request. Ablockedchild is a failed one (its only producer maps a providerfailedtask to it), so it is live work again and an idle main agent beside it readsworking, notwaiting. - Docs and tests realigned — the reference doc, the liveness and structured suites, and the fold expectations follow the narrower arm;
isLiveChildWork's comment now says an in-placeblockedfailure pinsworking. - Comment-only corrections —
2db5562fixed the relay test comment and theisLiveChildWorkdoc.
Verified: failed → blocked at claude-background-task-frames.ts:139 and blocked: 'failed' in native-chat-background-task-row.ts confirm the state means failure, not a human request; the Codex roster is typed working | waiting only, so no reachable Codex input changes. Five touched suites pass (66 tests).
ℹ️ The PR description still describes the reversed policy
The "Mechanism" bullet, point #1 of "Two things a reviewer should know", and the third rejected alternative all still say the waiting arm is produced by a child in waiting or blocked, and list "Keeping blocked children out of the waiting arm" as a rejected alternative. Commit 5fecbe2 implements exactly that rejected alternative: only waiting produces the arm, and a blocked child stays live working work. A reviewer reading the description would be sent looking for the opposite of what the head does.
Technical details
# PR description contradicts the narrowed waiting arm
## Affected sites
- PR description, "Mechanism" bullet 1 — "A live child whose own state is `waiting` or `blocked` produces it."
- PR description, "Two things a reviewer should know" #1 — says the structured pin now expects `waiting` for a `waiting` or `blocked` subagent; a `blocked` child still reads `working`.
- PR description, "Alternatives considered" bullet 3 — lists "Keeping `blocked` children out of the waiting arm" as rejected; `5fecbe2` does it.
## Required outcome
- The description reflects the head: a child's `waiting` state is the waiting-arm input; `blocked` (a failed task) stays live work and does not surface as needs-you.DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
…the structured lane's real input The doc said the Claude hook lane's rows match Codex and that every lane feeds a child's wait into the fold. Neither holds: Claude keeps the wait in one slot the next main agent event overwrites, the structured lane turns a child's prompt into the main agent's own attention, and Codex drops its roster on a root Stop when it tracks no child transcripts. The parity story now drives the structured lane with the input it actually receives.
Review status: ready to merge at
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the delta since Pullfrog's prior review at f46d037 (commit 26f1197: the reference doc's divergence paragraph and one parity-story expectation), verified against the code it describes.
- Reference doc's divergence list corrected — the
agent-status-store.mdclaim that every lane feeds a child's wait into the fold and that Claude's rows match Codex is replaced with a precise list: only the Codex hook lane feeds the waiting-child input today, plus four named divergences — the cancel/watch-loop row, Claude's single-slot displaced wait, the structured lane's session-levelattention, and Codex dropping its roster on a rootStop. - Parity story drives the structured lane with its real input — the "settled main agent whose child is waiting on the user" story now feeds
status: 'attention'with a working child and pins the resultingblockeddivergence under aKNOWN DIVERGENCEcomment, replacing a syntheticstate: 'waiting'task that no in-repo structured producer emits.
Each documented mechanism checks out in code: codex-events.ts:216 drops the roster on a childless-transcript Stop; claude-events.ts:235-251,295 stashes the displaced state and overwrites waitingAgentId on a later child wait; structured-agent-session-projection.ts:232-239 returns attention for a pending approval/question; and the structured live-task producers only emit working (claude-background-task-frames.ts:150) or no state (codex-background-task-tracker.ts:61), so no waiting task reaches the fold from a real producer. pnpm test on the parity, structured, fold and liveness suites: 58 passed.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

ELI5
Orca shows one status per agent pane: working, waiting for you, or done. When the main agent has helpers (subagents) running, that one status has to combine what the main agent is doing with what the helpers are doing. Every kind of agent used one shared rule for that, except Codex, which had its own copy. This change teaches the shared rule the one thing Codex's copy could do that it could not (a helper waiting for your approval makes the whole pane say "waiting"), moves Codex onto the shared rule, and deletes Codex's copy.
What Changed
Before and after, as the user sees it. No visible change. A Codex pane whose helper is waiting for approval reads "waiting", and one whose main turn finished while a helper still runs reads "working", exactly as before. What changes is that the next fix to the shared rule now reaches Codex too, instead of silently skipping it.
Mechanism.
src/shared/agent-status-child-work-liveness.ts) gains awaitinganswer, ranked aboveworkingandmonitoring. Only a helper whose own state iswaitingproduces it. A helper'sblockedmeans its task failed (the only producer maps a failed task to it, and the task view labels it "failed"), so it still counts as ordinary live work, as before. Lost contact (unverifiable) is not a request for a human either.src/shared/agent-lead-status-fold.ts) reads that answer: a waiting helper makes the rowwaitingwhatever the main agent is doing, unless the main agent is itself asking, in which case the main agent's own word wins.codexRosterEffectiveState) is deleted. Codex now only gathers evidence (every tracked helper is an agent; its state feeds the shared summary), and one function,resolveCodexPaneStatus, folds the Codex main agent with that evidence. All three Codex publish paths go through it: main agent events, helper-driven events, and relayed rows that the main machine reconciles for SSH sessions.src/shared/main-agent-status-parity.test.ts), which checks that every agent type publishes what the shared rule says for its evidence.Why Codex results cannot change: a Codex helper is only ever
workingorwaiting, and a finished helper is removed rather than kept asdone. So "any tracked helper" and "any live helper" are the same thing, and every combination of main agent state and helper states gives the same answer under the old and new rule. Codex never produces themonitoringmode, because every Codex helper is agent work.Why
The agent status reference doc's rule is that every agent type derives the combined status from one shared rule. Codex was the last one with its own copy, so a future change to the shared policy would silently not reach it.
Alternatives considered:
Known issues (existing on main, not caused by this PR)
The same event sequences give the same results on
mainand on this branch. They are listed indocs/reference/agent-status-store.mdand will be fixed in a follow-up stacked on this PR:Linked Issue
Follow-up to #22452 (which added the main agent's own state to each row) and #22295. No separate issue.
Visual Proof
Validated in a hidden dev build of this branch at
f46d03790a(macOS). Codex is not installed on the test machine, so each scenario was driven by running Orca's installed Codex hook script inside a real Orca terminal with Codex-shaped events. The screenshots show the sidebar row and the tab.A

B

C

D

E

Testing
pnpm tc: pass.pnpm exec oxlinton changed files andpnpm run check:code-quality:changed: clean.pnpm testover the shared rule, helper summary, Codex roster, parity, native chat status, Codex main agent, hook listener andsrc/main/agent-hookssuites: pass.Deletion checks, each restored afterwards and each failing for the intended reason. These were run on the commits that introduced each piece, not re-run at the final head: the rule's waiting-helper branch, the main-agent-asks-first guard, the waiting evidence in the helper summary, the Codex helper state feeding the summary, the relayed-row re-derivation, and the
waiting-only rule (restoringblockedas waiting fails the two tests that pin a failed helper as live work).The Known issues sequences were run against
origin/mainand this branch, and matched.Platform: macOS. The change is shared logic with no platform branch.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
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.Notes
No wire change:
state,workingModeandmainAgentalready exist on the row, and helperwaitingis already an accepted helper state. For SSH, the main machine re-derives a relayed Codex row through the same rule; relays of any version send the helper list the rule reads, and old and new rules agree on it. Mobile reads the published row and needs no change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)