fix(chat): revalidate a stale persisted chat_url and fall back to a live endpoint

crates/rocm-dash-tui/src/app/chat.rs

@@ -292,6 +292,43 @@ pub(super) const fn startup_chat_outcome(
     }
 }
 
+/// Decide whether a persisted `chat_url` should be replaced by a freshly
+/// detected live endpoint (EAI-7360).
+///
+/// Pure — takes the already-computed reachability/detection results rather
+/// than doing I/O itself, so it is unit-testable without a tool executor or a
+/// live socket. The caller only feeds this a `live` detection result when
+/// `chat_url` came from persisted config and neither the managed-services
+/// registry nor a direct TCP probe already confirmed it reachable, so this
+/// never revalidates an explicit env override, and `resolve_llm_config`'s own
+/// CLI>config>env>probe precedence (tested directly in `llm.rs`) is
+/// completely untouched — this only ever supplies a *replacement* config
+/// before that call runs.
+///
+/// Returns `Some(live)` only when the configured endpoint is unreachable and
+/// a *genuinely different*, live endpoint was found; `None` otherwise (nothing
+/// to replace, or detection re-found the same URL modulo a trailing `/`/`/v1`).
+pub(super) fn stale_chat_url_replacement(
+    configured_base_url: &str,
+    configured_probe_ok: bool,
+    live: Option<crate::llm::LlmConfig>,
+) -> Option<crate::llm::LlmConfig> {
+    if configured_probe_ok {
+        return None;
+    }
+    let configured = normalize_base_url(configured_base_url);
+    live.filter(|l| normalize_base_url(&l.base_url) != configured)
+}
+
+/// Normalize an OpenAI-style base URL for change-detection comparison: drop a
+/// trailing `/` and a trailing `/v1` so a formatting-only difference (e.g.
+/// `http://h:8000` vs `http://h:8000/v1/`) is not mistaken for a server change
+/// and does not emit a spurious "server changed" notice.
+fn normalize_base_url(base_url: &str) -> String {
+    let trimmed = base_url.trim().trim_end_matches('/');
+    trimmed.strip_suffix("/v1").unwrap_or(trimmed).to_string()
+}
+
 /// Persist an accepted local endpoint to the user's `config.toml`: load the
 /// existing config (or defaults), set `tui.chat_url`/`tui.chat_model`, and write
 /// it back. Best-effort — returns a human error string on failure.
@@ -655,4 +692,75 @@ mod tests {
             StartupChatOutcome::Configured
         );
     }
+
+    fn live(base_url: &str) -> crate::llm::LlmConfig {
+        crate::llm::detected_llm_config(base_url, "m")
+    }
+
+    #[test]
+    fn stale_chat_url_replacement_none_when_configured_endpoint_reachable() {
+        // Reachable configured endpoint must never be swapped out, regardless
+        // of what a stray `live` detection result would have found.
+        assert_eq!(
+            stale_chat_url_replacement(
+                "http://127.0.0.1:9/v1",
+                true,
+                Some(live("http://127.0.0.1:11435/v1"))
+            ),
+            None
+        );
+    }
+
+    #[test]
+    fn stale_chat_url_replacement_none_when_nothing_live_found() {
+        assert_eq!(
+            stale_chat_url_replacement("http://127.0.0.1:9/v1", false, None),
+            None
+        );
+    }
+
+    #[test]
+    fn stale_chat_url_replacement_none_when_live_matches_configured() {
+        // Detection simply re-confirming the same (currently-unreachable-by-
+        // TCP-probe-timing) URL is not a "server changed" situation.
+        let url = "http://127.0.0.1:11435/v1";
+        assert_eq!(
+            stale_chat_url_replacement(url, false, Some(live(url))),
+            None
+        );
+    }
+
+    #[test]
+    fn stale_chat_url_replacement_swaps_to_a_different_live_endpoint() {
+        let found = live("http://127.0.0.1:13305/v1");
+        let replacement =
+            stale_chat_url_replacement("http://127.0.0.1:9/v1", false, Some(found.clone()))
+                .expect("a different live endpoint replaces the stale one");
+        assert_eq!(replacement.base_url, found.base_url);
+    }
+
+    #[test]
+    fn stale_chat_url_replacement_ignores_trailing_slash_and_v1_formatting() {
+        // A formatting-only difference (trailing `/`, presence/absence of the
+        // `/v1` suffix) is the SAME endpoint — no spurious "server changed".
+        for (configured, found) in [
+            ("http://127.0.0.1:8000/v1", "http://127.0.0.1:8000/v1/"),
+            ("http://127.0.0.1:8000/v1", "http://127.0.0.1:8000"),
+            ("http://127.0.0.1:8000", "http://127.0.0.1:8000/v1"),
+        ] {
+            assert_eq!(
+                stale_chat_url_replacement(configured, false, Some(live(found))),
+                None,
+                "{configured} vs {found} must be treated as the same endpoint"
+            );
+        }
+    }
+
+    #[test]
+    fn normalize_base_url_strips_trailing_slash_and_v1() {
+        assert_eq!(normalize_base_url("http://h:8000/v1/"), "http://h:8000");
+        assert_eq!(normalize_base_url("http://h:8000/v1"), "http://h:8000");
+        assert_eq!(normalize_base_url("http://h:8000/"), "http://h:8000");
+        assert_eq!(normalize_base_url("http://h:8000"), "http://h:8000");
+    }
 }

