feat(llm): support ChatGPT/Codex subscription auth for local reviews

[acoliver]

Summary

Adds local ChatGPT/Codex subscription authentication so ocr review can run against a
developer's ChatGPT plan instead of a metered API key. Fixes #1105.

This is local only. The GitHub Action cannot use this credential path.

The endpoint contract was measured directly rather than inferred. #275 concluded a
Codex subscription "exposes no reusable endpoint+token pair to point OCR at"; the
measurements in #1105 show otherwise, and this PR is the implementation.

What was measured

POST https://chatgpt.com/backend-api/codex/responses, with a ChatGPT plan token:

Request property	Result
stream omitted or false	400 {"detail":"Stream must be set to true"}
store: true or omitted	400 {"detail":"Store must be set to false"}
temperature	400 {"detail":"Unsupported parameter: temperature"}
max_output_tokens	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, refresh token rotates

Models are account-scoped. Which models a ChatGPT account can run depends on its plan
and changes over time, so the endpoint's refusal of a given model is a statement about
the account rather than about the API. gpt-5.6-sol, gpt-5.6-luna and gpt-5.6-terra
all return 200 on a Pro account. gpt-5.6-codex, gpt-5.1-codex and
codex-mini-latest are refused on the two accounts available to me, which I would not
generalise further.

Because that set cannot be known upstream, the preset's model list is a picker for
ocr config model and does not gate --model. This follows the resolver's existing
treatment of ambient-auth providers, where Bedrock identifiers are account-scoped for
the same reason.

Two results worth flagging because they contradict reasonable assumptions: every custom
header is optional, so the reserved-header list is not an obstacle; and the access token
lives ten days rather than an hour, so refresh is a robustness concern rather than a
correctness one.

Approach

Streaming Responses path. The existing client is non-streaming by construction and
deliberately drops stream from extra_body. Codex requires it, so this adds a
streaming path gated on the provider, leaving every other openai-responses provider
byte-for-byte unchanged.

Output is accumulated from response.output_item.done, not from the final response.
Codex sends response.completed with status and usage but an empty output
array, so reading it would produce an empty review. Items are sorted by output_index
because reasoning must precede the function_call it belongs to for #1070's replay to be
valid input. The accumulated items are spliced back into the terminal envelope and
re-unmarshalled so RawJSON() is populated and usage extraction takes the same path as
every other provider rather than silently falling back.

Three failure modes that would otherwise read as success are surfaced: a mid-stream
{"type":"error"} event (ssestream does not set stream.Err() for it, since it looks
only for a top-level error key), a truncated stream with no terminal event (which would
otherwise map to finish_reason: "stop"), and a non-terminal response status.

Error legibility. Codex emits two error shapes: {"detail":...} for gateway
rejections and {"error":{...}} for upstream errors. The SDK reads only the latter, so
every rejection in the table above surfaced as a bare 400 Bad Request. A middleware
rewrites the former into the shape the SDK reads.

Auth. ocr auth login (PKCE + loopback), --device (device code, for headless or
remote shells), --no-browser (paste the URL), plus status and logout. Constants
match the official Codex CLI. The loopback listener reuses the Host-header guard from
internal/viewer, since a callback carrying an authorization code is a worse
DNS-rebinding target than the viewer.

Storage. ~/.opencodereview/auth/codex.json, mode 0600 in a 0700 directory. The
auth/ subdirectory is deliberate: saveConfig creates the parent at 0755 and
os.MkdirAll does not chmod an existing directory, so a 0700 claim on the parent would
silently not hold. Writes are atomic (temp file, chmod, rename) because refresh rotates
the refresh token and a torn write would strand the user with neither credential. There
is no atomic-write precedent in the tree, so this is new code with its own tests.

Storage sits behind a small interface so an OS keyring backend is additive later, which
is what #236 asked for and what the official Codex CLI already does.

Not in scope

No new protocol constant: openai-responses is the correct wire protocol and only the
transport and credential source differ. No extra_headers relaxation, since every Codex
header turned out optional. No keyring in this PR.

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 rather than a structural guarantee:
nothing stops a workflow restoring the token file from actions/cache.

Test plan

• go build ./..., go vet ./..., gofmt -s -l clean
• make check (license headers, english-only, LF, go mod tidy)
• internal/llm and internal/codexauth suites pass
• Accumulator unit-tested directly on []ResponseStreamEventUnion, covering all
  three silent-failure modes and output_index ordering
• SSE fixture asserts encrypted_content survives into the NativeTurn replay
  payload, and that usage takes the raw-probe path (CacheWriteTokens), which fails
  if the splice degrades to the struct fallback
