feat(desktop): structured error recovery cards for chat

[eulicesl]

Summary

• Problem: The chat surface shows every failure through a single inline errorMessage banner — a flat red string with no recovery path. Auth lapses, timeouts, a missing bridge runtime, user interrupts, and empty results all look the same and offer the user nothing to do.
• What changed: Introduces ChatErrorState (five user-visible failure classes mapped from BridgeError) and ChatErrorCard, an inline message-level card with a primary recovery CTA. ChatProvider now drives the card and dispatches each recovery action to a concrete handler.
• What did NOT change (scope boundary): Paywall/product sheets (ClaudeAuthSheet, showOmiThresholdAlert, browser-extension setup) are intentionally left as-is — they are product flows, not generic error recovery. No telemetry, backend, or bridge-protocol changes. The legacy errorMessage banner is retained as the fallback for unmappable errors, so no error path becomes invisible.

Linked Issue / Bounty

• N/A — independent contribution.

Root Cause (bug fixes only)

• N/A — new feature.

How It Works

• ChatErrorState.from(_ bridgeError:) lifts a BridgeError into one of five recoverable states (authRequired, timeout, bridgeUnavailable, interrupted, noDataFound). Cases that shouldn't preempt the existing banner (encodingError, quotaExceeded, free-form agentError) return nil and fall through.
• sendMessage's catch block prefers the structured card when the error maps, clearing errorMessage; unmappable errors keep the legacy banner. Only one surface is active per turn.
• Each ChatErrorState exposes a primaryRecovery action. recoverFromError() dispatches it: .retry re-issues the captured last prompt; .signIn opens the sign-in URL; .openSettings posts the existing navigateToAIChatSettings notification; .installRuntime opens the Node install page; .switchMode toggles the bridge mode via the existing switchBridgeMode(to:); .dismiss clears.
• ChatPage renders ChatErrorCard above the legacy banner so its CTA gets the prominent slot.

Change Type

• Feature

Scope

• Desktop app (Swift/macOS)

Security Impact

• New permissions or capabilities introduced? No
• Auth or token handling changed? No — .signIn opens a public URL; it does not touch tokens or the auth store.
• Data access scope changed (screen data, OCR, user data)? No
• New or changed network calls? No app-originated calls. .signIn / .installRuntime hand a public URL to the default browser via NSWorkspace.open.
• Plugin or tool execution surface changed? No
• The "Show details" disclosure surfaces only a redacted, user-safe description of the failure class — never raw error text.

Testing

• Built locally — xcrun swift build -c debug --package-path Desktop → Build complete!
• Manual verification: compile + unit tests on macOS. See "Human Verification" for what was not runtime-verified.
• Existing tests pass
• New tests added — ChatErrorStateTests.swift (11 tests): full BridgeError → ChatErrorState mapping, every state has a recovery action, and a guard locking the 6-case recovery-action set so a new case forces a recoverFromError switch update.

Human Verification

• Verified scenarios: clean debug build against current main; all 11 ChatErrorState tests pass; full test target compiles.
• Edge cases checked (via tests): unmappable BridgeError cases correctly fall through to the legacy banner; bridgeUnavailable reason split (node-missing / runtime-missing / crashed / unknown).
• What I did not verify (and why): I did not drive the running app to capture a per-state screenshot. The cards are hard to trigger deterministically without a fake-bridge harness (out of scope for this PR), so runtime UI evidence is a known gap — see Evidence.

Evidence

• Test output:

  Test Suite 'ChatErrorStateMappingTests' passed — Executed 2 tests, 0 failures
  Test Suite 'ChatErrorStateTests' passed       — Executed 9 tests, 0 failures
  Build complete!
  

• No runtime screenshot — honest gap (see Human Verification). Happy to add one if a reviewer wants it.

Docs

• N/A — no docs impact

Performance, Privacy, and Reliability

• No measurable performance impact (one extra optional view + an enum map on the error path). Privacy: the card never renders raw error text, only a redacted class description. Reliability: strictly additive — unmapped errors retain today's behavior.

Migration / Backward Compatibility

• Backward compatible? Yes
• Migration needed? No — purely additive; the legacy banner remains for unmapped cases.

Risks and Mitigations

• Risk: Card and legacy banner could both show. Mitigation: the catch block sets one and clears the other; only one is active per turn.
• Risk: A future BridgeError case silently fails to map. Mitigation: testFactoryMappabilityIsStableUnderRefactor locks the mapping; changing it forces a test update.
• Known limitation: Recovery actions are deliberately minimal — .signIn opens the sign-in URL rather than triggering native in-app OAuth. This is the honest minimum; deepening it is a natural follow-up.

AI Disclosure

• Tools used: Claude Code (Claude Opus).
• AI-assisted portions: implementation and this PR description.
• How I verified correctness: ran the local debug build and the unit-test suite (all green); manually reviewed every diff and confirmed no telemetry/personal data is touched.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[greptile-apps]

