Repository navigation
fix(agent-status): show a failed or stopped OpenCode turn, and the screens #22944 missed - #23105
brennanb2025 wants to merge 18 commits into
Conversation
There was a problem hiding this comment.
ℹ️ No critical issues — one narrow, pre-existing edge noted below.
Reviewed changes
This run reviewed all 13 commits (135 files) of brennanb2025/agent-status-failed-turn-ending against its stack base brennanb2025/codex-cancel-with-subagent.
- One shared display derivation — new
src/shared/agent-status-display-state.tsderiveswaiting > failed > working > interrupted > done > decayfrommainAgentplus the child-work residual; every desktop surface, the CLI, mobile, the pet and fleet attention now read it, and a failure survives reader staleness. - OpenCode turn endings — the status plugin publishes
root_state/root_turn_error_nameand the listener maps them to amainAgentverdict (MessageAbortedError→ cancellation, any other name → failure).opencode2/mimo-codedeliberately publish nomainAgent, asserted by test. - Cancel beats a same-turn failure —
resolveCancelVerdictLatchkeeps a latched cancellation when a provider failure for the same turn arrives. - Reader plumbing —
mainAgentridesRuntimeWorktreeAgentRow, fleet stage evidence and dashboardturnEnding; notifications switch fromagentInterruptedtoagentTurnEnding; hibernation, the star prompt, auto-sleep and fleet attention now require a clean completion.
Verification. 161 tests across the new display-state, producer-parity, OpenCode-normalization, cancel-latch, sidebar and Cmd+J suites pass locally. Three independent specialist passes (OpenCode root-state delivery, cancel-latch vs every reader, display precedence across all surfaces) each returned no defect, and the shared precedence is applied consistently.
ℹ️ A same-turn failure can still outrank a cancel when the relay's prompt cache is lost
The new rule ("a provider failure after a user cancel keeps the cancellation") sits behind the pre-existing restatesAnotherPrompt guard, which returns early before the new failure branch. A main-agent failure whose resolved prompt is empty is treated as a changed prompt and applied, so the row reads Failed. This is narrow — it needs a relay restart between the prompt and the failure — and pre-existing, but it does contradict the PR's stated guarantee.
Technical details
# Cancel latch bypassed by an empty prompt after a relay restart
## Affected sites
- `src/main/agent-hooks/server/server-cancel-verdict-latch.ts:111-125` — `restatesAnotherPrompt` is evaluated before the new `incoming.payload.mainAgent?.state === 'done'` branch, so an empty incoming prompt short-circuits to `apply`.
- `src/shared/agent-hook-listener/prompt-fields.ts:114` — `resolvePrompt` returns `''` when the per-pane cache is empty (e.g. a restarted relay).
## Required outcome
- A same-turn main-agent failure should keep the latched cancellation whether or not the relay can restate the prompt.
## Concrete sequence
1. Relayed Claude: `UserPromptSubmit 'long task'` → stored prompt `'long task'`, `working`.
2. Ctrl+C → `inferInterrupt` writes `mainAgent { done, cancellation }`; latch armed.
3. The relay restarts and loses `lastPromptByPaneKey`.
4. The aborted request fails → relay posts `StopFailure`; `resolvePrompt` returns `''`.
5. `restatesAnotherPrompt` sees `'' !== 'long task'` → `apply`; row becomes `mainAgent { done, failure }` → Failed.
## Open questions for the human
- Is this reachable in the relayed/SSH lanes you care about, and would you rather reorder the guard or exempt main-agent events with an unknown prompt as a follow-up?DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
55f434e to
19c3015
Compare
a04af01 to
12ccbfb
Compare
|
| export function isClearableActivityThread(thread: AgentPaneThread): boolean { | ||
| const id = activityThreadStatusId(thread) | ||
| return id === 'done' || id === 'interrupted' | ||
| return id === 'done' || id === 'interrupted' || id === 'failed' |
There was a problem hiding this comment.
Active child work gets cleared
If the main agent fails while a background child is still working, the pane remains active but its Activity thread is classified as failed. This check then lets “Clear completed” clear that thread, stamping an activity cutoff and potentially dismissing its retained snapshot before the child finishes. Only treat the thread as clearable after the child work settles.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the delta since the prior pullfrog review: the new commit 3e383f77ea (plus the restack previously reviewed at 12ccbfba) against stack base brennanb2025/codex-cancel-with-subagent.
- Shared wire resolver — extracted the unparsed-row guard (
wireAgentDisplayState→resolveWireAgentPaneDisplayState) out ofsrc/cli/agent-display-state.tsintosrc/shared/agent-status-display-state.ts, so the CLI and the phone read a row published by another build through one function. - Phone reads the shared resolver —
agentDotStatenow callsresolveWireAgentPaneDisplayState, so amainAgent.statearm this build does not know is dropped and falls back to the row's own state instead of falling through the precedence todone;AgentDotStateis now derived from the sharedAgentStatusDisplayStatevocabulary. - Regression test — added the unknown-main-agent-arm case for phone rows, asserting
workingandinterruptedsurvive rather than reading asdone.
Verification. 19 mobile tests (agent-row-display, workspace-list-ordering) and 41 root tests (agent-status-display-state, worker-list-display-state, workspace-format) pass; pnpm --dir mobile run typecheck is clean. No import cycle — agent-status-types does not depend on the display module.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
3e383f7 to
85d6e43
Compare
Comments Outside DiffThese findings could not be posted inline.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOpenCode hook data now reports root-turn state and error names, including failure and user-interruption outcomes for OpenCode 2. The listener normalizes this data into main-agent statuses, and cancellation handling preserves inferred cancellations when same-turn provider failures arrive. Main-agent verdicts now feed orchestration attention and appear in dashboard cards, agent rows, editor menus, and worker-list output. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to Failed or user-stopped OpenCode 2 turns will not show the intended verdict. Include OpenCode 2 in verdict resolution before merging. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The change affects whether failed work is shown and brought to a coordinator’s attention. One supported OpenCode version still appears to miss the new failure verdict, although no new external access or privilege escalation was identified. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: b46d188c-a619-4113-8591-5eed9613a5f3
📥 Commits
Reviewing files that changed from the base of the PR and between f6324f2 and 85d6e436e8cfee0bbb310fccfe27e4d22b779fb1.
📒 Files selected for processing (51)
docs/reference/agent-status-store.mdmobile/src/components/AgentSpinner.tsxmobile/src/components/WorktreeListRow.tsxmobile/src/host-screen/host-workspace-list.tsxmobile/src/worktree/workspace-list-ordering.test.tsmobile/src/worktree/workspace-list-ordering.tsmobile/src/worktree/workspace-list-types.tssrc/cli/handlers/orchestration/worker-list-display-state.test.tssrc/cli/handlers/orchestration/worker-terminal-handlers.tssrc/main/agent-hooks/server-cancel-verdict-failure.test.tssrc/main/agent-hooks/server-opencode-normalization.test.tssrc/main/agent-hooks/server/server-cancel-verdict-latch.tssrc/main/ipc/dashboard-payload-validation.test.tssrc/main/ipc/dashboard-payload-validation.tssrc/main/opencode/hook-plugin-background-child-completion.test.tssrc/main/opencode/hook-plugin-lifecycle-delivery.test.tssrc/main/opencode/hook-plugin-opencode2-turn-ending.test.tssrc/main/opencode/hook-plugin-root-turn-state.test.tssrc/main/opencode/hook-service.test.tssrc/main/opencode/opencode2-turn-ending-captures.test-fixture.tssrc/main/opencode/status-plugin-delivery-source.tssrc/main/opencode/status-plugin-factory-source.tssrc/main/opencode/status-plugin-lifecycle-source.tssrc/main/opencode/status-plugin-message-preview-source.tssrc/main/opencode/status-plugin-ownership-source.tssrc/main/opencode/status-plugin-runtime-state-source.tssrc/main/opencode2/status-plugin-setup-source.tssrc/main/runtime/orchestration/worker-attention-context.test.tssrc/main/runtime/orchestration/worker-attention-context.tssrc/renderer/src/components/dashboard-popout/AgentKanbanCard.test.tsxsrc/renderer/src/components/dashboard-popout/AgentKanbanCard.tsxsrc/renderer/src/components/dashboard-popout/dashboard-agent-status-patch.test.tssrc/renderer/src/components/dashboard-popout/dashboard-agent-status-patch.tssrc/renderer/src/components/dashboard/DashboardAgentRow.test.tsxsrc/renderer/src/components/dashboard/DashboardAgentRow.tsxsrc/renderer/src/components/dashboard/build-dashboard-snapshot.test.tssrc/renderer/src/components/dashboard/build-dashboard-snapshot.tssrc/renderer/src/components/dashboard/dashboard-card-bucket.tssrc/renderer/src/components/dashboard/dashboard-row-bucket.test.tssrc/renderer/src/components/dashboard/dashboard-row-bucket.tssrc/renderer/src/components/editor/ReviewNotesSendMenuContent.test.tsxsrc/renderer/src/components/editor/ReviewNotesSendMenuContent.tsxsrc/shared/agent-hook-listener/listener-state.tssrc/shared/agent-hook-listener/providers/opencode-family-events.tssrc/shared/dashboard-snapshot.tssrc/shared/orchestration-fleet-agent-status-evidence.tssrc/shared/orchestration-fleet-attention.test.tssrc/shared/orchestration-fleet-attention.tssrc/shared/orchestration-fleet-projection.test.tssrc/shared/orchestration-fleet-projection.tssrc/shared/orchestration-fleet-worker-projection.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
ℹ️ No critical issues — one minor color-consistency note inline.
Reviewed changes
This run re-reviewed the rebuilt branch at 85d6e436 (8 commits, 51 files) since the prior pullfrog review at 3e383f77; it covers the reader surfaces #22944 did not reach.
- Pop-out board and notes picker — dashboard cards carry an optional
verdictMark(agentVerdictDisplayMark) beside the livedotState; it outranks the dot, is never muted by being seen, files the card under Done, and is accepted by the dashboard payload validator. The notes picker and the dashboard row's fallback label read the same mark. - Phone workspace dot —
getWorktreeStatusfolds its agent rows' verdicts (agentDotState) over the host rollup in desktop order (human wait > failed > working > interrupted), and the agent-attention sort ranks failed after a pending question. - Orchestration — worker stage evidence now carries the row's
mainAgent; fleet attention re-derives onefailurefrom it on every read, andorca orchestration worker-listprintsfailed/interruptedin the stage slot. - Phone stop color — the worktree-level spinner renders
interruptedmuted.
Verification: the OpenCode/root-turn, cancel-latch, fleet, worker-list, dashboard, picker and mobile suites pass (268 tests across the touched files); tc:web, tc:cli and mobile typecheck are clean. tc:node fails on a pre-existing main-only import in src/main/codex/codex-background-task-frames.ts, untouched here.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ The follow-up commit is clean; the earlier mobile-stop color note remains open inline.
Reviewed changes
This run reviewed the one commit pushed since the prior pullfrog review of 85d6e436 (5704fb4126).
- Seen verdict cards settle into Idle — a new
dashboardCardBucketmoves a card whose verdict is already acknowledged (verdictMarkset,dotState === 'done',unseen === false) into the Idle column, mirroring how a seen clean completion settles, while the dot keeps its Failed / Interrupted mark. An unseen verdict still files under Done, and a failure whose subagents still run (dotState !== 'done') stays under Done. - One bucket source — the pop-out patch and the dashboard row projection both call
dashboardCardBucket, so the board and the in-app dashboard agree;dashboardCardDisplayStatestill returns the verdict mark first, so the settled card keeps its red dot.
Verification: the dashboard / kanban / patch suites pass (79 tests) and tc:web is clean.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the single commit pushed since the prior pullfrog review of 5704fb41: 2183a990 makes a stale turn verdict stop colouring the phone's workspace-list dot, matching the desktop sidebar card, which reads only fresh rows.
- Stale verdicts decay on the phone dot —
getWorktreeStatusnow skips an agent row whoseupdatedAtis older than the 30-minute window unless the host still owns it (structuredHostOwned), so a failed or stopped verdict past the window falls back to the host rollup instead of pinning the dot red/grey indefinitely. - Structured-host rows stay exempt — a
structuredHostOwnedrow is still read regardless of age, mirroring the desktopisExplicitAgentStatusFreshgate that the sidebar card applies to those same rows. The human-wait branch is now under the same gate but keeps its previous effective freshness for non-structured rows. - Test — the old "keeps showing failed past the staleness window" case was replaced by a decay assertion for both a failure and a cancellation, plus the structured-host exemption.
Verification: mobile workspace-list-ordering, agent-row-display and workspace-list-sections suites pass (58 tests) and mobile tsc is clean.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the single commit pushed since the prior pullfrog review of 2183a99: 711196f keeps a failed main agent's pending subagent question under Needs You on the pop-out board, instead of collapsing it into Done.
- A question outranks a failure in the card column —
dashboardCardBucketnow derives the live bucket with the verdict removed and returns it unchanged when it isattention; a verdict otherwise files Done until seen then Idle, and a failure still outranks live subagent work. The dot still reads Failed throughdashboardCardDisplayState, so the card sits under Needs You with its question shown and turns Failed once answered — matchingresolveWorktreeStatus, where a pending question precedeshasFailed. - Tests — a patch-level case asserts a
waitingrow with a failedmainAgentstaysbucket: 'attention'with itsaskSummaryandverdictMark: 'failed'; two row-projection cases cover the same precedence and the failure whose subagents went silent, which files like a completion (Done until seen, then Idle).
Verification: the dashboard row-bucket, pop-out patch and build-snapshot suites pass (46 tests).
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
711196f to
9b08c05
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the three commits pushed since the prior Pullfrog review of 711196f: the mobile workspace-list dot is reverted, fleet failure attention now keys on the shared display mark, and a parity test pins the pop-out card to the desktop row dot and worktree ladder.
- Mobile workspace-list dot reverted —
e26986aremoves the phone's hand-rolledgetWorktreeStatusverdict fold and restores the six mobile files tomain; the host will derive and publish the worktree display status in a follow-up after #23467. The branch no longer touchesmobile/, and the one open Pullfrog thread (mobile stop color) is retired as moot. - Fleet failure keys on the display mark —
f0e2c0achangesisFleetMainTurnFailedtoagentVerdictDisplayMark(...) === 'failed'. This is behaviour-preserving:agentVerdictDisplayMarkreturns'failed'exactly when the underlying verdict is'failure', so the two predicates are equivalent. Its invariant test is now table-driven over every status × outcome. - Pop-out / desktop parity test —
9b08c05addsdashboard-card-bucket.test.ts, running single-agent scenarios (clean, failed, stopped, live or questioning subagents, working, monitoring, waiting, blocked; each seen and unseen) through both the card projection and the desktop row dot +resolveWorktreeStatusladder, asserting the card dot equals the row dot and the column follows the worktree status. A real oracle, not a tautology.
Verification: dashboard-card-bucket.test.ts (22) and orchestration-fleet-attention.test.ts (7) pass.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run followed up on the single commit pushed after the prior review at 9b08c05: afb12f1 hardens the fleet attention test.
- Fleet failure pinned to the display mark under any row state — the
status × outcometable inorchestration-fleet-attention.test.tsnow also iterates everyAGENT_STATUS_STATESvalue as the combined row state and assertsisFleetMainTurnFailed(mainAgent)equalsagentVerdictDisplayMark({ state: rowState, mainAgent }) === 'failed'. That invariant holds because the'failed'mark ignores the row state (only the cancellation arm reads it), and it documents that the fleet path — which never sees a combined row state — derives its failure from the main agent's record alone.
Verification: orchestration-fleet-attention.test.ts (7) passes.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Include OpenCode 2 in root-state verdict resolution. · opencode-family-events.ts:88-92
src/shared/agent-hook-listener/providers/opencode-family-events.ts:88-92
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude OpenCode 2 in root-state verdict resolution.
OpenCode 2 converts failed executions and user interruptions into
session.errordata before emitting idle. The shared plugin carries that verdict on the rootSessionIdlepayload. This guard discards it becausesourceisopencode2, so the published status lacksmainAgent.outcomeand legacyinterrupted.Suggested fix
const mainAgent = - source === 'opencode' + (source === 'opencode' || source === 'opencode2') ? resolveOpenCodeMainAgent(state, paneKey, eventName, hookPayload) : undefined
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 8dd098b3-b0b1-45b2-81f8-f9bff0f33b89
📥 Commits
Reviewing files that changed from the base of the PR and between 711196f6ba79e960d518521a85e9ddebcaa96900 and 2cd080346483b8b6d7ea66d29f5880387aa42acc.
📒 Files selected for processing (4)
src/main/agent-hooks/server-cancel-verdict-failure.test.tssrc/renderer/src/components/dashboard/dashboard-card-bucket.test.tssrc/shared/orchestration-fleet-attention.test.tssrc/shared/orchestration-fleet-attention.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the single commit pushed since the prior Pullfrog review of afb12f1: 2cd0803 adds one test.
- Same-prompt resend pins a new turn —
server-cancel-verdict-failure.test.tsgainslets a failure through when the user resends the same prompt after the stopon the three-lane table (relayed Claude, Grok, OpenCode): after Ctrl+C, resending the identical prompt and then posting the provider failure must readmainAgent.outcome: 'failure', not the latched cancellation. It is falsifiable (the latch'skeepLatchedCancellationwould make it'cancellation') and not redundant with the existingfirst/secondcase, since the identical prompt makesrestatesAnotherPromptreturn false and the release then rides a different path (Claude viaopensNewTurn; Grok/OpenCode viahasExplicitPrompt). No production code changed; the file passes (14 tests).
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
… ended An OpenCode turn that died on a provider error, or that the user stopped, read as a clean Done: the plugin dropped `session.error` and the host published no main-agent record, so the Failed and Interrupted marks main already draws for Claude and Grok never applied. The plugin now sends the root session's own state (`root_state`) and the name of the error that ended its turn (`root_turn_error_name`) beside its pane fold. The listener turns them into `mainAgent`: `MessageAbortedError` is a cancellation, any other name a failure. A plugin that predates the fields publishes no `mainAgent`, so nothing is inferred from the fold.
…the user OpenCode 2 has no `session.error`. Its only turn verdict is the run-ending event, and the setup bridge mapped `session.execution.failed` and `session.execution.interrupted` to the same plain idle as a success, so a failed or stopped OpenCode 2 turn read as Done. The bridge now restates that ending as a `session.error` before the idle: a failed execution by its error type (or `UnknownError`), a user's interrupt as `MessageAbortedError`. Other interrupt reasons (shutdown, superseded, inactivity) stay a plain idle. A test replays event sequences captured from OpenCode 2.0.14 through the generated plugin.
…ts a provider failure A user who pressed Ctrl+C on a relayed (SSH) Claude, Grok or OpenCode turn saw the row flip from Interrupted to Failed when the aborted request then reported its error (`StopFailure`, `stop_failure`, `session.error`). Those listeners never learn of the cancel Orca inferred from the keystroke, so the failure arrives as the provider's own verdict and the store's cancel hold released on any settled main-agent record. A failure no longer releases the hold: it ends the same turn the user already stopped, so the row keeps the cancellation and no completion stamp. A new prompt still opens a new turn whose failure shows as Failed, and a plain settle still releases the hold. The tests also pin that a Ctrl+C at an OpenCode root that already finished, while a busy child holds the pane open, cancels nothing.
… failure attention When a worker's agent turn failed (a Claude API error, an OpenCode provider error) and the worker never reported done, the fleet showed it as a quiet `done` (or `working` while a subagent ran) with no attention, so the coordinator was never told. The fleet evidence now keeps the row's `mainAgent`, the worker's stage carries it (an optional wire field; older hosts send none), and attention adds one `failure` whenever the main agent's latest turn failed, re-derived from that record on every read so the next turn clears it. A restored, unconfirmed row or an identity-only row contributes no record.
…worker-list `orca orchestration worker-list` printed the folded activity, so a worker whose turn failed read `[running/done]`. The stage slot now shows `failed` or `interrupted` from the worker's main-agent record through the same verdict rule the app uses. A worker with no fresh row keeps `unknown`, and a main-agent state this build cannot name carries no verdict.
…st dot The phone's workspace list dot read only the host's rollup, which has no failed state, so a worktree whose agent failed showed a green Done dot (or the yellow spinner while a subagent ran); only the expanded agent row was red. A user-stopped worktree read green Done too. The dot now folds in the verdicts its agent rows already carry, in desktop's container order: a pending question first, then failed, live work, interrupted, and the host rollup. Agent-attention sort ranks failed right after a pending question and a stop with no signal. A host that hides the worktree keeps it inactive.
…d and notes picker Three desktop surfaces still read the folded row state where #22944 already draws the main agent's verdict on the agent's own row. The pop-out agent board filed a failed agent under Done with a green dot (or under Working while subagents ran) and showed a user stop as a green Done; the review-notes "send to agent" picker labelled a failed agent Done or Working; and a dashboard row with no captured prompt labelled a failed agent from the fold. Dashboard cards now carry the verdict mark (`failed` / `interrupted`, from `agentVerdictDisplayMark`) as an optional field. It outranks the live dot, is never muted by being seen, and files the card under Done, since a failure ranks like a completion. The picker and the fallback label draw the same mark.
…ot the failure red
… seen completion The pop-out board kept every failed or user-stopped card in the Done column until that agent started another turn, even after it was seen, while a seen completion settles into Idle. A failure ranks like a completion for attention and a stop is not news, so a seen verdict on a finished row now settles into Idle too; its dot keeps the Failed or Interrupted mark. A failure whose subagents still work stays under Done.
… the desktop card The desktop worktree card reads only fresh agent rows, so a failed or stopped turn older than the staleness window stops colouring its dot. The phone list dot kept a stale failure red, and on hosts without desktop Smart Sort ranks it pinned that worktree above live work until the agent's next turn.
…ain agent failed The pop-out column read the card's display state, where a failed main agent outranks a waiting row, so a subagent's pending question moved to Done with its ask summary dropped. The column now follows main's attention rule (a question first, as the worktree card and Smart Sort rank it); the dot keeps the Failed mark. A failure whose subagents went silent files like a completion.
The phone's workspace list dot re-derived the desktop worktree card's precedence ladder and freshness fold by hand on the phone, and each review loop found a new way that copy drifted from the desktop. It also aged host rows on the phone's clock. The list dot returns to main's behaviour; the phone's agent rows already show Failed / Interrupted (#22944). A follow-up will have the host derive the worktree's display status once, from the same ladder the desktop card uses, and publish it to the phone, after #23467 changes the same verdict vocabulary. No other mobile change in this branch is needed, so the branch no longer touches mobile/.
…ed mark A worker's fleet failure attention compared the main agent's raw outcome to 'failure'. Key it on the display rule instead (agentVerdictDisplayMark === 'failed'), so any verdict the agent rows draw as Failed also raises fleet failure attention and the two cannot drift when the verdict vocabulary grows. It still re-derives from the record on every read, so the next turn clears it.
…tree ladder One table of single-agent scenarios (clean, failed, stopped, failed or stopped with live subagents, failed with a subagent's question, working, monitoring, waiting, blocked; each seen and unseen) runs through the snapshot builder's card projection and through main's own desktop functions (the agent-row dot, applyAgentPaneActivityFlags, resolveWorktreeStatus). It asserts the card dot matches the row dot, and the column is Needs you exactly when the worktree card reads permission, Working exactly when it reads working or monitoring, and otherwise Done until seen, then Idle.
…row state The fleet sees only the main agent's record, so its failure attention must match the agent display's Failed mark whatever the combined row reads (subagents working or asking). The table fed the main agent's own state as the row state, so a display rule that began depending on the combined state would have passed; it now crosses every row state with every main-agent record.
…past a stop reads Failed
…aunch entry too A pane opened from Orca's OpenCode 2 entry runs the opencode2 plugin variant, which already sends the root session's state and turn error, but the listener only read them from the opencode variant, so a failed OpenCode 2 turn launched that way still read Done. Live QA confirmed the same plugin reports reach the host on that route; mimo-code stays unread.
…nCode 2 launch entry's hook path The listener change that admits opencode2 was pinned only by a normalizer unit test; this runs the same inferred-cancel, same-turn failure, new-turn failure and plain-settle cases through the hook server on /hook/opencode2.
8a2becb to
39bbf8a
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the single commit pushed since the prior pullfrog review at 2cd0803: ef7aa83 also reads OpenCode 2's root turn state, so a failed or stopped turn from Orca's OpenCode 2 launch entry shows Failed / Interrupted.
- OpenCode 2 launch entry now publishes the verdict —
normalizeOpenCodeFamilyEventresolvesmainAgentforsource === 'opencode' || source === 'opencode2'(wasopencodeonly), so theopencode2plugin variant that posts to/hook/opencode2is no longer dropped;mimo-codestays excluded. This closes the gap the coderabbit thread flagged, and the shared setup bridge already publishedroot_state/root_turn_error_nameon that route. - Tests split and widened — the old
opencode2 | mimo-code"publishes no main agent" case is split:opencode2now asserts afailureverdict,mimo-codestill assertsundefined.hook-plugin-opencode2-turn-ending.test.tsnow replays its real OpenCode 2.0.14 captures through both generated plugin variants (getOpenCodePluginSource→/hook/opencode,getOpenCode2PluginSource→/hook/opencode2) and asserts each posts to its own route, so the branch is exercised end to end rather than only at the normalizer.
Verification: server-opencode-normalization.test.ts (43) and hook-plugin-opencode2-turn-ending.test.ts (22) pass — 65 tests.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the commits pushed since the prior pullfrog review at ef7aa83 (the branch was rebased onto main, so every SHA changed): the OpenCode 2 launch-entry fix itself, plus a test-only follow-up.
- OpenCode 2 launch entry reads its own root verdict —
normalizeOpenCodeFamilyEventresolvesmainAgentforsource === 'opencode' || source === 'opencode2', so theopencode2plugin variant that posts to/hook/opencode2is no longer dropped;mimo-codestays excluded. Same content asef7aa83, re-verified after the rebase. - Cancel-then-failure lane for the OpenCode 2 launch entry —
server-cancel-verdict-failure.test.tsgains anopenCode2LaunchEntrylane (agentType: 'opencode2', posting to/hook/opencode2, reusing the OpenCode payloads) in itsdescribe.each, so the hold / keep-cancellation rules run through that path too. It is falsifiable against the fix: withoutopencode2in the guard, the "new prompt" cases cannot readmainAgent.outcome: 'failure', because the row would carry no main agent at all.
Verification: server-cancel-verdict-failure.test.ts (18), server-opencode-normalization.test.ts (43) and hook-plugin-opencode2-turn-ending.test.ts (22) pass.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
Review status: ready for your review (not merged)The problem. When an agent's turn ends because of an error (the model request is rejected, the context overflows) or because you press Esc, some Orca screens still showed it as a clean green "Done". #22944 (on What changes for you.
What happened in review.
Validation.
Not verified live: OpenCode over SSH (unit-tested through the relay), Windows and Linux, the orchestration worker list and fleet attention (unit-tested), and the phone (unchanged by this PR). Before merging, you should know:
Not merged. Merging is your call. |
Adapt the root verdict proposal from brennanb2025 in PR #23105 to the current TUI-owned lifecycle, keeping hook-store authority and existing mainAgent semantics.
…tion (#24612) * fix(opencode): retain failed and stopped TUI turn outcomes Adapt the root verdict proposal from brennanb2025 in PR #23105 to the current TUI-owned lifecycle, keeping hook-store authority and existing mainAgent semantics. * fix(opencode): let auto-approved permissions settle before attention * fix(opencode): reconcile cached outcomes with completed session turns * fix(opencode): bind terminal verdicts to ending event timestamps * fix(opencode): publish approval cards for permission requests
|
Closing this combined patch. #24612 already records OpenCode failures and cancellations; the remaining board, picker and worker-list display gaps should be handled separately on current main. |





ELI5
When an OpenCode turn ends because of an error (the model service rejected the request, the context overflowed) or because you pressed Esc, Orca showed it as a turn that finished well: a green Done. Claude and Grok already show Failed since #22944; this PR brings OpenCode (1.x and 2.x) up to the same place, and fixes the last few screens that still showed a failed turn as Done or Working: the pop-out agent board, the "send notes to agent" picker, and the orchestration worker list, where a failed worker also never raised attention to its coordinator.
What Changed
This PR fixes a failed or user-stopped agent turn reading as a clean finish in the places #22944 did not reach. It builds on #22944 (main already records the verdict as
mainAgent.outcomeand draws Failed / Interrupted on the sidebar, tabs, worktree status, activity views, notifications, auto-sleep, Smart Sort,worktree psrows and phone agent rows); it adds no second verdict and no new display rules.Scope. Changes: OpenCode 1.x and 2.x status (local and over SSH, since Orca ships the same plugin to the relay), the pop-out agent board, the review-notes agent picker, the dashboard row's fallback label,
orca orchestration worker-list, and orchestration fleet attention. Not affected: Claude, Grok and Codex producers; the combined lifecyclestatethat waits, automations and sleeping read; every surface #22944 already covers.Before → after, as the user sees it:
opencodeor from Orca's own OpenCode 2 launch entry. A failed or stopped OpenCode 2 turn read Done → now Failed / Interrupted. A context overflow whose compaction succeeds still reads Done.activity=done,attention=none→ fleet attention now raises onefailure, andworker-listprintsfailed(orinterrupted) in the stage slot.Mechanism.
root_state) and the name of the error that ended its turn (root_turn_error_name) beside the pane fold it already sends; the listener turns them intomainAgent(MessageAbortedError= cancellation, any other name = failure). A plugin that predates the fields publishes nomainAgent, so nothing is inferred from the fold. OpenCode 2 has nosession.error, so its setup bridge restates the run-endingsession.execution.failed(by its error type) and a user'ssession.execution.interruptedas one before the idle.server-cancel-verdict-latch.ts) no longer releases on a settledmainAgentwhose outcome isfailure: that failure ends the same turn the user already stopped.verdictMarktaken from main'sagentVerdictDisplayMark; the picker and the fallback label read the same function.mainAgent, the worker's stage carries it (optional), and attention re-derivesfailurefrom it on every read by the display rule (agentVerdictDisplayMarkdraws the main agent as Failed), so a turn the rows show as Failed always raises it and the next turn clears it.Why
mainAgent.outcomeand names the display rule (agentVerdictDisplayMark). Each change here is either a producer that never wrote that fact (OpenCode) or a reader that still read the combined state; extending them removes the gap without a parallel module that could disagree.mainAgent; fleet attention re-derivesfailureon each read instead of latching it, so it clears itself when the next turn starts.Out of scope:
mainAgentyet.orca worktree pstext output: its--jsonagent rows already carrymainAgent/interruptedfrom fix(native-chat): a request that failed reads as failed #22944; the text output prints no agent status for any state, so adding a status column is a separate feature.Linked Issue
Linear: STA-7949. Builds on #22944.
Visual Proof
Isolated background Orca dev instances on a separate Mac (hidden windows, a throwaway home folder, real agent CLIs pointed at a local mock model server). Before =
mainat this PR's base; after = this branch. Same scenario and moment on both sides.OpenCode 1.18.33, the model request is rejected (400). Before: the tab and worktree card show a green Done check. After: both show Failed (red), with the "Failed" tooltip, and the notification says "OpenCode failed".
OpenCode 1.18.33, the user presses Esc mid-reply. Before: green Done. After: Interrupted (the tooltip says "Interrupted"; its dot colour is the shared status dot, which #23467 makes muted).
OpenCode 2.0.14 opened from Orca's OpenCode 2 entry, request rejected (400). Before: green Done. After: Failed on the row, tab and worktree card.
OpenCode 2.0.14 from the OpenCode 2 entry, Esc mid-reply. Before: green Done. After: Interrupted.
Pop-out agent board, a failed OpenCode turn the user has not looked at yet. Before: a green Done card. After: the card sits under Done with a Failed dot.
Pop-out agent board after the user has seen a failed Claude turn. Before: the card settles into Idle and loses its mark (grey dot). After: it settles into Idle like a seen completion but keeps its Failed dot.
"Send notes to agent" picker with a failed OpenCode 2 agent. Before: labelled Done. After: labelled Failed.
Testing
Live (isolated background Orca dev instances on a separate Mac, real CLIs against a local mock model server; before =
mainat this PR's base, after = this branch):opencode, 400 / EscAutomated: every new behaviour has a test through its real boundary: the hook server (local and through the SSH relay) for the cancel hold, the generated plugin for OpenCode 1.x, a replay of event sequences captured from OpenCode 2.0.14 through the plugin's setup bridge for OpenCode 2, the fleet projection and worker attention context, the
worker-listcommand handler, the dashboard snapshot builder / pop-out patch / payload validator / kanban card, the notes picker and dashboard row renders, and one table-driven test that runs each single-agent scenario through the pop-out card and through main's own desktop row dot and worktree-card ladder (applyAgentPaneActivityFlags/resolveWorktreeStatus) and checks they agree. Each was checked by deleting its production line, confirming the named test fails, and restoring it.Not tested live: OpenCode over SSH (the relay ships the same plugin; unit-tested through the relay), Windows and Linux, the orchestration worker list and fleet attention (unit-tested), and a context overflow whose compaction succeeds on OpenCode 2 (the mock's compaction reply is rejected by 2.0.14, so only the failed-compaction path ran live). Notification titles on the OpenCode 2 entry read "opencode2 failed" because main already labels that route by its agent id ("opencode2 finished" before this change).
AI Disclosure
Author: @BrennanKB5
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
root_state/root_turn_error_name(host-generated plugin; an old plugin sends none and gets nomainAgent),stage.mainAgenton orchestration worker rows (an old host sends none; an old CLI ignores it), andverdictMarkon pop-out dashboard cards (same app build on both ends).failureis an existing fleet attention category.mainAgentthe same way; a relay still running an older build ignores the new fields and publishes none, as today. The cancel hold's new rule is tested through the relay for Claude, and locally for Grok and OpenCode (the rule lives in the desktop store, so the lane does not change it).Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)