• Temperature and MaxOutputTokens asserted absent for codex, present for a normal
  openai-responses provider
• Token store: 0700/0600, chmod-before-rename, original survives a failed rename,
  with the Windows guard
• TestLookupProvider_CodexDetails per .opencodereview/rule.json
• Provider table and an ## ocr auth section added in all five locales
• Verified live end to end: ocr auth login completes a real PKCE sign-in
  (Signed in to ChatGPT account 79f9***bfe1), writes 0600 inside a 0700
  directory, and ocr auth status then reports the plan parsed from the id_token
  and exits 0
• ocr llm test succeeds on gpt-5.6-sol against chatgpt.com/backend-api/codex
  using the credential that login produced, and a full ocr review completes on it

The full suite passes with a clean HOME (HOME=$(mktemp -d) go test ./..., exit 0,
24 packages, no failures). Six tests in cmd/opencodereview and internal/config/rules
read the caller's real ~/.opencodereview/ when one exists, so a contributor with an
existing OCR configuration will see them fail locally while CI is green. That is
pre-existing and not touched here.

Review notes

This branch was reviewed by both Open Code Review and Kodus before submission; findings
from each are summarised in a comment below.

Happy to split this if you would prefer: the streaming Responses path is independently
useful and could land ahead of the auth work.

[CLAassistant]

All committers have signed the CLA.

[acoliver]

What the review tools found in this branch

This branch was reviewed by Open Code Review and by Kodus before submission. Recording
what each found, including where they were wrong, since it is relevant to judging the
change.

Open Code Review, reviewing itself

OCR reviewed this branch using the very feature it adds: gpt-5.6-luna through a
ChatGPT subscription, --effort high. Two passes, status: complete both times.

First pass, 11 findings. Eight were real and are fixed in 37ba23f:

Finding	Why it mattered
Refresh rejected a response omitting refresh_token	RFC 6749 §6 makes the field optional. A provider that does not rotate would have had its still-valid token discarded.
Zero or absent device-code interval	Produced a tight polling loop against the token endpoint. Now defaults to the RFC 8628 five seconds.
Polling could sleep past its own expiry	A slow_down-increased interval could exceed the remaining 15-minute lifetime.
Any local process could kill a login	A single request to the loopback callback with a bad state aborted a legitimate in-progress browser login. Now returns 400 and keeps listening.
ocr auth status exited non-zero when signed out	Which is the normal state on a fresh install.
Stale lock reclaimed without checking the owner	Locks now carry a PID and are only broken when the owner is gone.
expires_in overflow	A large value could produce an ExpiresAt in the past, refreshing on every request.

Three did not survive checking:

• A high-severity claim that os.Rename cannot replace an existing file on Windows,
  so every refresh after the first login would fail. Go's os.Rename calls
  MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING). It replaces.
• A claim that TestLookupProvider_CodexDetails was missing. It is at
  internal/llm/providers_test.go:188.
• A registry field-order complaint against an entry that already complies.

Second pass over the remediated branch, 9 findings. Three were real, fixed in
8f4299e:

• id_token was not carried across a refresh, so the account id and plan derived from it
  were blanked on every rotation. The first pass had fixed exactly this for
  refresh_token and missed its sibling.
• The gateway shaping flags gated on a protocol override but not a url override, while
  the credential guard checks both. An explicit api_key plus a redirected url would
  have applied Codex request shaping to a gateway that never asked for it.
• The callback bound the name localhost rather than 127.0.0.1, which a modified hosts
  file or resolver could point at a routable address.

The remaining six were repeats of findings already disproved in the first pass, one
deliberately documented limitation, one benign case, and one whose stated impact is
contradicted by measurement: it claimed the discarded AccountID means the "required"
chatgpt-account-id header cannot be sent, but that header is optional. Omitting it
returns 200, and ocr llm test succeeds without it.

Worth noting for its own sake: a stateless reviewer re-raises adjudicated findings on a
second pass. Three of nine second-pass findings were things the first pass had already
been shown to be wrong about.

Kodus, reviewing the companion branch

Kodus reviewed the parallel Kodus change (kodustech/kodus-ai#1806) and produced three
findings, none actionable.

Two of them, one root cause, claimed the credential compare-and-swap is key-order
sensitive because it builds its comparand with JSON.stringify. That would be a real
defect in the mechanism protecting the credential. The SQL is
"configValue" = :expected::jsonb, and the cast on that same line is what makes the
comparison order-insensitive. Checked against a live PostgreSQL:

SELECT '{"a":1,"b":{"x":1,"y":2}}'::jsonb = '{"b":{"y":2,"x":1},"a":1}'::jsonb;
 t

The third asked for path validation on an environment variable that names a credential
file. Declined: an attacker able to set environment variables in the worker already has
code execution, so the check defends a position that is already lost.

Reliability note

Every OCR run in this exercise returned status: complete when driven through a ChatGPT
subscription. In earlier work against a rate-limited API key, ten out of ten runs
returned partial, losing between 5 and 41 file bundles apiece. The bundle fan-out that
makes OCR fast needs concurrency headroom, and a subscription supplies it.

[acoliver]

Field results: reviewing unrelated pull requests through a subscription

To check this beyond self-review, I ran the built binary against two merged pull requests
in an unrelated repository (vybestack/llxprt-code), using gpt-5.6-luna through a
ChatGPT subscription at --effort high.

PR	Files	Result	Findings
#3390	2	status: complete	4
#3363	6	status: complete	6

The findings were substantive rather than stylistic. On #3390 it noted that the new
spawn() error handling only covers the synchronous throw and misses the asynchronous
error event, and that a descriptor close runs after the child already exists. On #3363,
a caching change, it observed that memoising a mutable FunctionDeclaration changes the
getter's observable contract, so a caller mutating the schema after the first call no
longer has that mutation observed, and that the same cache implementation is duplicated
across two validators so any future invalidation fix has to be made twice.

Why status: complete is the interesting part

Every run in this exercise returned complete. In earlier work driving the same tool
against a rate-limited API key, ten consecutive runs all returned partial, losing
between 5 and 41 file bundles each; one 50-file pull request produced zero findings
because 41 of its 47 bundles failed.

OCR's per-bundle fan-out is what makes it fast, and it needs concurrency headroom to
finish. A subscription supplies that headroom, which is a practical argument for this
feature beyond cost: the tool is materially more reliable when it is not competing with
itself for a single-concurrency key.

Runs took roughly 15 minutes each, comparable to an API key without contention.

[github-actions]

🔍 OpenCodeReview found 3 issue(s) in this PR.

• ✅ Successfully posted inline: 3 comment(s)

[acoliver]

End-to-end verification of the login flow

Earlier verification in this PR used a credential copied from an existing
~/.codex/auth.json, which exercised the resolver and the transport but never the login
code itself. That gap is now closed, and closing it is what surfaced the model bug
corrected in #1105.

ocr auth login --no-browser, real PKCE sign-in against auth.openai.com:

Open this URL to sign in:
https://auth.openai.com/oauth/authorize?...&code_challenge_method=S256&state=...
Signed in to ChatGPT account 79f9***bfe1.

The listener binds 127.0.0.1:1455 rather than resolving the localhost name:

ocr  14848  acoliver  4u  IPv4  TCP 127.0.0.1:1455 (LISTEN)

The credential lands with the intended permissions:

drwx------  .opencodereview/auth
-rw-------  .opencodereview/auth/codex.json

And ocr auth status now reports a field that the copied credential could not populate,
because a refresh-only token carries no id_token:

Account: 79f9***bfe1
Plan:    pro
Expires: 2026-09-07T19:54:15-03:00

That Plan: pro line read (unknown) under the copied credential. The id_token parsing
had never actually run until this sign-in.

Then, on the credential that login produced:

$ ocr llm test
Model:  gpt-5.6-sol
✓ Connection test successful

$ ocr review --from <base> --to <head> --format json
status: complete   model: gpt-5.6-sol   7 findings

Storage follows the existing layout rather than introducing a new one: Path() derives
from os.UserHomeDir() and returns ~/.opencodereview/auth/codex.json, alongside the
config.json and sessions/ the project already keeps there. The auth/ subdirectory
exists so its mode can be 0700, since saveConfig creates the parent at 0755 and
os.MkdirAll will not tighten an existing directory.

[acoliver]

Correction: the "pre-existing failures" note in this PR is wrong

The test plan says six tests fail on this checkout, unrelated to this branch: four
TestRunRulesCheck_ObjCSniff* in cmd/opencodereview, plus
TestNewResolver_FileFilterNilWhenEmpty and TestResolveDetail_SystemDefault in
internal/config/rules.

They are not failing tests. They fail only when the run inherits a developer HOME that
already contains a ~/.opencodereview/ directory, because they read the real user
configuration instead of an isolated one. Given a clean HOME, the whole suite passes:

$ HOME=$(mktemp -d) go test ./...
exit 0     24 packages ok     0 failures

I identified the cause correctly and then described the symptom as if the repository
were at fault. It was my environment. Please read the "Pre-existing failures on this
checkout" paragraph as withdrawn: this branch passes the full suite with no exceptions
to declare.

Worth noting separately, since it is a genuine (pre-existing) sharp edge rather than a
mistake in this PR: those tests depending on the caller's HOME means a contributor with
an existing OCR configuration sees failures that CI does not. That is not this branch's
to fix, but it cost me a wrong claim, and it may be worth a t.Setenv("HOME", t.TempDir())
in those tests.

[acoliver]

Review-thread dispositions (github-actions findings):

Fixed — device.go poll body (41d8de5): the token-poll request embeds server-derived values (device_auth_id, user_code), and %q fmt-escapes as Go, which can emit sequences that are not valid JSON. The body is now built with json.Marshal. The client_id site keeps %q — it is an ASCII package constant.

Declined, with reasons:

1. client_id %q → json.Marshal — ClientID is an ASCII compile-time constant; %q produces the exact same output with no error path to carry.
2. middleware "might drain the body" — rewriteDetailErrorMiddleware is the only middleware touching the body, it reads and restores it in one place, and SDK middleware ordering here is deterministic; there is no other actor that could drain it between read and rewrite.

[acoliver]

Final OCR review (10 findings) adjudicated against the code; 8 accepted and fixed in aa1e2d3, 2 declined:

Fixed (aa1e2d3):

• provider_tui.go: stale api-key state reported for ambient/external-auth providers — result() returned the previously active provider's saved key; state is now cleared on the skip path.
• auth logout: now holds the cross-process refresh lock, so a concurrent refresh can no longer save rotated credentials after the teardown's Clear.
• oauth.go lock: owner PID is read back after the exclusive create and release only removes a lock that still names this process, closing the two-waiter stale-lock race.
• device.go poll: transport failures retry on the poll interval (network blips no longer discard a live code), and one final poll happens after the capped sleep before declaring expiry (a completed authorization during the final sleep is no longer lost).
• LoginLoopback: the authorization URL is always printed; xdg-open can spawn successfully and still fail without an error.
• OAuth non-2xx: the allowlisted error code (invalid_grant etc.) is surfaced; free-text fields are never interpolated since a server echo could carry token material (TestExchangeCodeErrorsNeverContainTokens enforced this).
• auth status: "(expired; will refresh on next use)" when a refresh token exists.

Declined:

• Mid-run 401-triggered refresh (resolver.go:610): the measured token lifetime is expires_in=864000 (10 days) with a 5-minute refresh margin, so a review run cannot outlive an access token that was valid at resolution. The next command refreshes transparently. Adding a 401-retry layer would hedge against a scenario the live contract rules out.
• exchangeCode form-encoded vs refresh JSON (oauth.go:248): not a live defect. The device login run against auth.openai.com on this branch completed through the form-encoded exchange (the stored credential was produced by that flow), and the JSON refresh path also returns 200. The endpoint accepts both encodings.

Verified: go build, go vet, gofmt clean; internal/codexauth and the auth/TUI tests in cmd/opencodereview pass. The four TestRunRulesCheck_* failures are pre-existing on this machine's clean tree (rules-check binary resolution in the local environment) and do not appear in CI.

[acoliver]

Findings-only follow-up review of aa1e2d3 (2 medium findings, both fixed in 03fd38a):

• Stale-lock co-hold window (oauth.go): the read-back verification detects a stolen create, but the stale-break os.Remove further down was unconditional, so a waiter paused between its staleness check and the remove could still delete a freshly re-acquired lock. The stale-break now removes only when the lock content still matches the bytes observed before the liveness probe, and the comment states the residual single-read gap honestly (full closure needs flock/LockFileEx).
• Logout abort on lock-acquisition failure (auth_cmd.go): when the refresh lock could not be taken (unreadable stale lock, contention, cancellation), logout returned before Clear, leaving tokens on disk. Acquisition failure now falls through to a best-effort Clear with a revocation-skipped caveat; a Clear that failed inside the locked section still surfaces its error rather than being retried.

The follow-up also re-verified the remaining fix areas from aa1e2d3 (device final poll, transport-error retention, error-code allowlist, login URL printing, TUI stale-key clear) and confirmed them correct. go build/go vet/gofmt clean; internal/codexauth and the auth/TUI command tests pass.

[acoliver]

Note this reviewed itself many times with OCR to test the thing :-)