crates/rocm-dash-tui/src/app/mod.rs

@@ -40,7 +40,8 @@ mod summary;
 
 use chat::{
     StartupChatOutcome, build_chat_agent, build_local_agent, detect_local_chat,
-    discover_configured_chat_model, persist_chat_endpoint, startup_chat_outcome,
+    discover_configured_chat_model, persist_chat_endpoint, stale_chat_url_replacement,
+    startup_chat_outcome,
 };
 use summary::{parse_plan_result, summarize_json_value, summarize_slash_tool};
 
@@ -79,7 +80,9 @@ pub struct ResolvedArgs {
     /// exit back to the launcher when the overlay is closed at its root. `None`
     /// (the default) is the normal full dashboard — every path stays unchanged.
     pub focus: Option<Focus>,
-    /// Chat endpoint base URL, CLI-flag value already merged over config.
+    /// Chat endpoint base URL, sourced from persisted config (`tui.chat_url`).
+    /// There is no `--chat-url` CLI flag today, so this tier is exactly the
+    /// persisted config — which is what startup revalidates when it goes stale.
     pub chat_url: Option<String>,
     /// Chat model, CLI-flag value already merged over config.
     pub chat_model: Option<String>,
@@ -1748,11 +1751,16 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu
         let probe_ok = match startup_outcome {
             StartupChatOutcome::Local => true,
             StartupChatOutcome::OAuth => false,
-            StartupChatOutcome::Configured => tokio::task::spawn_blocking(move || {
-                crate::llm::probe_endpoint(&probe_target, crate::llm::PROBE_TIMEOUT)
-            })
-            .await
-            .unwrap_or(false),
+            StartupChatOutcome::Configured => {
+                // Clone so `probe_target` stays available for the EAI-7360
+                // stale-URL recovery and its notice below.
+                let target = probe_target.clone();
+                tokio::task::spawn_blocking(move || {
+                    crate::llm::probe_endpoint(&target, crate::llm::PROBE_TIMEOUT)
+                })
+                .await
+                .unwrap_or(false)
+            }
         };
         let llm = detected.or_else(|| {
             crate::llm::resolve_llm_config(
@@ -1780,6 +1788,43 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu
             ),
             other => other,
         };
+        // EAI-7360: a persisted `tui.chat_url` can go stale — the server it
+        // pointed at may have been stopped or replaced since the dash last wrote
+        // it. This is the `Configured`-but-unreachable case: the reachable
+        // `Configured` case already ran `discover_configured_chat_model` above,
+        // and a `Local` outcome auto-detected its own live endpoint. Only this
+        // persisted-config tier is revalidated — `chat_env_url` is a separate
+        // tier, and `resolve_llm_config`'s CLI>config>env>probe precedence (the
+        // literal contract unit-tested in `llm.rs`) is untouched: this only
+        // substitutes a *replacement* `LlmConfig` after that call, never changes
+        // how the call itself resolves.
+        //
+        // Require NO configured api_key/auth_header: the replacement is a
+        // keyless `detected_llm_config`, and since `ResolvedArgs` can't
+        // distinguish an explicit `--chat-url` from persisted config, a remote
+        // gateway URL + `OPENAI_API_KEY` that merely TCP-times-out must never
+        // silently swap to a local keyless engine and drop the credential.
+        // Confining the swap to a no-auth `chat_url` keeps it on the true
+        // EAI-7360 target: a dead *persisted* local endpoint.
+        let stale_replacement = if startup_outcome == StartupChatOutcome::Configured
+            && !probe_ok
+            && args.chat_url.is_some()
+            && args.chat_api_key.is_none()
+            && args.chat_auth_header.is_none()
+        {
+            let live = detect_local_chat(state.tool_executor.clone()).await;
+            stale_chat_url_replacement(&probe_target, probe_ok, live)
+        } else {
+            None
+        };
+        if let Some(live) = &stale_replacement {
+            state.chat.push(ChatTurn::system(format!(
+                "Configured chat server at {probe_target} is unreachable; switched to the live \
+                 local engine at {} (model: {}). Run `/detect save` to persist it.",
+                live.base_url, live.model
+            )));
+        }
+        let llm = stale_replacement.or(llm);
         state.set_chat_config(llm, args.chat_auto_consent);
         // No reachable local endpoint AND no key/url configured → the no-key
         // ChatGPT OAuth default (device-code login surfaced in the chat tab).