Merge pull request #7108 from ddlw/style/rustfmt-mcp

style(tui): apply rustfmt to mcp and restore the lint gate. Required checks green on f8a6346159 (Lint, Test (windows-latest), npm wrapper smoke). Maintainer commits on the branch: feature-gated Ollama dead_code expectation, Windows unused mut, merges of main incl. 715bb2ed02 resolved to the branch's gating. Supersedes #7106 (same mcp.rs rustfmt) and #7109 (.gitattributes line now included). CW-7525.

.gitattributes

@@ -63,6 +63,7 @@ crates/tui/extension-host/dist/** text eol=lf
 # Reviewed extension-host fixtures carry exact source SHA-256 receipts.
 # A Windows checkout must not rewrite their admitted source closure to CRLF.
 crates/tui/tests/fixtures/extension_host/** text eol=lf
+crates/tui/extension-host/src/dsh/dsh-message-compat.js text eol=lf
 
 # Rustfmt writes LF; keep Rust sources stable across Windows/Linux/macOS.
 *.rs text eol=lf

crates/protocol/src/op.rs

@@ -208,7 +208,7 @@ pub struct ReviewOperation {
 #[serde(tag = "kind", rename_all = "snake_case")]
 pub enum Op {
     Review {
-        turn: TurnSpec,
+        turn: Box<TurnSpec>,
         request: ReviewOperation,
     },
     /// Drive one model turn: `role=user` content plus the resolved route
@@ -629,7 +629,7 @@ mod tests {
         };
         vec![
             Op::Review {
-                turn,
+                turn: Box::new(turn),
                 request: ReviewOperation {
                     version: 1,
                     repository: "owner/repo".into(),

crates/runtime/src/config_persistence/test_support.rs

@@ -49,6 +49,12 @@ impl ModelEnvGuard {
     }
 }
 
+impl Default for ModelEnvGuard {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl Drop for ModelEnvGuard {
     fn drop(&mut self) {
         // Safety: test-only environment restoration under the same lock.

crates/runtime/src/safe_label.rs

@@ -393,6 +393,166 @@ fn error_word_is_allowlisted(word: &str) -> bool {
     !looks_opaque(word)
 }
 
+const MAX_RECEIPT_FIELD_CHARS: usize = 96;
+
+pub fn safe_receipt_field(raw: &str) -> String {
+    let sanitized = sanitize_persisted_route_label(raw);
+    let mut out = String::with_capacity(raw.len().min(MAX_RECEIPT_FIELD_CHARS));
+    let mut previous_separator = false;
+    for ch in sanitized.chars() {
+        if out.chars().count() >= MAX_RECEIPT_FIELD_CHARS {
+            break;
+        }
+        let safe = if ch.is_alphanumeric() || matches!(ch, '.' | '_' | '-' | '/' | ':' | '+') {
+            ch
+        } else {
+            '_'
+        };
+        let separator = safe == '_';
+        if separator && previous_separator {
+            continue;
+        }
+        out.push(safe);
+        previous_separator = separator;
+    }
+    if out.is_empty() { "-".to_string() } else { out }
+}
+
+pub fn sanitize_persisted_route_label(raw: &str) -> String {
+    const MAX_PERSISTED_ROUTE_LABEL_CHARS: usize = 256;
+    let value = raw.trim();
+    let lower = value.to_ascii_lowercase();
+
+    if value.is_empty() {
+        return "-".to_string();
+    }
+
+    // URLs are not route labels. Endpoints have a dedicated, validated hash
+    // field; persisting a URL here risks leaking userinfo, query credentials,
+    // or fragments through a custom provider/model name.
+    if value.contains("://") {
+        return "redacted-url".to_string();
+    }
+
+    let authorization_value = ["bearer ", "basic ", "digest ", "token ", "apikey "]
+        .iter()
+        .any(|scheme| lower.starts_with(scheme))
+        || lower.contains("authorization:")
+        || lower.contains("proxy-authorization:");
+    if authorization_value {
+        return "redacted-credential".to_string();
+    }
+
+    // Reject credential assignments regardless of common casing or separator:
+    // FOO_API_KEY=..., access-token:..., password = ....
+    for (index, ch) in value.char_indices() {
+        if !matches!(ch, '=' | ':') {
+            continue;
+        }
+        let name = lower[..index]
+            .trim()
+            .trim_matches(|ch: char| matches!(ch, '"' | '\'' | '{' | '[' | ','));
+        let name = name.rsplit([' ', ',', ';']).next().unwrap_or(name);
+        let normalized = name.replace('-', "_");
+        if normalized.ends_with("api_key")
+            || normalized.ends_with("token")
+            || normalized.ends_with("secret")
+            || normalized.ends_with("password")
+            || normalized.ends_with("passwd")
+        {
+            return "redacted-credential".to_string();
+        }
+    }
+
+    // Common credential token prefixes. These are intentionally checked at
+    // word boundaries so model ids containing an incidental "sk" survive.
+    let credential_prefix = lower
+        .split(|ch: char| ch.is_whitespace() || matches!(ch, '=' | ':' | ',' | ';' | '"' | '\''))
+        .filter(|part| !part.is_empty())
+        .any(|part| {
+            [
+                "sk-",
+                "sk_",
+                "rk-",
+                "pk-",
+                "ghp_",
+                "gho_",
+                "ghu_",
+                "ghs_",
+                "github_pat_",
+                "hf_",
+                "glpat-",
+                "xoxb-",
+                "xoxp-",
+                "xoxa-",
+                "akia",
+                "aiza",
+                "eyj",
+            ]
+            .iter()
+            .any(|prefix| part.starts_with(prefix))
+        });
+    if credential_prefix {
+        return "redacted-credential".to_string();
+    }
+
+    let windows_absolute = value.as_bytes().get(1) == Some(&b':')
+        && value
+            .as_bytes()
+            .get(2)
+            .is_some_and(|separator| matches!(separator, b'/' | b'\\'));
+    let contains_local_root = [
+        "/users/",
+        "/volumes/",
+        "/home/",
+        "/private/",
+        "\\users\\",
+        "file://",
+        "/.ssh/",
+        "\\.ssh\\",
+    ]
+    .iter()
+    .any(|needle| lower.contains(needle));
+    let looks_like_relative_path = value.contains('\\')
+        || lower.starts_with(".ssh/")
+        || lower.starts_with(".ssh\\")
+        || lower.split('/').any(|segment| {
+            matches!(
+                segment,
+                "." | ".."
+                    | ".ssh"
+                    | ".config"
+                    | "secrets"
+                    | "secret"
+                    | "credentials"
+                    | "credential"
+                    | "relative"
+                    | "workspace"
+                    | "tmp"
+            )
+        });
+    if std::path::Path::new(value).is_absolute()
+        || windows_absolute
+        || value.starts_with("~/")
+        || value.starts_with("./")
+        || value.starts_with("../")
+        || contains_local_root
+        || looks_like_relative_path
+    {
+        return "redacted-local-path".to_string();
+    }
+    let bounded: String = value
+        .chars()
+        .filter(|ch| !ch.is_control())
+        .take(MAX_PERSISTED_ROUTE_LABEL_CHARS)
+        .collect();
+    if bounded.is_empty() {
+        "-".to_string()
+    } else {
+        bounded
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -569,163 +729,3 @@ mod tests {
         assert!(!safe.contains('\n'));
     }
 }
-
-const MAX_RECEIPT_FIELD_CHARS: usize = 96;
-
-pub fn safe_receipt_field(raw: &str) -> String {
-    let sanitized = sanitize_persisted_route_label(raw);
-    let mut out = String::with_capacity(raw.len().min(MAX_RECEIPT_FIELD_CHARS));
-    let mut previous_separator = false;
-    for ch in sanitized.chars() {
-        if out.chars().count() >= MAX_RECEIPT_FIELD_CHARS {
-            break;
-        }
-        let safe = if ch.is_alphanumeric() || matches!(ch, '.' | '_' | '-' | '/' | ':' | '+') {
-            ch
-        } else {
-            '_'
-        };
-        let separator = safe == '_';
-        if separator && previous_separator {
-            continue;
-        }
-        out.push(safe);
-        previous_separator = separator;
-    }
-    if out.is_empty() { "-".to_string() } else { out }
-}
-
-pub fn sanitize_persisted_route_label(raw: &str) -> String {
-    const MAX_PERSISTED_ROUTE_LABEL_CHARS: usize = 256;
-    let value = raw.trim();
-    let lower = value.to_ascii_lowercase();
-
-    if value.is_empty() {
-        return "-".to_string();
-    }
-
-    // URLs are not route labels. Endpoints have a dedicated, validated hash
-    // field; persisting a URL here risks leaking userinfo, query credentials,
-    // or fragments through a custom provider/model name.
-    if value.contains("://") {
-        return "redacted-url".to_string();
-    }
-
-    let authorization_value = ["bearer ", "basic ", "digest ", "token ", "apikey "]
-        .iter()
-        .any(|scheme| lower.starts_with(scheme))
-        || lower.contains("authorization:")
-        || lower.contains("proxy-authorization:");
-    if authorization_value {
-        return "redacted-credential".to_string();
-    }
-
-    // Reject credential assignments regardless of common casing or separator:
-    // FOO_API_KEY=..., access-token:..., password = ....
-    for (index, ch) in value.char_indices() {
-        if !matches!(ch, '=' | ':') {
-            continue;
-        }
-        let name = lower[..index]
-            .trim()
-            .trim_matches(|ch: char| matches!(ch, '"' | '\'' | '{' | '[' | ','));
-        let name = name.rsplit([' ', ',', ';']).next().unwrap_or(name);
-        let normalized = name.replace('-', "_");
-        if normalized.ends_with("api_key")
-            || normalized.ends_with("token")
-            || normalized.ends_with("secret")
-            || normalized.ends_with("password")
-            || normalized.ends_with("passwd")
-        {
-            return "redacted-credential".to_string();
-        }
-    }
-
-    // Common credential token prefixes. These are intentionally checked at
-    // word boundaries so model ids containing an incidental "sk" survive.
-    let credential_prefix = lower
-        .split(|ch: char| ch.is_whitespace() || matches!(ch, '=' | ':' | ',' | ';' | '"' | '\''))
-        .filter(|part| !part.is_empty())
-        .any(|part| {
-            [
-                "sk-",
-                "sk_",
-                "rk-",
-                "pk-",
-                "ghp_",
-                "gho_",
-                "ghu_",
-                "ghs_",
-                "github_pat_",
-                "hf_",
-                "glpat-",
-                "xoxb-",
-                "xoxp-",
-                "xoxa-",
-                "akia",
-                "aiza",
-                "eyj",
-            ]
-            .iter()
-            .any(|prefix| part.starts_with(prefix))
-        });
-    if credential_prefix {
-        return "redacted-credential".to_string();
-    }
-
-    let windows_absolute = value.as_bytes().get(1) == Some(&b':')
-        && value
-            .as_bytes()
-            .get(2)
-            .is_some_and(|separator| matches!(separator, b'/' | b'\\'));
-    let contains_local_root = [
-        "/users/",
-        "/volumes/",
-        "/home/",
-        "/private/",
-        "\\users\\",
-        "file://",
-        "/.ssh/",
-        "\\.ssh\\",
-    ]
-    .iter()
-    .any(|needle| lower.contains(needle));
-    let looks_like_relative_path = value.contains('\\')
-        || lower.starts_with(".ssh/")
-        || lower.starts_with(".ssh\\")
-        || lower.split('/').any(|segment| {
-            matches!(
-                segment,
-                "." | ".."
-                    | ".ssh"
-                    | ".config"
-                    | "secrets"
-                    | "secret"
-                    | "credentials"
-                    | "credential"
-                    | "relative"
-                    | "workspace"
-                    | "tmp"
-            )
-        });
-    if std::path::Path::new(value).is_absolute()
-        || windows_absolute
-        || value.starts_with("~/")
-        || value.starts_with("./")
-        || value.starts_with("../")
-        || contains_local_root
-        || looks_like_relative_path
-    {
-        return "redacted-local-path".to_string();
-    }
-    let bounded: String = value
-        .chars()
-        .filter(|ch| !ch.is_control())
-        .take(MAX_PERSISTED_ROUTE_LABEL_CHARS)
-        .collect();
-    if bounded.is_empty() {
-        "-".to_string()
-    } else {
-        bounded
-    }
-}

crates/runtime/src/test_support.rs

@@ -493,6 +493,15 @@ pub fn read_shell_sentinel(path: &Path) -> String {
         .to_string()
 }
 
+/// Build a syntactically valid, non-secret JWT fixture without embedding a
+/// high-entropy token-shaped literal in Git history.
+pub fn future_test_jwt(label: &str) -> String {
+    use base64::Engine as _;
+
+    let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(br#"{"exp":9999999999}"#);
+    format!("test.{payload}.{label}")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -550,12 +559,3 @@ mod tests {
         );
     }
 }
-
-/// Build a syntactically valid, non-secret JWT fixture without embedding a
-/// high-entropy token-shaped literal in Git history.
-pub fn future_test_jwt(label: &str) -> String {
-    use base64::Engine as _;
-
-    let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(br#"{"exp":9999999999}"#);
-    format!("test.{payload}.{label}")
-}

crates/sandbox/src/read_guard.rs

@@ -425,7 +425,7 @@ pub fn with_test_active_denylist<R>(list: ReadDenylist, callback: impl FnOnce()
             *ACTIVE.write().unwrap() = self.0.take();
         }
     }
-    let previous = std::mem::replace(&mut *ACTIVE.write().unwrap(), Some(Arc::new(list)));
+    let previous = (*ACTIVE.write().unwrap()).replace(Arc::new(list));
     let _restore = RestoreActive(previous);
     callback()
 }

crates/tui/src/client.rs

@@ -111,7 +111,7 @@ pub(crate) struct RemoteControlInferencePermit {
 }
 
 #[cfg(any(test, all(feature = "test-thread-scoped-state", debug_assertions)))]
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(all(not(test), debug_assertions), expect(dead_code))]
 pub(crate) async fn acquire_runtime_chat_inference_ownership() -> RuntimeChatInferenceOwnership {
     let gate = runtime_chat_inference_gate().write_owned().await;
     RuntimeChatInferenceOwnership { _gate: gate }

crates/tui/src/client/chat.rs

@@ -1429,7 +1429,7 @@ impl CodewhaleClient {
                 // sanitizer, dump the offending indices so we can diagnose where
                 // they came from on the next failure.
                 if format!("{error:#}").contains("reasoning_content") {
-                    log_thinking_mode_violations(&body);
+                    log_thinking_mode_violations(body);
                 }
                 return Err(error);
             }

crates/tui/src/client/ollama_catalog.rs

@@ -5,7 +5,10 @@ use codewhale_config::catalog::{
 };
 use serde::Deserialize;
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 pub(crate) const TAGS_PROBE_TIMEOUT: Duration = Duration::from_secs(2);
 
 /// Upper bound on `/api/show` lookups per probe. A developer box can hold
@@ -208,7 +211,10 @@ pub(crate) fn ollama_catalog_delta(
     }
 }
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 pub(crate) fn probe_client() -> anyhow::Result<reqwest::Client> {
     // The first-run probe can run before any provider client has installed
     // the rustls crypto provider; the shared builder installs it (the bare
@@ -218,7 +224,10 @@ pub(crate) fn probe_client() -> anyhow::Result<reqwest::Client> {
         .build()?)
 }
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 pub(crate) async fn fetch_text(url: &str) -> anyhow::Result<String> {
     let response = probe_client()?.get(url).send().await?;
     if !response.status().is_success() {
@@ -229,7 +238,10 @@ pub(crate) async fn fetch_text(url: &str) -> anyhow::Result<String> {
         .map_err(|reason| anyhow::anyhow!("Ollama catalog response rejected: {reason:?}"))
 }
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 pub(crate) async fn fetch_tag_profile(origin: &str, tag: &str) -> anyhow::Result<OllamaTagProfile> {
     let response = probe_client()?
         .post(format!("{origin}/api/show"))

crates/tui/src/core/engine.rs

@@ -3802,7 +3802,7 @@ impl Engine {
                                 controls.active = control.clone();
                                 control
                             };
-                            Box::pin(self.handle_message_operation(turn, Some(request))).await;
+                            Box::pin(self.handle_message_operation(*turn, Some(request))).await;
                         }
                         Op::SendMessage(spec) => {
                             self.admitted_turn_control = {

crates/tui/src/core/engine/tests/test_cases_05.rs

@@ -1152,7 +1152,7 @@ async fn user_steer_during_parent_answer_still_gets_a_reply_with_healthy_childre
         .expect("steering must preempt an indefinitely silent request");
         assert_eq!(status, TurnOutcomeStatus::Completed, "{phase}: {error:?}");
         assert_eq!(client.calls.load(Ordering::SeqCst), 3, "{phase}");
-        let requests = client.requests.lock().unwrap();
+        let requests = client.requests.lock().unwrap().clone();
         assert!(requests[1].messages.iter().any(|message| message.content.iter().any(|block| {
             matches!(block, ContentBlock::Text { text, .. } if text.contains("Also read state.txt and include its evidence."))
         })), "{phase}");
@@ -1179,7 +1179,6 @@ async fn user_steer_during_parent_answer_still_gets_a_reply_with_healthy_childre
             requests[0].system, requests[1].system,
             "{phase}: pinned system prefix changed"
         );
-        drop(requests);
         let mut events = handle.rx_event.write().await;
         let events = std::iter::from_fn(|| events.try_recv().ok()).collect::<Vec<_>>();
         assert!(events.iter().all(|event| !matches!(event, Event::ToolCallStarted { model_call: Some(call), .. } if call.provider_id == "call-must-not-run")), "{phase}");

crates/tui/src/core/ops.rs

@@ -240,7 +240,7 @@ pub struct TurnSpec {
 #[derive(Debug)]
 pub enum Op {
     Review {
-        turn: TurnSpec,
+        turn: Box<TurnSpec>,
         request: codewhale_protocol::op::ReviewOperation,
     },
     /// Send a message to the AI

crates/tui/src/core/protocol_parity.rs

@@ -1103,7 +1103,7 @@ pub fn event_to_protocol(event: &Event, ids: &ProtocolIds) -> wire::EventMsg {
 pub fn op_to_protocol(op: &Op) -> wire_op::Op {
     match op {
         Op::Review { turn, request } => wire_op::Op::Review {
-            turn: turn_spec_to_wire(turn),
+            turn: Box::new(turn_spec_to_wire(turn)),
             request: request.clone(),
         },
         Op::SendMessage(spec) => wire_op::Op::SendMessage(turn_spec_to_wire(spec)),

crates/tui/src/extension_host/mcp_tests.rs

@@ -91,7 +91,7 @@ fn request(
     };
     let frame = json!({"jsonrpc":"2.0","id":id,"method":"proc/write","params":params});
     match parse_host_message(frame, HostTier::Builtin).unwrap() {
-        HostMessage::Request { request, .. } => request,
+        HostMessage::Request { request, .. } => *request,
         _ => panic!("decoded process write must be a request"),
     }
 }

crates/tui/src/extension_host/protocol.rs

@@ -782,7 +782,7 @@ pub enum HostNotification {
 pub enum HostMessage {
     Request {
         id: u64,
-        request: HostRequest,
+        request: Box<HostRequest>,
     },
     Notification(HostNotification),
     Response {
@@ -864,64 +864,64 @@ pub fn parse_host_message(value: Value, tier: HostTier) -> Result<HostMessage, P
                 .map_err(|reason| perr(format!("{method}: {reason}")))?;
             HostMessage::Request {
                 id,
-                request: HostRequest::Register(register),
+                request: Box::new(HostRequest::Register(register)),
             }
         }
         ("registry/unregister", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::Unregister(params(&method, p)?),
+            request: Box::new(HostRequest::Unregister(params(&method, p)?)),
         },
         ("user-questions/ask", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::UserQuestion(params(&method, p)?),
+            request: Box::new(HostRequest::UserQuestion(params(&method, p)?)),
         },
         ("session/checkpoint", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::SessionCheckpoint(params(&method, p)?),
+            request: Box::new(HostRequest::SessionCheckpoint(params(&method, p)?)),
         },
         ("core/call", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::CoreCall(params(&method, p)?),
+            request: Box::new(HostRequest::CoreCall(params(&method, p)?)),
         },
         ("net/start", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::NetStart(params(&method, p)?),
+            request: Box::new(HostRequest::NetStart(params(&method, p)?)),
         },
         ("net/fetch", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::NetFetch(params(&method, p)?),
+            request: Box::new(HostRequest::NetFetch(params(&method, p)?)),
         },
         ("net/read", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::NetRead(params(&method, p)?),
+            request: Box::new(HostRequest::NetRead(params(&method, p)?)),
         },
         ("net/release", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::NetRelease(params(&method, p)?),
+            request: Box::new(HostRequest::NetRelease(params(&method, p)?)),
         },
         ("net/close", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::NetClose(params(&method, p)?),
+            request: Box::new(HostRequest::NetClose(params(&method, p)?)),
         },
         ("proc/close", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::ProcClose(params(&method, p)?),
+            request: Box::new(HostRequest::ProcClose(params(&method, p)?)),
         },
         ("proc/write", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::ProcWrite(params(&method, p)?),
+            request: Box::new(HostRequest::ProcWrite(params(&method, p)?)),
         },
         ("proc/read", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::ProcRead(params(&method, p)?),
+            request: Box::new(HostRequest::ProcRead(params(&method, p)?)),
         },
         ("exec/redeem", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::ExecutionRedeem(params(&method, p)?),
+            request: Box::new(HostRequest::ExecutionRedeem(params(&method, p)?)),
         },
         ("proc/launch", Some(id)) => HostMessage::Request {
             id,
-            request: HostRequest::ProcLaunch(params(&method, p)?),
+            request: Box::new(HostRequest::ProcLaunch(params(&method, p)?)),
         },
         ("host/hello", None) => {
             let hello: HelloParams = params(&method, p)?;
@@ -974,7 +974,7 @@ impl HostMessage {
     #[must_use]
     pub fn to_value(&self) -> Value {
         match self {
-            Self::Request { id, request } => match request {
+            Self::Request { id, request } => match request.as_ref() {
                 HostRequest::Register(p) => request_value(*id, "registry/register", to_value(p)),
                 HostRequest::Unregister(p) => {
                     request_value(*id, "registry/unregister", to_value(p))

crates/tui/src/extension_host/protocol/tests.rs

@@ -808,11 +808,10 @@ fn production_methods_follow_their_tier_rows_and_reserved_families_stay_builtin_
         for tier in HostTier::ALL {
             let parsed = parse_host_message(frame.clone(), tier).unwrap();
             assert_eq!(parsed.to_value(), frame);
-            let HostMessage::Request {
-                request: HostRequest::UserQuestion(params),
-                ..
-            } = parsed
-            else {
+            let HostMessage::Request { request, .. } = parsed else {
+                panic!("wrong question variant")
+            };
+            let HostRequest::UserQuestion(params) = *request else {
                 panic!("wrong question variant")
             };
             assert_eq!(params.questions[0].options.len(), count);

crates/tui/src/extension_host/supervisor.rs

@@ -165,6 +165,7 @@ pub(crate) struct HostLaunch {
     /// build pins ([`tier::BUILTIN_MODULES`]), which the host bundle embedded
     /// from the same build must agree with ([`check_hello_identity`]).
     pub builtin_modules: &'static [BuiltinModule],
+    #[cfg(not(windows))]
     pub native_snapshot_roots: Vec<PathBuf>,
     /// Exact verified Native LPAC plan; Builtin retains its labelled exception.
     #[cfg(windows)]
@@ -551,7 +552,6 @@ pub(crate) fn plan_launch(
             memory_cap,
             memory: MemoryEnforcement::JobObject,
             builtin_modules: tier::BUILTIN_MODULES,
-            native_snapshot_roots: Vec::new(),
             windows: Some(sandbox),
         });
     }
@@ -569,8 +569,12 @@ pub(crate) fn plan_launch(
         bundle,
         &snapshot_roots,
     );
-    let mut launch = host_launch(tier, runtime, args, data, memory_cap, wrapped)?;
-    launch.native_snapshot_roots = snapshot_roots;
+    let launch = host_launch(tier, runtime, args, data, memory_cap, wrapped)?;
+    #[cfg(not(windows))]
+    let launch = HostLaunch {
+        native_snapshot_roots: snapshot_roots,
+        ..launch
+    };
     Ok(launch)
 }
 
@@ -810,15 +814,15 @@ fn native_readable_paths(
             return Err("Native writable data overlaps a runtime or package read grant".into());
         }
         canonical.push(resolved);
-        if path.is_file() {
-            if let (Some(parent), Some(name)) = (path.parent(), path.file_name()) {
-                canonical.push(
-                    parent
-                        .canonicalize()
-                        .map_err(|error| error.to_string())?
-                        .join(name),
-                );
-            }
+        if path.is_file()
+            && let (Some(parent), Some(name)) = (path.parent(), path.file_name())
+        {
+            canonical.push(
+                parent
+                    .canonicalize()
+                    .map_err(|error| error.to_string())?
+                    .join(name),
+            );
         }
     }
     paths.extend(canonical);
@@ -1095,6 +1099,7 @@ fn host_launch(
         memory_cap,
         memory: MemoryEnforcement::planned(runtime.kind),
         builtin_modules: tier::BUILTIN_MODULES,
+        #[cfg(not(windows))]
         native_snapshot_roots: Vec::new(),
         #[cfg(windows)]
         windows: None,
@@ -1556,7 +1561,7 @@ pub(crate) struct HostProcess {
     /// How the cap is enforced for this process, settled at the handshake.
     memory: Arc<std::sync::OnceLock<MemoryEnforcement>>,
     pub sandbox: HostSandbox,
-    #[cfg_attr(windows, expect(dead_code))]
+    #[cfg(not(windows))]
     native_snapshot_roots: Vec<PathBuf>,
     tree: Arc<codewhale_runtime::process_tree::ProcessTree>,
     #[cfg(windows)]
@@ -1732,7 +1737,7 @@ impl HostProcess {
                 if !snapshot_roots.iter().any(|grant| canonical.starts_with(grant) && canonical != *grant) {
                     return Err("Native snapshot store was not granted to this host; restart Codewhale to admit this store".into());
                 }
-                let current = native_snapshot_roots(&[authority.state_path.clone()])?;
+                let current = native_snapshot_roots(std::slice::from_ref(&authority.state_path))?;
                 if !current.iter().any(|grant| snapshot_roots.contains(grant) && canonical.starts_with(grant)) {
                     return Err("Native snapshot store changed after host launch".into());
                 }
@@ -1941,6 +1946,7 @@ impl HostProcess {
             memory_cap: launch.memory_cap,
             memory,
             sandbox: launch.sandbox.clone(),
+            #[cfg(not(windows))]
             native_snapshot_roots: launch.native_snapshot_roots.clone(),
             tree,
             #[cfg(windows)]
@@ -2396,7 +2402,7 @@ impl Reader {
                 };
                 let _ = call.tx.send(result);
             }
-            HostMessage::Request { id, request } => self.start_host_request(id, request)?,
+            HostMessage::Request { id, request } => self.start_host_request(id, *request)?,
             HostMessage::Notification(notification) => match notification {
                 HostNotification::Hello(hello) => {
                     if let Some(tx) = self.handshake.lock().expect("handshake lock").hello.take() {
@@ -3012,7 +3018,7 @@ mod tests {
     fn request(rig: &Rig, id: u64, plugin: &str) -> Result<(), String> {
         rig.reader.handle(HostMessage::Request {
             id,
-            request: register_request(plugin),
+            request: Box::new(register_request(plugin)),
         })
     }
 
@@ -3060,10 +3066,10 @@ mod tests {
         rig.reader
             .handle(HostMessage::Request {
                 id: 6,
-                request: HostRequest::Unregister(protocol::UnregisterParams {
+                request: Box::new(HostRequest::Unregister(protocol::UnregisterParams {
                     owner: owner("p"),
                     handle: 9,
-                }),
+                })),
             })
             .unwrap();
         assert_eq!(

crates/tui/src/hooks/config.rs

@@ -653,7 +653,7 @@ condition = { type = "exit_code", code = 1 }
             problem.event.is_none(),
             "the override is not one hook's problem: {problem:?}"
         );
-        assert!(problem_summary(&problem).contains("`[hooks]` setting"));
+        assert!(problem_summary(problem).contains("`[hooks]` setting"));
 
         // Even unvalidated, the accessors refuse the value rather than hand a
         // zero budget to the executor.

crates/tui/src/lib.rs

@@ -1553,6 +1553,48 @@ fn validate_serve_mode_selection(
     Ok(http_selected)
 }
 
+#[derive(Args, Debug, Clone)]
+struct McpAddArgs {
+    /// Server name
+    name: String,
+    /// Command to launch stdio server
+    #[arg(long, conflicts_with = "url")]
+    command: Option<String>,
+    /// URL for streamable HTTP/SSE server
+    #[arg(long, conflicts_with = "command")]
+    url: Option<String>,
+    /// Explicit URL transport override. Use "sse" for legacy SSE endpoints.
+    #[arg(long, requires = "url")]
+    transport: Option<String>,
+    /// Environment variable containing a bearer token for URL-based servers
+    #[arg(long, requires = "url")]
+    bearer_token_env_var: Option<String>,
+    /// Pre-registered OAuth client ID, bound to --oauth-issuer
+    #[arg(long, requires_all = ["url", "oauth_issuer"])]
+    oauth_client_id: Option<String>,
+    #[arg(
+        long,
+        requires = "url",
+        help = "Exact authorization issuer bound to this OAuth client"
+    )]
+    oauth_issuer: Option<String>,
+    #[arg(
+        long,
+        requires = "url",
+        help = "HTTPS URL of a hosted OAuth client metadata document"
+    )]
+    oauth_client_metadata_url: Option<String>,
+    /// Exact OAuth resource pin checked against server discovery
+    #[arg(long, requires = "url")]
+    oauth_resource: Option<String>,
+    /// OAuth scope to request during login. Repeat or comma-separate.
+    #[arg(long = "scope", requires = "url", value_delimiter = ',')]
+    scopes: Vec<String>,
+    /// Arguments for command-based servers
+    #[arg(long = "arg", allow_hyphen_values = true)]
+    args: Vec<String>,
+}
+
 #[derive(Subcommand, Debug, Clone)]
 enum McpCommand {
     /// List configured MCP servers
@@ -1577,46 +1619,7 @@ enum McpCommand {
         server: Option<String>,
     },
     /// Add an MCP server entry
-    Add {
-        /// Server name
-        name: String,
-        /// Command to launch stdio server
-        #[arg(long, conflicts_with = "url")]
-        command: Option<String>,
-        /// URL for streamable HTTP/SSE server
-        #[arg(long, conflicts_with = "command")]
-        url: Option<String>,
-        /// Explicit URL transport override. Use "sse" for legacy SSE endpoints.
-        #[arg(long, requires = "url")]
-        transport: Option<String>,
-        /// Environment variable containing a bearer token for URL-based servers
-        #[arg(long, requires = "url")]
-        bearer_token_env_var: Option<String>,
-        /// Pre-registered OAuth client ID, bound to --oauth-issuer
-        #[arg(long, requires_all = ["url", "oauth_issuer"])]
-        oauth_client_id: Option<String>,
-        #[arg(
-            long,
-            requires = "url",
-            help = "Exact authorization issuer bound to this OAuth client"
-        )]
-        oauth_issuer: Option<String>,
-        #[arg(
-            long,
-            requires = "url",
-            help = "HTTPS URL of a hosted OAuth client metadata document"
-        )]
-        oauth_client_metadata_url: Option<String>,
-        /// Exact OAuth resource pin checked against server discovery
-        #[arg(long, requires = "url")]
-        oauth_resource: Option<String>,
-        /// OAuth scope to request during login. Repeat or comma-separate.
-        #[arg(long = "scope", requires = "url", value_delimiter = ',')]
-        scopes: Vec<String>,
-        /// Arguments for command-based servers
-        #[arg(long = "arg", allow_hyphen_values = true)]
-        args: Vec<String>,
-    },
+    Add(Box<McpAddArgs>),
     /// Authenticate to a URL-based MCP server using OAuth
     Login {
         /// Server name
@@ -12224,19 +12227,20 @@ async fn run_mcp_command(
             }
             Ok(())
         }
-        McpCommand::Add {
-            name,
-            command,
-            url,
-            transport,
-            bearer_token_env_var,
-            oauth_client_id,
-            oauth_issuer,
-            oauth_client_metadata_url,
-            oauth_resource,
-            scopes,
-            args,
-        } => {
+        McpCommand::Add(args) => {
+            let McpAddArgs {
+                name,
+                command,
+                url,
+                transport,
+                bearer_token_env_var,
+                oauth_client_id,
+                oauth_issuer,
+                oauth_client_metadata_url,
+                oauth_resource,
+                scopes,
+                args,
+            } = *args;
             if command.is_none() && url.is_none() {
                 bail!("Provide either --command or --url for `mcp add`.");
             }
@@ -20932,10 +20936,10 @@ mod mcp_add_arg_tests {
         let Some(Commands::Mcp { command }) = cli.command else {
             panic!("expected mcp command");
         };
-        let McpCommand::Add { args, .. } = command else {
+        let McpCommand::Add(args) = command else {
             panic!("expected mcp add subcommand");
         };
-        assert_eq!(args, vec!["-y".to_string()]);
+        assert_eq!(args.args, vec!["-y".to_string()]);
     }
 }
 

crates/tui/src/local_ollama.rs

@@ -59,7 +59,10 @@ pub(crate) fn should_adopt_live_local_ollama(
 }
 
 /// Resolve the OpenAI-compat Ollama base URL (`…/v1`) from config defaults.
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 pub(crate) fn ollama_v1_base_url(config: &Config) -> String {
     config
         .resolve_provider_pin_identity("ollama")
@@ -69,7 +72,10 @@ pub(crate) fn ollama_v1_base_url(config: &Config) -> String {
         .unwrap_or_default()
 }
 
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 fn record_ollama_tags_into_lake(
     endpoint_v1: &str,
     tags: &[String],
@@ -100,7 +106,10 @@ fn record_ollama_tags_into_lake(
 /// Probe local Ollama for a live catalog. Prefers native `/api/tags`, falls
 /// back to OpenAI-compat `/v1/models`. Returns `None` when nothing useful
 /// answered — never invents a tag.
-#[cfg_attr(windows, allow(dead_code))]
+#[cfg_attr(
+    all(not(test), feature = "test-thread-scoped-state", debug_assertions),
+    expect(dead_code)
+)]
 pub(crate) async fn probe_live_local_ollama_catalog(
     config: &Config,
 ) -> Option<LiveLocalOllamaCatalog> {

crates/tui/src/project_context.rs

@@ -1902,8 +1902,7 @@ fn load_rules_from_dir(
 }
 
 /// User-owned global instructions may intentionally resolve through links.
-#[cfg(test)]
-#[cfg_attr(windows, expect(dead_code))]
+#[cfg(all(test, unix))]
 fn load_global_context_file(path: &Path) -> Result<String, ProjectContextError> {
     load_global_context_file_limited(path, DEFAULT_FILE_MAX_BYTES)
 }

crates/tui/src/prompts.rs

@@ -1556,6 +1556,15 @@ pub(crate) fn measure_static_prefix(system: Option<&SystemPrompt>, tools: &[Tool
 
 const GUARD_PASSES_PER_SCOPE: usize = 2;
 
+type GuardInstructionResult = Result<
+    (
+        Option<SystemPrompt>,
+        Option<crate::project_context::InstructionAssembly>,
+        Vec<crate::project_context::InstructionNotice>,
+    ),
+    crate::project_context::InstructionRefusal,
+>;
+
 fn replace_installed_instruction_block(
     prompt: &mut Option<SystemPrompt>,
     raw: &crate::project_context::ProjectContext,
@@ -1598,14 +1607,7 @@ pub(crate) fn guard_instruction_prefix(
     mut assembly: Option<crate::project_context::InstructionAssembly>,
     budgets: crate::project_context::InstructionBudgets,
     tools: &[Tool],
-) -> Result<
-    (
-        Option<SystemPrompt>,
-        Option<crate::project_context::InstructionAssembly>,
-        Vec<crate::project_context::InstructionNotice>,
-    ),
-    crate::project_context::InstructionRefusal,
-> {
+) -> GuardInstructionResult {
     use crate::project_context::{
         InstructionAction, InstructionBudgetPolicy, InstructionLimit, InstructionNotice,
         InstructionRefusal, InstructionScope,

crates/tui/src/route_preferences.rs

@@ -524,6 +524,195 @@ pub fn unset(path: &Path, key: &str) -> Result<()> {
     })
 }
 
+pub(crate) fn set_provider_model_document(
+    doc: &mut toml_edit::DocumentMut,
+    identity: &ProviderIdentity,
+    model: &str,
+) -> anyhow::Result<()> {
+    let mut config =
+        codewhale_runtime::config::parse_config_base(&doc.to_string()).map_err(|_| {
+            anyhow::anyhow!("Could not parse destination route identity; contents omitted")
+        })?;
+    crate::plugins::providers::apply_startup_providers(&mut config)?;
+    persistence::set_verified_provider_model_document(doc, &config, identity, model)
+}
+
+/// One persistent owner and atomic write for an explicitly saved route.
+///
+/// Also returns the undo for exactly this write, taken under the config lock,
+/// so a caller whose follow-up apply step is rejected can take back only the
+/// switch.
+pub(crate) fn persist_provider_selection(
+    config_path: Option<&Path>,
+    identity: &ProviderIdentity,
+    model: Option<&str>,
+) -> anyhow::Result<(std::path::PathBuf, codewhale_config::ConfigDocumentUndo)> {
+    let path = persistence::config_toml_path(config_path)?;
+    let ((), undo) =
+        codewhale_config::mutate_config_document_undoable_with_migration(&path, |doc, moved| {
+            persistence::migrate_legacy_route_preferences(&path, doc)?;
+            let mut config = codewhale_runtime::config::parse_config_after_locked_migration(
+                &doc.to_string(),
+                moved,
+            )
+            .map_err(|_| anyhow::anyhow!("Could not parse destination route; contents omitted"))?;
+            crate::plugins::providers::apply_startup_providers(&mut config)?;
+            config
+                .verify_provider_identity(identity)
+                .map_err(anyhow::Error::msg)?;
+            if let Some(model) = model {
+                persistence::set_verified_provider_model_document(doc, &config, identity, model)?;
+            }
+            persistence::set_document_value(
+                doc,
+                &["provider"],
+                identity.persisted_id().unwrap_or(identity.key.as_str()),
+            )?;
+            reconcile_root_model_aliases(doc, &config, identity)
+        })?;
+    Ok((path, undo))
+}
+
+/// Keep a root `default_text_model` alias from stranding a route switch,
+/// without discarding the choice it holds.
+///
+/// The root alias is the *active* route's fallback: `Config` resolves it only
+/// when the selected route has no model of its own. A switch that leaves the
+/// incoming route without a leaf therefore hands it an alias naming the route
+/// on its way out, and `Config::load` rejects the file the caller just wrote —
+/// a committed switch that produces an unloadable config — or, on a
+/// pass-through incoming route that loads it, a model the incoming route never
+/// chose while the outgoing route comes back on its catalog default.
+///
+/// When the outgoing route has no leaf and was resolving the alias, relocate
+/// that value onto the outgoing route's own canonical leaf, because it is real
+/// saved state, and only then clear the alias. Any other alias the incoming
+/// route already shadows with its own leaf is inert and stays: deleting a
+/// saved choice to satisfy validation of a value nothing resolves is data
+/// loss, not a repair. Likewise an unnamed custom route stores its
+/// model in the alias itself and has no leaf to receive it. If the incoming
+/// route cannot shadow that value, refuse the switch atomically and ask for an
+/// explicit destination model instead of saving an unloadable configuration.
+///
+/// `previous` is the document's configuration before the switch; `incoming` is
+/// the identity now selected. Every route writer calls this, so no writer can
+/// keep a private rule about which route owns the root alias.
+pub(crate) fn reconcile_root_model_aliases(
+    doc: &mut toml_edit::DocumentMut,
+    previous: &codewhale_runtime::config::Config,
+    incoming: &codewhale_runtime::config::ProviderIdentity,
+) -> anyhow::Result<()> {
+    // A freshly admitted id-less root keeps its legacy alias as a receipt.
+    // Its effective model is written to the canonical leaf created by this
+    // same-lock migration; the alias is never another route's saved choice.
+    if incoming.provider == ProviderKind::Custom && incoming.persisted_id().is_none() {
+        return Ok(());
+    }
+    // The outgoing route was resolving the alias as its own model. Move it
+    // onto that route's leaf even when the incoming route could load it: a
+    // pass-through incoming route would otherwise inherit a model it never
+    // chose, and the outgoing route would come back on its catalog default.
+    if let Some((outgoing, value)) = previous.root_model_alias_owned_by_outgoing(incoming) {
+        persistence::set_verified_provider_model_document(doc, previous, &outgoing, &value)?;
+        unset_root_model_aliases(doc)?;
+        return Ok(());
+    }
+    // Below, only `default_text_model` is read. The legacy root `model` key is
+    // never what blocks a load: `Config::default_model` already refuses to
+    // route a foreign legacy value to a provider that cannot serve it, and
+    // `Config::validate` does not consult it, so relocating it would move a
+    // value nothing is asking about. It is still cleared together with the
+    // alias that shadows it (`unset_root_model_aliases`).
+    const ROOT_KEY: &str = "default_text_model";
+    let Some(value) = doc
+        .get(ROOT_KEY)
+        .and_then(toml_edit::Item::as_str)
+        .map(str::to_owned)
+    else {
+        return Ok(());
+    };
+    let mut switched = codewhale_runtime::config::parse_config_base(&doc.to_string())
+        .map_err(|_| anyhow::anyhow!("Could not parse switched route; contents omitted"))?;
+    crate::plugins::providers::apply_startup_providers(&mut switched)?;
+    // `Config::validate` is the single authority on what the incoming route can
+    // serve, so a writer cannot disagree with the loader. Act only when this
+    // alias is what the loader rejects: a document already broken for an
+    // unrelated reason is not this writer's to rewrite.
+    let mut without_alias = switched.clone();
+    without_alias.default_text_model = None;
+    if switched
+        .provider_config_for(incoming)
+        .and_then(|entry| entry.model.as_deref())
+        .is_some()
+        || switched.validate().is_ok()
+        || without_alias.validate().is_err()
+    {
+        return Ok(());
+    }
+    // An empty or control-bearing alias names no saved model. Nothing to
+    // relocate, and clearing it loses nothing.
+    if value.trim().is_empty() || value.chars().any(char::is_control) {
+        unset_root_model_aliases(doc)?;
+        return Ok(());
+    }
+    // No leaf can hold this value. Keep the only copy of the user's choice
+    // rather than discard it for a document the incoming route loads as soon as
+    // it saves a model of its own.
+    let outgoing = previous.active_provider_identity().ok();
+    if outgoing.as_ref().is_some_and(|outgoing| {
+        outgoing != incoming
+            && outgoing.provider == ProviderKind::Custom
+            && outgoing.persisted_id().is_none()
+    }) {
+        // This same-lock canonicalizer copied the root model before moving
+        // its endpoint. Clear an alias only when that exact outgoing leaf
+        // proves the choice survives; otherwise retain the refusal.
+        if outgoing.as_ref().is_some_and(|outgoing| {
+            previous
+                .provider_config_for(outgoing)
+                .and_then(|entry| entry.model.as_deref())
+                == Some(value.as_str())
+        }) {
+            return unset_root_model_aliases(doc);
+        }
+        anyhow::bail!(
+            "Choose a model for the destination provider before switching from a legacy custom connection; the saved configuration was not changed"
+        );
+    }
+    let Some(outgoing) = outgoing.as_ref().filter(|outgoing| *outgoing != incoming) else {
+        return Ok(());
+    };
+    let mut scoped = switched;
+    scoped
+        .scope_to_provider_identity(outgoing)
+        .map_err(anyhow::Error::msg)?;
+    if scoped
+        .provider_config_for(outgoing)
+        .and_then(|entry| entry.model.as_deref())
+        .is_none()
+    {
+        persistence::set_verified_provider_model_document(doc, previous, outgoing, &value)?;
+    }
+    // The outgoing route either already saved its own choice or has just
+    // received this one, so the alias is now a shadowed duplicate that only
+    // blocks the incoming route.
+    unset_root_model_aliases(doc)?;
+    Ok(())
+}
+
+/// Clear the root model alias: `default_text_model` and the legacy root
+/// `model` that `Config::default_model` falls back to when the former is
+/// unset. While `default_text_model` is present the legacy key resolves on no
+/// route, so removing only the alias would resurrect a stale legacy model on
+/// the incoming route (a pass-through route would then use a model nobody
+/// chose for it). When the legacy key *is* the alias, the caller has already
+/// moved its value onto the outgoing route's leaf.
+fn unset_root_model_aliases(doc: &mut toml_edit::DocumentMut) -> anyhow::Result<()> {
+    persistence::unset_document_value(doc, &["default_text_model"])?;
+    persistence::unset_document_value(doc, &["model"])?;
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -899,192 +1088,3 @@ model = "Other-X"
         Ok(())
     }
 }
-
-pub(crate) fn set_provider_model_document(
-    doc: &mut toml_edit::DocumentMut,
-    identity: &ProviderIdentity,
-    model: &str,
-) -> anyhow::Result<()> {
-    let mut config =
-        codewhale_runtime::config::parse_config_base(&doc.to_string()).map_err(|_| {
-            anyhow::anyhow!("Could not parse destination route identity; contents omitted")
-        })?;
-    crate::plugins::providers::apply_startup_providers(&mut config)?;
-    persistence::set_verified_provider_model_document(doc, &config, identity, model)
-}
-
-/// One persistent owner and atomic write for an explicitly saved route.
-///
-/// Also returns the undo for exactly this write, taken under the config lock,
-/// so a caller whose follow-up apply step is rejected can take back only the
-/// switch.
-pub(crate) fn persist_provider_selection(
-    config_path: Option<&Path>,
-    identity: &ProviderIdentity,
-    model: Option<&str>,
-) -> anyhow::Result<(std::path::PathBuf, codewhale_config::ConfigDocumentUndo)> {
-    let path = persistence::config_toml_path(config_path)?;
-    let ((), undo) =
-        codewhale_config::mutate_config_document_undoable_with_migration(&path, |doc, moved| {
-            persistence::migrate_legacy_route_preferences(&path, doc)?;
-            let mut config = codewhale_runtime::config::parse_config_after_locked_migration(
-                &doc.to_string(),
-                moved,
-            )
-            .map_err(|_| anyhow::anyhow!("Could not parse destination route; contents omitted"))?;
-            crate::plugins::providers::apply_startup_providers(&mut config)?;
-            config
-                .verify_provider_identity(identity)
-                .map_err(anyhow::Error::msg)?;
-            if let Some(model) = model {
-                persistence::set_verified_provider_model_document(doc, &config, identity, model)?;
-            }
-            persistence::set_document_value(
-                doc,
-                &["provider"],
-                identity.persisted_id().unwrap_or(identity.key.as_str()),
-            )?;
-            reconcile_root_model_aliases(doc, &config, identity)
-        })?;
-    Ok((path, undo))
-}
-
-/// Keep a root `default_text_model` alias from stranding a route switch,
-/// without discarding the choice it holds.
-///
-/// The root alias is the *active* route's fallback: `Config` resolves it only
-/// when the selected route has no model of its own. A switch that leaves the
-/// incoming route without a leaf therefore hands it an alias naming the route
-/// on its way out, and `Config::load` rejects the file the caller just wrote —
-/// a committed switch that produces an unloadable config — or, on a
-/// pass-through incoming route that loads it, a model the incoming route never
-/// chose while the outgoing route comes back on its catalog default.
-///
-/// When the outgoing route has no leaf and was resolving the alias, relocate
-/// that value onto the outgoing route's own canonical leaf, because it is real
-/// saved state, and only then clear the alias. Any other alias the incoming
-/// route already shadows with its own leaf is inert and stays: deleting a
-/// saved choice to satisfy validation of a value nothing resolves is data
-/// loss, not a repair. Likewise an unnamed custom route stores its
-/// model in the alias itself and has no leaf to receive it. If the incoming
-/// route cannot shadow that value, refuse the switch atomically and ask for an
-/// explicit destination model instead of saving an unloadable configuration.
-///
-/// `previous` is the document's configuration before the switch; `incoming` is
-/// the identity now selected. Every route writer calls this, so no writer can
-/// keep a private rule about which route owns the root alias.
-pub(crate) fn reconcile_root_model_aliases(
-    doc: &mut toml_edit::DocumentMut,
-    previous: &codewhale_runtime::config::Config,
-    incoming: &codewhale_runtime::config::ProviderIdentity,
-) -> anyhow::Result<()> {
-    // A freshly admitted id-less root keeps its legacy alias as a receipt.
-    // Its effective model is written to the canonical leaf created by this
-    // same-lock migration; the alias is never another route's saved choice.
-    if incoming.provider == ProviderKind::Custom && incoming.persisted_id().is_none() {
-        return Ok(());
-    }
-    // The outgoing route was resolving the alias as its own model. Move it
-    // onto that route's leaf even when the incoming route could load it: a
-    // pass-through incoming route would otherwise inherit a model it never
-    // chose, and the outgoing route would come back on its catalog default.
-    if let Some((outgoing, value)) = previous.root_model_alias_owned_by_outgoing(incoming) {
-        persistence::set_verified_provider_model_document(doc, previous, &outgoing, &value)?;
-        unset_root_model_aliases(doc)?;
-        return Ok(());
-    }
-    // Below, only `default_text_model` is read. The legacy root `model` key is
-    // never what blocks a load: `Config::default_model` already refuses to
-    // route a foreign legacy value to a provider that cannot serve it, and
-    // `Config::validate` does not consult it, so relocating it would move a
-    // value nothing is asking about. It is still cleared together with the
-    // alias that shadows it (`unset_root_model_aliases`).
-    const ROOT_KEY: &str = "default_text_model";
-    let Some(value) = doc
-        .get(ROOT_KEY)
-        .and_then(toml_edit::Item::as_str)
-        .map(str::to_owned)
-    else {
-        return Ok(());
-    };
-    let mut switched = codewhale_runtime::config::parse_config_base(&doc.to_string())
-        .map_err(|_| anyhow::anyhow!("Could not parse switched route; contents omitted"))?;
-    crate::plugins::providers::apply_startup_providers(&mut switched)?;
-    // `Config::validate` is the single authority on what the incoming route can
-    // serve, so a writer cannot disagree with the loader. Act only when this
-    // alias is what the loader rejects: a document already broken for an
-    // unrelated reason is not this writer's to rewrite.
-    let mut without_alias = switched.clone();
-    without_alias.default_text_model = None;
-    if switched
-        .provider_config_for(incoming)
-        .and_then(|entry| entry.model.as_deref())
-        .is_some()
-        || switched.validate().is_ok()
-        || without_alias.validate().is_err()
-    {
-        return Ok(());
-    }
-    // An empty or control-bearing alias names no saved model. Nothing to
-    // relocate, and clearing it loses nothing.
-    if value.trim().is_empty() || value.chars().any(char::is_control) {
-        unset_root_model_aliases(doc)?;
-        return Ok(());
-    }
-    // No leaf can hold this value. Keep the only copy of the user's choice
-    // rather than discard it for a document the incoming route loads as soon as
-    // it saves a model of its own.
-    let outgoing = previous.active_provider_identity().ok();
-    if outgoing.as_ref().is_some_and(|outgoing| {
-        outgoing != incoming
-            && outgoing.provider == ProviderKind::Custom
-            && outgoing.persisted_id().is_none()
-    }) {
-        // This same-lock canonicalizer copied the root model before moving
-        // its endpoint. Clear an alias only when that exact outgoing leaf
-        // proves the choice survives; otherwise retain the refusal.
-        if outgoing.as_ref().is_some_and(|outgoing| {
-            previous
-                .provider_config_for(outgoing)
-                .and_then(|entry| entry.model.as_deref())
-                == Some(value.as_str())
-        }) {
-            return unset_root_model_aliases(doc);
-        }
-        anyhow::bail!(
-            "Choose a model for the destination provider before switching from a legacy custom connection; the saved configuration was not changed"
-        );
-    }
-    let Some(outgoing) = outgoing.as_ref().filter(|outgoing| *outgoing != incoming) else {
-        return Ok(());
-    };
-    let mut scoped = switched;
-    scoped
-        .scope_to_provider_identity(outgoing)
-        .map_err(anyhow::Error::msg)?;
-    if scoped
-        .provider_config_for(outgoing)
-        .and_then(|entry| entry.model.as_deref())
-        .is_none()
-    {
-        persistence::set_verified_provider_model_document(doc, previous, outgoing, &value)?;
-    }
-    // The outgoing route either already saved its own choice or has just
-    // received this one, so the alias is now a shadowed duplicate that only
-    // blocks the incoming route.
-    unset_root_model_aliases(doc)?;
-    Ok(())
-}
-
-/// Clear the root model alias: `default_text_model` and the legacy root
-/// `model` that `Config::default_model` falls back to when the former is
-/// unset. While `default_text_model` is present the legacy key resolves on no
-/// route, so removing only the alias would resurrect a stale legacy model on
-/// the incoming route (a pass-through route would then use a model nobody
-/// chose for it). When the legacy key *is* the alias, the caller has already
-/// moved its value onto the outgoing route's leaf.
-fn unset_root_model_aliases(doc: &mut toml_edit::DocumentMut) -> anyhow::Result<()> {
-    persistence::unset_document_value(doc, &["default_text_model"])?;
-    persistence::unset_document_value(doc, &["model"])?;
-    Ok(())
-}

crates/tui/src/runtime_threads.rs

@@ -17152,7 +17152,10 @@ impl RuntimeThreadManager {
             submission_id: None,
         };
         let op = match &input_source {
-            RuntimeTurnInputSource::Review(request) => Op::Review { turn: spec, request: request.clone() },
+            RuntimeTurnInputSource::Review(request) => Op::Review {
+                turn: Box::new(spec),
+                request: request.clone(),
+            },
             _ => Op::SendMessage(spec),
         };
 

crates/tui/src/runtime_threads/tests.rs

@@ -20231,7 +20231,7 @@ fn restart_rebuild_restores_tool_call_identity_from_persisted_items() -> Result<
     let turn = manager.store.load_turn(&checkpoint_turn.id)?;
     let items = manager.prepared_items_for_turns(std::slice::from_ref(&turn))?;
     assert_eq!(
-        manager.history_from_records(&[turn.clone()], &items)?,
+        manager.history_from_records(std::slice::from_ref(&turn), &items)?,
         expected
     );
     first.metadata.as_mut().unwrap()["tool_input"] = json!(r#"{"path":"changed"}"#);

crates/tui/src/tools/user_input.rs

@@ -245,17 +245,16 @@ impl UserInputRequest {
                     ));
                 }
             }
-            if let Some(UserInputIntent::PlanReview { approve, .. }) = &question.intent {
-                if !labels.contains(approve)
+            if let Some(UserInputIntent::PlanReview { approve, .. }) = &question.intent
+                && (!labels.contains(approve)
                     || question
                         .detail
                         .as_ref()
-                        .is_none_or(|detail| detail.trim().is_empty())
-                {
-                    return Err(ToolError::invalid_input(
-                        "Plan review requires detail and an offered approve choice",
-                    ));
-                }
+                        .is_none_or(|detail| detail.trim().is_empty()))
+            {
+                return Err(ToolError::invalid_input(
+                    "Plan review requires detail and an offered approve choice",
+                ));
             }
         }
         Ok(())

crates/util/src/utils.rs

@@ -16,6 +16,7 @@ mod response_body;
 pub use response_body::read_response_body_capped;
 
 /// A writer that counts bytes written without storing them.
+#[derive(Default)]
 pub struct CountingWriter {
     count: usize,
 }