Greptile Summary

This PR introduces a structured error-recovery layer for the macOS chat surface: a ChatErrorState enum mapping BridgeError cases to five user-visible failure classes, a ChatErrorCard SwiftUI view with per-state copy and a primary recovery CTA, and wiring in ChatProvider / ChatPage to drive the card while keeping the legacy errorMessage banner as a fallback for unmapped cases.

• ChatErrorState.from(_:) lifts eight BridgeError cases into five recoverable states; the remaining three fall through to the existing banner, ensuring no error path disappears.
• recoverFromError() dispatches each ChatErrorRecoveryAction to a concrete handler (retry, sign-in URL, settings notification, Node install URL, mode switch, dismiss); 11 unit tests cover mapping and recovery-action assignments.

Confidence Score: 4/5

The new error card layer is additive and strictly scoped; the legacy banner remains as a fallback so no error path disappears.

The main concern is that currentError is never cleared at the start of sendMessage, so a stale error card can remain visible throughout a new user-initiated send and even persist after a successful response. The BridgeError mapping, card rendering, recovery dispatch, and unit tests are otherwise well-structured.

desktop/Desktop/Sources/Providers/ChatProvider.swift - the sendMessage preamble needs currentError = nil to match the existing errorMessage = nil clear-on-new-send contract.

Important Files Changed

Filename	Overview
desktop/Desktop/Sources/Chat/ChatErrorState.swift	New file defining ChatErrorState enum, BridgeUnavailableReason, ChatErrorRecoveryAction, and the BridgeError to ChatErrorState factory; two recovery actions (.switchMode, .openSettings) are defined but unreachable from any primaryRecovery case.
desktop/Desktop/Sources/MainWindow/Pages/ChatErrorCard.swift	New SwiftUI view rendering the inline error card with state-specific copy, accent color, SF Symbol, primary CTA, and optional detail disclosure; straightforward and well-structured.
desktop/Desktop/Sources/MainWindow/Pages/ChatPage.swift	Adds ChatErrorCard rendering above the legacy error banner; recovery and dismiss callbacks are correctly wired.
desktop/Desktop/Sources/Providers/ChatProvider.swift	Adds currentError/@published, lastFailedPrompt, recoverFromError(), and dismissCurrentError(); stale card issue: currentError is not cleared at the start of sendMessage alongside errorMessage.
desktop/Desktop/Tests/ChatErrorStateTests.swift	11 tests covering BridgeError mapping and primaryRecovery values; testEveryRecoveryActionIsAddressableByProvider only counts array elements and does not verify all 6 actions are reachable from any error state.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User sends message] --> B[sendMessage called]
    B --> C{BridgeError thrown?}
    C -->|No| D[Success: response shown]
    C -->|Yes - stopped| F[ChatErrorState.from stopped]
    C -->|Yes - other| G[ChatErrorState.from bridgeError]
    F --> H{maps to card?}
    G --> H
    H -->|nil| I[errorMessage = localizedDescription\ncurrentError = nil]
    H -->|some card| J[currentError = card\nlastFailedPrompt saved\nerrorMessage = nil]
    J --> K[ChatErrorCard rendered above legacy banner]
    K --> L{User action}
    L -->|Taps CTA| M[recoverFromError dispatches action]
    L -->|Taps X| N[dismissCurrentError - card cleared]
    M --> O1[retry - sendMessage with saved prompt]
    M --> O2[signIn - open omi.me]
    M --> O3[installRuntime - open nodejs.org]
    M --> O4[openSettings - post notification]
    M --> O5[switchMode - toggle bridge mode]
    M --> O6[dismiss - card cleared]

Loading

Comments Outside Diff (1)

1. desktop/Desktop/Sources/Providers/ChatProvider.swift, line 2512-2514 (link)

   Stale error card persists into the next user-initiated send

   errorMessage is cleared at the start of sendMessage (line 2513), but currentError is not. If the user types a new message in the input field directly (bypassing the CTA), the old ChatErrorCard stays visible for the entire new request duration — and even after a successful response, because the success path has no currentError = nil assignment anywhere. The card only disappears if the new send also fails with an unmappable error, or if the user explicitly taps the dismiss button.

   Adding currentError = nil alongside errorMessage = nil at line 2513 would match the existing clear-on-new-send contract.

_{Reviews (1): Last reviewed commit: "feat(desktop): structured error recovery..." | Re-trigger Greptile}

[kodjima33]

Desktop feature: structured error recovery cards, additive with legacy fallback. Approve only — feature >500 lines and mergeStateStatus BLOCKED.

[eulicesl]

Rebased onto the latest main, checks green, and all review threads resolved — this one's ready to merge whenever convenient. Thanks for the review!