From cbef583b8a3a938e03f4cfaecdb726558bb1e605 Mon Sep 17 00:00:00 2001 From: Adam Dalloul <47503782+Adam-Dalloul@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:06:58 -0700 Subject: [PATCH 1/2] fix(acp): stop replaying a config option the agent never advertised MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saved selector preferences are replayed on every connect, and an id the connected agent does not offer is rejected every time. The reporter's logs carry 29 of these in a week, the only ERROR class in them: [ACP] failed to apply preferred config 'fast'='off' on connect: Internal error: {"details": "Unknown config option: fast"} The entry cannot clear itself. `saveConfigPreference` is the only writer and it fires on a user pick, but once the agent stops advertising the option its selector leaves the UI, so there is no pick left to make and the value is re-sent forever. Nor is the value the problem: an agent that advertises no `effort` option rejects `effort` identically at every value, so clamping the value could never have helped either. `apply_preferred_session_options` now sends a preference only when the agent currently advertises the id. `mode` keeps the documented exception, because codex-acp's `applySessionConfigOption` switches on `configId` alone with no advertised-list check, and a rejection of a deliberately speculative send logs at debug rather than error. Nothing is deleted. The store is keyed per agentType but an option's presence is narrower than the agent: cursor-agent hangs `fast` and the thinking parameters off the model in effect, so the same agent advertises the id under one model and not under another. Dropping the entry would retire a preference the user still has a use for. Keeping it costs a skipped round-trip, and it applies again as soon as the option returns — including within the same replay, since `model` goes first and the option set that comes back with it is what the remaining ids are checked against. --- src-tauri/src/acp/connection.rs | 185 ++++++++++++++++++++++++++++-- src/lib/selector-prefs-storage.ts | 15 ++- 2 files changed, 185 insertions(+), 15 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 5399f9851f..10cea4d8b8 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -3006,6 +3006,11 @@ async fn emit_selectors_ready(state: &Arc>, emitter: &Event /// id this way, and the frontend's `isModelConfigOption` accepts either. const MODEL_CONFIG_OPTION_ID: &str = "model"; +/// The conventional id of a mode selector carried as a config option. ACP +/// reserves none here either, but codex-acp — the agent that ships one — spells +/// it this way. See [`UNADVERTISED_CONFIG_OPTION_IDS`]. +const MODE_CONFIG_OPTION_ID: &str = "mode"; + /// Synthesized config-option id for Grok's model picker (drives the composer's /// grouped model selector via the frontend's `isModelConfigOption`). const GROK_MODEL_OPTION_ID: &str = MODEL_CONFIG_OPTION_ID; @@ -7025,6 +7030,50 @@ fn is_model_config_option(option: &SessionConfigOption) -> bool { || option.id.to_string() == MODEL_CONFIG_OPTION_ID } +/// Config-option ids codeg still sends when the connected agent did not +/// advertise them. +/// +/// The replay is otherwise gated on the advertised list (see +/// [`applies_preferred_config_option`]). `mode` is the one documented +/// exception: codex-acp's `applySessionConfigOption` switches on `configId` +/// alone with no advertised-list check (verified in the 1.7.0 bundle), so a +/// build that omits `mode` from its options may still honour it — and mode is +/// the pick whose loss changes what the agent is allowed to do. +const UNADVERTISED_CONFIG_OPTION_IDS: &[&str] = &[MODE_CONFIG_OPTION_ID]; + +/// Whether a saved config preference should be put on the wire at all. +/// +/// `advertised` is whether the agent's CURRENT option list carries the id — the +/// list as it stands at this point of the replay, not the one the session +/// opened with. +/// +/// Preferences are stored per agent type and only ever written by a user pick +/// (`saveConfigPreference` in `src/lib/selector-prefs-storage.ts`). Once an +/// agent stops advertising an option its selector leaves the UI, so there is no +/// pick left to make and the entry cannot be rewritten — it is replayed on every +/// single connect. For an id the agent has never heard of the answer is always +/// the same rejection, which used to be logged at ERROR each time: +/// +/// ```text +/// [ACP] failed to apply preferred config 'fast'='off' on connect: +/// Internal error: {"details": "Unknown config option: fast"} +/// ``` +/// +/// Gating on the advertised list costs nothing a user can see, and the +/// preference is NOT deleted — deleting is what the keying cannot support. The +/// store is keyed by agent type, but an option's presence is narrower than that: +/// Cursor hangs `fast` and the thinking parameters off the CURRENT model, so the +/// same agent advertises the id under one model and not under another. Dropping +/// the entry on a miss would silently retire a preference the user still has a +/// use for. Keeping it costs a skipped round-trip and applies again the moment +/// the agent (or the selected model) offers the option once more — which the +/// replay reaches in the same pass, because `model` goes first (see +/// [`order_preferred_config_values`]) and the option set that comes back with it +/// is what the remaining ids are checked against. +fn applies_preferred_config_option(advertised: bool, config_id: &str) -> bool { + advertised || UNADVERTISED_CONFIG_OPTION_IDS.contains(&config_id) +} + /// Saved preferences in application order: the model selector first, then every /// other id in its natural (sorted) order. /// @@ -7039,7 +7088,9 @@ fn is_model_config_option(option: &SessionConfigOption) -> bool { /// /// A preferred id the agent never advertised is ordered as a non-model option /// unless it is literally `model` — the fallback stays deliberately narrow -/// because an unadvertised id is still sent (see `apply_preferred_session_options`). +/// because ordering runs before the advertised-list gate in +/// `apply_preferred_session_options`, and an id that is missing here may well be +/// advertised by the time its turn comes (the model switch re-scopes the list). fn order_preferred_config_values<'a>( options: &[SessionConfigOption], preferred: &'a BTreeMap, @@ -7175,13 +7226,9 @@ async fn apply_preferred_session_options( let ordered = order_preferred_config_values(&options, preferred_config_values); for (config_id, value_id) in ordered { // Skip the round-trip when the agent's current value already matches. - // Note: codex-acp advertises "mode" as a config option (so the match - // check below normally fires), but we still do NOT skip when a - // requested config_id is absent from the advertised options — an agent - // may accept `set_config_option` for an id it never advertised. codex - // does: its `applySessionConfigOption` switches on `configId` alone, - // with no advertised-list check (verified in the 1.7.0 bundle). So let - // the agent decide. + // `options` is the LIVE list — reassigned from every successful set + // below — so an id scoped to the model is checked against the option + // set the model preference just brought in, not the pre-replay one. let advertised = options.iter().find(|o| o.id.to_string() == *config_id); let already_matches = advertised.is_some_and(|o| config_option_already_holds(o, value_id.as_str())); @@ -7190,12 +7237,31 @@ async fn apply_preferred_session_options( } // Encode against what the agent advertised for this id. An id the agent // never advertised falls back to the select form — the same value shape - // codeg has always sent (see the note above on unadvertised "mode"). + // codeg has always sent for the allowlisted ids below. let is_boolean = advertised.is_some_and(|o| matches!(o.kind, SessionConfigKind::Boolean(_))); + let is_advertised = advertised.is_some(); + // An id the agent has never heard of can only be rejected, on this + // connect and on every connect after it — see + // [`applies_preferred_config_option`] for why the entry cannot clear + // itself and why dropping it here would be wrong. + if !applies_preferred_config_option(is_advertised, config_id) { + tracing::debug!( + "[ACP] preferred config '{config_id}'='{value_id}' is not advertised by this \ + agent; leaving it saved and not applying it" + ); + continue; + } let value = encode_config_option_value(is_boolean, value_id); match set_session_config_option_inner(cx, &session_id, config_id.clone(), value).await { Ok(updated) => options = updated, + // A deliberately speculative send (an allowlisted id the agent did + // not advertise) losing its gamble is the expected outcome, not an + // incident. Only a rejection of something the agent DID offer is. + Err(e) if !is_advertised => tracing::debug!( + "[ACP] agent declined unadvertised preferred config '{config_id}'='{value_id}' \ + on connect: {e}" + ), Err(e) => tracing::error!( "[ACP] failed to apply preferred config '{config_id}'='{value_id}' \ on connect: {e}" @@ -22029,8 +22095,9 @@ mod tests { .collect(); assert_eq!(ordered, vec!["llm", "effort"]); - // Nothing model-shaped: the order is untouched, and an id the agent - // never advertised is still replayed (it is not codeg's call to drop). + // Nothing model-shaped: the order is untouched. Ordering keeps an id + // the agent has not advertised — the advertised gate runs later, on the + // list as it stands when that id's turn comes. let preferred = BTreeMap::from([ ("a_thing".to_string(), "1".to_string()), ("z_thing".to_string(), "2".to_string()), @@ -22042,6 +22109,102 @@ mod tests { assert_eq!(ordered, vec!["a_thing", "z_thing"]); } + /// The replay must not push an id the connected agent never advertised. + /// Preferences are keyed per agent type and rewritten only by a user pick, + /// so once the selector leaves the UI there is no pick left to make and the + /// entry is replayed on every connect forever — the reporter's log carried + /// 29 of these in a week, every one of them the same rejection: + /// + /// ```text + /// [ACP] failed to apply preferred config 'fast'='off' on connect: + /// Internal error: {"details": "Unknown config option: fast"} + /// ``` + #[test] + fn preferred_config_replay_skips_ids_the_agent_never_advertised() { + // Advertised: applied, exactly as before. + for id in ["fast", "effort", "reasoning_effort", MODEL_CONFIG_OPTION_ID] { + assert!(applies_preferred_config_option(true, id), "{id}"); + } + // Not advertised: no request, so no ERROR. `fast` is the reported id; + // an agent that advertises no effort option rejects `effort` the same + // way at EVERY value, which is why clamping the value cannot help. + for id in ["fast", "effort", "reasoning_effort", MODEL_CONFIG_OPTION_ID] { + assert!(!applies_preferred_config_option(false, id), "{id}"); + } + // `mode` keeps the documented exception: codex-acp's + // `applySessionConfigOption` switches on `configId` alone, with no + // advertised-list check, so a build that omits it may still honour it. + assert!(applies_preferred_config_option(false, MODE_CONFIG_OPTION_ID)); + assert!(applies_preferred_config_option(true, MODE_CONFIG_OPTION_ID)); + } + + /// A preference scoped to the current model must still land. Cursor hangs + /// `fast` off the model in effect, so the list the session opens with may + /// not carry it while the list that comes back from the `model` pick does. + /// That is why the gate reads the LIVE list — and why a miss must never + /// delete the entry: the store is keyed per agent type, but the option's + /// presence is narrower than the agent. + #[test] + fn a_model_scoped_preference_still_applies_after_the_model_pick() { + let advertises = + |opts: &[SessionConfigOption], id: &str| opts.iter().any(|o| o.id.to_string() == id); + let model_only: Vec = serde_json::from_value(serde_json::json!([ + { + "type": "select", + "id": "model", + "name": "Model", + "category": "model", + "currentValue": "composer-2.5", + "options": [ + {"value": "composer-2.5", "name": "Composer 2.5"}, + {"value": "claude-opus-5", "name": "Claude Opus 5"} + ] + }, + ])) + .expect("parses"); + let after_model_pick: Vec = + serde_json::from_value(serde_json::json!([ + { + "type": "select", + "id": "model", + "name": "Model", + "category": "model", + "currentValue": "claude-opus-5", + "options": [{"value": "claude-opus-5", "name": "Claude Opus 5"}] + }, + { + "type": "select", + "id": "fast", + "name": "Fast", + "category": "model_config", + "currentValue": "on", + "options": [{"value": "on", "name": "On"}, {"value": "off", "name": "Off"}] + }, + ])) + .expect("parses"); + + let preferred = BTreeMap::from([ + ("fast".to_string(), "off".to_string()), + ("model".to_string(), "claude-opus-5".to_string()), + ]); + // `model` leads, so the list `fast` is checked against is the one the + // model pick returned, not the one the session opened with. + let ordered: Vec<&str> = order_preferred_config_values(&model_only, &preferred) + .into_iter() + .map(|(id, _)| id.as_str()) + .collect(); + assert_eq!(ordered, vec!["model", "fast"]); + + assert!(!applies_preferred_config_option( + advertises(&model_only, "fast"), + "fast" + )); + assert!(applies_preferred_config_option( + advertises(&after_model_pick, "fast"), + "fast" + )); + } + /// The claude shape: a model select plus the effort option that hangs off it. fn asserted_drift_options(model: &str, effort: &str) -> Vec { serde_json::from_value(serde_json::json!([ diff --git a/src/lib/selector-prefs-storage.ts b/src/lib/selector-prefs-storage.ts index 64790dc979..18a2af5981 100644 --- a/src/lib/selector-prefs-storage.ts +++ b/src/lib/selector-prefs-storage.ts @@ -4,10 +4,17 @@ * Persists user's selector preferences (mode & config option selections) * per agentType to localStorage, so they survive session restarts. * - * Structure hash is stored alongside values — when the saved value no - * longer exists in the current option set (item renamed / removed) the - * backend's `set_session_config_option` will reject the application and - * the stale value is naturally dropped on the next user pick. + * An entry can outlive the option it names (renamed, removed, or scoped to + * a model that is no longer selected). It is not dropped here, and it + * cannot be: the selector is gone from the UI, so there is no further user + * pick to rewrite it with, and this store is keyed per agentType while an + * option's presence is narrower than that — cursor-agent hangs `fast` and + * the thinking parameters off the model in EFFECT, so the same agent + * advertises the id under one model and not under another. The backend + * owns the decision instead: it applies a saved value only when the agent + * currently advertises the id, and leaves the entry alone otherwise, so it + * applies again as soon as the option comes back (see + * `applies_preferred_config_option` in `src-tauri/src/acp/connection.rs`). * * Preferences are shipped to the backend at `acp_connect` time (see * `getSavedPrefsForConnect`) which applies them to the agent BEFORE From 78286a57f9dfc169d2699781b861ae61ced8f07d Mon Sep 17 00:00:00 2001 From: Adam Dalloul <47503782+Adam-Dalloul@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:07:12 -0700 Subject: [PATCH 2/2] fix(acp): skip the session/load a failed resume already answered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reopening a conversation whose codex rollout is gone puts the same failure on the wire twice, then reports it as a raw JSON-RPC body: [ACP] session/resume failed (Internal error: {"details": "no rollout found for thread id 019a…"}); falling back to session/load [ACP] session/load failed (Internal error: {"details": "no rollout found for thread id 019a…"}), falling back to session/new Falling through on a resume failure is right in general — resume is unstable and not equivalent to load, so a resume-specific error must never deny a load that might still succeed. This one cause is the exception: both methods resolve the same per-thread rollout record, so "no rollout found for thread id" is the store reporting the record does not exist, and asking a second time cannot make it appear. The resume error is now carried into the load block as its own result. The whole ladder runs unchanged on it — classification, the banner, the session/new fallback — minus one guaranteed-to-fail request and its duplicate warning, and the log names the method that actually answered. `classify_session_load_failure` also learns the wording. It is -32002 in everything but the code, exactly like the claude 0.58.1 cases already matched on the wire message, so it takes the existing `resource_not_found` verdict and the user sees the localized "session failed to load, reload or start a new conversation" banner instead of the raw error body. Custom agents, whose history is codeg's own transcript, keep their silent local recovery. --- src-tauri/src/acp/connection.rs | 188 ++++++++++++++++++++++++++++---- 1 file changed, 168 insertions(+), 20 deletions(-) diff --git a/src-tauri/src/acp/connection.rs b/src-tauri/src/acp/connection.rs index 10cea4d8b8..0256a64b0c 100644 --- a/src-tauri/src/acp/connection.rs +++ b/src-tauri/src/acp/connection.rs @@ -5515,6 +5515,12 @@ async fn run_connection( // parser, not the ACP wire). On any non-terminal resume failure // we fall through to the session/load block below, so the // effective chain is resume → load → new. + // + // Set when resume failed with a cause `session/load` cannot + // answer differently (see `resume_failure_settles_load`); the + // load block then takes this error as its own result instead of + // re-asking. `None` keeps the plain resume → load fall-through. + let mut settled_by_resume: Option = None; if supports_resume { let resume_req = build_resume_session_request( agent_type, @@ -5614,20 +5620,31 @@ async fn run_connection( Err(e) => { // resume is unstable and NOT guaranteed equivalent to // session/load, so a resume-specific failure must - // never deny a load that might still succeed. EVERY - // resume error — ResourceNotFound, "Authentication - // required", "Method not found", or anything else — - // falls through to the session/load block below, - // which already owns all terminal decisions - // (SessionLoadFailed for not-found, silent stop for - // auth, fallback to session/new otherwise). No - // user-facing event is emitted here: load re-derives - // the same outcome a moment later, so emitting now - // would double up (not-found) or flash a transient - // error that self-heals when load succeeds. - tracing::warn!( - "[ACP] session/resume failed ({e}); falling back to session/load" - ); + // never deny a load that might still succeed. Almost + // every resume error — ResourceNotFound, + // "Authentication required", "Method not found", or + // anything else — falls through to the session/load + // block below, which already owns all terminal + // decisions (SessionLoadFailed for not-found, silent + // stop for auth, fallback to session/new otherwise). + // No user-facing event is emitted here: load + // re-derives the same outcome a moment later, so + // emitting now would double up (not-found) or flash a + // transient error that self-heals when load succeeds. + // + // The exception is a cause both methods resolve out + // of the same record, where load cannot re-derive + // anything else. That error is carried into the block + // below as the load result, so the ladder runs + // unchanged on it — minus one guaranteed-to-fail + // request and its duplicate warning. + if resume_failure_settles_load(&e.to_string()) { + settled_by_resume = Some(e); + } else { + tracing::warn!( + "[ACP] session/resume failed ({e}); falling back to session/load" + ); + } // fall through to the session/load block below } } @@ -5647,8 +5664,15 @@ async fn run_connection( // agents that advertise `loadSession: true` and then answer // "Method not found" are real, so the whole error ladder below // stays exactly as it was. - let attempted_load = init_resp.agent_capabilities.load_session; - let load_result = if attempted_load { + // + // A resume that already settled the answer skips the request + // too, and hands its own error to the ladder below. + let skipped_load = settled_by_resume.is_some(); + let attempted_load = + !skipped_load && init_resp.agent_capabilities.load_session; + let load_result = if let Some(e) = settled_by_resume { + Err(e) + } else if attempted_load { let load_req = build_load_session_request( agent_type, SessionId::new(sid.clone()), @@ -5876,9 +5900,16 @@ async fn run_connection( let forgotten_session = classify_session_load_failure(e.code, &err_str); let recovers_locally = recovers_load_failure_locally(agent_type, forgotten_session); + // Name the method that actually answered: when the load + // was skipped this error came off session/resume. + let answered_by = if skipped_load { + "session/resume" + } else { + "session/load" + }; if let Some(code) = forgotten_session.filter(|_| !recovers_locally) { tracing::warn!( - "[ACP] session/load failed ({err_str}); surfacing as session_load_failed={code}" + "[ACP] {answered_by} failed ({err_str}); surfacing as session_load_failed={code}" ); emit_with_state( &state, @@ -5900,7 +5931,13 @@ async fn run_connection( .await; return Ok(()); } - if attempted_load { + if skipped_load { + tracing::warn!( + "[ACP] {answered_by} failed ({err_str}); session/load reads the \ + same record and was skipped. The agent no longer holds this \ + conversation, so a new session is opened for {sid}" + ); + } else if attempted_load { tracing::warn!( "[ACP] session/load failed ({err_str}), falling back to session/new" ); @@ -5925,8 +5962,13 @@ async fn run_connection( // would be pure noise. // A load codeg deliberately never sent is not a failure // to report — the capability gate above is the expected - // path for agents that don't implement it. - if attempted_load && !err_str.contains("Method not found") && !recovers_locally + // path for agents that don't implement it. A load + // skipped because resume already settled it is the + // opposite: there IS a real failure, it just came off + // the other method, so it reports exactly as before. + if (attempted_load || skipped_load) + && !err_str.contains("Method not found") + && !recovers_locally { emit_with_state( &state, @@ -8385,6 +8427,38 @@ fn stop_reason_to_str(reason: StopReason) -> &'static str { } } +/// codex-acp's wording when a thread's rollout record is not on disk. Both +/// `session/resume` and `session/load` answer with it, because both resolve the +/// same record. See [`resume_failure_settles_load`]. +const ROLLOUT_NOT_FOUND: &str = "no rollout found for thread id"; + +/// Whether a `session/resume` failure has already settled what `session/load` +/// would answer for the same session id, so the second request can be skipped. +/// +/// Deliberately one cause, not a family. `session/resume` is unstable and NOT +/// equivalent to `session/load`, so a resume-specific failure must never deny a +/// load that might still succeed — every other error keeps falling through to +/// the load block, which owns all terminal decisions. This one cannot disagree: +/// the two methods read the SAME per-thread rollout record, so "no rollout found +/// for thread id " is the store saying the record does not exist, and asking +/// a second time cannot make it appear. Today it buys a round-trip and a +/// duplicate warning on every reconnect of a conversation whose rollout has been +/// pruned: +/// +/// ```text +/// [ACP] session/resume failed (Internal error: {"details": "no rollout found +/// for thread id 019a…"}); falling back to session/load +/// [ACP] session/load failed (Internal error: {"details": "no rollout found +/// for thread id 019a…"}), falling back to session/new +/// ``` +/// +/// The failure is not swallowed: the resume error is handed to the same ladder +/// the load error would have entered, so classification, the `SessionLoadFailed` +/// banner and the `session/new` fallback all behave exactly as before. +fn resume_failure_settles_load(message: &str) -> bool { + message.contains(ROLLOUT_NOT_FOUND) +} + /// Classify a `session/load` failure into a stable frontend `code` when the /// historical session cannot be restored — either the agent has no record of /// it (`ResourceNotFound`, -32002) or the agent process/session died mid-load. @@ -8402,6 +8476,10 @@ fn stop_reason_to_str(reason: StopReason) -> &'static str { /// the same banner rather than the silent `session/new` fallback, which would /// orphan a history the user is one command away from restoring. /// +/// The input is whichever method actually answered: normally `session/load`, +/// or `session/resume` when its failure already settled the outcome and the +/// load was skipped (see [`resume_failure_settles_load`]). +/// /// Returns `None` for failures that must keep the existing behavior: /// "Method not found" (agent lacks resume → silent `session/new` fallback), /// "Authentication required" (silent stop), and any other error (emit @@ -8434,6 +8512,15 @@ fn classify_session_load_failure( if message.contains("already has an active writer") { return Some("session_busy"); } + // codex-acp when the thread's rollout file is no longer on disk (pruned, + // rotated, or deleted): a generic -32603 whose body reads "no rollout found + // for thread id ". That is -32002 in everything but the code — the + // agent is reporting it has no record of the session — so it takes the + // same verdict, and the user gets the localized banner instead of the raw + // JSON-RPC body. + if message.contains(ROLLOUT_NOT_FOUND) { + return Some("resource_not_found"); + } // Upstream signals for an unrecoverable session (claude-agent-acp 0.58.1): // - "process exited" → "Claude Code process exited with code 1", // "The Claude Agent process exited unexpectedly…" @@ -15454,6 +15541,67 @@ mod tests { )); } + /// A thread whose rollout is gone. codex-acp resolves `session/resume` and + /// `session/load` out of that same record, so both answer with the same + /// generic -32603 and the reporter's log carried the pair on every + /// reconnect: + /// + /// ```text + /// [ACP] session/resume failed (Internal error: {"details": "no rollout + /// found for thread id 019a…"}); falling back to session/load + /// [ACP] session/load failed (Internal error: {"details": "no rollout + /// found for thread id 019a…"}), falling back to session/new + /// ``` + #[test] + fn classify_load_failure_names_a_pruned_rollout() { + let missing = "Internal error: {\n \"details\": \"no rollout found for thread id \ + 019a1f7e-2c44-70b1-9d33-4f8b1c05e6aa\"\n}"; + // Resume already settled it: a second lookup of a record that does not + // exist cannot answer anything else. + assert!(resume_failure_settles_load(missing)); + // -32002 in everything but the code, so it takes the same verdict and + // the user gets the localized banner instead of the raw JSON-RPC body. + assert_eq!( + classify_session_load_failure(sacp::schema::ErrorCode::InternalError, missing), + Some("resource_not_found"), + ); + // Codex reads history back out of that same rollout store, so it stops + // with the banner rather than silently opening a fresh session. + assert!(!recovers_load_failure_locally( + AgentType::Codex, + Some("resource_not_found") + )); + // A custom agent's history is codeg's own transcript, so it keeps the + // silent local recovery. + let custom = AgentType::custom("glm-acp-agent").expect("valid id"); + assert!(recovers_load_failure_locally( + custom, + Some("resource_not_found") + )); + } + + /// `session/resume` is unstable and NOT equivalent to `session/load`, so + /// only a cause the two methods cannot disagree on may cancel the second + /// attempt. Everything else keeps the resume → load → new chain intact. + #[test] + fn other_resume_failures_still_fall_through_to_session_load() { + for message in [ + "Method not found", + "Authentication required", + "session abc not found", + "Internal error: { \"details\": \"Claude Code process exited with code 1\" }", + "Internal error: { \"details\": \"session 019bf0c4 is archived. Run \ + `codex unarchive 019bf0c4` to restore it.\" }", + "Internal error: { \"details\": \"thread 01a0626c already has an active writer\" }", + ] { + assert!(!resume_failure_settles_load(message), "{message}"); + } + // The capability gate's synthetic error is not a resume verdict either. + let gate = sacp::Error::method_not_found() + .data("agent does not advertise the loadSession capability"); + assert!(!resume_failure_settles_load(&gate.to_string())); + } + /// After a codex fork, the sibling row codeg creates to keep the pre-fork /// history points at the PARENT thread — whose writer the forking process /// still holds, because `session/fork` only unsubscribes the child. Opening