feat(review): report rejected logins as logged out, and review with environment credentials

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

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

src/cli/main.ts

@@ -835,7 +835,11 @@ export function main(argv: string[], out: Output): number | Promise<number> {
 		.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 <effort> <base>...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<number> {
 				"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 <remote/branch>", "Base branch to review against (default: origin's default branch)")
 		.addOption(new Option("--harness <name>", "Reviewer to run instead of the other harness").choices(HARNESSES))

src/cli/review.ts

@@ -159,6 +159,8 @@ interface Reply {
 	structured?: unknown;
 	usage?: Omit<Usage, "turn">;
 	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<Reply> {
-	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<Cli> {
+	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<Reply> {
+	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);