Repository navigation
feat(desktop,host-service): surface agent failures as a "failed" status - #5697
Conversation
Terminal-agent status was derived from a Start/Stop/PermissionRequest vocabulary with no failure category, so API failures were silent: an error that kept the process alive left the pane stuck on "working", and a crash silently dropped the binding to idle. Add a first-class "failed" lifecycle event and PaneStatus (red, styled like "permission"), fed by two detectors: - Claude Code StopFailure hook (primary): fires on API errors (rate_limit, overloaded, auth, billing, server_error, ...) while the session stays alive. Registered in the Claude wrapper config and mapped StopFailure -> Failed; the binding flips to failed and self-clears when the agent retries. - Abnormal process exit (universal fallback): the pty onExit now classifies non-zero/crash-signal exits (excluding clean exits and user interrupts) and, when an agent was bound, marks the terminal "failed" so the binding survives the liveness filter instead of silently vanishing. The failed session status is surfaced by the live binding reads (but never as an attach target), flows through StatusIndicator, the status priority aggregation, and the dock-badge/unread counts.
📝 WalkthroughWalkthroughAgent failure events are registered, normalized, persisted, and broadcast as ChangesAgent failure lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Claude
participant NotificationsRouter
participant HostService
participant Desktop
Claude->>NotificationsRouter: Send StopFailure hook
NotificationsRouter->>HostService: Normalize and store Failed event
HostService-->>Desktop: Broadcast failed terminal status
Desktop-->>Desktop: Show failure indicator and attention state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit 505a571 on Jul 14, 2026 11:52pm UTC. |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Architecture diagram
sequenceDiagram
participant Claude as Claude Code CLI
participant Host as Host Service
participant Router as Event Router
participant Store as Agent Store & DB
participant Pty as PTY Session
participant Exit as Exit Analyzer
participant Bus as Event Bus
participant UI as UI Renderer
participant Attach as Attach API
Note over Claude,UI: Agent Failure Flow
%% Claude StopFailure path
Claude->>Host: POST /hook { eventType: "StopFailure", ... }
Host->>Router: route hook
Router->>Router: mapEventType("StopFailure") → "Failed"
Router->>Store: recordEvent(terminalId, eventType="Failed")
Store->>Store: update binding lastEventType="Failed"
Store-->>Router: ok
Router->>Bus: broadcastAgentLifecycle(Failed)
Bus-->>UI: lifecycle event (Failed)
%% UI handling
UI->>UI: deriveTerminalAgentStatus(...) → "failed"
UI->>UI: StatusIndicator: red pulsing dot
UI->>UI: include in attention workspace count
%% Abnormal exit path
Pty->>Pty: onExit(code, signal)
Pty->>Exit: isAbnormalAgentExit(code, signal)
alt Abnormal exit (crash, non-zero, not user interrupt)
Exit-->>Pty: true
Pty->>Store: markAgentBindingFailed(terminalId, occurredAt)
Store->>Store: update binding lastEventType="Failed"
Store-->>Pty: ok
Pty->>Store: update session status="failed"
Pty->>Bus: broadcastAgentLifecycle(Failed)
Bus-->>UI: lifecycle event (Failed)
UI->>UI: same as above
else Normal exit (clean, user interrupt)
Exit-->>Pty: false
Pty->>Store: update session status="exited"
end
Note over Store,Attach: Failed bindings kept in listLiveByWorkspace,<br>but attach API rejects them
Attach->>Store: findLiveActive(workspace, agent)
alt status == "failed"
Attach-->>Client: error "terminal has exited"
else status == "active"
Attach-->>Client: attach success
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Greptile SummaryThis PR introduces a first-class
Confidence Score: 3/5The core failed-state machinery is sound for the in-band StopFailure path; the abnormal-exit path has an undocumented state divergence in the in-memory store that could produce wrong answers from store.get() after a crash. The abnormal-exit detection in terminal.ts writes the failure state directly to the DB without updating TerminalAgentStore's in-memory byTerminal Map. The new store test only exercises the recordEvent path, not the markAgentBindingFailed DB-direct path, leaving the divergence untested. Additionally, USER_INTERRUPT_EXIT_CODES misses exit code variants for SIGHUP (129) and SIGKILL (137). The PR explicitly notes it has not been end-to-end verified in the running app. packages/host-service/src/terminal/terminal.ts (markAgentBindingFailed bypasses in-memory store) and packages/host-service/src/terminal/agent-exit.ts (missing exit code variants 129 and 137)
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal/terminal.ts | Adds markAgentBindingFailed that writes directly to the DB without updating TerminalAgentStore's in-memory byTerminal Map, creating a state divergence where store.get() returns stale data after abnormal exits |
| packages/host-service/src/terminal/agent-exit.ts | New utility classifying abnormal pty exits; logic is mostly correct but USER_INTERRUPT_EXIT_CODES only covers SIGINT (130) and SIGTERM (143), omitting SIGHUP (129) and SIGKILL (137) shell-encoding variants |
| packages/host-service/src/terminal-agents/persistence.ts | Correctly extends listLiveByWorkspace and listLive to include 'failed' sessions while keeping findLiveActive restricted to 'active' only |
| packages/host-service/src/events/map-event-type.ts | Adds Failed lifecycle event type with correct ordering (failure check before Stop); maps StopFailure and multiple aliases correctly |
| apps/desktop/src/shared/tabs-types.ts | Adds 'failed' PaneStatus; getHighestPriorityStatus early-exit on 'permission' is still correct; acknowledgedStatus leaves 'failed' uncleared (intentional, like permission) |
| packages/host-service/src/terminal-agents/store.test.ts | New test verifies Failed event is recorded on binding via recordEvent(), but does not cover the markAgentBindingFailed DB-direct path used in the abnormal-exit code path |
| apps/desktop/src/renderer/hooks/host-service/useV2NotificationStatus/useV2NotificationStatus.ts | Correctly includes 'failed' in unread and attention-count checks |
| apps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsx | Adds red pulsing 'failed' indicator config, styled consistently with 'permission' |
| packages/host-service/src/trpc/router/notifications/notifications.test.ts | Good coverage of StopFailure to Failed routing through the notification hook path |
| apps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.ts | Registers StopFailure hook correctly alongside Stop using the same managedHookCommand |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CLI as Claude Code CLI
participant NR as Notifications Router
participant Store as TerminalAgentStore
participant DB as SQLite DB
participant EB as EventBus
participant UI as Renderer
Note over CLI,UI: Path 1 - In-band StopFailure
CLI->>NR: "POST /hook {eventType: StopFailure}"
NR->>Store: "recordEvent({eventType: Failed})"
Store->>DB: "upsert binding lastEventType=Failed"
NR->>EB: broadcastAgentLifecycle(Failed)
EB->>UI: refresh - listByWorkspace shows failed
Note over CLI,UI: Path 2 - Abnormal process exit
CLI->>DB: "markAgentBindingFailed - lastEventType=Failed"
CLI->>DB: "terminalSessions.status=failed"
CLI->>EB: broadcastAgentLifecycle(Failed)
EB->>UI: refresh - listByWorkspace shows failed
Note over Store: store.get() returns stale pre-crash binding
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CLI as Claude Code CLI
participant NR as Notifications Router
participant Store as TerminalAgentStore
participant DB as SQLite DB
participant EB as EventBus
participant UI as Renderer
Note over CLI,UI: Path 1 - In-band StopFailure
CLI->>NR: "POST /hook {eventType: StopFailure}"
NR->>Store: "recordEvent({eventType: Failed})"
Store->>DB: "upsert binding lastEventType=Failed"
NR->>EB: broadcastAgentLifecycle(Failed)
EB->>UI: refresh - listByWorkspace shows failed
Note over CLI,UI: Path 2 - Abnormal process exit
CLI->>DB: "markAgentBindingFailed - lastEventType=Failed"
CLI->>DB: "terminalSessions.status=failed"
CLI->>EB: broadcastAgentLifecycle(Failed)
EB->>UI: refresh - listByWorkspace shows failed
Note over Store: store.get() returns stale pre-crash binding
Comments Outside Diff (1)
-
apps/desktop/src/shared/tabs-types.ts, line 91-94 (link)The
acknowledgedStatuscomment doesn't documentfailed, leaving its persistence behavior implicit. Sincefailedis intentionally designed to persist likepermission(it clears only on retry or terminal close, not on acknowledgement), the omission may lead a future reader to accidentally add a clearing case.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/shared/tabs-types.ts Line: 91-94 Comment: The `acknowledgedStatus` comment doesn't document `failed`, leaving its persistence behavior implicit. Since `failed` is intentionally designed to persist like `permission` (it clears only on retry or terminal close, not on acknowledgement), the omission may lead a future reader to accidentally add a clearing case. How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/host-service/src/terminal/terminal.ts:906-922
**`markAgentBindingFailed` bypasses the in-memory store cache**
`markAgentBindingFailed` writes `lastEventType = "Failed"` directly to the DB without going through `TerminalAgentStore.recordEvent()`. The store's `listByWorkspace`, `list`, and `findActive` delegate to the persistence layer, so those reads correctly reflect the failure. However, `store.get(terminalId)` reads solely from the in-memory `byTerminal` Map and will return the stale pre-crash binding (e.g. `lastEventType: "Start"`) for the rest of the host-service process lifetime. The function's own doc comment says "the store's reads are persistence-backed," but `get()` is explicitly not — it's the fresh-launch wait path. For the in-band `StopFailure` route, `recordEvent("Failed")` keeps both layers consistent. The abnormal-exit route has no equivalent, creating an undocumented divergence. If any code calls `store.get(terminalId)` after an abnormal exit and branches on `lastEventType`, it will observe wrong state.
### Issue 2 of 3
packages/host-service/src/terminal/agent-exit.ts:3
`USER_INTERRUPT_EXIT_CODES` only covers SIGINT (130) and SIGTERM (143), but omits the SIGHUP (129) and SIGKILL (137) shell-encoding variants. A bash/zsh wrapper that catches SIGHUP or is killed by the OOM killer's final SIGKILL while an inner process handles the signal and re-exits with `128 + signal` would report `code = 129` or `code = 137` with `signal = 0`. The function currently classifies both as abnormal failures even though the corresponding signals (1 and 9) are in `USER_INTERRUPT_SIGNALS`. Symmetric coverage prevents this over-classification.
```suggestion
const USER_INTERRUPT_EXIT_CODES = new Set([129, 130, 137, 143]);
```
### Issue 3 of 3
apps/desktop/src/shared/tabs-types.ts:91-94
The `acknowledgedStatus` comment doesn't document `failed`, leaving its persistence behavior implicit. Since `failed` is intentionally designed to persist like `permission` (it clears only on retry or terminal close, not on acknowledgement), the omission may lead a future reader to accidentally add a clearing case.
```suggestion
* - "review" → "idle" (user saw the completion)
* - "permission" → unchanged (persists until agent resumes)
* - "failed" → unchanged (persists until agent retries or terminal closes)
* - "working" → unchanged (persists until agent stops)
* - "idle" → unchanged
```
Reviews (1): Last reviewed commit: "feat(desktop,host-service): surface agen..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function markAgentBindingFailed( | ||
| db: HostDb, | ||
| terminalId: string, | ||
| occurredAt: number, | ||
| ): boolean { | ||
| const binding = db.query.terminalAgentBindings | ||
| .findFirst({ | ||
| where: eq(terminalAgentBindings.terminalId, terminalId), | ||
| columns: { terminalId: true }, | ||
| }) | ||
| .sync(); | ||
| if (!binding) return false; | ||
| db.update(terminalAgentBindings) | ||
| .set({ lastEventType: "Failed", lastEventAt: occurredAt }) | ||
| .where(eq(terminalAgentBindings.terminalId, terminalId)) | ||
| .run(); | ||
| return true; |
There was a problem hiding this comment.
markAgentBindingFailed bypasses the in-memory store cache
markAgentBindingFailed writes lastEventType = "Failed" directly to the DB without going through TerminalAgentStore.recordEvent(). The store's listByWorkspace, list, and findActive delegate to the persistence layer, so those reads correctly reflect the failure. However, store.get(terminalId) reads solely from the in-memory byTerminal Map and will return the stale pre-crash binding (e.g. lastEventType: "Start") for the rest of the host-service process lifetime. The function's own doc comment says "the store's reads are persistence-backed," but get() is explicitly not — it's the fresh-launch wait path. For the in-band StopFailure route, recordEvent("Failed") keeps both layers consistent. The abnormal-exit route has no equivalent, creating an undocumented divergence. If any code calls store.get(terminalId) after an abnormal exit and branches on lastEventType, it will observe wrong state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal/terminal.ts
Line: 906-922
Comment:
**`markAgentBindingFailed` bypasses the in-memory store cache**
`markAgentBindingFailed` writes `lastEventType = "Failed"` directly to the DB without going through `TerminalAgentStore.recordEvent()`. The store's `listByWorkspace`, `list`, and `findActive` delegate to the persistence layer, so those reads correctly reflect the failure. However, `store.get(terminalId)` reads solely from the in-memory `byTerminal` Map and will return the stale pre-crash binding (e.g. `lastEventType: "Start"`) for the rest of the host-service process lifetime. The function's own doc comment says "the store's reads are persistence-backed," but `get()` is explicitly not — it's the fresh-launch wait path. For the in-band `StopFailure` route, `recordEvent("Failed")` keeps both layers consistent. The abnormal-exit route has no equivalent, creating an undocumented divergence. If any code calls `store.get(terminalId)` after an abnormal exit and branches on `lastEventType`, it will observe wrong state.
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,19 @@ | |||
| // Exit codes for user-initiated interrupts (128 + SIGINT/SIGTERM) — the user | |||
| // stopped the agent, not a failure. | |||
| const USER_INTERRUPT_EXIT_CODES = new Set([130, 143]); | |||
There was a problem hiding this comment.
USER_INTERRUPT_EXIT_CODES only covers SIGINT (130) and SIGTERM (143), but omits the SIGHUP (129) and SIGKILL (137) shell-encoding variants. A bash/zsh wrapper that catches SIGHUP or is killed by the OOM killer's final SIGKILL while an inner process handles the signal and re-exits with 128 + signal would report code = 129 or code = 137 with signal = 0. The function currently classifies both as abnormal failures even though the corresponding signals (1 and 9) are in USER_INTERRUPT_SIGNALS. Symmetric coverage prevents this over-classification.
| const USER_INTERRUPT_EXIT_CODES = new Set([130, 143]); | |
| const USER_INTERRUPT_EXIT_CODES = new Set([129, 130, 137, 143]); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal/agent-exit.ts
Line: 3
Comment:
`USER_INTERRUPT_EXIT_CODES` only covers SIGINT (130) and SIGTERM (143), but omits the SIGHUP (129) and SIGKILL (137) shell-encoding variants. A bash/zsh wrapper that catches SIGHUP or is killed by the OOM killer's final SIGKILL while an inner process handles the signal and re-exits with `128 + signal` would report `code = 129` or `code = 137` with `signal = 0`. The function currently classifies both as abnormal failures even though the corresponding signals (1 and 9) are in `USER_INTERRUPT_SIGNALS`. Symmetric coverage prevents this over-classification.
```suggestion
const USER_INTERRUPT_EXIT_CODES = new Set([129, 130, 137, 143]);
```
How can I resolve this? If you propose a fix, please make it concise.…ailed test coverage, doc accuracy - isAbnormalAgentExit: treat the 128+signal shell encodings (129 SIGHUP, 137 SIGKILL) as user-interrupt, symmetric with USER_INTERRUPT_SIGNALS, so both encodings classify identically (greptile P2). - notifications StopFailure test: assert agent identity survives onto the Failed broadcast and the store binding (cubic P3). - markAgentBindingFailed: document that the direct-DB write can leave store.get() briefly stale, why its sole caller is unaffected, and when the entry clears (greptile P1).
|
Addressed the automated review findings in 06ed7cb: greptile P2 — cubic P3 — Failed test doesn't verify agent identity survives ✅ Fixed. The greptile P1 — Also: verified end-to-end over CDP (both detectors turn the pane red) — see the updated PR description. host-service typecheck + lint clean; touched test suites green. |
…res only Drop the abnormal-exit heuristic (isAbnormalAgentExit / agent-exit.ts, the markAgentBindingFailed onExit path, and the failed session-status liveness plumbing). Detection is now purely hook-reported: Claude Code's StopFailure maps to the Failed lifecycle event while the session stays alive, so the binding shows failed via the normal active read — no extra machinery, and no exit-code false positives.
|
Scoped this down to CLI-reported failures only (505a571): removed the abnormal-exit heuristic ( |
Problem
Terminal-agent status was derived from a
Start/Stop/PermissionRequestvocabulary with no failure category, so a failed agent turn was silent — the pane stayed on "working".What this does
Adds a first-class
Failedlifecycle event and a redfailedPaneStatus, driven purely by CLI-reported failures: Claude Code emits aStopFailurehook on API errors (rate_limit,overloaded,authentication_failed,billing_error,server_error, …). We register that hook and mapStopFailure→Failed.Because
StopFailurefires while the session stays alive, the binding just flips toFailedand surfaces through the normal live read — no session-status or liveness plumbing. It self-clears when the agent retries (nextStart). Thefailedstatus is a red pulsing dot (likepermission) and counts toward the unread / dock-badge attention totals.Scope
Deliberately limited to failures a CLI actually reports via a hook — no exit-code/crash inference (that path had false-positive risk on ordinary non-zero shell exits and is not worth the complexity). Non-Claude CLIs that don't emit a failure hook simply won't show
failed; that's fine.Tests
mapEventType(StopFailure→Failed),deriveTerminalAgentStatus, the store recordingFailedwithout dropping the binding, and the fullStopFailure→Failednotifications-router path (asserting agent identity survives). Typecheck + lint clean.Verified end-to-end (CDP)
Drove the running dev desktop: POSTing a
StopFailurehook for a live terminal agent flipped the bindingStart→Failedand turned the Terminal tab, pane header, and sidebar workspace badge from amber "working" to red "failed".Summary by cubic
Surfaces agent failures as a first-class failed status across
apps/desktopandpackages/host-service. Claude Code API errors now show a red “failed” state instead of staying “working”.New Features
Failedlifecycle event andfailedPaneStatus(red, pulsing). Shown in status indicators and attention/unread counts; priority just belowpermission.StopFailurehook and map toFailedwhile the session stays alive; clears on nextStart. Detection is hook-reported only (no exit-code heuristics).failedbindings in live reads so panes show failure; never use them as attach targets.Bug Fixes
Failedbroadcasts and keep the binding in the store.Written for commit 505a571. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes