diff --git a/crates/ide-agent/src/acp/mod.rs b/crates/ide-agent/src/acp/mod.rs index 46b4143d2..f25a155a5 100644 --- a/crates/ide-agent/src/acp/mod.rs +++ b/crates/ide-agent/src/acp/mod.rs @@ -27,6 +27,29 @@ use std::sync::Arc; use std::sync::atomic::{AtomicU8, Ordering}; use tokio::sync::oneshot; +/// ACP's error code for a request that needs the user to sign in. +pub(crate) const AUTH_REQUIRED: i64 = -32000; + +// Covers signed-out users and refused accounts (Claude Code: "API Error: 403"). +pub(crate) fn auth_required(code: i64, message: &str) -> bool { + if code == AUTH_REQUIRED { + return true; + } + let lower = message.to_lowercase(); + let words: Vec<&str> = lower.split(|c: char| !c.is_alphanumeric()).filter(|w| !w.is_empty()).collect(); + words.iter().any(|w| { + *w == "auth" + || w.starts_with("authenticat") + || w.starts_with("authoriz") + || w.starts_with("unauthenticat") + || w.starts_with("unauthoriz") + || w.starts_with("oauth") + }) || words.windows(3).any(|w| w == ["not", "logged", "in"]) + || words + .windows(2) + .any(|w| w == ["logged", "out"] || w == ["sign", "in"] || w == ["log", "in"] || w == ["login", "required"]) +} + /// The capabilities this client offers, in v1 form. fn client_capabilities() -> Value { json!({ diff --git a/crates/ide-agent/src/acp/tests.rs b/crates/ide-agent/src/acp/tests.rs index 3bdcf02d6..89afd2958 100644 --- a/crates/ide-agent/src/acp/tests.rs +++ b/crates/ide-agent/src/acp/tests.rs @@ -16,6 +16,42 @@ const CANCEL: &str = include_str!("../../tests/fixtures/acp-v2/cancel.jsonl"); const RESUME: &str = include_str!("../../tests/fixtures/acp-v2/resume.jsonl"); const INITIALIZE_V1: &str = include_str!("../../tests/fixtures/acp-v2/initialize-v1.jsonl"); +#[test] +fn auth_required_matches_authentication_words_and_sign_in_phrases() { + for (code, message, expected) in [ + (AUTH_REQUIRED, "This account is not eligible.", true), + (codes::INTERNAL_ERROR, "Auth required", true), + (codes::INTERNAL_ERROR, "Authentication failed", true), + (codes::INTERNAL_ERROR, "Failed to authenticate. API Error: 401", true), + (codes::INTERNAL_ERROR, "Failed to authenticate. API Error: 403", true), + (codes::INTERNAL_ERROR, "Not authenticated", true), + (codes::INTERNAL_ERROR, "Authorization failed", true), + (codes::INTERNAL_ERROR, "Authorize your account", true), + (codes::INTERNAL_ERROR, "Unauthenticated", true), + (codes::INTERNAL_ERROR, "Unauthentication error", true), + (codes::INTERNAL_ERROR, "Unauthorized", true), + (codes::INTERNAL_ERROR, "Unauthorization error", true), + (codes::INTERNAL_ERROR, "OAuth token has expired", true), + (codes::INTERNAL_ERROR, "OAuth2 token has expired", true), + (codes::INTERNAL_ERROR, "You are not logged in", true), + (codes::INTERNAL_ERROR, "You are logged out", true), + (codes::INTERNAL_ERROR, "Please sign in", true), + (codes::INTERNAL_ERROR, "Please log in", true), + (codes::INTERNAL_ERROR, "Login required", true), + (codes::INTERNAL_ERROR, "author", false), + (codes::INTERNAL_ERROR, "authors", false), + (codes::INTERNAL_ERROR, "authority", false), + (codes::INTERNAL_ERROR, "authored", false), + (codes::INTERNAL_ERROR, "reauthenticate", false), + (codes::INTERNAL_ERROR, "preauthorized", false), + (codes::INTERNAL_ERROR, "signal input failed", false), + (codes::INTERNAL_ERROR, "catalog index failed", false), + (codes::INTERNAL_ERROR, "login succeeded", false), + ] { + assert_eq!(auth_required(code, message), expected, "code={code}, message={message:?}"); + } +} + /// The messages of a fixture going `dir` (`in` or `out`). pub(crate) fn frames(fixture: &str, dir: &str) -> Vec { fixture diff --git a/crates/ide-agent/src/lib.rs b/crates/ide-agent/src/lib.rs index 387a67a64..e1b6f47e6 100644 --- a/crates/ide-agent/src/lib.rs +++ b/crates/ide-agent/src/lib.rs @@ -1394,13 +1394,22 @@ impl Task { }; return Failure { message, limit: Some(resets) }; } - match e { - // Signed out, or the account was refused (Claude Code: "API Error: 403"). - RpcError::Server { message, .. } if message.to_lowercase().contains("auth") => { - Failure::from(self.live.lock().as_ref().map(|live| live.sign_in_message.clone()).unwrap_or(message)) - } - e => Failure::from(self.describe(e)), + let guidance = self.live.lock().as_ref().map(|live| live.sign_in_message.clone()); + Failure::from(self.sign_in_error(&e, guidance.as_deref()).unwrap_or_else(|| self.describe(e))) + } + + fn sign_in_error(&self, e: &RpcError, guidance: Option<&str>) -> Option { + let RpcError::Server { code, message, .. } = e else { return None }; + if !acp::auth_required(*code, message) { + return None; } + tracing::warn!(task = %self.id, code, %message, "agent sign-in failed"); + let Some(guidance) = guidance else { return Some(message.clone()) }; + Some(if message.trim().is_empty() || guidance.contains(message) { + guidance.to_owned() + } else { + format!("{guidance}\n\n{message}") + }) } async fn ensure_session(self: &Arc) -> Result { @@ -1419,7 +1428,6 @@ impl Task { let spec = tokio::task::spawn_blocking(move || spec.resolve_for_launch(&launch_cwd)) .await .map_err(|e| e.to_string())?; - let sign_in_message = spec.sign_in_message(); self.stderr.lock().clear(); self.config_sent.lock().clear(); let weak = Arc::downgrade(self); @@ -1439,6 +1447,7 @@ impl Task { .map_err(|e| format!("Couldn't start {} (`{}`): {e}", spec.name, spec.command))?; let init = peer.initialize(&client_info()).await.map_err(|e| self.describe(e))?; + let sign_in_message = spec.sign_in_message(&init.auth_methods); let can_load = init.load_session(); let (capabilities, embedded) = capabilities_of(&init); // Agents that compare canonical paths fail in a symlinked directory (macOS `/tmp`). @@ -1463,12 +1472,7 @@ impl Task { let v: Value = peer .request("session/new", json!({ "cwd": cwd_s, "mcpServers": [] }), None) .await - .map_err(|e| match e { - RpcError::Server { ref message, .. } if message.to_lowercase().contains("auth") => { - sign_in_message.clone() - } - e => self.describe(e), - })?; + .map_err(|e| self.sign_in_error(&e, Some(&sign_in_message)).unwrap_or_else(|| self.describe(e)))?; let id = v.get("sessionId").and_then(Value::as_str).ok_or("the agent returned no session")?.to_owned(); (id, v) } diff --git a/crates/ide-agent/src/native/codex/mod.rs b/crates/ide-agent/src/native/codex/mod.rs index 945b9a908..624fd49bc 100644 --- a/crates/ide-agent/src/native/codex/mod.rs +++ b/crates/ide-agent/src/native/codex/mod.rs @@ -8,6 +8,7 @@ mod translate; mod tests; use super::Shim; +use crate::acp::AUTH_REQUIRED; use crate::provider::ProviderSpec; use ide_jsonrpc::{Framing, Handler, Peer, RpcError, codes}; use parking_lot::Mutex; @@ -27,8 +28,6 @@ const OUTPUT_EVERY: Duration = Duration::from_millis(200); const EARLY_CANCEL: Duration = Duration::from_secs(1); /// How long the client's connection outlives codex. const EXIT_GRACE: Duration = Duration::from_millis(100); -/// ACP's error code for a request that needs the user to sign in. -const AUTH_REQUIRED: i64 = -32000; pub(crate) fn connect( spec: &ProviderSpec, @@ -204,7 +203,7 @@ fn from_codex(e: RpcError) -> ResponseError { code, codes::PARSE_ERROR | codes::INVALID_REQUEST | codes::METHOD_NOT_FOUND | codes::INVALID_PARAMS ); - let code = if !protocol && message.to_lowercase().contains("auth") { AUTH_REQUIRED } else { code }; + let code = if !protocol && crate::acp::auth_required(code, &message) { AUTH_REQUIRED } else { code }; ResponseError { code, message, data } } RpcError::Closed => error(codes::INTERNAL_ERROR, "Codex exited"), diff --git a/crates/ide-agent/src/native/codex/tests.rs b/crates/ide-agent/src/native/codex/tests.rs index 43499711c..ec0a73d04 100644 --- a/crates/ide-agent/src/native/codex/tests.rs +++ b/crates/ide-agent/src/native/codex/tests.rs @@ -623,6 +623,23 @@ async fn auth_failure_names_auth() { assert!(start.get("jsonrpc").is_some(), "codex accepts the jsonrpc field"); } +#[test] +fn request_errors_classify_authentication_without_matching_author_or_protocol_errors() { + for (code, message, expected) in [ + (codes::INTERNAL_ERROR, "Missing author field", codes::INTERNAL_ERROR), + (codes::INTERNAL_ERROR, "Unknown authority", codes::INTERNAL_ERROR), + (codes::INTERNAL_ERROR, "Authentication failed: expired token", AUTH_REQUIRED), + (codes::INTERNAL_ERROR, "Unauthorized", AUTH_REQUIRED), + (codes::METHOD_NOT_FOUND, "authentication method not found", codes::METHOD_NOT_FOUND), + (AUTH_REQUIRED, "Account is not eligible", AUTH_REQUIRED), + ] { + let response = from_codex(RpcError::Server { code, message: message.into(), data: Some(json!({"detail": 1})) }); + assert_eq!(response.code, expected, "{message}"); + assert_eq!(response.message, message); + assert_eq!(response.data, Some(json!({"detail": 1}))); + } +} + #[tokio::test(flavor = "multi_thread")] async fn a_stop_sent_right_after_the_prompt_interrupts_the_turn() { let (results, _client, frames, _) = replay(Scenario::Interrupt).await; diff --git a/crates/ide-agent/src/provider.rs b/crates/ide-agent/src/provider.rs index ab97a4a7d..432067cfb 100644 --- a/crates/ide-agent/src/provider.rs +++ b/crates/ide-agent/src/provider.rs @@ -5,6 +5,7 @@ use crate::{AgentCommandReadOnlyReason, AgentProvider, AgentSource}; use ide_core::{IdeError, IdeResult}; use serde::{Deserialize, Serialize}; +use serde_json::Value; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; @@ -92,9 +93,26 @@ impl ProviderSpec { render_login_hint(self, program, on_path.as_deref(), cfg!(windows)) } - pub(crate) fn sign_in_message(&self) -> String { + pub(crate) fn sign_in_message(&self, auth_methods: &[Value]) -> String { let name = &self.name; - let hint = self.login_hint(Path::new(&self.command)); + let program = Path::new(&self.command); + let terminal_args = matches!(self.source, AgentSource::Registry { .. }) + .then(|| { + auth_methods.iter().find_map(|method| { + if method.get("type").and_then(Value::as_str) != Some("terminal") { + return None; + } + method.get("args")?.as_array()?.iter().map(Value::as_str).collect::>>() + }) + }) + .flatten(); + let hint = match terminal_args { + Some(args) => { + let command = render_login_command(program, args, cfg!(windows)); + format!("Run `{command}` in a terminal.") + } + None => self.login_hint(program), + }; format!("{name} needs you to sign in. {hint}") } @@ -124,14 +142,8 @@ fn render_login_hint(spec: &ProviderSpec, program: &Path, on_path: Option<&Path> if on_path.is_some_and(|path| same_path(path, program)) { return spec.login_hint.clone(); } - let path = program.to_string_lossy(); - let quoted = if windows { - format!("& '{}'", path.replace('\'', "''")) - } else { - format!("'{}'", path.replace('\'', "'\\''")) - }; let (args, suffix) = rest.split_once('`').expect("built-in login hints delimit their command"); - let command = format!("{quoted}{args}"); + let command = render_login_command(program, args.split_whitespace(), windows); match suffix { " in a terminal." => format!("Run `{command}` in a terminal."), " in a terminal and sign in." => format!("Run `{command}` in a terminal and sign in."), @@ -139,6 +151,22 @@ fn render_login_hint(spec: &ProviderSpec, program: &Path, on_path: Option<&Path> } } +fn render_login_command<'a>(program: &Path, args: impl IntoIterator, windows: bool) -> String { + let quote = |word: &str| { + if windows { format!("'{}'", word.replace('\'', "''")) } else { format!("'{}'", word.replace('\'', "'\\''")) } + }; + let mut command = format!("{}{}", if windows { "& " } else { "" }, quote(&program.to_string_lossy())); + for arg in args { + command.push(' '); + if !arg.is_empty() && arg.chars().all(|c| c.is_ascii_alphanumeric() || "-_=./".contains(c)) { + command.push_str(arg); + } else { + command.push_str("e(arg)); + } + } + command +} + /// Whether `id` names a built-in agent that needs no registry install (native drivers /// and `npx -y` adapters). CLI built-ins (`copilot`, `gemini`, …) can be installed and /// updated from the ACP Registry into `/agents/.toml`. @@ -370,6 +398,7 @@ fn is_executable(p: &Path) -> bool { #[cfg(test)] mod tests { use super::*; + use serde_json::json; #[test] fn login_hints_cover_all_builtin_commands_and_preserve_separate_login_tools() { @@ -429,12 +458,43 @@ mod tests { cursor.env.push(("PATH".into(), dir.path().to_string_lossy().into_owned())); let resolved = cursor.clone().resolve_for_launch(dir.path()); assert_eq!(Path::new(&resolved.command), program); - assert!(resolved.sign_in_message().contains(&*program.to_string_lossy())); + assert!(resolved.sign_in_message(&[]).contains(&*program.to_string_lossy())); cursor.command = format!("tools/{command}"); let resolved = cursor.resolve_for_launch(dir.path()); assert_eq!(Path::new(&resolved.command), dir.path().join("tools").join(command)); } + #[test] + fn terminal_login_commands_quote_the_binary_and_each_argument() { + for (windows, expected) in [ + (false, "'/agent'\\''s tools/$cli' auth login --account 'user name' 'a'\\''b' '$token' ''"), + (true, "& '/agent''s tools/$cli' auth login --account 'user name' 'a''b' '$token' ''"), + ] { + assert_eq!( + render_login_command( + Path::new("/agent's tools/$cli"), + ["auth", "login", "--account", "user name", "a'b", "$token", ""], + windows + ), + expected + ); + } + let mut spec = builtin_spec("cursor").unwrap(); + spec.id = "custom".into(); + spec.source = AgentSource::Registry { version: "1.4.5".into(), update: None }; + spec.login_hint = "Sign in with the agent's own command line first.".into(); + for method in [ + json!({"type": "terminal"}), + json!({"type": "terminal", "args": ["login", null]}), + json!({"type": "agent", "args": ["login"]}), + ] { + assert_eq!( + spec.sign_in_message(&[method]), + "Cursor needs you to sign in. Sign in with the agent's own command line first." + ); + } + } + #[cfg(unix)] #[test] fn registry_login_hint_names_the_installed_executable() { diff --git a/crates/ide-agent/src/tests.rs b/crates/ide-agent/src/tests.rs index 3589f366c..70c106fd0 100644 --- a/crates/ide-agent/src/tests.rs +++ b/crates/ide-agent/src/tests.rs @@ -35,6 +35,8 @@ struct FakeAgent { /// Without `loadSession` (a session lives only as long as its process). no_load: bool, auth_on_new: bool, + request_error: Option<(i64, String)>, + auth_methods: Vec, /// Hold the late mode reply until the prompt's permission request is already waiting. hold_late_mode: bool, empty_config: AtomicBool, @@ -128,7 +130,10 @@ impl Handler for FakeAgent { if (method == "session/new" && self.auth_on_new) || (method == "session/prompt" && params.pointer("/prompt/0/text") == Some(&json!("auth"))) { - return Err((codes::INTERNAL_ERROR, "Authentication required".into())); + return Err(self + .request_error + .clone() + .unwrap_or((codes::INTERNAL_ERROR, "Authentication required".into()))); } if self.hold_late_mode && method == "session/set_config_option" @@ -143,8 +148,10 @@ impl Handler for FakeAgent { if params.pointer("/clientCapabilities/subagents").is_some_and(Value::is_object) { self.calls.lock().push("subagents".into()); } - Ok(json!({"protocolVersion": 1, "agentCapabilities": {"loadSession": !self.no_load, - "sessionCapabilities": {"fork": {}}, "_meta": {"openidea": {"rollback": true, "steer": true, "backgroundTasks": true}}}})) + Ok( + json!({"protocolVersion": 1, "authMethods": self.auth_methods, "agentCapabilities": {"loadSession": !self.no_load, + "sessionCapabilities": {"fork": {}}, "_meta": {"openidea": {"rollback": true, "steer": true, "backgroundTasks": true}}}}), + ) } "session/new" => { if self.empty_config.load(Ordering::Acquire) { @@ -392,7 +399,110 @@ async fn authentication_failures_name_the_session_executable_at_start_and_during .expect("authentication error"); assert!(error.starts_with("Cursor needs you to sign in. Run `"), "{error}"); assert!(error.contains(&*program.to_string_lossy()), "{error}"); - assert!(error.ends_with(" login` in a terminal."), "{error}"); + assert!(error.ends_with(" login` in a terminal.\n\nAuthentication required"), "{error}"); + mgr.shutdown(); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn sign_in_errors_preserve_details_and_leave_unrelated_errors_unchanged() { + for auth_on_new in [true, false] { + for (code, message, sign_in) in [ + (-32000, "This Google account is not eligible for this agent.", true), + (codes::INTERNAL_ERROR, "Authentication failed: token expired for account alice.", true), + (codes::INTERNAL_ERROR, "UNAUTHORIZED: account refused", true), + (codes::INTERNAL_ERROR, "You are not logged in to this installation.", true), + (codes::INTERNAL_ERROR, "Please sign in with your work account.", true), + (codes::INTERNAL_ERROR, "Missing author field", false), + (codes::INTERNAL_ERROR, "Unknown authority in configuration", false), + ] { + let tmp = tempfile::tempdir().unwrap(); + let agent = + Arc::new(FakeAgent { auth_on_new, request_error: Some((code, message.into())), ..Default::default() }); + let mgr = manager(tmp.path(), agent); + let task = mgr + .create(NewAgentTask { + project_root: None, + prompt: text("again"), + provider: "cursor".into(), + config: vec![], + isolation: Isolation::Local, + base: None, + }) + .await + .unwrap(); + if !auth_on_new { + wait_for(&mgr, &task.id, AgentStatus::Ready).await; + mgr.prompt(&task.id, text("auth"), false).await.unwrap(); + } + wait_for(&mgr, &task.id, AgentStatus::Failed).await; + let entries = mgr.entries(&task.id, None).unwrap(); + let error = entries + .iter() + .find_map(|e| match e { + AgentEntry::Error { message, .. } => Some(message), + _ => None, + }) + .unwrap(); + if sign_in { + assert!(error.starts_with("Cursor needs you to sign in. Run `"), "{error}"); + assert!(error.ends_with(&format!("\n\n{message}")), "{error}"); + } else { + assert_eq!(error, message); + } + mgr.shutdown(); + } + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn registry_sign_in_errors_use_the_advertised_terminal_login() { + for terminal in [true, false] { + let tmp = tempfile::tempdir().unwrap(); + let agent = Arc::new(FakeAgent { + auth_on_new: true, + request_error: Some((-32000, "This account is not eligible.".into())), + auth_methods: vec![json!({"type": if terminal { "terminal" } else { "agent" }, + "id": "cli", "name": "CLI", "args": ["auth", "login"]})], + ..Default::default() + }); + let mgr = manager(tmp.path(), agent); + let program = tmp.path().join("Registry Tools").join("antigravity.cmd"); + let mut spec = provider::builtin_spec("cursor").unwrap(); + spec.id = "antigravity".into(); + spec.name = "Google Antigravity".into(); + spec.command = program.to_string_lossy().into_owned(); + spec.source = AgentSource::Registry { version: "1.4.5".into(), update: None }; + spec.login_hint = "Sign in with the agent's own command line first.".into(); + mgr.shared.providers.write().push(spec); + let task = mgr + .create(NewAgentTask { + project_root: None, + prompt: text("again"), + provider: "antigravity".into(), + config: vec![], + isolation: Isolation::Local, + base: None, + }) + .await + .unwrap(); + wait_for(&mgr, &task.id, AgentStatus::Failed).await; + let entries = mgr.entries(&task.id, None).unwrap(); + let error = entries + .iter() + .find_map(|e| match e { + AgentEntry::Error { message, .. } => Some(message), + _ => None, + }) + .unwrap(); + assert!(error.starts_with("Google Antigravity needs you to sign in. "), "{error}"); + if terminal { + assert!(error.contains(&*program.to_string_lossy()), "{error}"); + assert!(error.contains(" auth login` in a terminal."), "{error}"); + } else { + assert!(error.contains("Sign in with the agent's own command line first."), "{error}"); + } + assert!(error.ends_with("\n\nThis account is not eligible."), "{error}"); mgr.shutdown(); } } diff --git a/src/features/agents/loginHint.test.ts b/src/features/agents/loginHint.test.ts index 056c105cb..449c5f06e 100644 --- a/src/features/agents/loginHint.test.ts +++ b/src/features/agents/loginHint.test.ts @@ -24,3 +24,16 @@ it("preserves external agent errors", async () => { const { result } = renderHook(() => useAgentErrorText("Agent says: authentication failed")); expect(result.current).toBe("Agent says: authentication failed"); }); + +it("translates only the guidance when the agent adds its own explanation", async () => { + await setLocale("zh"); + const details = "Authentication failed: token expired.\nRun `account repair` in a terminal."; + const { result } = renderHook(() => + useAgentErrorText( + `Google Antigravity needs you to sign in. Run \`'/registry/antigravity' auth login\` in a terminal.\n\n${details}`, + ), + ); + expect(result.current).toBe( + `Google Antigravity 需要你登录。在终端中运行 \`'/registry/antigravity' auth login\`。\n\n${details}`, + ); +}); diff --git a/src/features/agents/loginHint.ts b/src/features/agents/loginHint.ts index 231dc0a26..6b8a6945e 100644 --- a/src/features/agents/loginHint.ts +++ b/src/features/agents/loginHint.ts @@ -4,6 +4,6 @@ import { useTx, useTxTemplate } from "@/platform/i18n"; export function useAgentErrorText(message: string): string { const tx = useTx(); const translate = useTxTemplate(); - const auth = /^(.+) needs you to sign in\. (.+)$/s.exec(message); - return auth ? tx`${auth[1]} needs you to sign in. ${translate(auth[2]!)}` : message; + const auth = /^(.+?) needs you to sign in\. ([^\n]+)([\s\S]*)$/.exec(message); + return auth ? `${tx`${auth[1]} needs you to sign in. ${translate(auth[2]!)}`}${auth[3]}` : message; }