[codex] Recover ACP runtime errors from snapshots

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<SessionLastError>,
 
     /// 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<ConfigStaleKind>,
+    /// 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<SessionLastError>,
     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();

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: [],
   })),

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

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")
+  })
+})

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<string, ToolCallState>

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
 }