diff --git a/docs-site/src/content/docs/reference/cli/providers-accounts.md b/docs-site/src/content/docs/reference/cli/providers-accounts.md index 1b5575e889d..224fa1bff7a 100644 --- a/docs-site/src/content/docs/reference/cli/providers-accounts.md +++ b/docs-site/src/content/docs/reference/cli/providers-accounts.md @@ -218,6 +218,13 @@ as OrcaRouter are reused until the provider revokes them); API-key login provide key, validate it when possible, and save the resulting provider config. The command prints the currently accepted OAuth and API-key provider ids when the name is missing or unknown. +Failed OAuth logins exit with status 1 and print a fixed diagnostic: the HTTP status and standard +OAuth error code (such as `invalid_grant`) when the provider returned one, `network_error` or +`timeout` when the provider could not be reached, `configuration_error` with the reason when a +local setting such as a Codex account namespace blocks the login, and `oauth_authentication_failed` +otherwise. Pool 2 (`anthropic2`) also reports `configuration_error` before prompting when a custom +provider owns that name or its config file cannot be read or parsed. + Use the same command to **reauthenticate** after `ocx status` / `ocx doctor` reports reauthentication required or a terminal refresh failure (or use Reauthenticate in the dashboard). Codex pool accounts are not one of those OAuth or API-key providers, but `ocx login codex` reaches diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 470fba1b3a6..79fc19f3bbf 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1247,6 +1247,7 @@ "oauth-health.test.ts": "oauth", "oauth-log.test.ts": "oauth", "oauth-login-cli-browser-launch.test.ts": "oauth", + "oauth-login-cli-errors.test.ts": "oauth", "oauth-login-cli-live-update.test.ts": "oauth", "oauth-login-open-browser.test.ts": "oauth", "oauth-login-summary.test.ts": "oauth", diff --git a/src/oauth/index.ts b/src/oauth/index.ts index e1d3b3b3f42..738e796f019 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -453,6 +453,14 @@ export class OAuthProviderPublicationError extends Error { } } +/** A local configuration conflict found before any credential is written; its text is fixed. */ +export class OAuthProviderNamespaceCollisionError extends Error { + constructor(message: string) { + super(message); + this.name = "OAuthProviderNamespaceCollisionError"; + } +} + export class OAuthReauthIdentityMismatchError extends Error { constructor() { super("Signed-in account does not match the selected account. Sign in with the same account."); @@ -485,6 +493,7 @@ export function publicOAuthAuthenticationErrorMessage(error: unknown): string { (error instanceof OAuthLoginRequiredError && isOAuthProvider(error.provider)) || error instanceof OAuthAccountPausedError || error instanceof OAuthProviderPublicationError + || error instanceof OAuthProviderNamespaceCollisionError // Reauth identity outcomes carry fixed, account-free remediation text. Dropping them to the // generic message hides WHICH failure the user must fix (sign in with the selected account). || error instanceof OAuthReauthIdentityMismatchError @@ -1506,7 +1515,7 @@ export function upsertOAuthProvider(config: OcxConfig, provider: string): void { if (!def) return; if (provider === GOOGLE_ANTIGRAVITY_PROVIDER) migrateLegacyAntigravityStaticCatalog(config); const namespaceCollision = codexAccountNamespaceProviderCollisionError(config.codexAccountNamespaces, provider); - if (namespaceCollision) throw new Error(namespaceCollision); + if (namespaceCollision) throw new OAuthProviderNamespaceCollisionError(namespaceCollision); const existing = config.providers[provider]; const providerConfig = def.resolveProviderConfig?.(config) ?? def.providerConfig; // Clone operator state, including xAI wire choices and their migration version. @@ -1603,7 +1612,7 @@ export async function runLogin( preflightConfig.codexAccountNamespaces, provider, ); - if (namespaceCollision) throw new Error(namespaceCollision); + if (namespaceCollision) throw new OAuthProviderNamespaceCollisionError(namespaceCollision); } if (provider === "anthropic2" && observeInitialConfigState() !== "exists") { throw new OAuthProviderPublicationError(provider, false); diff --git a/src/oauth/login-cli-errors.ts b/src/oauth/login-cli-errors.ts new file mode 100644 index 00000000000..057d7aa1151 --- /dev/null +++ b/src/oauth/login-cli-errors.ts @@ -0,0 +1,44 @@ +import { AnthropicTokenError } from "./anthropic"; +import { OAuthProviderNamespaceCollisionError, OAuthProviderPublicationError, publicOAuthAuthenticationErrorMessage } from "./index"; +import { XaiTokenRequestError } from "./xai"; +import { AnthropicInstanceCollisionError, AnthropicInstanceConfigReadError } from "./store-anthropic-instance"; + +// Standard RFC 6749 / RFC 6750 / RFC 8628 error codes. A provider's `error` field is printed only +// when it is one of these, so a body that echoes request parameters there still prints nothing. +const STANDARD_OAUTH_ERROR_CODES = new Set([ + "invalid_request", "invalid_client", "invalid_grant", "unauthorized_client", + "unsupported_grant_type", "invalid_scope", "access_denied", "unsupported_response_type", + "server_error", "temporarily_unavailable", "invalid_token", "insufficient_scope", + "authorization_pending", "slow_down", "expired_token", +]); + +/** Fixed, machine-readable reason for a failed login; never derived from free-form text. */ +export function oauthLoginFailureDiagnostic(error: unknown): string { + if (error instanceof OAuthProviderNamespaceCollisionError || error instanceof OAuthProviderPublicationError + || error instanceof AnthropicInstanceCollisionError || error instanceof AnthropicInstanceConfigReadError) { + return "configuration_error"; + } + const status = error instanceof AnthropicTokenError ? error.httpStatus + : error instanceof XaiTokenRequestError ? error.status : undefined; + const oauthError = error instanceof AnthropicTokenError || error instanceof XaiTokenRequestError + ? error.oauthError : undefined; + const parts: string[] = []; + if (typeof status === "number" && Number.isInteger(status) && status >= 400 && status <= 599) parts.push(`HTTP ${status}`); + if (typeof oauthError === "string" && STANDARD_OAUTH_ERROR_CODES.has(oauthError)) parts.push(oauthError); + if (parts.length > 0) return parts.join(" "); + const name = error instanceof Error ? error.name : ""; + if (name === "TimeoutError" || name === "AbortError") return "timeout"; + // fetch() reports refused connections and DNS failures as a TypeError carrying a string code; a + // programming TypeError carries none and stays in the generic bucket. + const code = error instanceof TypeError ? (error as { code?: unknown }).code : undefined; + if ((error instanceof AnthropicTokenError && error.requestNotSent) + || (error instanceof XaiTokenRequestError && error.status === undefined) + || (typeof code === "string" && code !== "")) return "network_error"; + return "oauth_authentication_failed"; +} + +/** Keep terminal login failures within the public OAuth error vocabulary. */ +export function reportOAuthLoginError(error: unknown): never { + console.error(`\n❌ OAuth login failed (${oauthLoginFailureDiagnostic(error)}): ${publicOAuthAuthenticationErrorMessage(error)}`); + process.exit(1); +} diff --git a/src/oauth/login-cli.ts b/src/oauth/login-cli.ts index 6197047b996..b63bf4aa906 100644 --- a/src/oauth/login-cli.ts +++ b/src/oauth/login-cli.ts @@ -15,6 +15,7 @@ import type { OcxConfig, OcxProviderConfig } from "../types"; import { configuredAdminToken } from "../lib/admin-secrets"; import { codexAccountNamespaceProviderCollisionError } from "../codex/account-namespace-match"; import { assertAnthropicInstanceLoginConfig } from "./store-anthropic-instance"; +import { reportOAuthLoginError } from "./login-cli-errors"; /** * Seams a test drives in place of a browser, a terminal and a real provider. Production passes @@ -122,6 +123,12 @@ export function loginUsageMessage(): string { export async function handleLogin(provider?: string, deps: LoginCliDeps = {}): Promise { const name = (provider ?? "").trim().toLowerCase(); + try { + // Check raw ownership before tolerant loading can print diagnostics or fall back to defaults. + if (name === "anthropic2") assertAnthropicInstanceLoginConfig({ providers: {} }, name); + } catch (error) { + reportOAuthLoginError(error); + } // A removed provider id reached through its alias still logs in — the merged // successor owns the flow. Warn rather than silently reroute so scripts and // docs that still say `devin-cli` surface the rename to whoever runs them. @@ -159,7 +166,7 @@ export async function handleOAuthLogin(name: string, deps: LoginCliDeps = {}): P return await ask("Paste redirect URL or code (or wait for browser): "); }, }); - }); + }).catch(reportOAuthLoginError); // A device or polling provider never prompts, so nothing above waited on the launcher. It is // still owed an answer before this claims the login worked. await browser.settled(); diff --git a/src/oauth/store-anthropic-instance.ts b/src/oauth/store-anthropic-instance.ts index 2333ebf06d7..dde516a5e45 100644 --- a/src/oauth/store-anthropic-instance.ts +++ b/src/oauth/store-anthropic-instance.ts @@ -32,6 +32,13 @@ export class AnthropicInstanceCollisionError extends Error { } } +export class AnthropicInstanceConfigReadError extends Error { + constructor() { + super("Cannot verify Anthropic Pool 2 ownership in the existing config file."); + this.name = "AnthropicInstanceConfigReadError"; + } +} + export function assertAnthropicInstanceLoginConfig(config: Pick, provider: string, credentialWritten = false): void { if (provider !== "anthropic2") return; const row = config.providers.anthropic2; @@ -44,11 +51,11 @@ export function assertAnthropicInstanceLoginConfig(config: Pick)[provider]; diff --git a/structure/providers/anthropic-account-pool.md b/structure/providers/anthropic-account-pool.md index cf0c0b9ba8e..07fd3fd8e4d 100644 --- a/structure/providers/anthropic-account-pool.md +++ b/structure/providers/anthropic-account-pool.md @@ -25,7 +25,9 @@ existing auth-store write lock. Display identities do not establish a duplicate. Refresh intent paths and credential-owner checks retain the actual instance. Pool 2 config publication rechecks the latest provider row under the config mutation lock; a collision preserves the custom row and reports any already-written orphan -credential. No default-provider change accompanies that publication. +credential. No default-provider change accompanies that publication. CLI preflight checks raw +config ownership before tolerant loading and reports a custom-row collision or unreadable/malformed +config through the fixed `configuration_error` reporter before prompting. Regression coverage: `tests/providers/provider-anthropic-instance.test.ts`, `tests/config/config-anthropic-instance-pool.test.ts`, diff --git a/structure/runtime.md b/structure/runtime.md index 13c0097ced6..d4e5739596f 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -53,7 +53,7 @@ this wire projection does not change the usage ledger. ## CLI readiness diagnostics -The [CLI management contract](cli-management.md#cli-readiness-diagnostics) owns terminal-safe human diagnostics, structured-output fidelity and capability argument failures. Its [data-key and observation contract](cli-management.md#data-key-probes-and-observation-streams) also owns explicit stdin cancellation, credential separation and bounded follow lifetimes. +The [CLI management contract](cli-management.md#cli-readiness-diagnostics) owns terminal-safe human diagnostics, structured-output fidelity and capability argument failures. Its [data-key and observation contract](cli-management.md#data-key-probes-and-observation-streams) also owns explicit stdin cancellation, credential separation and bounded follow lifetimes. `src/oauth/login-cli.ts` delegates failed provider logins to `src/oauth/login-cli-errors.ts`: the terminal prints the public OAuth message with a fixed reason (a validated HTTP status plus an allowlisted RFC 6749/8628 error code, `network_error`, `timeout`, `configuration_error`, or `oauth_authentication_failed`) and exits 1, so provider response text never reaches the terminal (`tests/oauth/oauth-login-cli-errors.test.ts`). A status-less `XaiTokenRequestError` represents a connection failure after retries and reports `network_error` without exposing its cause. Pool 2 checks raw config ownership before tolerant config loading or prompting; custom-row collisions and unreadable or malformed config are fixed `configuration_error` diagnostics. ## CLI resolve and stop contracts for embedding shells diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 44d37a1e5b3..604f01b61ba 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -838,6 +838,7 @@ "oauth-callback-server.test.ts": "oauth", "oauth-device-code-contract.test.ts": "oauth", "oauth-first-add-hint.test.ts": "gui", "oauth-health.test.ts": "oauth", "oauth-log.test.ts": "oauth", "oauth-login-cli-browser-launch.test.ts": "oauth", + "oauth-login-cli-errors.test.ts": "oauth", "oauth-login-cli-live-update.test.ts": "oauth", "oauth-login-open-browser.test.ts": "oauth", "oauth-login-summary.test.ts": "oauth", "oauth-manual-code.test.ts": "oauth", "oauth-open-browser-choice.test.ts": "oauth", "oauth-open-url-result.test.ts": "oauth", diff --git a/tests/oauth/oauth-login-cli-errors.test.ts b/tests/oauth/oauth-login-cli-errors.test.ts new file mode 100644 index 00000000000..73eccf8e763 --- /dev/null +++ b/tests/oauth/oauth-login-cli-errors.test.ts @@ -0,0 +1,153 @@ +import { expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { repoPath, repoRoot } from "../helpers/repo-root"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const marker = "oauth-cli-provider-response-marker"; +const moduleUrl = (...name: string[]) => JSON.stringify(pathToFileURL(repoPath("src", ...name)).href); +const publicMessage = "OAuth authentication failed. Check the OpenCodex account status and retry."; + +/** Run one `ocx login` in a child process: the failure path ends in process.exit. */ +async function runLoginChild(script: string): Promise<{ stdout: string; stderr: string; code: number }> { + const home = mkdtempSync(join(tmpdir(), "ocx-login-errors-")); + const child = Bun.spawn([process.execPath, "--eval", script], { + cwd: repoRoot(), + env: { ...process.env, OPENCODEX_HOME: join(home, "ocx"), CODEX_HOME: join(home, "codex") }, + stdin: "ignore", stdout: "pipe", stderr: "pipe", timeout: 10_000, + }); + try { + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), new Response(child.stderr).text(), child.exited, + ]); + return { stdout, stderr, code }; + } finally { + child.kill(); + await child.exited; + removeTreeWithRetry(home); + } +} + +for (const scenario of [ + { provider: "anthropic", status: 400, body: JSON.stringify({ error: marker }), diagnostic: "HTTP 400" }, + { provider: "anthropic", status: 400, body: JSON.stringify({ error: "invalid_grant", error_description: marker }), diagnostic: "HTTP 400 invalid_grant" }, + { provider: "anthropic", status: 503, body: marker, diagnostic: "HTTP 503" }, + { provider: "xai", status: 400, body: JSON.stringify({ error: marker, error_description: marker }), diagnostic: "HTTP 400" }, + { provider: "xai", status: 400, body: JSON.stringify({ error: "invalid_grant", error_description: marker }), diagnostic: "HTTP 400 invalid_grant" }, + { provider: "anthropic", status: 200, body: marker, diagnostic: "oauth_authentication_failed" }, + { provider: "anthropic", status: 0, body: marker, diagnostic: "network_error" }, + { provider: "xai", status: 0, body: marker, diagnostic: "network_error" }, +]) { + const label = scenario.status === 0 ? "an unreachable token endpoint" : `token HTTP ${scenario.status} (${scenario.diagnostic})`; + test(`${scenario.provider} login prints a fixed error for ${label}`, async () => { + let requests = 0; + const server = Bun.serve({ + hostname: "127.0.0.1", port: 0, + fetch: () => { + requests++; + return new Response(scenario.body, { status: scenario.status }); + }, + }); + const endpoint = JSON.stringify(`http://127.0.0.1:${server.port}/token`); + // A stopped listener leaves a port that refuses connections. + if (scenario.status === 0) await server.stop(true); + try { + const { stdout, stderr, code } = await runLoginChild(` + import { handleLogin } from ${moduleUrl("oauth", "login-cli.ts")}; + import { OAUTH_PROVIDERS } from ${moduleUrl("oauth", "index.ts")}; + import { AnthropicOAuthFlow } from ${moduleUrl("oauth", "anthropic.ts")}; + import { postXaiToken } from ${moduleUrl("oauth", "xai.ts")}; + const localFetch = globalThis.fetch; + globalThis.fetch = (_input, init) => localFetch(${endpoint}, init); + OAUTH_PROVIDERS[${JSON.stringify(scenario.provider)}].login = async ctrl => { + if (${JSON.stringify(scenario.provider)} === "xai") { + return await postXaiToken(${endpoint}, { grant_type: "authorization_code", code: "test-code" }, undefined, { sleep: async () => {}, random: () => 0 }); + } + return await new AnthropicOAuthFlow(ctrl).exchangeToken("test-code", "test-state", "http://127.0.0.1/callback"); + }; + await handleLogin(${JSON.stringify(scenario.provider)}, { ask: async () => "" }); + `); + expect(requests).toBe(scenario.status === 0 ? 0 : 1); + expect(stderr).not.toContain(marker); + expect(stderr.trim()).toBe(`❌ OAuth login failed (${scenario.diagnostic}): ${publicMessage}`); + expect(code).toBe(1); + expect(stdout).not.toContain("Logged in"); + } finally { + await server.stop(true); + } + }); +} + +test("a non-Error login rejection prints fixed text and exits unsuccessfully", async () => { + const { stdout, stderr, code } = await runLoginChild(` + import { handleLogin } from ${moduleUrl("oauth", "login-cli.ts")}; + await handleLogin("anthropic", { + ask: async () => "", + runLogin: async () => { throw ${JSON.stringify(marker)}; }, + }); + `); + expect(stderr).not.toContain(marker); + expect(stderr.trim()).toBe(`❌ OAuth login failed (oauth_authentication_failed): ${publicMessage}`); + expect(code).toBe(1); + expect(stdout).not.toContain("Logged in"); +}); + +test("a Codex account namespace collision is reported as configuration before any login starts", async () => { + const { stdout, stderr, code } = await runLoginChild(` + import { handleLogin } from ${moduleUrl("oauth", "login-cli.ts")}; + import { OAUTH_PROVIDERS } from ${moduleUrl("oauth", "index.ts")}; + import { loadConfig, saveConfig } from ${moduleUrl("config.ts")}; + const config = loadConfig(); + config.codexAccountNamespaces = { anthropic: "pool-a" }; + saveConfig(config); + if (!loadConfig().codexAccountNamespaces?.anthropic) throw new Error("fixture namespace was not persisted"); + OAUTH_PROVIDERS.anthropic.login = async () => { console.log("login started"); throw new Error(${JSON.stringify(marker)}); }; + await handleLogin("anthropic", { ask: async () => "" }); + `); + expect(stderr.trim()).toBe( + "❌ OAuth login failed (configuration_error): provider name must not collide with a configured Codex account namespace", + ); + expect(stdout).not.toContain("login started"); + expect(code).toBe(1); +}); + +for (const scenario of [ + { + label: "an existing custom anthropic2 row", + setup: `writeFileSync(configPath, JSON.stringify({ providers: { anthropic2: { adapter: "openai", baseUrl: "https://example.invalid/v1", apiKey: ${JSON.stringify(marker)} } } }));`, + message: "anthropic2 is already configured as a custom provider; rename the custom provider before logging in to Pool 2.", + }, + { + label: "malformed config", + setup: `writeFileSync(configPath, ${JSON.stringify(`{ "providers": ${marker}`)});`, + message: publicMessage, + }, + { + label: "unreadable config", + setup: "mkdirSync(configPath);", + message: publicMessage, + }, +]) { + test(`anthropic2 preflight reports configuration_error for ${scenario.label} before prompting`, async () => { + const { stdout, stderr, code } = await runLoginChild(` + import { mkdirSync, writeFileSync } from "node:fs"; + import { dirname } from "node:path"; + import { handleLogin } from ${moduleUrl("oauth", "login-cli.ts")}; + import { getConfigPath } from ${moduleUrl("config", "paths.ts")}; + const configPath = getConfigPath(); + mkdirSync(dirname(configPath), { recursive: true }); + ${scenario.setup} + await handleLogin("anthropic2", { + ask: async () => { console.log("prompt started"); return ""; }, + runLogin: async () => { console.log("login started"); throw new Error(${JSON.stringify(marker)}); }, + openUrl: async () => { console.log("browser started"); return { status: "started" }; }, + }); + `); + expect(code).toBe(1); + expect(stderr.trim()).toBe(`❌ OAuth login failed (configuration_error): ${scenario.message}`); + expect(stdout.trim()).toBe(""); + expect(stderr).not.toContain(marker); + }); +}