diff --git a/src-tauri/src/acp/session_state.rs b/src-tauri/src/acp/session_state.rs index 5e49b92a..f6e80685 100644 --- a/src-tauri/src/acp/session_state.rs +++ b/src-tauri/src/acp/session_state.rs @@ -265,15 +265,15 @@ pub struct SessionState { /// "init complete" without waiting for an event that already fired. pub selectors_ready: bool, - /// Most recent `AcpEvent::Error` payload, or `None` if no error has - /// landed since the connection started. The probe path reads this - /// after `wait_for_session_options` errors so it can fold the - /// agent's own error message into the returned `AcpError` instead - /// of surfacing a generic "connection not found" once the - /// connection task has cleaned up its map entry. + /// Most recent unresolved `AcpEvent::Error` payload. Cleared when a new + /// prompt starts, matching the frontend reducer's live-event behavior. The + /// probe path reads this after `wait_for_session_options` errors so it can + /// fold the agent's own error message into the returned `AcpError` instead + /// of surfacing a generic "connection not found" once the connection task + /// has cleaned up its map entry. /// - /// Not exposed on `to_snapshot()` today — chat-side error UX already - /// flows through the live `AcpEvent::Error` channel. + /// Exposed on `to_snapshot()` so clients that reconnect after missing the + /// live `AcpEvent::Error` can still surface the latest agent failure. pub last_error: Option, /// Single-fire signal that fires when `SessionStarted` applies (i.e. @@ -474,6 +474,12 @@ impl SessionState { } } AcpEvent::StatusChanged { status } => { + if matches!(status, ConnectionStatus::Prompting) { + // Match the live frontend reducer: a new prompt starts a + // new error scope, so stale recoverable errors must not be + // resurrected by a later snapshot attach. + self.last_error = None; + } self.status = status.clone(); } AcpEvent::SessionModes { modes } => { @@ -1086,6 +1092,7 @@ impl SessionState { selectors_ready: self.selectors_ready, config_stale: self.config_stale, config_stale_kind: self.config_stale_kind, + last_error: self.last_error.clone(), event_seq: self.event_seq, } } @@ -1152,6 +1159,10 @@ pub struct LiveSessionSnapshot { /// byte-identical with the pre-feature wire shape. #[serde(default, skip_serializing_if = "Option::is_none")] pub config_stale_kind: Option, + /// Most recent agent/runtime error for this live connection. Omitted when + /// no error has occurred so older clients and common snapshots stay small. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub last_error: Option, pub event_seq: u64, } @@ -1347,6 +1358,44 @@ mod tests { ); } + #[test] + fn snapshot_carries_last_error_and_clears_on_next_prompt() { + let mut s = fresh_state(); + s.apply_event(&AcpEvent::Error { + message: "ACP protocol error: forbidden".into(), + agent_type: "claude_code".into(), + code: Some("forbidden".into()), + terminal: true, + }); + + let snap = s.to_snapshot(); + assert_eq!( + snap.last_error, + Some(SessionLastError { + message: "ACP protocol error: forbidden".into(), + code: Some("forbidden".into()), + }) + ); + + let json = serde_json::to_string(&snap).expect("serialize"); + let back: LiveSessionSnapshot = serde_json::from_str(&json).expect("deserialize"); + assert_eq!(back.last_error, snap.last_error); + + let empty_json = serde_json::to_string(&fresh_state().to_snapshot()).expect("serialize"); + assert!( + !empty_json.contains("last_error"), + "no-error snapshot must omit last_error" + ); + + s.apply_event(&AcpEvent::StatusChanged { + status: ConnectionStatus::Prompting, + }); + assert!( + s.to_snapshot().last_error.is_none(), + "new prompts clear stale snapshot-recoverable errors" + ); + } + #[test] fn latest_live_reply_prefers_answer_after_last_tool_call() { let mut s = fresh_state(); diff --git a/src/contexts/acp-connections-context.test.tsx b/src/contexts/acp-connections-context.test.tsx index 97985720..4489dee3 100644 --- a/src/contexts/acp-connections-context.test.tsx +++ b/src/contexts/acp-connections-context.test.tsx @@ -66,6 +66,22 @@ vi.mock("@/lib/selector-prefs-storage", () => ({ vi.mock("@/lib/snapshot-denormalize", () => ({ denormalizeSnapshot: vi.fn(() => ({ connectionId: "owner-conn", + status: "connected", + sessionId: null, + modes: null, + configOptions: null, + availableCommands: null, + usage: null, + liveMessage: null, + pendingPermission: null, + pendingAskQuestion: null, + pendingUserMessage: null, + promptCapabilities: null, + selectorsReady: false, + supportsFork: false, + configStale: false, + configStaleKind: null, + lastError: null, eventSeq: 0, activeDelegations: [], })), diff --git a/src/contexts/acp-connections-context.tsx b/src/contexts/acp-connections-context.tsx index ca2f1d07..906493f5 100644 --- a/src/contexts/acp-connections-context.tsx +++ b/src/contexts/acp-connections-context.tsx @@ -1014,6 +1014,7 @@ function connectionsReducer( current.availableCommands ?? action.patch.availableCommands const mergedPromptCapabilities = action.patch.promptCapabilities ?? current.promptCapabilities + const recoveredError = current.error ?? action.patch.lastError // Race guard: the snapshot may have been generated BEFORE events // that have since arrived and been applied to in-memory state. @@ -1028,7 +1029,8 @@ function connectionsReducer( mergedModes === current.modes && mergedConfigOptions === current.configOptions && mergedAvailableCommands === current.availableCommands && - mergedPromptCapabilities === current.promptCapabilities + mergedPromptCapabilities === current.promptCapabilities && + recoveredError === current.error ) { return state } @@ -1041,6 +1043,7 @@ function connectionsReducer( promptCapabilities: mergedPromptCapabilities, selectorsReady: mergedSelectorsReady, supportsFork: mergedSupportsFork, + error: recoveredError, }) return next } @@ -1066,6 +1069,7 @@ function connectionsReducer( // preserved via `...current`. configStale: action.patch.configStale, configStaleKind: action.patch.configStaleKind, + error: action.patch.lastError, lastAppliedSeq: action.patch.eventSeq, }) return next diff --git a/src/lib/snapshot-denormalize.test.ts b/src/lib/snapshot-denormalize.test.ts index cc2325c0..f64babb5 100644 --- a/src/lib/snapshot-denormalize.test.ts +++ b/src/lib/snapshot-denormalize.test.ts @@ -74,3 +74,26 @@ describe("denormalizeSnapshot — config staleness", () => { expect(patch.configStaleKind).toBeNull() }) }) + +describe("denormalizeSnapshot — last_error", () => { + it("carries last_error.message into the patch", () => { + const patch = denormalizeSnapshot( + baseSnapshot({ + last_error: { + message: " ACP protocol error: Forbidden ", + code: "forbidden", + }, + }) + ) + expect(patch.lastError).toBe("ACP protocol error: Forbidden") + expect(patch.status).toBe("connected") + }) + + it("defaults lastError to null when the field is absent", () => { + const snap = baseSnapshot() + delete (snap as { last_error?: unknown }).last_error + const patch = denormalizeSnapshot(snap) + expect(patch.lastError).toBeNull() + expect(patch.status).toBe("connected") + }) +}) diff --git a/src/lib/snapshot-denormalize.ts b/src/lib/snapshot-denormalize.ts index e4387856..b8bcd3d4 100644 --- a/src/lib/snapshot-denormalize.ts +++ b/src/lib/snapshot-denormalize.ts @@ -24,9 +24,9 @@ import type { /** * Snapshot-derived subset of ConnectionState. Fields not present here - * (pendingQuestion, claudeApiRetry, error, contextKey, agentType, - * workingDir) are frontend-only or set elsewhere and must not be touched - * by HYDRATE_FROM_SNAPSHOT. + * (pendingQuestion, claudeApiRetry, contextKey, agentType, workingDir) are + * frontend-only or set elsewhere and must not be touched by + * HYDRATE_FROM_SNAPSHOT. */ export interface SnapshotPatch { // Carries the snapshot's source connection_id so the reducer can reject @@ -61,6 +61,8 @@ export interface SnapshotPatch { * that the one-shot `session_config_stale` event won't replay. */ configStale: boolean configStaleKind: ConfigStaleKind | null + /** Latest ACP runtime error carried by the snapshot. `null` means none. */ + lastError: string | null eventSeq: number /** Live sub-agent delegations carried by the snapshot. Consumed directly at * the attach call sites to re-seed `DelegationProvider` bindings (see @@ -80,6 +82,7 @@ export function denormalizeSnapshot(wire: LiveSessionSnapshot): SnapshotPatch { for (const tc of wire.active_tool_calls) { toolMap.set(tc.id, tc) } + const lastError = normalizeSnapshotLastError(wire.last_error) return { connectionId: wire.connection_id, @@ -117,11 +120,22 @@ export function denormalizeSnapshot(wire: LiveSessionSnapshot): SnapshotPatch { supportsFork: wire.fork_supported, configStale: wire.config_stale ?? false, configStaleKind: wire.config_stale_kind ?? null, + lastError, eventSeq: wire.event_seq, activeDelegations: wire.active_delegations ?? [], } } +function normalizeSnapshotLastError( + lastError: LiveSessionSnapshot["last_error"] +): string | null { + const message = + lastError && typeof lastError.message === "string" + ? lastError.message.trim() + : "" + return message || null +} + function denormalizeLiveMessage( wire: WireLiveMessage, toolMap: Map diff --git a/src/lib/types.ts b/src/lib/types.ts index 627a78d8..77d9032c 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -1267,6 +1267,12 @@ export interface FeedbackItem { delivered_at?: string | null } +/** Snapshot of the most recent ACP runtime error. */ +export interface SessionLastError { + message: string + code?: string | null +} + export interface LiveSessionSnapshot { connection_id: string conversation_id: number | null @@ -1308,6 +1314,8 @@ export interface LiveSessionSnapshot { config_stale?: boolean /** Which settings surface drifted; present only while `config_stale`. */ config_stale_kind?: ConfigStaleKind | null + /** Latest agent/runtime error recoverable after reconnect. */ + last_error?: SessionLastError | null event_seq: number }