Merge PR #3041 from Hmbown: harvest error-message fixes — better tool denial and provider errors

fix: harvest error-message fixes from PR #2933 — better tool denial + subagent conflict messages

crates/tui/src/core/engine/dispatch.rs

@@ -99,6 +99,15 @@ pub(super) fn caller_allowed_for_tool(
     requested == "direct"
 }
 
+/// Whole-word check for "mode"/"modes" — a plain `contains("mode")` also
+/// matched "model", letting provider model errors skip the actionable-hint
+/// suffix (#3020).
+fn mentions_mode_word(lower: &str) -> bool {
+    lower
+        .split(|ch: char| !ch.is_ascii_alphanumeric())
+        .any(|word| word == "mode" || word == "modes")
+}
+
 pub(super) fn format_tool_error(err: &ToolError, tool_name: &str) -> String {
     match err {
         ToolError::InvalidInput { message } => {
@@ -117,7 +126,16 @@ pub(super) fn format_tool_error(err: &ToolError, tool_name: &str) -> String {
         ),
         ToolError::NotAvailable { message } => {
             let lower = message.to_ascii_lowercase();
-            if lower.contains("current tool catalog") || lower.contains("did you mean:") {
+            // #3020: Pass through self-explanatory messages that already name the
+            // cause (mode switch, allow_shell, feature flag).  Avoids appending a
+            // conflicting "Check mode, feature flags" suffix on top of
+            // "switch to Agent, Goal, or YOLO mode" which confuses the model.
+            if lower.contains("current tool catalog")
+                || lower.contains("did you mean:")
+                || mentions_mode_word(&lower)
+                || lower.contains("allow_shell")
+                || lower.contains("feature flag")
+            {
                 message.clone()
             } else {
                 format!(
@@ -125,9 +143,20 @@ pub(super) fn format_tool_error(err: &ToolError, tool_name: &str) -> String {
                 )
             }
         }
-        ToolError::PermissionDenied { message } => format!(
-            "Tool '{tool_name}' was denied: {message}. Adjust approval mode or request permission."
-        ),
+        ToolError::PermissionDenied { message } => {
+            let lower = message.to_ascii_lowercase();
+            // #3020: Pass through messages that already name the denial cause.
+            if mentions_mode_word(&lower)
+                || lower.contains("allow_shell")
+                || lower.contains("denied by user")
+            {
+                message.clone()
+            } else {
+                format!(
+                    "Tool '{tool_name}' was denied: {message}. Adjust approval mode or request permission."
+                )
+            }
+        }
     }
 }
 

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

@@ -486,6 +486,33 @@ fn tool_error_messages_include_actionable_hints() {
     let timeout = ToolError::Timeout { seconds: 5 };
     let formatted = format_tool_error(&timeout, "exec_shell");
     assert!(formatted.contains("timed out"));
+
+    // #3020: Plan-mode denials already explain the fix — pass through
+    // verbatim, with no conflicting "Adjust approval mode" suffix.
+    let plan_denied = ToolError::permission_denied(
+        "'exec_shell' is not available in Plan mode — switch to Agent, Goal, or YOLO mode to run commands and code.",
+    );
+    let formatted = format_tool_error(&plan_denied, "exec_shell");
+    assert_eq!(
+        formatted,
+        "'exec_shell' is not available in Plan mode — switch to Agent, Goal, or YOLO mode to run commands and code."
+    );
+
+    // Bare denials still get the actionable suffix.
+    let bare_denied = ToolError::permission_denied("nope");
+    let formatted = format_tool_error(&bare_denied, "exec_shell");
+    assert!(
+        formatted.contains("Adjust approval mode or request permission"),
+        "{formatted}"
+    );
+
+    // "model" must not satisfy the "mode" pass-through check.
+    let model_denied = ToolError::permission_denied("requested model is not allowed");
+    let formatted = format_tool_error(&model_denied, "agent_open");
+    assert!(
+        formatted.contains("Adjust approval mode or request permission"),
+        "{formatted}"
+    );
 }
 
 #[test]

crates/tui/src/tools/subagent/mod.rs

@@ -1496,8 +1496,19 @@ impl SubAgentManager {
                 .values()
                 .find(|existing| existing.session_name == name)
             {
+                // #3020: Include elapsed time so the parent can distinguish a
+                // live worker from a stale/failed earlier spawn (#2656).
+                let elapsed = existing.started_at.elapsed();
+                let since = if elapsed.as_secs() < 120 {
+                    format!("{}s ago", elapsed.as_secs())
+                } else {
+                    let mins = elapsed.as_secs() / 60;
+                    let secs = elapsed.as_secs() % 60;
+                    format!("{mins}m{secs}s ago")
+                };
                 return Err(anyhow!(
-                    "Sub-agent session name '{name}' is already in use by agent_id '{}' (status: {}). \
+                    "Sub-agent session name '{name}' is already in use by agent_id '{}' \
+                     (status: {}, started {since}). \
                      Reuse that agent_id with agent_eval/agent_close, or open with a different name.",
                     existing.id,
                     subagent_status_name(&existing.status)
@@ -5619,7 +5630,26 @@ fn annotate_child_model_error(err: &str, model: &str) -> String {
             "{err}\n(child model `{model}` may be unavailable under the current access profile — \
              retry agent_open with a different `model`, or remove `model` to inherit the parent's)"
         ),
-        _ => err.to_string(),
+        _ => {
+            // #3020 (#2653): Provider rejections like "Model Not Exist" or
+            // "does not exist or you do not have access" often classify as
+            // `Internal` rather than `Authorization`/`State`.  Catch these
+            // patterns in the raw error text and annotate anyway.
+            let lower = err.to_ascii_lowercase();
+            if lower.contains("model not exist")
+                || lower.contains("model_not_found")
+                || lower.contains("does not exist")
+                || lower.contains("no such model")
+                || lower.contains("invalid model")
+            {
+                format!(
+                    "{err}\n(child model `{model}` may be unavailable under the current access profile — \
+                     retry agent_open with a different `model`, or remove `model` to inherit the parent's)"
+                )
+            } else {
+                err.to_string()
+            }
+        }
     }
 }
 

crates/tui/src/tools/subagent/tests.rs

@@ -1489,6 +1489,12 @@ async fn spawn_duplicate_session_name_error_names_conflicting_agent() {
         msg.contains("running"),
         "includes the conflicting status: {msg}"
     );
+    // #3020: elapsed time lets the parent distinguish a live worker from a
+    // stale earlier spawn.
+    assert!(
+        msg.contains("started ") && msg.contains(" ago"),
+        "includes elapsed time since spawn: {msg}"
+    );
 }
 
 #[tokio::test]
@@ -2074,6 +2080,23 @@ fn annotate_child_model_error_adds_actionable_hint() {
 
     let unrelated = annotate_child_model_error("connection reset by peer", "kimi-k2");
     assert_eq!(unrelated, "connection reset by peer");
+
+    // #3020: provider rejections that classify as Internal (not
+    // Authorization/State) still get the hint via raw-text matching.
+    let not_exist = annotate_child_model_error("Model Not Exist", "kimi-k2");
+    assert!(
+        not_exist.contains("retry agent_open"),
+        "DeepSeek-style rejection gets the hint: {not_exist}"
+    );
+
+    let openai_style = annotate_child_model_error(
+        "The model `gpt-5.5-nano` does not exist or you do not have access to it.",
+        "gpt-5.5-nano",
+    );
+    assert!(
+        openai_style.contains("retry agent_open"),
+        "OpenAI-style rejection gets the hint: {openai_style}"
+    );
 }
 
 #[test]