fix(agents): preserve sign-in failure details (#2141)

Any agent error containing the substring "auth" was replaced by generic sign-in guidance and the agent's own message was dropped, so a real failure could not be told apart and errors about an "author" looked like a sign-in problem.

Sign-in failures are now recognised by ACP's auth-required code or by whole authentication words and phrases. The guidance keeps the agent's text after it, and the code and message are logged. A Registry agent that advertises a terminal login now gets that command for the exact binary being launched.

Siblings covered: session creation, prompts and the native Codex adapter all use the same check.

Refs #2080
Цей коміт міститься в:
l0ng-ai authored and GitHub committed 2026-11-08 20:34:39 +08:00
1 джерело 6394f5904a
коміт b57fba8b43
9 files changed
+294 -32

No files matched your search

+23
Переглянути файл
@@ -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!({
+36
Переглянути файл
@@ -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<Value> {
fixture
+17 -13
Переглянути файл
@@ -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<String> {
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<Self>) -> Result<Live, String> {
@@ -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)
}
+2 -3
Переглянути файл
@@ -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"),
+17
Переглянути файл
@@ -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;
+70 -10
Переглянути файл
@@ -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::<Option<Vec<_>>>()
})
})
.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<Item = &'a str>, 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(&quote(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 `<config>/agents/<id>.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() {
+114 -4
Переглянути файл
@@ -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<Value>,
/// 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();
}
}
+13
Переглянути файл
@@ -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}`,
);
});
+2 -2
Переглянути файл
@@ -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;
}