alibaba / alibaba/open-code-review
Support ChatGPT/Codex subscription auth for local reviews (measured contract; follow-up to #275)
- Dominant language
- Go
- Stars
- 24.4k
- Forks
- 1.8k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 105
Description
## Problem Statement
OpenCodeReview currently requires an API key for every provider. Developers who already
pay for a ChatGPT plan cannot point `ocr review` at that subscription, so using OCR
locally means a second, metered provider account on top of a subscription they already
have.
#275 asked for this and concluded it was not possible. That thread ends with:
> A true Codex API transport (OCR calling the Codex session's model directly) doesn't
> look feasible today — unlike `ANTHROPIC_BASE_URL`/`ANTHROPIC_AUTH_TOKEN`, a Codex
> subscription session exposes no reusable endpoint+token pair to point OCR at.
That conclusion is incorrect, and this issue exists to show the measurements that
disprove it and to propose a design. #585 asks for the same capability more generally.
Delegation mode (#383) solves an adjacent problem well: it lets a host agent do the
reviewing with no LLM configured on the OCR side. It does not help someone who wants
`ocr review` itself to run against their ChatGPT plan, which is what this asks for.
## The endpoint contract, measured
A Codex subscription does expose a reusable endpoint and token pair. The Codex CLI
writes `~/.codex/auth.json` containing `tokens.access_token`, `tokens.refresh_token`,
`tokens.id_token` and `tokens.account_id`, and
`POST https://chatgpt.com/backend-api/codex/responses` accepts that access token as a
bearer credential.
Measured against the live endpoint on 2026-08-28 with a ChatGPT plan token:
| Request property | Result |
|---|---|
| `stream` omitted or `false` | `400 {"detail":"Stream must be set to true"}` |
| `stream: true` | `200`, SSE |
| `store: true` or omitted | `400 {"detail":"Store must be set to false"}` |
| `store: false` | `200` |
| `temperature` present | `400 {"detail":"Unsupported parameter: temperature"}` |
| `max_output_tokens` present | `400 {"detail":"Unsupported parameter: max_output_tokens"}` |
| `reasoning: {effort: "high"}` | `200` |
| `include: ["reasoning.encrypted_content"]` | `200` |
| `text.format` `json_schema` | `200` |
| function tools | `200`, emits `function_call` items |
| Omit `chatgpt-account-id` / `OpenAI-Beta` / `originator` / `User-Agent` | `200` for each |
| Refresh at `auth.openai.com/oauth/token` | `200`, `expires_in=864000` (10 days), refresh token rotates |
Model availability is account-scoped and narrower than the API:
| Model | Result |
|---|---|
| `gpt-5.6-luna` | `200` |
| `gpt-5.6-terra` | `200` |
| `gpt-5.6-sol` | `400 "not supported when using Codex with a ChatGPT account"` |
| `gpt-5.1-codex`, `gpt-5.6-codex`, `codex-mini-latest`, `gpt-4o` | same 400 |
Two results are worth calling out because they contradict what one would reasonably
assume:
- **Every custom header is optional.** `chatgpt-account-id`, `OpenAI-Beta`,
`originator` and `User-Agent` can all be omitted and the request still succeeds. The
reserved-header list in `internal/llm/resolver.go` is therefore not an obstacle.
- **The access token is valid for ten days**, not one hour. Refresh is a robustness
concern rather than a correctness one.
## What actually blocks it
Not authentication. `internal/llm/responses_client.go` is non-streaming by
construction and deliberately drops `stream` from `extra_body`:
```go
// forwarding it here makes the API answer with SSE and every call fails to decode.
// Drop the key rather than forward it.
if k == "stream" {
continue
}
```
Codex requires `stream: true`. So this needs a streaming Responses path, which is the
bulk of the work. `Store: openai.Bool(false)` and
`Include: [ResponseIncludableReasoningEncryptedContent]` are already exactly right.
One measured subtlety that matters for #1070: Codex's `response.completed` event
carries `status` and `usage` but an **empty `output` array**. Output has to be
accumulated from `response.output_item.done`, which carries each whole item including
`reasoning` items with `encrypted_content`. Reading the final response instead would
silently drop reasoning and degrade multi-turn tool loops under `store: false`, with no
error. That is exactly the configuration #1070 was written for.
## Proposed Solution
Local only. Never the GitHub Action.
**A `codex` provider preset**, `Protocol: openai-responses`,
`BaseURL: https://chatgpt.com/backend-api/codex`, models `gpt-5.6-luna` and
`gpt-5.6-terra`. No new protocol constant; only the transport and credential source
differ.
**A new `Provider.ExternalAuth` flag.** A preset with no `EnvVar` and no `AmbientAuth`
cannot be configured today: `apiKeyStepCanConfirm` in `provider_tui.go`,
`checkAPIKeyRequirement` in `provider_cmd.go`, and the TUI step-skip logic all refuse
it. `ExternalAuth` would short-circuit the key requirement at those three sites only.
Reusing `AmbientAuth` would be wrong: it also disables the `--model` allowlist and
suppresses `api_key_cmd`, and the allowlist matters here because the model list is
short and account-scoped.
**A credential branch in `tryProviderConfig`**, between the `api_key`/`api_key_cmd`
selection and the missing-credential error, loading the cached token into `apiKey`.
That satisfies both the credential check and the completeness gate with no relaxation
of either.
**A streaming path in the Responses client**, gated on the provider so every other
`openai-responses` provider keeps the existing non-streaming behaviour. Accumulate from
`response.output_item.done`, sort by `output_index` (reasoning items must precede the
`function_call` they belong to for replay to be valid), and rebuild a real
`responses.Response` via the exported `UnmarshalJSON` so `RawJSON()` is populated and
usage extraction takes the same path as every other provider.
Three failure modes the loop must not swallow, all measured against the pinned SDK:
1. A mid-stream `{"type":"error"}` event leaves `stream.Err()` nil, because
`ssestream` only sets it for a top-level `error` key.
2. A truncated stream leaves `status` empty, which `mapResponsesFinishReason` maps
through `default:` to `"stop"`, reporting a clean finish.
3. The terminal-status guard that raises errors for failed/cancelled/queued responses
is bypassed entirely unless it is extracted and shared.
**Suppress `temperature` and `max_output_tokens`** for this provider only; both now
hard-400.
**Error legibility.** Codex emits two error shapes: `{"detail":"..."}` for gateway
rejections and `{"error":{...}}` for upstream errors. The SDK extracts only
`gjson.GetBytes(contents,"error").Raw`, so every gateway rejection in the table above
currently surfaces as a bare `400 Bad Request`. A small middleware rewriting `detail`
into the `error` shape before SDK parsing makes the three most likely first-run
failures diagnosable.
**An `ocr auth` command group:**
```
ocr auth login # PKCE + loopback, opens a browser
ocr auth login --device # device code, for headless or remote shells
ocr auth login --no-browser # print the authorize URL to paste
ocr auth status # masked account, plan, expiry
ocr auth logout # clear local token, best-effort revoke
```
Both flows exist in the official Codex CLI and its constants are confirmed working:
issuer `https://auth.openai.com`, `client_id app_EMoamEEZ73f0CkXaXp7hrann`, loopback
`http://localhost:1455/auth/callback` with S256 PKCE, device code via
`/deviceauth/callback`, scopes
`openid profile email offline_access api.connectors.read api.connectors.invoke`.
The loopback listener should reuse the Host-header guard from
`internal/viewer/server.go`, since a callback carrying an authorization code is a worse
DNS-rebinding target than the viewer. Login must not run inside the bubbletea TUI, for
the stdin-contention reason documented in `internal/llm/keycmd.go`.
**Token storage** at `~/.opencodereview/auth/codex.json`, mode `0600` in a `0700`
directory. The `auth/` subdirectory matters: `saveConfig` creates
`~/.opencodereview` at `0755` and `os.MkdirAll` does not chmod an existing directory,
so a `0700` claim on the parent would silently not hold. The write must be atomic
(temp file, chmod, rename) because **refresh rotates the refresh token** and a torn
write strands the user with neither credential. There is no atomic-write precedent in
the tree today, so this is new code that needs its own tests.
Behind a small storage interface, so an OS keyring backend can be added later without
touching the resolver. That is what #236 asked for, and the official Codex CLI already
stores its own credentials through a keyring backend.
## Keeping it out of CI
The preset carries no `EnvVar`, so `action.yml` (whose entire credential surface is
`OCR_LLM_*`) cannot supply this credential, and `ocr auth` is the only writer of the
token file.
Stated honestly: that is a convention, not a structural guarantee. Nothing prevents a
workflow restoring the token file from `actions/cache`. Removing the environment
variable removes the easy path, not every path.
## Alternatives Considered
- **Delegation mode (#383).** Solves a different problem well. It does not let
`ocr review` itself use the subscription.
- **A local proxy.** Works today but requires the user to run and maintain a second
process, and gets none of the parameter suppression above, so it fails on
`temperature` and `max_output_tokens` unless the proxy strips them.
- **Reusing `AmbientAuth`.** Rejected: it disables the model allowlist and suppresses
`api_key_cmd`, neither of which is wanted.
- **A fifth protocol constant.** Rejected: `openai-responses` is the correct wire
protocol, and a new constant would touch three validation lists and a docs table in
five locales for no semantic gain.
## Affected Area
Review Agent / LLM interaction, Configuration
## Acceptance criteria
- A developer can run `ocr auth login`, then `ocr review` against their ChatGPT plan
with no API key configured.
- Reviews work at `--effort high` with reasoning preserved across tool-call turns.
- Gateway errors surface with their `detail` text rather than a bare 400.
- Partial or failed streams surface as errors, never as a clean `stop`.
- The GitHub Action cannot use this credential path.
- Existing `openai-responses` providers are behaviourally unchanged.
## Additional Context
I have a working design document covering the above with file-level detail, and I would
like to implement this. Happy to split it: the streaming Responses path is independently
useful and could land before the auth work.
Could this be assigned to me (@acoliver)?
Contributor guide
Research direction
Start with internal/llm/responses_client.go, then trace provider_tui.go, provider_cmd.go, and tryProviderConfig for credential handling. Read internal/viewer/server.go and internal/llm/keycmd.go before designing the auth flow and storage under ~/.opencodereview/auth. Done means local Codex login and review work with preserved reasoning, clear stream errors, unchanged existing providers, and no GitHub Action credential path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authentication, backend, cli, developer-experience
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 32/100