fix(delivery): stop state-projected supervisor method from stalling review loops - #3750
fix(delivery): stop state-projected supervisor method from stalling review loops#3750eltmon wants to merge 1 commit into
Conversation
![panopticon-agent[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…eview loops A codex app-server work agent was stamped supervisorEnabled=true at spawn even though its launcher never wraps in the PTY supervisor, and the state projection turned that into deliveryMethod: 'supervisor'. deliverAgentMessage treated the persisted value as a strict per-call opt-in (PAN-1769 semantics), so every state-routed delivery to the socketless agent threw socket-missing with no fallback. On 2026-08-16 this swallowed the inspect verdict for PAN-3743: the work agent parked waiting, the review loop stalled, and only a console.error recorded it until the PAN-2583 workspace-verdict sweep healed the loop ~30 minutes later. Three changes, root to symptom: - supervisor-channels: decideSupervisorForWorkAgent rejects codex on the app-server transport (the app-server branch of buildCodexCommand skips the wrap), so the supervisor stamp is never applied to a launch that cannot have a socket. Matches shouldUseSupervisorForConversation and the launcher's own config read. The codex.transport access is optional-chained like runtime-command.ts so a partial config can't throw mid-spawn. - delivery: deliverAgentMessage routes a state-derived 'supervisor' through resilientDeliveryMethod (PAN-1988), so a stale projection falls through to the live app-server/channels/tmux tiers. An explicit deliveryMethod caller argument keeps the strict throw-on-failure contract. - inspect-agent: a verdict that fails to deliver now also surfaces an operator-visible needs-you mark via surfaceIssueFeedbackNeedsYou (the PAN-2228 pattern), instead of console-only. Best-effort: the error log is preserved verbatim and a needs-you failure can't mask the original error. Refs PAN-3078, PAN-2848, PAN-3257, PAN-2580, PAN-3560. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 14 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
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 |
1 participant
Why
On 2026-08-16 the review loop for PAN-3743 (#3749) stalled: the inspect specialist posted its verdict, but the work agent was never told, and it parked waiting. The loop only healed ~30 minutes later when the PAN-2583 workspace-verdict fallback sweep re-delivered the result through a different path.
Root cause, end to end:
agent-pan-3743is a codex app-server work agent. The app-server branch ofbuildCodexCommandnever wraps the launcher in the PTY supervisor — yet the spawn stampedsupervisorEnabled: trueanyway, and the state projection turned that intodeliveryMethod: 'supervisor'in the agent's durable state.deliverAgentMessageresolved the method from that persisted state and treated it as a strict PAN-1769 supervisor opt-in — no fallback tier, throw on failure.pty-agent-pan-3743.sock(nothing was ever wrapped), so the delivery threwsocket-missing. The only record was aconsole.errorin the dashboard log; the work agent sat parked and the review loop stalled.The known-hazard patches (PAN-1988 resilient mapping, the kickoff special-case) fixed individual call sites but left the door itself unsafe: any caller that resolves the method from agent state inherited the strict contract for a value that state can project spuriously.
What changed
Three changes, root to symptom:
src/lib/agents/supervisor-channels.ts(root) —decideSupervisorForWorkAgentnow rejects codex on the app-server transport, so the supervisor stamp is never applied to a launch that cannot have a supervisor socket. Only the work-tui transport (which the launcher does wrap) stays eligible. This matches the existing conversation-side precedent inshouldUseSupervisorForConversationand reads the merged config the same way the launcher does. Thecodex?.transportaccess is optional-chained (same idiom asruntime-command.ts) so a partial config can't throw mid-spawn.src/lib/agents/delivery.ts(door) —deliverAgentMessagenow routes a state-deriveddeliveryMethodthroughresilientDeliveryMethod, so a stale 'supervisor' projection falls through the cascade to the live app-server / channels / tmux tiers instead of throwing. An explicitdeliveryMethodcaller argument keeps the strict throw-on-failure PAN-1769 contract — only the persisted hint was reclassified.src/lib/cloister/inspect-agent.ts(hardening) — a verdict that fails to deliver now also surfaces an operator-visible needs-you mark viasurfaceIssueFeedbackNeedsYou(the PAN-2228 pattern), instead of console-only. Best-effort: the original error log is preserved verbatim and a needs-you failure can never mask the delivery error.Tests
src/lib/agents/__tests__/delivery.test.ts— new describe block reproducing the stall: state-stamped supervisor + live app-server socket delivers via app-server; no sockets falls through to tmux; an explicit'supervisor'argument with no socket still throws (strict contract preserved).src/lib/agents/__tests__/supervisor-codex-transport.test.ts(new) — codex app-server → supervisor-ineligible; codex work-tui → eligible; claude-code never consults the codex transport.src/lib/cloister/__tests__/inspect-agent.test.ts— needs-you is surfaced on bothok: falseand thrown delivery failures, and a needs-you failure still leaves the original error log intact.Gates:
npm run typecheck✓,npm run lint✓,npm test✓ (confirmation run: 1470 files / 13,902 tests, zero failures; an earlier run showed 7 load-flake failures in 6 files unrelated to this diff — hook timeouts and a tmp-dir cleanup race under parallel-worker pressure — all green in isolation and in the confirmation run).Manual unstick (verified verb)
If a review loop stalls this way again before the fix is deployed:
pan unstick <id>clears the issue's persistent stuck marker and resets stale review/test/merge approval state so the pipeline can run again; addpan review restart PAN-3743if a fresh review convoy is required. For PAN-3743 itself neither is needed — the loop self-healed via the PAN-2583 fallback sweep and #3749 is green.Refs #3078, #2848, #3257, #2580, #3560 (all still open; this PR fixes the state-projection defect they share symptoms with but does not claim to close them).