diff --git a/README.md b/README.md index e7f000c5..f6cf7b62 100644 --- a/README.md +++ b/README.md @@ -806,9 +806,16 @@ command waits as long as the host stays busy. branch before it is pushed for review: Codex when run from Claude Code, and Claude Code when run from Codex. `--harness` names the reviewer instead; an unknown harness, or a missing or logged-out CLI, is an error rather than a -same-model review. The reviewer uses its CLI's own login, with a -pinned model and effort that `quest review --help` lists; `--model` and -`--effort` override them. +same-model review, as is a login the API rejects mid-round, such as an expired +or revoked token. The reviewer uses its CLI's own stored login; a CLI with +none keeps the credential it reads from the environment (`ANTHROPIC_API_KEY`, +`ANTHROPIC_AUTH_TOKEN`, or `CLAUDE_CODE_OAUTH_TOKEN` for Claude Code, +`CODEX_API_KEY` for Codex), unless a stripped `ANTHROPIC_BASE_URL` or +`CLAUDE_CODE_USE_BEDROCK`, `_VERTEX`, or `_FOUNDRY` would send it elsewhere. +A logged-out error names the profile the CLI checked, `CLAUDE_CONFIG_DIR` or +`CODEX_HOME`, or the default when unset. +Every review runs with a pinned model and effort that `quest review --help` +lists; `--model` and `--effort` override them. ```sh quest review # round 1 against origin's default branch diff --git a/quest/m0/README.md b/quest/m0/README.md index d2f77e48..f2bcc28f 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -89,7 +89,6 @@ ownership-manifest design: - [Current landing status](/quest/m0/status-landing.md) - `quest status` drops a stale `landing-stopped` line and ignores superseded cancelled checks - [Kix decision UI](/quest/m0/kix/README.md) - planning and implementation through prompts and summaries, run on the maintainer's own runners - [Codex cloud off](/quest/m0/codex-cloud-off.md) - automatic Codex cloud reviews stop once no open PR relies on them -- [Review handles expired and environment logins](/quest/m0/review-login.md) - `quest review` says to log in again instead of failing with a 401, and reviews with credentials from the environment - [Bun EFAULT report](/quest/m0/bun-efault.md) - condition: Bun fixes concurrent `rmSync` on Windows; report it, then drop the workaround - [Codex project-config skip](/quest/m0/codex-project-config.md) - condition: Codex keeps a supported way to skip a project's `.codex/` layer; then reviews keep the user's config - [Gate bench](/quest/m0/gate-bench.md) - measure headroom admission on an idle host diff --git a/quest/m0/review-login.md b/quest/m0/review-login.md deleted file mode 100644 index 83923c06..00000000 --- a/quest/m0/review-login.md +++ /dev/null @@ -1,53 +0,0 @@ -# [S] Review handles expired and environment logins - -## Goal - -When a reviewer CLI's login has expired or been revoked, `quest review` fails -with the same "`` is not logged in; run ``" error it gives a -signed-out CLI, instead of a raw 401 from the reviewer's turn. The error -names the Claude profile it checked. A reviewer whose only credentials come -from the environment still reviews. - -## Plan - -Decided 2026-10-29 (user, /quest-plan of the follow-ups from -[kixelated/rind#31](https://github.com/kixelated/rind/pull/31)), where -`codex login status` exited 0 on a revoked token -(`refresh_token_invalidated`) and every review turn then failed with a 401. - -- Map the failure, don't probe: when `turn` in `src/cli/review.ts` fails on - an authentication error (a 401, or a revoked or expired token), raise the - `ReviewInputError` the status check raises. This costs no extra request and - covers Claude Code too. -- Keep the `status` check for the signed-out case. -- Cover it with the fake-CLI tests: one reviewer that exits with each CLI's - auth error text. - -Decided 2026-10-30 (user, /quest-plan of the follow-ups from a moq-dev/moq -`/quest-complete` session): ✅ extend this quest / drop. - -- `quest review --harness claude` reported "claude is not logged in" in - sessions where Claude Code's credentials came only from `ANTHROPIC_*` or - `CLAUDE*` variables. `strippedEnvironment` (`src/cli/agent.ts`, used at - `src/cli/review.ts`'s status check) removes every one except - `CLAUDE_CONFIG_DIR`, so the status check fails. Codex-written PRs then got - no Claude review (moq-dev/moq#7084, moq-dev/moq#7251). -- Let a reviewer keep the credential its own CLI would read from the - environment, while still stripping variables that would redirect billing or - configuration. Weigh that against why `STRIPPED` exists before choosing - which names pass. If a credential is stripped on purpose, the error names the - variable instead of saying "not logged in". -- Test with a fake reviewer that authenticates only through an environment - variable. - -Decided 2026-10-31 (user, /quest-plan of the follow-ups from a moq-dev/moq -`/quest-complete` run): the "not logged in" error names the -`CLAUDE_CONFIG_DIR` profile it checked, or the default when unset. Sessions -running with `~/.claude-personal` were told "claude is not logged in" with no -hint of which profile was read -([moq-dev/moq#7245](https://github.com/moq-dev/moq/pull/7245), -[moq-dev/moq#7210](https://github.com/moq-dev/moq/pull/7210)). The error is -raised in `src/cli/review.ts`'s status check. -[moq-dev/quest#316](https://github.com/moq-dev/quest/pull/316) (preserve -reviewer connection credentials) covers the environment-credential part -above and removes it from this quest when it lands. diff --git a/src/cli/main.ts b/src/cli/main.ts index 7a8cb0fe..9d994a5c 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -835,7 +835,11 @@ export function main(argv: string[], out: Output): number | Promise { .description( "Review the checkout's committed branch against its base with the other harness: Codex when run from " + "Claude Code, Claude Code when run from Codex, detected from the variables each sets. --harness names " + - "the reviewer instead; there is no other fallback. The reviewer uses its CLI's own login, " + + "the reviewer instead; there is no other fallback. The reviewer uses its CLI's own stored login or, " + + "without one, the credential it reads from the environment (ANTHROPIC_API_KEY, ANTHROPIC_AUTH_TOKEN, or " + + "CLAUDE_CODE_OAUTH_TOKEN for Claude Code, CODEX_API_KEY for Codex) unless a stripped ANTHROPIC_BASE_URL or " + + "CLAUDE_CODE_USE_BEDROCK, _VERTEX, or _FOUNDRY would send it elsewhere. A logged-out error names the " + + "profile it checked, CLAUDE_CONFIG_DIR or CODEX_HOME, or the default when unset. It runs " + `with ${models}. --model and --effort override them.\n\n` + "The first round runs the reviewer's own review (`codex exec review --base`, or Claude Code's " + "`/code-review ...HEAD`), then resumes that session for one turn that applies the base's " + @@ -851,7 +855,7 @@ export function main(argv: string[], out: Output): number | Promise { "Prints the reviewer, model, and effort, session, round, base, head, a usage line per turn (input tokens, the " + "part read from the prompt cache, and output), the finding count, then each finding. --json prints " + "the same as one object. Exits 0 once the round finishes, whatever it found, 2 for invalid arguments " + - "or a checkout or CLI that cannot be reviewed, and 1 for other failures.", + "or a checkout or CLI that cannot be reviewed, including a login the API rejects, and 1 for other failures.", ) .option("--base ", "Base branch to review against (default: origin's default branch)") .addOption(new Option("--harness ", "Reviewer to run instead of the other harness").choices(HARNESSES)) diff --git a/src/cli/review.ts b/src/cli/review.ts index d917c95b..772dcca9 100644 --- a/src/cli/review.ts +++ b/src/cli/review.ts @@ -159,6 +159,8 @@ interface Reply { structured?: unknown; usage?: Omit; error?: string; + /** The API's HTTP status when the turn failed on an API error, if the CLI reports one. */ + status?: number; } interface Reader { @@ -171,6 +173,18 @@ interface Reviewer { status: string[]; /** How the user logs in. */ login: string; + /** The variable naming the configuration directory, and so the stored login, the status check reads. */ + profile: string; + /** Credentials the CLI reads from the environment, passed only when it has no stored login. */ + credentials: string[]; + /** + * The variables in `env` that send the CLI's requests, and their credentials, + * to another endpoint or provider. They stay stripped, and while one is set + * no credential passes either, since it is meant for that endpoint. + */ + reroutes(env: NodeJS.ProcessEnv): string[]; + /** Matches the CLI's error for a login or credential the API rejected: expired, revoked, or invalid. */ + unauthorized: RegExp; review(context: Context, session: string): Turn; resume(context: Context, session: string, prompt: string): Turn; reader(): Reader; @@ -201,6 +215,17 @@ const text = (value: unknown) => (typeof value === "string" ? value : undefined) const claude: Reviewer = { status: ["claude", "auth", "status"], login: "claude auth login", + profile: "CLAUDE_CONFIG_DIR", + credentials: ["ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"], + // Claude Code turns a provider on only for `1`, `true`, `yes`, or `on`, so `0` or `false` reroutes nothing. + reroutes: (env) => [ + ...(env.ANTHROPIC_BASE_URL ? ["ANTHROPIC_BASE_URL"] : []), + ...["CLAUDE_CODE_USE_BEDROCK", "CLAUDE_CODE_USE_VERTEX", "CLAUDE_CODE_USE_FOUNDRY"].filter((name) => + /^\s*(1|true|yes|on)\s*$/i.test(env[name] ?? ""), + ), + ], + // Such as "Failed to authenticate. API Error: 401 ...", "Invalid API key · ...", or "Login expired · Please run /login". + unauthorized: /API Error: 401|Failed to authenticate|Invalid API key|Please run \/login/, // The command's level sets its own effort, overriding `--effort`, so it is given the same one. review: (context, session) => ({ argv: [...claudeFlags(context), "--session-id", session], @@ -243,6 +268,7 @@ const claude: Reviewer = { structured: result.structured_output, usage: { input, cached, output }, error: result.is_error === true ? (text(result.result) ?? "claude reported an error") : undefined, + status: typeof result.api_error_status === "number" ? result.api_error_status : undefined, }; }, }; @@ -286,6 +312,14 @@ const claudeEnv = () => ({ CLAUDE_CODE_PROMPT_CACHE_TTL: "1h" }); const codex: Reviewer = { status: ["codex", "login", "status"], login: "codex login", + profile: "CODEX_HOME", + // `codex exec` reads CODEX_API_KEY, though `codex login status` does not count it; it ignores OPENAI_API_KEY. + credentials: ["CODEX_API_KEY"], + // OPENAI_BASE_URL is never stripped, so a passed key goes where the CLI already sends its requests. + reroutes: () => [], + // Such as "unexpected status 401 Unauthorized: ..." or "Your access token could not be refreshed + // because your refresh token was revoked. Please log out and sign in again." + unauthorized: /\b401 Unauthorized\b|sign in again/, review: (context) => ({ argv: ["codex", "exec", "review", "--base", context.base, "--json", ...codexFlags(context)], stdin: "", @@ -409,12 +443,65 @@ function run(argv: string[], cwd: string, env: NodeJS.ProcessEnv, stdin: string, }); } -/** Run one reviewer turn, failing on any error it reports. */ -async function turn(reviewer: Reviewer, call: Turn, cwd: string, env: NodeJS.ProcessEnv): Promise { - const reader = reviewer.reader(); - const { code, stderr } = await run(call.argv, cwd, { ...env, ...call.env }, call.stdin, reader); +/** The reviewer CLI with the environment it runs in and the error for a login the API rejects. */ +interface Cli { + reviewer: Reviewer; + env: NodeJS.ProcessEnv; + /** Thrown when a turn fails on authentication, as a logged-out status check fails. */ + unauthorized: string; +} + +const names = (list: string[]) => list.join(" and "); + +/** The stored login the status check read, so a session running with another profile can tell. */ +function profile(reviewer: Reviewer, env: NodeJS.ProcessEnv): string { + const dir = env[reviewer.profile]; + return dir ? `the profile ${reviewer.profile}=${dir}` : `its default profile (${reviewer.profile} is unset)`; +} + +/** + * Prepare the reviewer: the user's environment minus `STRIPPED`, so the CLI + * uses its stored login, as any launched agent does. A CLI with no stored + * login keeps the credentials it would read from the environment instead, + * unless a stripped variable would have sent them elsewhere. A shell's API key + * never overrides a stored subscription login, so a review is billed as the + * user's own CLI sessions are. + */ +async function prepare(harness: HarnessName, cwd: string, env: NodeJS.ProcessEnv): Promise { + const reviewer = REVIEWERS[harness]; + const stripped = strippedEnvironment(env); + const notLoggedIn = `${harness} is not logged in to ${profile(reviewer, env)}`; + const loggedOut = `${notLoggedIn}; run \`${reviewer.login}\``; + if ((await run(reviewer.status, cwd, stripped, "")).code === 0) { + return { reviewer, env: stripped, unauthorized: loggedOut }; + } + const reroutes = reviewer.reroutes(env); + if (reroutes.length > 0) { + throw new ReviewInputError( + `${notLoggedIn}, and quest review strips ${names(reroutes)}, which would reroute it; ` + + `run \`${reviewer.login}\``, + ); + } + const credentials = reviewer.credentials.filter((name) => env[name]); + if (credentials.length === 0) throw new ReviewInputError(loggedOut); + return { + reviewer, + env: { ...stripped, ...Object.fromEntries(credentials.map((name) => [name, env[name]])) }, + unauthorized: `${harness} rejected ${names(credentials)}; replace it, or run \`${reviewer.login}\``, + }; +} + +/** Run one reviewer turn, failing on any error it reports, and as logged out on an authentication error. */ +async function turn(cli: Cli, call: Turn, cwd: string): Promise { + const reader = cli.reviewer.reader(); + const { code, stderr } = await run(call.argv, cwd, { ...cli.env, ...call.env }, call.stdin, reader); const reply = reader.reply(); if (code !== 0 || reply.error !== undefined) { + // A login can expire or be revoked while the status check still passes. A CLI may report it + // only on stderr, behind a reader's fallback error such as "claude printed no result". + if (reply.status === 401 || cli.reviewer.unauthorized.test(`${reply.error ?? ""}\n${stderr}`)) { + throw new ReviewInputError(cli.unauthorized); + } const detail = reply.error ?? stderr.trim().split("\n").slice(-5).join("\n"); throw new Error(`${call.argv[0]} failed${detail ? `: ${detail}` : ` with exit status ${code}`}`); } @@ -654,9 +741,7 @@ export async function review(options: ReviewOptions, deps: ReviewDependencies): ); } - const env = strippedEnvironment(deps.env); - const status = await run(reviewer.status, root, env, ""); - if (status.code !== 0) throw new ReviewInputError(`${harness} is not logged in; run \`${reviewer.login}\``); + const cli = await prepare(harness, root, deps.env); // The reviewer's sandbox has no network, so the base is fetched first. const baseRef = options.base ?? `origin/${defaultBranch(root, "origin")}`; @@ -690,12 +775,12 @@ export async function review(options: ReviewOptions, deps: ReviewDependencies): // A resumed session would keep instructions the base or the change has since replaced. const resume = previous !== undefined && state?.given.instructions === given.instructions; if (!resume) { - const first = await turn(reviewer, reviewer.review(context, randomUUID()), root, env); + const first = await turn(cli, reviewer.review(context, randomUUID()), root); if (first.session === undefined) throw new Error(`${harness} did not report its session`); session = first.session; if (reported(first.usage)) usage.push({ turn: "review", ...first.usage }); const prompt = firstPrompt(head, rules, options.responses); - reply = await turn(reviewer, reviewer.resume(context, session, prompt), root, env); + reply = await turn(cli, reviewer.resume(context, session, prompt), root); } else { session = state!.session; const prompt = laterPrompt({ @@ -706,7 +791,7 @@ export async function review(options: ReviewOptions, deps: ReviewDependencies): rules, rulesRemoved: state!.given.rules !== null, }); - reply = await turn(reviewer, reviewer.resume(context, session, prompt), root, env); + reply = await turn(cli, reviewer.resume(context, session, prompt), root); } if (reported(reply.usage)) usage.push({ turn: "findings", ...reply.usage }); const found = findings(reply); diff --git a/test/review.test.ts b/test/review.test.ts index 04b6a22e..c37037e6 100644 --- a/test/review.test.ts +++ b/test/review.test.ts @@ -18,19 +18,37 @@ const FINDING = { file: "src/lib.ts", line: 2, severity: "high", title: "Off by /** * Fake reviewer CLIs. Each logs its argv, stdin, and the variables it was - * given, then answers as the FAKE_* variables say: FAKE_LOGGED_OUT fails the - * status check, and FAKE_REPLY replaces the findings reply. + * given, then answers as the FAKE_* variables say. FAKE_LOGGED_OUT removes the + * stored login, so only a credential variable the real CLI reads + * authenticates; like the real ones, `claude auth status` counts such a + * variable and `codex login status` does not. FAKE_REJECTED has the API + * reject the login in each turn with the real CLI's error, in the style it + * names. FAKE_REPLY replaces the findings reply. */ const FAKES: Record = { claude: ` const fs = require("node:fs"); const argv = process.argv.slice(2); const stdin = fs.readFileSync(0, "utf8"); -const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => /^(CLAUDE|CODEX|FAKE_HOST|PATH$)/.test(k))); +const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => /^(CLAUDE|CODEX|ANTHROPIC|FAKE_HOST|PATH$)/.test(k))); fs.appendFileSync(process.env.FAKE_LOG, JSON.stringify({ argv, stdin, env, cwd: process.cwd() }) + "\\n"); -if (argv[0] === "auth") process.exit(process.env.FAKE_LOGGED_OUT ? 1 : 0); +const credential = ["ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"].some((k) => process.env[k]); +const loggedIn = !process.env.FAKE_LOGGED_OUT || credential; +if (argv[0] === "auth") process.exit(loggedIn ? 0 : 1); const at = (flag) => argv[argv.indexOf(flag) + 1]; const result = (fields) => console.log(JSON.stringify({ type: "result", is_error: false, ...fields })); +const rejected = process.env.FAKE_REJECTED ?? (loggedIn ? undefined : "status"); +if (rejected) { + const session_id = at("--session-id") ?? at("--resume"); + if (rejected === "status") { + result({ session_id, is_error: true, api_error_status: 401, result: "Failed to authenticate. API Error: 401 Invalid bearer token" }); + } else if (rejected === "stderr") { + console.error("Login expired · Please run /login"); + } else { + result({ session_id, is_error: true, result: "OAuth token revoked · Please run /login" }); + } + process.exit(1); +} const usage = { input_tokens: 0, cache_read_input_tokens: 0, cache_creation_input_tokens: 0, output_tokens: 0 }; if (argv.includes("--session-id")) { result({ @@ -57,10 +75,23 @@ if (argv.includes("--session-id")) { const fs = require("node:fs"); const argv = process.argv.slice(2); const stdin = fs.readFileSync(0, "utf8"); -const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => /^(CLAUDE|CODEX|FAKE_HOST|PATH$)/.test(k))); +const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => /^(CLAUDE|CODEX|ANTHROPIC|FAKE_HOST|PATH$)/.test(k))); fs.appendFileSync(process.env.FAKE_LOG, JSON.stringify({ argv, stdin, env, cwd: process.cwd() }) + "\\n"); if (argv[0] === "login") process.exit(process.env.FAKE_LOGGED_OUT ? 1 : 0); const event = (value) => console.log(JSON.stringify(value)); +const loggedIn = !process.env.FAKE_LOGGED_OUT || process.env.CODEX_API_KEY; +const rejected = process.env.FAKE_REJECTED ?? (loggedIn ? undefined : "turn"); +if (rejected === "turn") { + const message = "unexpected status 401 Unauthorized: Missing bearer or basic authentication in header, url: https://api.openai.com/v1/responses"; + event({ type: "thread.started", thread_id: "thread-1" }); + event({ type: "error", message: "Reconnecting... 1/5 (" + message + ")" }); + event({ type: "turn.failed", error: { message } }); + process.exit(1); +} +if (rejected) { + console.error("Error: Your access token could not be refreshed because your refresh token was revoked. Please log out and sign in again."); + process.exit(1); +} const message = (text) => event({ type: "item.completed", item: { type: "agent_message", text } }); const zero = { input_tokens: 0, cached_input_tokens: 0, output_tokens: 0 }; if (argv[1] === "review") { @@ -93,6 +124,18 @@ interface Setup { calls(): { argv: string[]; stdin: string; env: Record; cwd: string }[]; } +/** Credentials and reroutes the running session may hold, cleared so each test sets its own. */ +const HOST_CREDENTIALS = [ + "ANTHROPIC_API_KEY", + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_BASE_URL", + "CLAUDE_CODE_OAUTH_TOKEN", + "CLAUDE_CODE_USE_BEDROCK", + "CLAUDE_CODE_USE_VERTEX", + "CLAUDE_CODE_USE_FOUNDRY", + "CODEX_API_KEY", +]; + /** * A clone whose origin's main holds instructions and review rules, checked out * on a branch that edits its own AGENTS.md and a file in `src/`. @@ -127,8 +170,11 @@ function setup(host: "CLAUDECODE" | "CODEX_THREAD_ID" | null = "CLAUDECODE"): Se const log = join(dir, "calls.log"); vi.stubEnv("PATH", `${bin}:${process.env.PATH}`); vi.stubEnv("FAKE_LOG", log); - vi.stubEnv("CLAUDECODE", undefined); - vi.stubEnv("CODEX_THREAD_ID", undefined); + // The session's own profile would change the logged-out errors. + const unset = [...HOST_CREDENTIALS, "CLAUDECODE", "CODEX_THREAD_ID", "CLAUDE_CONFIG_DIR", "CODEX_HOME"]; + for (const name of [...unset, "FAKE_LOGGED_OUT", "FAKE_REJECTED"]) { + vi.stubEnv(name, undefined); + } if (host === "CLAUDECODE") vi.stubEnv("CLAUDECODE", "1"); if (host === "CODEX_THREAD_ID") vi.stubEnv("CODEX_THREAD_ID", "parent"); return { @@ -189,10 +235,157 @@ fakes("a missing or logged-out reviewer CLI is a usage error", async () => { vi.stubEnv("FAKE_LOGGED_OUT", "1"); const out = await quest("--root", t.repo, "review"); expect(out.code).toBe(2); - expect(out.stderr).toContain("codex is not logged in; run `codex login`"); + expect(out.stderr).toContain( + "codex is not logged in to its default profile (CODEX_HOME is unset); run `codex login`", + ); expect(t.calls().map((c) => c.argv)).toEqual([["login", "status"]]); }); +fakes("a logged-out error names the profile its status check read", async () => { + const codex = "codex is not logged in to the profile CODEX_HOME=/work; run `codex login`"; + const claude = "claude is not logged in to the profile CLAUDE_CONFIG_DIR=/work; run `claude auth login`"; + // Signed out at the status check, and rejected by the API after it passed. + for (const [host, variable, flag, value, message] of [ + ["CLAUDECODE", "CODEX_HOME", "FAKE_LOGGED_OUT", "1", codex], + ["CLAUDECODE", "CODEX_HOME", "FAKE_REJECTED", "turn", codex], + ["CODEX_THREAD_ID", "CLAUDE_CONFIG_DIR", "FAKE_LOGGED_OUT", "1", claude], + ["CODEX_THREAD_ID", "CLAUDE_CONFIG_DIR", "FAKE_REJECTED", "status", claude], + ] as const) { + const s = setup(host); + vi.stubEnv(variable, "/work"); + vi.stubEnv(flag, value); + const result = await quest("--root", s.repo, "review"); + expect(result.code, result.stderr).toBe(2); + expect(result.stderr).toContain(message); + } +}); + +fakes("a login the API rejects fails as logged out, whatever the status check said", async () => { + for (const [host, variant, message] of [ + [ + "CLAUDECODE", + "turn", + "codex is not logged in to its default profile (CODEX_HOME is unset); run `codex login`", + ], + [ + "CLAUDECODE", + "stderr", + "codex is not logged in to its default profile (CODEX_HOME is unset); run `codex login`", + ], + [ + "CODEX_THREAD_ID", + "status", + "claude is not logged in to its default profile (CLAUDE_CONFIG_DIR is unset); run `claude auth login`", + ], + [ + "CODEX_THREAD_ID", + "text", + "claude is not logged in to its default profile (CLAUDE_CONFIG_DIR is unset); run `claude auth login`", + ], + [ + "CODEX_THREAD_ID", + "stderr", + "claude is not logged in to its default profile (CLAUDE_CONFIG_DIR is unset); run `claude auth login`", + ], + ] as const) { + const s = setup(host); + vi.stubEnv("FAKE_REJECTED", variant); + const result = await quest("--root", s.repo, "review"); + expect(result.stderr, variant).toContain(message); + expect(result.code, variant).toBe(2); + // The status check passed, and the first turn failed. + expect(s.calls(), variant).toHaveLength(2); + } +}); + +fakes("a reviewer with no stored login reviews with the credential its CLI reads from the environment", async () => { + const s = setup(); + vi.stubEnv("FAKE_LOGGED_OUT", "1"); + vi.stubEnv("CODEX_API_KEY", "codex-key"); + vi.stubEnv("CLAUDE_CODE_OAUTH_TOKEN", "session-token"); + const codex = await quest("--root", s.repo, "review"); + expect(codex.stderr).toBe(""); + expect(codex.code).toBe(0); + for (const call of s.calls().slice(1)) { + expect(call.env.CODEX_API_KEY).toBe("codex-key"); + // The other harness's credentials and the session's own variables still never reach it. + expect(call.env.CLAUDE_CODE_OAUTH_TOKEN).toBeUndefined(); + expect(call.env.CLAUDECODE).toBeUndefined(); + } + + const t = setup("CODEX_THREAD_ID"); + vi.stubEnv("FAKE_LOGGED_OUT", "1"); + vi.stubEnv("ANTHROPIC_API_KEY", "anthropic-key"); + vi.stubEnv("CODEX_API_KEY", "codex-key"); + vi.stubEnv("CLAUDE_CODE_ENTRYPOINT", "sdk-ts"); + const claude = await quest("--root", t.repo, "review"); + expect(claude.stderr).toBe(""); + expect(claude.code).toBe(0); + for (const call of t.calls().slice(1)) { + expect(call.env.ANTHROPIC_API_KEY).toBe("anthropic-key"); + expect(call.env.CLAUDE_CODE_ENTRYPOINT).toBeUndefined(); + expect(call.env.CODEX_API_KEY).toBeUndefined(); + expect(call.env.CODEX_THREAD_ID).toBeUndefined(); + } +}); + +fakes("a stored login wins over an environment credential, so a shell's key never changes the billing", async () => { + const s = setup("CODEX_THREAD_ID"); + vi.stubEnv("ANTHROPIC_API_KEY", "anthropic-key"); + expect((await quest("--root", s.repo, "review")).code).toBe(0); + for (const call of s.calls()) expect(call.env.ANTHROPIC_API_KEY).toBeUndefined(); +}); + +fakes("an environment credential the API rejects is named in the error", async () => { + const s = setup(); + vi.stubEnv("FAKE_LOGGED_OUT", "1"); + vi.stubEnv("CODEX_API_KEY", "codex-key"); + vi.stubEnv("FAKE_REJECTED", "turn"); + const result = await quest("--root", s.repo, "review"); + expect(result.code).toBe(2); + expect(result.stderr).toContain("codex rejected CODEX_API_KEY; replace it, or run `codex login`"); +}); + +fakes("a credential meant for another endpoint stays stripped, and the error names what reroutes it", async () => { + const s = setup("CODEX_THREAD_ID"); + vi.stubEnv("FAKE_LOGGED_OUT", "1"); + vi.stubEnv("ANTHROPIC_AUTH_TOKEN", "gateway-token"); + vi.stubEnv("ANTHROPIC_BASE_URL", "https://gateway.example"); + const result = await quest("--root", s.repo, "review"); + expect(result.code).toBe(2); + expect(result.stderr).toContain( + "claude is not logged in to its default profile (CLAUDE_CONFIG_DIR is unset), and quest review strips ANTHROPIC_BASE_URL, which would reroute it; run `claude auth login`", + ); + expect(s.calls().map((c) => c.argv)).toEqual([["auth", "status"]]); +}); + +fakes.each(["0", "false", ""])("a provider switch set to %j reroutes nothing", async (value) => { + const s = setup("CODEX_THREAD_ID"); + vi.stubEnv("FAKE_LOGGED_OUT", "1"); + vi.stubEnv("ANTHROPIC_API_KEY", "anthropic-key"); + vi.stubEnv("CLAUDE_CODE_USE_BEDROCK", value); + vi.stubEnv("CLAUDE_CODE_USE_VERTEX", value); + const result = await quest("--root", s.repo, "review"); + expect(result.code, result.stderr).toBe(0); + const turns = s.calls().filter((c) => c.argv[0] !== "auth"); + expect(turns.length).toBeGreaterThan(0); + for (const call of turns) { + expect(call.env.ANTHROPIC_API_KEY).toBe("anthropic-key"); + expect(call.env.CLAUDE_CODE_USE_BEDROCK).toBeUndefined(); + } +}); + +fakes("an enabled provider switch withholds the credential, and the error names it", async () => { + const s = setup("CODEX_THREAD_ID"); + vi.stubEnv("FAKE_LOGGED_OUT", "1"); + vi.stubEnv("ANTHROPIC_API_KEY", "anthropic-key"); + vi.stubEnv("CLAUDE_CODE_USE_VERTEX", "True"); + const result = await quest("--root", s.repo, "review"); + expect(result.code).toBe(2); + expect(result.stderr).toContain("quest review strips CLAUDE_CODE_USE_VERTEX, which would reroute it"); + expect(s.calls().map((c) => c.argv)).toEqual([["auth", "status"]]); +}); + fakes("the reviewer CLI is found on PATH, never in the checkout it reviews", async () => { for (const host of ["CLAUDECODE", "CODEX_THREAD_ID"] as const) { const s = setup(host);