Skip to content

fix(review): preserve reviewer connection credentials - #316

Closed
kixelated wants to merge 1 commit into
mainfrom
fix/review-provider-environment
Closed

kixelated wants to merge 1 commit into
mainfrom
fix/review-provider-environment

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

quest review reused the managed-agent environment scrubber, removing Claude environment credentials and gateway endpoints before checking login. CLIProxy users were reported as logged out. Codex's exec-only API key also failed the stored-login check.

Approach

Restore only the selected reviewer's connection variables for the login check and every review turn. Skip Codex's stored-login check when CODEX_API_KEY is supplied, since that key is accepted by exec/review only. Keep managed-agent account isolation and reviewer configuration restrictions intact.

Narrow the existing review-login quest to its remaining expired/revoked-login handling. The user's explicit request to route reviews through CLIProxy supersedes its older instruction to strip endpoint overrides.

Impact

  • Reviews honor the selected CLI's environment credentials and provider endpoint. No new flags, exported API, or wire format changes.

Alternatives

A PATH wrapper proved the gateway works but leaves Quest's environment bug in place. Passing every provider variable through would also restore plugin, permission, and model overrides.

Validation

  • The new environment-authentication cases reproduced the old failure before the fix.
  • All 22 review tests pass. The review and managed-agent suites passed together (46 passed, 1 skipped) before the four cloud-routing cases were added; those additional cases pass in the 22-test review suite.
  • nix develop --command bun run check passes (CLI/core TypeScript and repository formatting).
  • Live quest review --harness claude through CLIProxy with Claude Opus 5.5 passes the initial review and structured-findings turns using the fixed source, ordinary environment variables, and the unmodified Claude executable. This smoke test used an empty diff, not this patch.

Validation pending

nix develop --command just check and just test remained queued by quest gate for over thirteen minutes without being admitted. They were cancelled rather than bypassing host admission. Full local validation and the Claude review of this committed patch are still required before this draft is ready.

Follow-ups

Expired/revoked-login error handling remains in quest/m0/review-login.md.

(Written by GPT-6.1 Sol)

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Closing in favor of moq-dev/quest#339, which is now queued. That PR covers the Codex part of this fix: when codex login status fails, it passes CODEX_API_KEY and the review goes ahead. It also deletes the review-login quest that this PR narrowed.

This PR also routes reviews through a gateway or another provider: it keeps ANTHROPIC_BASE_URL, ANTHROPIC_CUSTOM_HEADERS, the Bedrock, Vertex and Foundry settings, and CODEX_ACCESS_TOKEN and CODEX_CA_CERTIFICATE. #339 still strips all of these. The maintainer chose on 2026-11-01 to plan that as its own follow-up quest, reusing this PR's tests. The branch stays available for it.

(Written by Claude Opus 5.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant