Compare commits

...
1 Commits
Author SHA1 Message Date
Brent Vatne 188f8521bf Detect provider rate limits and wait them out; subscription-aware concurrency (#32)
* Detect provider rate limits and wait them out; subscription-aware concurrency

A shared subscription credential can serve several PRs' reviews at once, so
throttling is a normal condition to ride out, not a failure — and it must be
distinguishable from a wedged request.

- watch the OpenCode server's own log (isolated per run) for 429/rate-limit
  ERROR lines — explicit throttle evidence, the only kind that exists outside
  the provider; a silently parked request emits nothing anywhere
- a stall WITH recent evidence waits in 90s beats (never consuming the one
  wedged retry, bounded by the pass deadline) instead of re-sending the pass's
  whole context into a limited account; without evidence, the wedged-retry
  behavior is unchanged
- explicit 429 errors retry on a patient 15s/45s/90s schedule instead of 2s/8s
- chunk.concurrency unset now resolves by auth mode: 3 on a subscription
  (oauth) credential, 6 on API keys — one account handles six parallel streams
  poorly (the parked-request stall signature on eas-cli#4084 hit both agents'
  chunk 2 simultaneously); an explicit value always wins
- rate-limit events surface at run end and in the run log (rateLimitEvents),
  so throttle frequency is measurable across PRs

* Support ChatGPT ACCESS tokens (rotation-safe); honest verdict when nothing ran

The refresh-token-as-static-secret strategy died in production (euxy#8: every
pass "Token refresh failed: 401" — refresh tokens are SINGLE-USE; the shared
copy was spent by its first CI use and the sign-in with it).

- oauthAuthJsonEntry detects the token kind: a JWT (ACCESS token) is used
  as-is with its exp claim as the expiry — no rotation involvement, safe as a
  shared/static secret; an opaque value keeps the refresh-token shape for
  sole-consumer setups
- an expired access token fails fast naming the re-mint; near-expiry warns
  (doctor + preflight) — the credential can't silently lapse mid-run
- setup-auth hands out the ACCESS token (default env CODEX_OAUTH_ACCESS_TOKEN)
  and never surfaces the refresh token, which stays in OpenCode's own store
  as its sole legitimate consumer
- an all-passes-failed run no longer renders as "Approve with comments":
  couldNotComplete overrides the decision label with "No review — every pass
  failed" (single and per-scope comments)
- README: retract the false "refresh-token reuse verified, static CI secret
  works" claim; document the access-token strategy
- ROADMAP: token-rotator cron job added as the most important next item
- this repo's config/workflow: tokenEnv renamed to CODEX_OAUTH_ACCESS_TOKEN
2026-07-26 19:44:03 -07:00
18 changed files with 536 additions and 63 deletions

No files matched your search

+1 -1
View File
@@ -39,7 +39,7 @@
// ECR_EXPECTED_TOKEN_ENV (repo variable) must equal this comma-separated set.
"auth": {
"providers": {
"openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_REFRESH_TOKEN" },
"openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" },
"openai-api": { "mode": "api-key", "tokenEnv": "OPENAI_API_KEY", "upstream": "openai" }
}
}
+4 -4
View File
@@ -53,7 +53,7 @@ jobs:
# published package), so the guard runs from source too — same engine as `ci`.
- name: Guard config tokenEnv
env:
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CODEX_OAUTH_REFRESH_TOKEN,OPENAI_API_KEY' }}
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CODEX_OAUTH_ACCESS_TOKEN,OPENAI_API_KEY' }}
run: bun run src/cli.ts verify-config
- name: Run AI review
@@ -66,13 +66,13 @@ jobs:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# Layer-1 auth lock: `ecr ci` re-checks the honored tokenEnv at runtime,
# catching what the guard step above can't. Keep in sync with the guard.
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CODEX_OAUTH_REFRESH_TOKEN,OPENAI_API_KEY' }}
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CODEX_OAUTH_ACCESS_TOKEN,OPENAI_API_KEY' }}
# Mixed credentials (see config.jsonc auth.providers): the ChatGPT/Codex
# subscription refresh token serves the default models; the usage-based
# subscription ACCESS token serves the default models; the usage-based
# API key (project-scoped, inference-only, budget-capped) serves the
# pro-tier security pass. ECR_EXPECTED_TOKEN_ENV (repo variable) locks
# the comma-separated set of BOTH names.
CODEX_OAUTH_REFRESH_TOKEN: ${{ secrets.CODEX_OAUTH_REFRESH_TOKEN }}
CODEX_OAUTH_ACCESS_TOKEN: ${{ secrets.CODEX_OAUTH_ACCESS_TOKEN }}
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
# Optional: override the model for every agent (uses your OpenCode login).
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
+21 -9
View File
@@ -55,7 +55,7 @@ subscription** sign-in (it runs OpenCode's browser login and extracts the token
for you). `doctor` offers to run it whenever a credential is missing.
In CI, store the same values as repo secrets (`OPENAI_API_KEY`; plus
`CODEX_OAUTH_REFRESH_TOKEN` for the mixed setup) — the scaffolded workflow
`CODEX_OAUTH_ACCESS_TOKEN` for the mixed setup) — the scaffolded workflow
forwards them.
**Have a ChatGPT Plus/Pro (Codex) subscription? Use both.** The recommended
@@ -344,7 +344,7 @@ coordinator, and per-repo `noise.additionalIgnores`.
{
"model": "openai/gpt-5.5", // default model for the specialists
"policy": { "includeSuggestions": false }, // suppress suggestion-severity findings
"chunk": { "maxChangedLines": 1000, "maxFiles": 20, "concurrency": 6 },
"chunk": { "maxChangedLines": 1000, "maxFiles": 20 }, // concurrency defaults: 6 (API key) / 3 (subscription)
"noise": { "additionalIgnores": ["packages/*/build/**"] },
"review": { "trigger": "all", // which PRs `ecr ci` reviews: "all"
"label": "ai-review", // (default, except ai-review:skip) or
@@ -406,6 +406,15 @@ change which model reviewed your code. Use an explicit override instead.
session, inside the same budget — instead of spending the whole cap on a dead
request. Progress lines say how long a reply has been silent, so this is legible in
the CI log.
- **Rate limits are detected and waited out, not fought.** The reviewer watches the
OpenCode server's own log for provider 429s (hard evidence, per run). A stall
*with* recent 429 evidence is throttling, not a wedge — the pass waits in 90s
beats (without consuming its one retry) instead of re-sending its whole context
into a limited account; explicit 429 errors retry on a slow 15s/45s/90s schedule.
Subscription (oauth) runs also default to `concurrency` 3 instead of 6, since one
account may be serving several PRs' reviews at once. Rate-limit events are
reported in the job log and the run log (`rateLimitEvents`), so throttling is a
visible fact about a run, never a mystery slowdown.
- **Soft landing on timeout** — at either cap, the run is interrupted and the agent
is asked to return the findings it already has, rather than discarding its work.
Tools are disabled for that request, so the salvage step can't resume investigating
@@ -490,7 +499,7 @@ set in `config.auth` (credentials come from OpenCode):
```jsonc
"auth": { "providers": {
"openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_REFRESH_TOKEN" },
"openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" },
"openai-api": { "mode": "api-key", "tokenEnv": "OPENAI_API_KEY", "upstream": "openai" }
} }
```
@@ -499,11 +508,14 @@ set in `config.auth` (credentials come from OpenCode):
(`upstream` names the SDK it's backed by): agents reference `openai-api/gpt-5.5-pro`
in frontmatter while everything else stays on `openai/gpt-5.5`. Notes:
- **The oauth `tokenEnv` holds the refresh token** from an `opencode auth login`
ChatGPT sign-in (copy `.openai.refresh` out of OpenCode's `auth.json`) —
access tokens are short-lived, so the refresh token is the durable secret and
OpenCode mints access tokens on demand. Refresh-token reuse across runs is
verified, so a static CI secret works.
- **The oauth `tokenEnv` holds the ACCESS token** from an `opencode auth login`
ChatGPT sign-in (`ecr setup-auth` extracts it) — a plain bearer, valid for
days, with no rotation involvement. Do **not** use the refresh token as a
shared secret: refresh tokens are single-use (rotation), so a static copy is
spent by its first use and the sign-in dies with it. Access tokens expire
(~10 days observed), so CI secrets need periodic re-minting — see the
token-rotator item in the [roadmap](./ROADMAP.md); `doctor` and the run
preflight warn before expiry.
- **The API key needs exactly two permissions** — a *Restricted* key with
*Model capabilities*: **Responses → Request** and **Chat completions →
Request**; everything else (including *List models*) stays None. Create it
@@ -511,7 +523,7 @@ set in `config.auth` (credentials come from OpenCode):
instructions too.)
- **In CI**, set the `ECR_EXPECTED_TOKEN_ENV` repo variable to the
comma-separated set of both env names
(`CODEX_OAUTH_REFRESH_TOKEN,OPENAI_API_KEY`) and pass both secrets in the
(`CODEX_OAUTH_ACCESS_TOKEN,OPENAI_API_KEY`) and pass both secrets in the
workflow.
- **Auditability**: every pass logs which provider/model answered it (job log,
step summary, run log), so the subscription/API split is visible per run.
+23
View File
@@ -12,6 +12,29 @@ that is meant to merge and then iterate in follow-up PRs. What's **in** phase 1
under "Recently shipped" below. The highest-value work **deferred to follow-up
PRs** (do these first, in this order):
0. **Codex access-token rotator (cron) — MOST IMPORTANT NEXT.** CI subscription
auth now uses ACCESS tokens (~10-day observed lifetime) because refresh tokens
are single-use: sharing one as a static secret spent it on first use and killed
the whole sign-in ("Your refresh token has already been used…", euxy#8,
2026-07-27). Until the rotator exists, the access-token secrets need a manual
re-mint (~weekly): `ecr setup-auth` → `gh secret set CODEX_OAUTH_ACCESS_TOKEN`
per repo. Design (constraints are load-bearing):
- a scheduled workflow (~twice weekly + workflow_dispatch) in ONE home repo is
the refresh token's SOLE consumer — nothing else may ever hold or use it;
- serialized via a concurrency group (a race forks the rotation family);
- each run: one form-urlencoded refresh call (client_id
`app_EMoamEEZ73f0CkXaXp7hrann`, endpoint `auth.openai.com/oauth/token`),
then persist the NEW refresh token to its own secret FIRST, then fan the new
access token out to `CODEX_OAUTH_ACCESS_TOKEN` on every consumer repo
(code-review-cli, eas-cli, euxy, …);
- needs a fine-grained PAT with Actions-secrets write on the consumer repos
(GITHUB_TOKEN cannot write secrets);
- failure mode: crashing between spending the old token and saving the new one
loses the family (recover: `opencode auth login` + reseed) — the rotation
grace window makes an immediate retry likely to succeed, so retry once
before alerting;
- `doctor`/preflight already warn when the access token is near expiry, which
doubles as the rotator's health check.
1. **Incremental / delta review** — review only what changed since the last review
(persistent per-file-version state). Biggest speed + cost + reliability win on
re-pushes; the last remaining lever for the monster-PR case. See §5.
+50 -1
View File
@@ -1,6 +1,12 @@
import { test, expect } from "bun:test";
import { prepareAuth, checkProviderAuth, checkOauthTokenShape } from "../core/auth.js";
import {
prepareAuth,
checkProviderAuth,
checkOauthTokenShape,
oauthAuthJsonEntry,
jwtExpiryMs,
} from "../core/auth.js";
import type { LoadedConfig } from "../config/schema.js";
// Internal auth is a LIST of entries; most tests exercise one credential, so the
@@ -329,3 +335,46 @@ test("a well-known provider key env may only feed that provider", () => {
).ok,
).toBe(true);
});
// ---- access-token support (rotation-safe subscription credentials) ----
test("a JWT access token is used as-is with its own expiry — never refreshed", () => {
// Refresh tokens are SINGLE-USE: sharing one as a static secret killed the
// whole sign-in (euxy#8, "Your refresh token has already been used"). An
// access token is a plain bearer with no rotation involvement.
const exp = Math.floor(Date.now() / 1000) + 5 * 24 * 3600;
const jwt = `eyJhbGciOiJSUzI1NiJ9.${Buffer.from(JSON.stringify({ exp })).toString("base64url")}.sig`;
const entry = oauthAuthJsonEntry("openai", jwt);
expect(entry).toEqual({ type: "oauth", access: jwt, refresh: "", expires: exp * 1000 });
// An opaque token is still treated as a refresh token (sole-consumer setups).
const opaque = oauthAuthJsonEntry("openai", "r".repeat(196));
expect(opaque).toEqual({ type: "oauth", access: "", refresh: "r".repeat(196), expires: 0 });
});
test("an EXPIRED access token fails fast with the re-mint instruction", () => {
const exp = Math.floor(Date.now() / 1000) - 2 * 24 * 3600;
const jwt = `eyJhbGciOiJSUzI1NiJ9.${Buffer.from(JSON.stringify({ exp })).toString("base64url")}.sig`;
const r = checkProviderAuth(cfg({ mode: "oauth", provider: "openai", tokenEnv: "TOK" }), {
TOK: jwt,
});
expect(r.ok).toBe(false);
expect(r.detail).toContain("EXPIRED");
expect(r.detail).toContain("setup-auth");
});
test("a nearly-expired access token warns but never blocks", () => {
const exp = Math.floor(Date.now() / 1000) + 12 * 3600; // 12h left
const jwt = `eyJhbGciOiJSUzI1NiJ9.${Buffer.from(JSON.stringify({ exp })).toString("base64url")}.sig`;
const r = checkProviderAuth(cfg({ mode: "oauth", provider: "openai", tokenEnv: "TOK" }), {
TOK: jwt,
});
expect(r.ok).toBe(true);
expect(r.warning).toContain("expires in");
});
test("jwtExpiryMs decodes exp and tolerates garbage", () => {
expect(jwtExpiryMs("not-a-jwt")).toBeNull();
expect(jwtExpiryMs("eyJ.%%%.sig")).toBeNull();
const jwt = `eyJ.${Buffer.from(JSON.stringify({ exp: 1000 })).toString("base64url")}.s`;
expect(jwtExpiryMs(jwt)).toBe(1000_000);
});
+26
View File
@@ -9,7 +9,9 @@ import {
isAuthError,
formatUsageSummary,
renderUsageMarkdown,
effectiveConcurrency,
} from "../core/review.js";
import type { LoadedConfig } from "../config/schema.js";
import type { PatchWorkspaceFile } from "../core/noise.js";
import type { CoordinatorOutput, Finding } from "../core/schema.js";
@@ -198,3 +200,27 @@ test("renderUsageMarkdown omits the hit rate when no prompt tokens were used", (
const s = renderUsageMarkdown({}, {}, {}, 0);
expect(s).not.toContain("hit rate");
});
// ---- auth-mode-aware concurrency ----
test("effectiveConcurrency: explicit config wins; subscription defaults lower", () => {
const cfg = (chunk: object, auth: object[]): LoadedConfig =>
({ chunk, auth }) as unknown as LoadedConfig;
// An explicit value always wins, whatever the auth mode.
expect(
effectiveConcurrency(cfg({ concurrency: 8 }, [{ mode: "oauth", provider: "openai" }])),
).toBe(8);
// A subscription (oauth) credential defaults lower: one account handles six
// parallel streams poorly, and several PRs may share the credential.
expect(effectiveConcurrency(cfg({}, [{ mode: "oauth", provider: "openai" }]))).toBe(3);
expect(
effectiveConcurrency(
cfg({}, [
{ mode: "oauth", provider: "openai" },
{ mode: "api-key", provider: "openai-api", upstream: "openai" },
]),
),
).toBe(3);
// Pure API-key runs keep the full default.
expect(effectiveConcurrency(cfg({}, [{ mode: "api-key", provider: "openai" }]))).toBe(6);
});
+4 -4
View File
@@ -4,10 +4,10 @@ import { planFromAuth, exportLine, opencodeAuthJsonPath } from "../commands/setu
test("plan: the mixed setup needs one ChatGPT login and one manual key", () => {
const plan = planFromAuth([
{ provider: "openai", mode: "oauth", tokenEnv: "CODEX_OAUTH_REFRESH_TOKEN" },
{ provider: "openai", mode: "oauth", tokenEnv: "CODEX_OAUTH_ACCESS_TOKEN" },
{ provider: "openai-api", mode: "api-key", tokenEnv: "OPENAI_API_KEY", upstream: "openai" },
]);
expect(plan.chatgptLogin).toEqual({ tokenEnv: "CODEX_OAUTH_REFRESH_TOKEN" });
expect(plan.chatgptLogin).toEqual({ tokenEnv: "CODEX_OAUTH_ACCESS_TOKEN" });
expect(plan.manualKeys).toEqual([
{ provider: "openai-api", tokenEnv: "OPENAI_API_KEY", upstream: "openai" },
]);
@@ -27,8 +27,8 @@ test("plan: anthropic oauth is unsupported (dead path), api-key without tokenEnv
});
test("export line is single-quoted so shells never expand the value", () => {
expect(exportLine("CODEX_OAUTH_REFRESH_TOKEN", "abc$HOME!x")).toBe(
"export CODEX_OAUTH_REFRESH_TOKEN='abc$HOME!x'",
expect(exportLine("CODEX_OAUTH_ACCESS_TOKEN", "abc$HOME!x")).toBe(
"export CODEX_OAUTH_ACCESS_TOKEN='abc$HOME!x'",
);
});
+77
View File
@@ -0,0 +1,77 @@
import { test, expect } from "bun:test";
import { mkdtemp, writeFile, appendFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import { isRateLimitError, stallAction } from "../core/opencode.js";
import { countRateLimitLines, opencodeLogFile, RateLimitWatch } from "../core/throttle.js";
const MIN = 60_000;
test("only ERROR lines with a throttle signature count as evidence", () => {
const chunk = [
// Real evidence, in OpenCode's logfmt shape.
'timestamp=x level=ERROR message="stream error" agent=security error.error="AI_APICallError: 429 Too Many Requests"',
'timestamp=x level=ERROR message="stream error" error.error="Rate limit reached for gpt-5.5"',
// NOT evidence: an INFO line that merely mentions retrying…
"timestamp=x level=INFO message=retry attempt=2",
// …an ERROR with no throttle signature…
'timestamp=x level=ERROR message="stream error" error.error="AI_APICallError: An error occurred"',
// …and a 429-looking number on an INFO line (timestamps contain .429Z!).
"timestamp=2026-07-23T05:20:31.429Z level=INFO message=process",
].join("\n");
expect(countRateLimitLines(chunk)).toBe(2);
});
test("the watch reads incrementally and reports recency", async () => {
const dir = await mkdtemp(path.join(tmpdir(), "ecr-throttle-"));
const file = path.join(dir, "opencode.log");
await writeFile(file, "timestamp=x level=INFO message=init\n");
const watch = new RateLimitWatch(file);
expect(await watch.check()).toBe(0);
expect(watch.recentlyLimited()).toBe(false);
await appendFile(file, 'timestamp=x level=ERROR message="stream error" error.error="429"\n');
expect(await watch.check()).toBe(1);
expect(watch.recentlyLimited()).toBe(true);
// Evidence goes stale outside the recency window.
expect(watch.recentlyLimited(Date.now() + 10 * MIN)).toBe(false);
// Already-read lines are never recounted.
expect(await watch.check()).toBe(1);
});
test("a missing log file is no evidence, not an error", async () => {
const watch = new RateLimitWatch("/nonexistent/opencode.log");
expect(await watch.check()).toBe(0);
expect(watch.recentlyLimited()).toBe(false);
});
test("log path honors XDG_DATA_HOME (the isolated run dir)", () => {
expect(opencodeLogFile({ XDG_DATA_HOME: "/isolated" })).toBe(
"/isolated/opencode/log/opencode.log",
);
});
// ---- patient stall behavior ----
test("a stall WITH rate-limit evidence waits instead of retrying — and keeps waiting", () => {
// Waiting must not consume the single wedged retry: patience is free while
// budget remains, whatever the retry history.
expect(stallAction(0, 20 * MIN, true)).toBe("wait");
expect(stallAction(1, 20 * MIN, true)).toBe("wait");
// Out of room: salvage what exists rather than waiting past the deadline.
expect(stallAction(0, 2 * MIN, true)).toBe("soft-land");
});
test("a stall WITHOUT evidence keeps the wedged-retry behavior", () => {
expect(stallAction(0, 20 * MIN, false)).toBe("retry");
expect(stallAction(1, 20 * MIN, false)).toBe("soft-land");
});
test("rate-limit errors are recognized for the slower backoff schedule", () => {
expect(isRateLimitError(new Error("HTTP 429 Too Many Requests"))).toBe(true);
expect(isRateLimitError(new Error("Rate limit reached for requests"))).toBe(true);
expect(isRateLimitError(new Error("server error 503"))).toBe(false);
});
+39 -18
View File
@@ -6,6 +6,7 @@ import readline from "node:readline/promises";
import { hasConfig, loadReviewConfig } from "../config/load.js";
import type { AuthConfigEntry } from "../config/schema.js";
import { jwtExpiryMs } from "../core/auth.js";
import { opencodeBinSource } from "../core/opencode.js";
import { errorMessage } from "../core/util.js";
@@ -62,16 +63,28 @@ export function opencodeAuthJsonPath(env: NodeJS.ProcessEnv = process.env): stri
return path.join(dataHome, "opencode", "auth.json");
}
/** The stored ChatGPT sign-in's refresh token, if OpenCode has one. */
async function readStoredRefreshToken(): Promise<string | null> {
/**
* The stored ChatGPT sign-in's ACCESS token, if OpenCode has a live one. The
* refresh token deliberately never leaves OpenCode's store: refresh tokens are
* SINGLE-USE (rotation) and OpenCode is their sole legitimate consumer — a copy
* in a shell config or CI secret dies on the next rotation and can take the
* whole sign-in with it. The access token is a plain bearer that stays valid for
* days and never touches rotation.
*/
async function readStoredAccessToken(): Promise<{ token: string; expiresMs: number } | null> {
try {
const raw = await readFile(opencodeAuthJsonPath(), "utf8");
const parsed = JSON.parse(raw) as Record<
string,
{ type?: string; refresh?: string } | undefined
{ type?: string; access?: string } | undefined
>;
const openai = parsed.openai;
return openai?.type === "oauth" && openai.refresh ? openai.refresh : null;
if (openai?.type !== "oauth" || !openai.access) {
return null;
}
const expiresMs = jwtExpiryMs(openai.access) ?? 0;
// An expired stored token means the sign-in needs redoing anyway.
return expiresMs > Date.now() ? { token: openai.access, expiresMs } : null;
} catch {
return null;
}
@@ -114,7 +127,7 @@ export async function setupAuthCommand(argv: string[] = []): Promise<void> {
} else {
err("No .expo-code-review config here — setting up the default ChatGPT/Codex flow.");
plan = planFromAuth([
{ provider: "openai", mode: "oauth", tokenEnv: "CODEX_OAUTH_REFRESH_TOKEN" },
{ provider: "openai", mode: "oauth", tokenEnv: "CODEX_OAUTH_ACCESS_TOKEN" },
]);
}
@@ -132,14 +145,17 @@ export async function setupAuthCommand(argv: string[] = []): Promise<void> {
if (process.env[tokenEnv]) {
err(`✓ ${tokenEnv} is already set in this shell — skipping the ChatGPT sign-in.`);
} else {
let refresh = await readStoredRefreshToken();
if (refresh) {
err("Found an existing ChatGPT sign-in in OpenCode.");
if (!(await confirm(`Reuse it for ${tokenEnv}?`, yes))) {
refresh = null;
let stored = await readStoredAccessToken();
if (stored) {
err(
`Found a live ChatGPT sign-in in OpenCode (access token valid ` +
`${Math.max(1, Math.round((stored.expiresMs - Date.now()) / 86_400_000))} more day(s)).`,
);
if (!(await confirm(`Use it for ${tokenEnv}?`, yes))) {
stored = null;
}
}
if (!refresh) {
if (!stored) {
err("This will run the bundled `opencode auth login` (interactive).");
err("When it prompts:");
err(" 1. select the provider: OpenAI");
@@ -156,19 +172,24 @@ export async function setupAuthCommand(argv: string[] = []): Promise<void> {
`\`opencode auth login\` exited with ${result.status ?? "a signal"}; nothing was changed.`,
);
}
refresh = await readStoredRefreshToken();
if (!refresh) {
stored = await readStoredAccessToken();
if (!stored) {
throw new Error(
"The login finished but no ChatGPT sign-in was stored — did you select " +
"The login finished but no live ChatGPT sign-in was stored — did you select " +
'OpenAI → "Sign in with ChatGPT"? Re-run `ecr setup-auth` to try again.',
);
}
}
}
if (refresh) {
// The REFRESH token is the durable secret: access tokens are short-lived,
// and OpenCode mints them from this on demand.
exports.push(exportLine(tokenEnv, refresh));
if (stored) {
// The ACCESS token: a plain bearer, valid for days, no rotation involved.
// (The refresh token stays in OpenCode's store — it is single-use, and
// copying it anywhere kills it on the next rotation.)
exports.push(exportLine(tokenEnv, stored.token));
err(
`Note: this access token expires in ~${Math.max(1, Math.round((stored.expiresMs - Date.now()) / 86_400_000))} day(s); ` +
`re-run \`ecr setup-auth\` then to refresh it (your OpenCode sign-in stays valid).`,
);
}
}
}
+10 -4
View File
@@ -40,10 +40,15 @@ export const ReviewConfigSchema = z.object({
maxChangedLines: z.number().int().positive().default(1000),
// Secondary guard so a chunk isn't an absurd number of tiny-diff files.
maxFiles: z.number().int().positive().default(20),
// Max concurrent reviewer calls across all agents/chunks.
concurrency: z.number().int().positive().default(6),
// Max concurrent reviewer calls across all agents/chunks. Unset ⇒ resolved
// from the auth mode: 6 for API-key runs, 3 when a subscription (oauth)
// credential is configured — one ChatGPT account handles six parallel
// streams poorly (requests get parked = the stall signature), and several
// PRs may be reviewing on the same credential at once. An explicit value
// here always wins. See effectiveConcurrency in core/review.ts.
concurrency: z.number().int().positive().optional(),
})
.default({ maxChangedLines: 1000, maxFiles: 20, concurrency: 6 }),
.default({ maxChangedLines: 1000, maxFiles: 20 }),
noise: z
.object({
additionalIgnores: z.array(z.string()).default([]),
@@ -284,7 +289,8 @@ export interface LoadedConfig {
chunk: {
maxChangedLines: number;
maxFiles: number;
concurrency: number;
/** Unset ⇒ resolved by effectiveConcurrency (auth-mode-aware default). */
concurrency?: number;
};
noise: {
additionalIgnores: string[];
+62 -6
View File
@@ -103,6 +103,34 @@ export function checkOauthTokenShape(
detail: `${tokenEnv} holds only ${token.length} characters, too short to be a real ${provider} token — it looks truncated. ${fix}`,
};
}
// A ChatGPT access token carries its own expiry — check it up front, so a
// lapsed credential is one clear message instead of N failed passes, and a
// nearly-lapsed one warns before it bites mid-run.
if (provider === "openai" && isJwtAccessToken(token)) {
const expires = jwtExpiryMs(token);
if (expires !== null) {
const remainingMs = expires - Date.now();
if (remainingMs <= 0) {
return {
ok: false,
detail:
`${tokenEnv} holds a ChatGPT access token that EXPIRED ${Math.ceil(-remainingMs / 86_400_000)} day(s) ago. ` +
`Mint a fresh one (\`ecr setup-auth\`, or your token-rotator job) and update ${tokenEnv}.`,
};
}
if (remainingMs < 3 * 86_400_000) {
return {
...ok,
detail: `oauth for ${provider}; token env ${tokenEnv} is set`,
warning:
`${tokenEnv}'s ChatGPT access token expires in ${Math.max(1, Math.round(remainingMs / 3_600_000))}h — ` +
`re-mint it soon (\`ecr setup-auth\`, or your token-rotator job).`,
};
}
}
return { ...ok, detail: `oauth for ${provider}; token env ${tokenEnv} is set` };
}
// Only anthropic's formats are known well enough to say anything about.
if (provider !== "anthropic") {
return ok;
@@ -291,11 +319,38 @@ export function checkProviderAuth(
};
}
/** A ChatGPT access token is a JWT (three base64url segments); refresh tokens are opaque. */
export function isJwtAccessToken(token: string): boolean {
return token.startsWith("eyJ") && token.split(".").length === 3;
}
/** A JWT's `exp` claim as epoch ms, decoded (not verified) — null when unreadable. */
export function jwtExpiryMs(token: string): number | null {
const payload = token.split(".")[1];
if (!payload) {
return null;
}
try {
const claims = JSON.parse(Buffer.from(payload, "base64url").toString("utf8")) as {
exp?: unknown;
};
return typeof claims.exp === "number" ? claims.exp * 1000 : null;
} catch {
return null;
}
}
/**
* The auth.json entry for one oauth credential. Provider-shaped:
* - openai: the durable secret is the REFRESH token (a ChatGPT/Codex sign-in's
* access tokens live ~1h, shorter than a worst-case run), so store it with
* `expires: 0` and let OpenCode's codex plugin mint access tokens on demand.
* The auth.json entry for one oauth credential. Shaped by what the token IS:
* - a JWT (an ACCESS token, e.g. from `ecr setup-auth` or a rotator job): use it
* as-is and never refresh — refresh tokens are SINGLE-USE (rotation), so a
* static/shared secret must not participate in rotation at all. Expiry comes
* from the JWT's own `exp` claim so OpenCode trusts it exactly as long as it
* is valid.
* - an opaque openai token (a REFRESH token): store it with `expires: 0` and let
* OpenCode's codex plugin mint the access token. Only safe when this run is
* the token's SOLE consumer — a value shared across runs/repos dies on first
* rotation (learned the hard way).
* - everything else: the token IS the access credential (e.g. long-lived
* setup-token style bearers), far-future expiry so OpenCode never tries to
* refresh a credential that has no refresh half.
@@ -304,10 +359,11 @@ export function oauthAuthJsonEntry(
provider: string,
token: string,
): Record<string, string | number> {
if (provider === "openai") {
if (provider === "openai" && !isJwtAccessToken(token)) {
return { type: "oauth", access: "", refresh: token, expires: 0 };
}
return { type: "oauth", access: token, refresh: "", expires: Date.now() + YEAR_MS };
const expires = isJwtAccessToken(token) ? jwtExpiryMs(token) : null;
return { type: "oauth", access: token, refresh: "", expires: expires ?? Date.now() + YEAR_MS };
}
/**
+2
View File
@@ -28,6 +28,8 @@ export interface RunLogRecord {
agentTokens?: Record<string, TokenUsage>;
/** provider/model that ACTUALLY answered each pass (not what was configured). */
agentModels?: Record<string, string>;
/** Provider rate-limit (429) events observed in the OpenCode server log this run. */
rateLimitEvents?: number;
// The reasoning trail behind the posted result: raw per-agent findings before
// coordination, the coverage gaps reported to the coordinator, and the findings
// the verifier rejected. Together these explain WHY the final finding set looks
+62 -9
View File
@@ -4,6 +4,7 @@ import path from "node:path";
import { createOpencode } from "@opencode-ai/sdk";
import type { LoadedConfig } from "../config/schema.js";
import { RateLimitWatch } from "./throttle.js";
import { toolMap } from "./tools.js";
import { errorMessage, sleep } from "./util.js";
@@ -11,6 +12,8 @@ export interface OpencodeHandle {
client: any;
url: string;
close: () => void;
/** Rate-limit evidence from this server's own log (see core/throttle.ts). */
rateLimit: RateLimitWatch;
}
/** Token usage as reported on an OpenCode assistant message's `info.tokens`. */
@@ -225,7 +228,9 @@ export async function startOpencode(config: unknown): Promise<OpencodeHandle> {
port: 0,
config: config as any,
});
return { client, url: server.url, close: () => server.close() };
// Watch THIS server's log for provider throttle evidence (prepareAuth has already
// pointed XDG_DATA_HOME at the run's isolated dir when auth is injected).
return { client, url: server.url, close: () => server.close(), rateLimit: new RateLimitWatch() };
}
/**
@@ -459,18 +464,31 @@ const FINALIZE_STALL_MS = 60 * 1000;
// Breathing room before the retry: if the silence came from provider-side throttling
// or backoff, reconnecting instantly is the worst move.
const STALL_RETRY_BACKOFF_MS = 20 * 1000;
// When the account is provably rate-limited, wait in longer beats: re-sending the
// pass's whole context into a throttled account only deepens the limit. Several
// waits fit inside a pass budget, and each is long enough for a limit window to move.
const RATE_LIMIT_WAIT_MS = 90 * 1000;
// Only retry when enough of the pass's budget remains for the fresh attempt to
// plausibly finish; otherwise go straight to the soft landing.
const STALL_RETRY_MIN_REMAINING_MS = STALL_MS + 60 * 1000;
/**
* What to do about a stalled attempt: start over from a clean session, or stop and
* try to salvage findings. Exactly ONE retry, and only with enough budget left for it
* to land — a second wedged attempt would just spend the rest of the pass's window,
* What to do about a stalled attempt: WAIT (the account is provably rate-limited —
* see core/throttle.ts — so patience beats re-sending the context; waits don't
* consume the one retry), start over from a clean session, or stop and salvage
* findings. Exactly ONE wedged retry, and only with enough budget left for it to
* land — a second wedged attempt would just spend the rest of the pass's window,
* which is the failure this whole mechanism exists to end. Exported for tests.
*/
export function stallAction(attempt: number, remainingMs: number): "retry" | "soft-land" {
return attempt === 0 && remainingMs > STALL_RETRY_MIN_REMAINING_MS ? "retry" : "soft-land";
export function stallAction(
wedgedRetries: number,
remainingMs: number,
rateLimited = false,
): "wait" | "retry" | "soft-land" {
if (rateLimited && remainingMs > STALL_RETRY_MIN_REMAINING_MS) {
return "wait";
}
return wedgedRetries === 0 && remainingMs > STALL_RETRY_MIN_REMAINING_MS ? "retry" : "soft-land";
}
const FINALIZE_PROMPT =
@@ -707,6 +725,7 @@ export async function promptAgent(
}
};
let wedgedRetries = 0;
for (let attempt = 0; ; attempt++) {
const session = unwrap<{ id: string }>(
await handle.client.session.create({
@@ -738,11 +757,29 @@ export async function promptAgent(
// wedged request to answer. Exactly one retry, and only when enough budget
// remains for it to land; after that the finalize is still worth a try as the
// only remaining salvage (in eas-cli#4084 the session did respond once aborted).
//
// EXCEPT when the server log shows the account is rate-limited: then the
// silence is throttling, not a wedge, and the patient move is to wait —
// re-sending the pass's whole context would deepen the limit. Waits repeat
// (never consuming the one wedged retry) until the evidence goes stale or
// the pass runs out of room, both bounded by the pass deadline.
if (error instanceof NoProgress) {
await abortQuietly(handle, session.id);
absorb(error);
await handle.rateLimit.check();
const remaining = deadline - Date.now();
if (stallAction(attempt, remaining) === "retry") {
const action = stallAction(wedgedRetries, remaining, handle.rateLimit.recentlyLimited());
if (action === "wait") {
args.onActivity?.(
`provider is rate-limiting this account (429 in the server log; ` +
`${handle.rateLimit.events} so far) — waiting ${Math.round(RATE_LIMIT_WAIT_MS / 1000)}s ` +
`instead of retrying (${Math.round(remaining / 60000)}m of budget left)`,
);
await sleep(RATE_LIMIT_WAIT_MS);
continue;
}
if (action === "retry") {
wedgedRetries++;
args.onActivity?.(
`stalled — no output for ${Math.round(error.idleMs / 1000)}s; ` +
`retrying once from a clean session (${Math.round(remaining / 60000)}m of budget left)`,
@@ -772,6 +809,20 @@ const CORRECTIVE_WAIT_MS = 2 * 60 * 1000;
/** Backoff (ms) before the 2nd and 3rd attempt of a transient-failing model call. */
const TRANSIENT_BACKOFF_MS = [2_000, 8_000];
/**
* Rate limits need patience, not persistence: a limited account stays limited for
* tens of seconds to minutes, so the 2s/8s schedule just burns the retries. Shared
* subscription credentials (several PRs reviewing at once) make this the common
* transient, hence the dedicated, slower schedule.
*/
const RATE_LIMIT_BACKOFF_MS = [15_000, 45_000, 90_000];
const RATE_LIMIT_ERROR = /\b429\b|rate.?limit|too many requests/i;
/** A transient error that is specifically a provider rate limit. */
export function isRateLimitError(error: unknown): boolean {
return !(error instanceof AgentTimeoutError) && RATE_LIMIT_ERROR.test(errorMessage(error));
}
/**
* A transient, retryable API failure — a one-off rate-limit (429), server error
@@ -821,13 +872,15 @@ async function withTransientRetry<T>(
try {
return await fn();
} catch (error) {
const waitMs = TRANSIENT_BACKOFF_MS[attempt];
// Rate limits get the slower, longer schedule — see RATE_LIMIT_BACKOFF_MS.
const schedule = isRateLimitError(error) ? RATE_LIMIT_BACKOFF_MS : TRANSIENT_BACKOFF_MS;
const waitMs = schedule[attempt];
if (waitMs === undefined || !isTransientApiError(error)) {
throw error;
}
onActivity?.(
`${label}: transient API error (${errorMessage(error)}); retry ${attempt + 1}/${
TRANSIENT_BACKOFF_MS.length
schedule.length
} in ${Math.round(waitMs / 1000)}s`,
);
await sleep(waitMs);
+9 -2
View File
@@ -154,7 +154,12 @@ export function renderMarkdown(
const dropped = withFp.filter(({ fp }) => dismissedByFp.has(fp));
const lines: string[] = [commentMarker(tag), "## 🤖 AI code review", ""];
lines.push(`**Decision:** ${decisionLabel(review.decision)}`, "", review.summary, "");
lines.push(
`**Decision:** ${review.couldNotComplete ? "No review — every pass failed" : decisionLabel(review.decision)}`,
"",
review.summary,
"",
);
if (review.incomplete.length > 0) {
lines.push(
@@ -334,7 +339,9 @@ export function renderAggregateMarkdown(
"| --- | --- | --- |",
];
for (const { result, kept } of perScope) {
lines.push(`| ${result.scope} | ${decisionLabel(result.review.decision)} | ${kept.length} |`);
lines.push(
`| ${result.scope} | ${result.review.couldNotComplete ? "No review — every pass failed" : decisionLabel(result.review.decision)} | ${kept.length} |`,
);
}
lines.push("");
+33 -2
View File
@@ -73,6 +73,21 @@ function makeRunId(): string {
* → coordinate → apply policy. Returns a CoordinatorOutput; the CLI commands are
* thin wrappers that supply a Source and render the result.
*/
/**
* Max concurrent reviewer calls: an explicit config value wins; otherwise 3 when a
* subscription (oauth) credential is configured, else 6. One ChatGPT account
* handles six parallel streams poorly — requests get parked server-side (the
* stall signature seen on eas-cli#4084), and several PRs may be reviewing on the
* same credential at once — so subscription runs trade a little wall-clock for a
* lot of reliability. Exported for tests.
*/
export function effectiveConcurrency(config: LoadedConfig): number {
if (config.chunk.concurrency) {
return config.chunk.concurrency;
}
return config.auth.some((entry) => entry.mode === "oauth") ? 3 : 6;
}
export async function runReview(
source: ReviewSource,
options: ReviewRunOptions,
@@ -247,10 +262,11 @@ export async function runReview(
);
// Only chunk (and add a cross-cutting pass) when the diff exceeds one chunk.
const chunked = chunks.length > 1;
const concurrency = effectiveConcurrency(config);
progress(
`Running ${selectedAgents.length} reviewer(s) [${selectedAgents.map((a) => a.id).join(", ")}] over ${chunks.length} chunk(s)` +
`${chunked ? " + cross-cutting pass" : ""} ` +
`(${kept.length} files, concurrency ${config.chunk.concurrency})…`,
`(${kept.length} files, concurrency ${concurrency})…`,
);
for (const agent of selectedAgents) {
@@ -437,7 +453,7 @@ export async function runReview(
// TIMEOUT, instead of dropping the work we break it into units that converge:
// subdivide the chunk, then a fast no-tools pass, and only report a coverage gap
// when even that can't finish inside the budget — so dropped work is never silent.
await runGrowableQueue(tasks, config.chunk.concurrency, async (task, enqueue) => {
await runGrowableQueue(tasks, concurrency, async (task, enqueue) => {
const minutes = Math.round(task.maxWaitMs / 60000);
try {
const { value, cost, truncated, tokens, model } = await promptAndParse(
@@ -598,6 +614,9 @@ export async function runReview(
"⚠️ The AI review could not complete: every review pass failed or timed out, " +
'so these changes were effectively NOT reviewed. Treat this as "no review", not "looks good".',
incomplete: coverageNotes,
// Presentation override: without it the comment header reads "Decision:
// Approve with comments" over a review that reviewed nothing (euxy#8).
couldNotComplete: true,
};
} else {
progress("Coordinating findings…");
@@ -691,6 +710,17 @@ export async function runReview(
output = { ...output, summary: reconcileSummary(output.summary, output.findings.length) };
}
// Surface provider throttling as a fact about the run: passes already waited or
// backed off, but the operator should still SEE that it happened (a run that
// was rate-limited is slower and may carry partial passes — that's the cause).
await handle!.rateLimit.check();
if (handle!.rateLimit.events > 0) {
progress(
` ⚠ provider rate-limited this run ${handle!.rateLimit.events} time(s) ` +
`(429s in the OpenCode server log) — passes waited it out rather than failing`,
);
}
// Every pass says which model actually answered it — in the job log, the step
// summary table, and the run log — so a wrong or substituted model is always
// visible, not just when the substitution warning fires.
@@ -716,6 +746,7 @@ export async function runReview(
agentFindings,
coverageNotes,
verifierDropped,
...(handle!.rateLimit.events > 0 ? { rateLimitEvents: handle!.rateLimit.events } : {}),
durationMs: Date.now() - started,
decision: output.decision,
findingCount: output.findings.length,
+8
View File
@@ -86,6 +86,14 @@ export const CoordinatorOutputSchema = z.object({
* cut-short review is never presented as complete.
*/
incomplete: z.array(z.string()).default([]),
/**
* True when EVERY pass failed — nothing was actually reviewed. Set by the
* engine, never the model. Reporters must not render an approving decision
* label for such a run (the decision enum has no "no review" member, and
* widening it would ripple through dismiss state and exit codes — this flag
* overrides the presentation instead).
*/
couldNotComplete: z.boolean().optional(),
});
export type CoordinatorOutput = z.infer<typeof CoordinatorOutputSchema>;
+98
View File
@@ -0,0 +1,98 @@
import { open } from "node:fs/promises";
import os from "node:os";
import path from "node:path";
/**
* Rate-limit evidence, read from the OpenCode server's own log.
*
* Why this exists: provider throttling reaches us in two shapes. An EXPLICIT
* failure (HTTP 429 / "rate limit" stream error) surfaces in OpenCode's log as a
* structured ERROR line — that is proof. A SILENT one (the request is accepted
* and parked server-side) produces no error anywhere and is indistinguishable
* from a wedged request from the outside. So: the log watcher turns the explicit
* case into a hard signal, and the stall path treats "stall + recent explicit
* evidence" as throttling — the one situation where the right move is to WAIT
* (re-sending the whole context into a limited account only makes it worse).
*
* During oauth runs prepareAuth points XDG_DATA_HOME at an isolated temp dir, so
* the log we read belongs to exactly this run's server — no cross-talk with a
* developer's own OpenCode sessions.
*/
/** The OpenCode server's log file under the active data dir. */
export function opencodeLogFile(env: NodeJS.ProcessEnv = process.env): string {
const dataHome = env.XDG_DATA_HOME || path.join(os.homedir(), ".local", "share");
return path.join(dataHome, "opencode", "log", "opencode.log");
}
/**
* Provider throttle signatures in OpenCode log lines. Matched only against ERROR
* lines (a chatty INFO line mentioning "retry" must not count as evidence).
*/
const RATE_LIMIT_PATTERN = /\b429\b|rate.?limit|too many requests|quota exceeded/i;
const ERROR_LINE = /\blevel=ERROR\b/;
/** Count rate-limit ERROR lines in a chunk of log text. Pure, for tests. */
export function countRateLimitLines(chunk: string): number {
let count = 0;
for (const line of chunk.split("\n")) {
if (ERROR_LINE.test(line) && RATE_LIMIT_PATTERN.test(line)) {
count++;
}
}
return count;
}
/** How recent explicit evidence must be for a stall to be read as throttling. */
const EVIDENCE_WINDOW_MS = 5 * 60 * 1000;
/**
* Incremental watcher over the OpenCode server log. `check()` reads only what was
* appended since the last call (cheap enough for poll loops); `recentlyLimited()`
* is the signal the stall path consults. Fails soft everywhere: a missing or
* unreadable log yields "no evidence", never an error.
*/
export class RateLimitWatch {
/** Total rate-limit ERROR lines seen this run. */
events = 0;
/** Wall-clock time evidence was last SEEN (observation time, not log time). */
lastSeenAt = 0;
private offset = 0;
constructor(private readonly file: string = opencodeLogFile()) {}
/** Scan newly-appended log lines for rate-limit evidence. */
async check(): Promise<number> {
try {
const handle = await open(this.file, "r");
try {
const { size } = await handle.stat();
if (size < this.offset) {
this.offset = 0; // rotated/truncated — rescan from the top
}
if (size === this.offset) {
return this.events;
}
const length = size - this.offset;
const buffer = Buffer.alloc(length);
await handle.read(buffer, 0, length, this.offset);
this.offset = size;
const found = countRateLimitLines(buffer.toString("utf8"));
if (found > 0) {
this.events += found;
this.lastSeenAt = Date.now();
}
} finally {
await handle.close();
}
} catch {
// No log yet (server just started) or unreadable — no evidence, no error.
}
return this.events;
}
/** True when explicit throttle evidence appeared within the recency window. */
recentlyLimited(now: number = Date.now()): boolean {
return this.lastSeenAt > 0 && now - this.lastSeenAt < EVIDENCE_WINDOW_MS;
}
}
+7 -3
View File
@@ -23,6 +23,9 @@
// Large diffs are split into focused chunks by changed-line count, plus a
// cross-cutting pass for multi-file issues. Diffs under maxChangedLines are one
// full-context pass. Defaults shown; raise/lower per your model + PR sizes.
// Concurrency defaults by auth mode: 6 with an API key, 3 on a subscription
// (oauth) credential — one account handles many parallel streams poorly, and
// several PRs may review on the same credential at once. Set it to override.
// "chunk": { "maxChangedLines": 1000, "maxFiles": 20, "concurrency": 4 },
// Which PRs `ecr ci` reviews. This is the source of truth for trigger policy;
@@ -56,11 +59,12 @@
// of the agents that need the pro tier, and set ECR_EXPECTED_TOKEN_ENV in the
// workflow to the comma-separated set of both env names.
// "auth": { "providers": {
// "openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_REFRESH_TOKEN" },
// "openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" },
// "openai-api": { "mode": "api-key", "tokenEnv": "OPENAI_API_KEY", "upstream": "openai" }
// } }
// (openai oauth: tokenEnv holds the REFRESH token from an `opencode auth login`
// ChatGPT sign-in — copy `.openai.refresh` from OpenCode's auth.json.)
// (openai oauth: tokenEnv holds the ACCESS token from an `opencode auth login`
// ChatGPT sign-in — `ecr setup-auth` extracts it. NEVER share the refresh
// token: it is single-use and dies on first rotation.)
"auth": {
"mode": "api-key",
"provider": "openai",