Repository navigation
feat(review): report rejected logins as logged out, and review with environment credentials - #339
Merged
Merged
Conversation
Quest-Claim: 819bc587da592102
…nvironment credentials A reviewer turn that fails on authentication (a 401, or an expired or revoked token the status check missed) now raises the same "not logged in" usage error as a signed-out CLI. A reviewer CLI with no stored login keeps the credential it reads from the environment, unless a stripped variable would send it to another endpoint, which the error then names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…derr Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
marked this pull request as ready for review
November 1, 2026 06:19
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
Claude Code turns CLAUDE_CODE_USE_BEDROCK, _VERTEX, and _FOUNDRY on only for 1, true, yes, or on, so a disabled switch no longer withholds an environment credential. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Landing
(Written by Claude Opus 5.5) |
kixelated
enabled auto-merge (squash)
November 1, 2026 21:15
kixelated
added a commit
that referenced
this pull request
Nov 1, 2026
#339 finishes and deletes review-login, including reviewing with credentials from the environment. Narrowing it here would conflict with that deletion, and the gateway quest would plan that work twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
moq-bot Bot
pushed a commit
that referenced
this pull request
Nov 1, 2026
* quest: plan the 2026-11-01 follow-ups (wip) * quest: plan the 2026-11-01 follow-ups Fifteen follow-ups the maintainer settled in a /quest-plan interview: land work (fixer TLS, squash-mismatch comment, landing status), CLI and agent fixes, runner review checks, and two m1 quests. Also brings existing quests in line with #349, #276's close, validateFix, #246, and #341. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * quest: leave review-login to #339 and build the gateway quest on it #339 finishes and deletes review-login, including reviewing with credentials from the environment. Narrowing it here would conflict with that deletion, and the gateway quest would plan that work twice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: moq-bot <moq-bot@users.noreply.github.com>
Contributor
|
quest land removed this pull request from the queue: GitHub refused /repos/moq-dev/quest/pulls/339: Pull Request has merge conflicts; the rest of the batch was not merged. |
# Conflicts: # test/review.test.ts
The quest's Goal asks the "not logged in" error to name the Claude profile its status check read. Every logged-out error, at the status check, after an API rejection, and with a stripped reroute, now names CLAUDE_CONFIG_DIR, or CODEX_HOME for Codex, or says the default profile when unset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
enabled auto-merge (squash)
November 2, 2026 02:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements and deletes
quest/m0/review-login.md.Summary
Rejected logins fail as logged out. When a reviewer turn fails on authentication,
quest reviewnow raises the same usage error (exit 2) as a signed-out CLI,<harness> is not logged in; run \`, in place of a raw 401. It doesn't send an extra probe request. This covers the case wherecodex login status` exits 0 on a revoked token.claude -preportsapi_error_status: 401on its result, and the check reads that field. As a fallback it also matches the result text:Failed to authenticate,API Error: 401,Invalid API key,Please run /login.401 Unauthorizedorsign in againin theturn.failedmessage. When no turn ran, it checks stderr instead, which is where a failed token refresh is reported (Your access token could not be refreshed because your refresh token was revoked. Please log out and sign in again.).Environment credentials. The status check still runs with
strippedEnvironment. If it fails, the reviewer keeps the credential its CLI reads from the environment and the review goes ahead:ANTHROPIC_API_KEY,ANTHROPIC_AUTH_TOKEN,CLAUDE_CODE_OAUTH_TOKENCODEX_API_KEYIf that credential is also rejected, the error names it:
codex rejected CODEX_API_KEY; replace it, or run \codex login``.Rerouting variables stay stripped.
ANTHROPIC_BASE_URL,CLAUDE_CODE_USE_BEDROCK,CLAUDE_CODE_USE_VERTEXandCLAUDE_CODE_USE_FOUNDRYare still never passed. While one of them is set, no Claude credential passes either, since it belongs to that other endpoint. The error names the variable instead of just saying "not logged in":claude is not logged in, and quest review strips ANTHROPIC_BASE_URL, which would reroute it; run \claude auth login``.Logged-out errors name the profile checked. Each one names the stored login the status check read, such as
claude is not logged in to the profile CLAUDE_CONFIG_DIR=/home/me/.claude-personal; run \claude auth login`, or... to its default profile (CLAUDE_CONFIG_DIR is unset). That covers the failed status check, a rejection after a passing check, and a stripped reroute. Codex namesCODEX_HOME` the same way.README and
quest review --helpdescribe the behavior.Evidence from the real CLIs
I probed both CLIs with bogus credentials in throwaway config dirs (Claude Code 2.1.316, codex-cli 0.165.0):
claude -p --output-format jsonwith a badANTHROPIC_API_KEYreturns{"is_error":true,"api_error_status":401,"result":"Invalid API key · Fix external API key"}. With a badCLAUDE_CODE_OAUTH_TOKENthe result text isFailed to authenticate. API Error: 401 Invalid bearer token.claude auth statusexits 0 when onlyANTHROPIC_API_KEY,ANTHROPIC_AUTH_TOKEN,CLAUDE_CODE_OAUTH_TOKEN, orCLAUDE_CODE_USE_BEDROCK/_VERTEXis set. So for Claude, stripping was the only cause of feat(gst): moqsrc takes a max-delay moq#7084 and fix(watch): accept structural read-side origins moq#7251.codex exec --jsonreadsCODEX_API_KEYand ignoresOPENAI_API_KEY: with onlyOPENAI_API_KEYset, the request goes out with no auth header.codex login statusprints "Not logged in" even whenCODEX_API_KEYis set. A failed request ends in{"type":"turn.failed","error":{"message":"unexpected status 401 Unauthorized: ..."}}.Choices made without asking
The quest's Plan left these to the implementer: "weigh that against why
STRIPPEDexists before choosing which names pass". Please check them when reviewing:ANTHROPIC_API_KEYfor other tools would otherwise move every review from a subscription login to API billing, and today's behavior for logged-in users stays exactly as it is. The other option is to always pass the credential and let the CLI's own order decide, which is closer to the Plan's wording but would change billing for users who have both.OPENAI_API_KEYstays stripped becausecodex execignores it. Harness markers such asCLAUDECODE,CLAUDE_CODE_ENTRYPOINTandCODEX_THREAD_IDstay stripped too, so the reviewer never acts as a sub-session.ANTHROPIC_BASE_URLand the provider switches stay stripped, and while one is set no credential passes, so a gateway token is never sent to api.anthropic.com.CLAUDE_CODE_USE_GATEWAY/_MANTLE/_ANTHROPIC_*also exist in the binary but aren't documented, so they are not named. The only effect of leaving them out is a less specific error message.<login>alone would not fix a bad variable. It doesn't name a profile, since no stored login was used.CLAUDE_CONFIG_DIR). Codex'sCODEX_HOMEselects its stored login in the same way and is kept bySTRIPPEDin the same way, so oneprofilefield per reviewer covers both and avoids a Claude-only branch.Test plan
test/review.test.ts: the fake CLIs gainedFAKE_LOGGED_OUT, where only a credential variable authenticates (the fakeclaude auth statuscounts it andcodex login statusdoesn't, like the real ones), andFAKE_REJECTED, which fails each turn with each CLI's real auth error in each style it uses. New tests:turn.failedand via stderrANTHROPIC_API_KEYANTHROPIC_BASE_URLwithholdsANTHROPIC_AUTH_TOKENand is named in the errorCLAUDE_CONFIG_DIRorCODEX_HOME, both when signed out and when rejected after a passing check (setup()unsets both, so the session's own profile can't change the expected messages)just checkandjust testpass atf9166c2(878 vitest tests plus the 432-test shell and lifecycle suites), run from a session withCLAUDE_CONFIG_DIRset.Merge from main
origin/mainat3c8cdce(never rebased). The one conflict was intest/review.test.ts, and both sides were kept. Each fake CLI's env filter now captures this PR'sANTHROPIC*and main'sPATH(moq-dev/quest#341). Main's new "reviewer CLI is found on PATH" test sits beside this PR's credential tests. Main sanitizes PATH insiderun(), so the status check and turns this PR adds get it too.4b89fb3was aquest landmerge of main into3f09da1, and its PR diff matches3f09da1's (only hunk offsets and surrounding context differ). The review below covers the whole branch at the new head.CLAUDE_CONFIG_DIRtest leak previously listed as a follow-up is fixed on main by moq-dev/quest#340, so it's dropped.Review
quest review), gpt-6.1-sol at medium effort, session01a19a18-98e5-77f1-a942-8b1c0eef506a, started fresh (--new) after mergingorigin/mainat3c8cdcewith a hand-resolved conflictf9166c2dfc642450a067d841f9cd5526e2057eaaagainstorigin/mainat3c8cdce: 0 findingse689a3d, the merge) reported 1 finding:src/cli/review.ts:463: the quest's Goal requires the "not logged in" error to name theCLAUDE_CONFIG_DIRprofile it checked (decided 2026-10-31), and the error didn't. Fixed inf9166c2: every logged-out error names the profile (CLAUDE_CONFIG_DIR, orCODEX_HOMEfor Codex) or the default when unset, with tests for both harnesses on both paths.01a198ee-48fe-7351-984a-6b82a7e92763(gpt-6.1-sol, medium) was clean on3f09da1after one fix (disabled provider switches counted as reroutes).01a195bd-4275-7441-98bb-779dc7963eda(gpt-6-astra,--deep) was clean on26a53ddafter one fix (stderr-only auth errors).🤖 Generated with Claude Code
(Written by Claude Opus 5.5)