mirror of
https://github.com/expo/code-review-cli.git
synced 2026-11-13 20:28:06 -10:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
188f8521bf |
No files matched your search
@@ -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" }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 }}
|
||||
@@ -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
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
@@ -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,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'",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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
@@ -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
@@ -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
@@ -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 };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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
@@ -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
@@ -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
@@ -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,
|
||||
|
||||
@@ -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>;
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
@@ -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",
|
||||
|
||||
Reference in new issue
Block a user