mirror of
https://github.com/expo/code-review-cli.git
synced 2026-11-14 09:58:11 -10:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Philippe Loulidi
|
fbcc822c2a |
No files matched your search
@@ -37,14 +37,13 @@
|
||||
|
||||
// Anthropic models always run through the Claude Code CLI (`claude -p`) — the
|
||||
// engine is inferred per agent from its model, so this entry only supplies the
|
||||
// credential. In CI the tokenEnv holds the shared review credential (an
|
||||
// `sk-ant-oat…` token minted by `claude setup-token`, or an `sk-ant-api…`
|
||||
// Console key); locally an active `claude` login is enough — the entry could
|
||||
// even be omitted. ECR_EXPECTED_TOKEN_ENV (repo variable / workflow fallback)
|
||||
// must equal this tokenEnv.
|
||||
// credential. In CI the tokenEnv holds a token minted by `claude setup-token`
|
||||
// (an `sk-ant-oat…` subscription token; an `sk-ant-api…` Console key works too);
|
||||
// locally an active `claude` login is enough — the entry could even be omitted.
|
||||
// ECR_EXPECTED_TOKEN_ENV (repo variable) must equal this tokenEnv.
|
||||
"auth": {
|
||||
"providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -68,7 +68,7 @@ jobs:
|
||||
# source, so the guard runs from that same base source — same engine as `ci`.
|
||||
- name: Guard config tokenEnv
|
||||
env:
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_OAUTH_TOKEN' }}
|
||||
run: bun run src/cli.ts verify-config
|
||||
|
||||
- name: Run AI review
|
||||
@@ -83,10 +83,10 @@ 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 || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Shared Anthropic review credential. The claude-code engine forwards
|
||||
# it to the Claude Code CLI as the only credential (any ambient
|
||||
# ANTHROPIC_* key is stripped). A setup-token or Console key both work.
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_OAUTH_TOKEN' }}
|
||||
# Claude Max/Team subscription token minted by `claude setup-token`
|
||||
# (~1 year lifetime). The claude-code engine forwards it to the Claude
|
||||
# Code CLI as the only credential (any ANTHROPIC_* key is stripped).
|
||||
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
@@ -48,19 +48,21 @@ npx @expo/code-review-cli setup-auth
|
||||
npx @expo/code-review-cli doctor
|
||||
```
|
||||
|
||||
`setup-auth` reads the repo's config and walks through each credential it needs.
|
||||
The scaffolded default is **Anthropic via the Claude Code CLI**: locally your
|
||||
`claude` login is enough, and for CI it helps you mint a token with
|
||||
`claude setup-token`. It also handles the alternatives (an OpenAI **API key**,
|
||||
a **ChatGPT/Codex subscription** sign-in). `doctor` offers to run it whenever a
|
||||
credential is missing.
|
||||
`setup-auth` reads the repo's config and walks through each credential it needs:
|
||||
an OpenAI **API key** (the scaffolded default — it prints where to create the key
|
||||
and the exact restricted permissions to grant), and/or a **ChatGPT/Codex
|
||||
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 credential as the repo secret the scaffolded workflow forwards
|
||||
(`CLAUDE_CODE_REVIEW_SHARED_API_TOKEN` by default — an `sk-ant-oat…` token from
|
||||
`claude setup-token`, or an `sk-ant-api…` Console key; the CLI reads either).
|
||||
In CI, store the same values as repo secrets (`OPENAI_API_KEY`; plus
|
||||
`CODEX_OAUTH_ACCESS_TOKEN` for the mixed setup) — the scaffolded workflow
|
||||
forwards them.
|
||||
|
||||
Prefer **OpenAI** (API key, or a ChatGPT/Codex subscription, or both mixed) or
|
||||
another provider? See [Other providers & auth modes](#other-providers) below.
|
||||
**Have a ChatGPT Plus/Pro (Codex) subscription? Use both.** The recommended
|
||||
production setup pairs the subscription (runs the default models at no marginal
|
||||
cost) with the usage-based key (covers only the pro-tier models the subscription
|
||||
excludes) — see [the mixed setup](#other-providers) below. Prefer
|
||||
**Anthropic/Claude** or another provider? Same section.
|
||||
|
||||
### Reviewing (already configured)
|
||||
|
||||
@@ -194,7 +196,7 @@ your-monorepo/
|
||||
// Central guardrails every scope inherits and CANNOT override.
|
||||
"defaults": {
|
||||
// The ONLY place auth/tokenEnv is honored (besides the root config.jsonc).
|
||||
"auth": { "providers": { "anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" } } },
|
||||
"auth": { "mode": "api-key", "provider": "openai", "tokenEnv": "OPENAI_API_KEY" },
|
||||
"enforceAgents": ["security"], // always runs on every scope, roster or not
|
||||
"commentTag": "expo-ai-code-reviewer" // per-scope markers derive from this
|
||||
},
|
||||
@@ -212,7 +214,7 @@ your-monorepo/
|
||||
// apps/api/.expo-code-review/config.jsonc (the api team owns this)
|
||||
{
|
||||
// NO "auth" block — locked centrally; a tokenEnv here is rejected by loader + CI guard.
|
||||
"model": "anthropic/claude-sonnet-5",
|
||||
"model": "openai/gpt-5.5",
|
||||
"policy": { "includeSuggestions": false },
|
||||
"noise": { "additionalIgnores": ["apps/api/**/__generated__/**"] }
|
||||
// shared.md, coordinator.md, agents/*.md live beside this file — the api team's roster.
|
||||
@@ -314,11 +316,6 @@ Ownership is enforced with CODEOWNERS: `/.expo-code-review/routing.jsonc @your-i
|
||||
- **Reporter** — posts/updates a single fingerprinted PR comment (CI), or prints
|
||||
a grouped summary (local). Findings below the configured severity floor are
|
||||
suppressed.
|
||||
- **Whole-review reuse** — automated CI stores a hash of each review job's inputs
|
||||
in the hidden state of that comment. If a restack leaves a scope's files and
|
||||
review configuration unchanged, its complete prior result is reused. Manual
|
||||
`/review`, partial/failed reviews, stack-aware review, and model-adjudicated
|
||||
feedback always run fresh.
|
||||
|
||||
Built on the [OpenCode](https://opencode.ai) SDK, which spawns the model provider
|
||||
and applies the provider's prompt caching automatically.
|
||||
@@ -390,7 +387,7 @@ show writes (there is nothing to re-read within the run).
|
||||
---
|
||||
description: One line the router uses to decide relevance.
|
||||
alwaysRun: true # run even when the router would skip this agent
|
||||
model: anthropic/claude-opus-5 # override the default model
|
||||
model: openai/gpt-5.5-pro # override the default model
|
||||
temperature: 0.1
|
||||
---
|
||||
|
||||
@@ -399,14 +396,14 @@ temperature: 0.1
|
||||
|
||||
For a real-world example, see eas-cli's
|
||||
[`.expo-code-review/`](https://github.com/expo/eas-cli/tree/main/.expo-code-review)
|
||||
— correctness/security/consistency agents, a stronger model for security, and
|
||||
per-repo `noise.additionalIgnores`.
|
||||
— correctness/security/consistency agents, a stronger model for security + the
|
||||
coordinator, and per-repo `noise.additionalIgnores`.
|
||||
|
||||
`config.jsonc` (JSONC — comments + trailing commas supported):
|
||||
|
||||
```jsonc
|
||||
{
|
||||
"model": "anthropic/claude-sonnet-5", // default model for the specialists
|
||||
"model": "openai/gpt-5.5", // default model for the specialists
|
||||
"policy": { "includeSuggestions": false }, // suppress suggestion-severity findings
|
||||
"chunk": { "maxChangedLines": 1000, "maxFiles": 20 }, // concurrency defaults: 6 (API key) / 3 (subscription)
|
||||
"noise": { "additionalIgnores": ["packages/*/build/**"] },
|
||||
@@ -415,8 +412,8 @@ per-repo `noise.additionalIgnores`.
|
||||
"skipLabel": "ai-review:skip" }, // "label" (only labeled PRs)
|
||||
"breakGlass": { "marker": "/skip-review" }, // PR body marker that skips the review
|
||||
"commentTag": "expo-ai-code-reviewer", // hidden tag used to find/update the comment
|
||||
"auth": { "providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" } } }
|
||||
"auth": { "mode": "api-key", "provider": "openai",
|
||||
"tokenEnv": "OPENAI_API_KEY" }
|
||||
}
|
||||
```
|
||||
|
||||
@@ -430,17 +427,15 @@ Precedence: **`REVIEWER_MODEL` env** (global override) → per-file **frontmatte
|
||||
setup, and a developer can override everything locally.
|
||||
|
||||
- **Specialist agents** (correctness/security/consistency) benefit from a
|
||||
reasoning-tier model — **`anthropic/claude-sonnet-5`** is the quality/speed
|
||||
sweet spot (the scaffolded default). The **Opus tier** finds more but is slower
|
||||
and more expensive, so scope it to the highest-stakes agent: **security runs on
|
||||
`anthropic/claude-opus-5`** (set in `security.md` frontmatter), the rest on the
|
||||
default.
|
||||
reasoning-tier model — **`openai/gpt-5.5`** is the quality/speed sweet spot
|
||||
(the scaffolded default). The **pro tier** finds more but is slower and more
|
||||
expensive, so scope it to the highest-stakes agent: **security runs on
|
||||
`openai/gpt-5.5-pro`** (set in `security.md` frontmatter), the rest on the default.
|
||||
- **The coordinator** makes the final call (dedupe / re-judge / decide) — worth a
|
||||
strong model; the scaffold pins it to `anthropic/claude-opus-5` in
|
||||
`coordinator.md` frontmatter.
|
||||
strong model; set it in `coordinator.md` frontmatter.
|
||||
- If latency/timeouts dominate on big PRs, moving the specialists to a faster model
|
||||
(e.g. `anthropic/claude-haiku-4-5`) is the most direct lever (a real recall
|
||||
tradeoff — measure it).
|
||||
(e.g. `openai/gpt-5.4-mini`) is the most direct lever (a real recall tradeoff —
|
||||
measure it).
|
||||
- **Every run logs which model actually answered each pass** — in the job log
|
||||
(`Models used — …`), the Actions step summary table, and the run log's
|
||||
`agentModels` — and warns loudly if a pass ran on a different model than
|
||||
@@ -655,34 +650,11 @@ visible even in CI (where the run log is ephemeral).
|
||||
<details>
|
||||
<summary><b>Other providers & auth modes</b></summary>
|
||||
|
||||
The scaffolded default is **Anthropic via the Claude Code CLI** — see First-time
|
||||
setup above and the Anthropic bullet below for how it authenticates and is
|
||||
sandboxed. Everything is set in `config.auth`; non-anthropic providers get their
|
||||
credentials through OpenCode.
|
||||
The recommended setup is an OpenAI API key — see Usage above. Alternatives, all
|
||||
set in `config.auth` (credentials come from OpenCode):
|
||||
|
||||
- **Anthropic / Claude (the default)** — use `anthropic/...` model ids and every
|
||||
anthropic pass runs through the **Claude Code CLI** (`claude -p
|
||||
--output-format json`), inferred from the model. The credential is (in order) a
|
||||
`tokenEnv` you name, an ambient `CLAUDE_CODE_OAUTH_TOKEN`, or your local
|
||||
`claude` login — an `auth` entry is entirely optional. Run `claude setup-token`
|
||||
for a Max/Team subscription token or point `tokenEnv` at an Anthropic Console
|
||||
API key (`sk-ant-api…`, forwarded as `ANTHROPIC_API_KEY`); the CLI reads
|
||||
either. `ecr setup-auth` walks you through it. Each pass is trust-isolated and
|
||||
read-only: it runs with `--safe-mode` (no `CLAUDE.md`/hooks/MCP/plugins),
|
||||
`--strict-mcp-config`, `--permission-mode dontAsk`, and only the
|
||||
`Read`/`Grep`/`Glob` tools — never `Bash`/`Edit`/`Write`/`WebFetch`/`WebSearch`.
|
||||
The child env is an allowlist that omits ambient `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`
|
||||
(only the configured credential is re-injected).
|
||||
```jsonc
|
||||
// The scaffolded default. No anthropic entry at all falls back to your
|
||||
// `claude` login.
|
||||
"auth": { "providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }
|
||||
} }
|
||||
```
|
||||
- **OpenAI: ChatGPT/Codex subscription (OAuth) + usage-based API key — the
|
||||
recommended mix if you review with OpenAI.** OpenAI permits subscription auth
|
||||
in third-party tools, and OpenCode
|
||||
- **ChatGPT/Codex subscription (OAuth) + usage-based API key — the recommended
|
||||
mix.** OpenAI permits subscription auth in third-party tools, and OpenCode
|
||||
ships the plugin for it — so the reviewer runs its default models on the
|
||||
subscription (zero marginal cost) and reserves the metered key for pro-tier
|
||||
models the subscription doesn't offer (`gpt-5.5-pro` is subscription-excluded).
|
||||
@@ -721,6 +693,25 @@ credentials through OpenCode.
|
||||
One caveat: OpenCode can't price alias models (they're config-declared), so
|
||||
pro passes report `$0` in the run log's cost column — token counts are
|
||||
correct, and the OpenAI project dashboard is the source of truth for spend.
|
||||
- **Anthropic / Claude** — use `anthropic/...` model ids and every anthropic pass
|
||||
runs through the **Claude Code CLI** (`claude -p --output-format json`), inferred
|
||||
from the model. The credential is (in order) a `tokenEnv` you name, an ambient
|
||||
`CLAUDE_CODE_OAUTH_TOKEN`, or your local `claude` login — an `auth` entry is
|
||||
entirely optional. Run `claude setup-token` for a Max/Team subscription token
|
||||
(forwarded as `CLAUDE_CODE_OAUTH_TOKEN`) or point `tokenEnv` at an Anthropic
|
||||
Console API key (`sk-ant-api…`, forwarded as `ANTHROPIC_API_KEY`); the CLI reads
|
||||
either. `ecr setup-auth` walks you through it. Each pass is trust-isolated and
|
||||
read-only: it runs with `--safe-mode` (no `CLAUDE.md`/hooks/MCP/plugins),
|
||||
`--strict-mcp-config`, `--permission-mode dontAsk`, and only the
|
||||
`Read`/`Grep`/`Glob` tools — never `Bash`/`Edit`/`Write`/`WebFetch`/`WebSearch`.
|
||||
The child env is an allowlist that omits ambient `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`
|
||||
(only the configured credential is re-injected).
|
||||
```jsonc
|
||||
// Optional — no anthropic entry at all falls back to your `claude` login.
|
||||
"auth": { "providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" }
|
||||
} }
|
||||
```
|
||||
- **Another provider** — the current path is the `REVIEWER_MODEL`
|
||||
env override: `opencode auth login` once (pick the provider), then run with
|
||||
e.g. `REVIEWER_MODEL=google/gemini-3-pro`. It overrides every agent's model
|
||||
@@ -733,9 +724,8 @@ any other) OpenCode provider, and each agent's `model` selects its engine.
|
||||
`REVIEWER_MODEL` still overrides every agent's model (and therefore every agent's
|
||||
engine), converging the whole run onto one engine.
|
||||
|
||||
There is no shared fallback key; if a run fails for lack of credentials, log in
|
||||
with `claude` (the default) or authenticate a provider in OpenCode. `ecr doctor`
|
||||
diagnoses setup.
|
||||
There is no shared fallback key; if a run fails for lack of credentials, authenticate
|
||||
a provider in OpenCode. `ecr doctor` diagnoses setup.
|
||||
|
||||
**Setup errors fail fast, with the fix in the message.** A bad credential or model id
|
||||
would otherwise fail every pass identically — a run that spends its whole budget
|
||||
|
||||
@@ -61,14 +61,6 @@ The absolute constraint is the carve-out (`suppress.ts:39-44`): a `critical` or
|
||||
|
||||
**The comment is the durable store.** An embedded `<!-- tag:state=... -->` comment holds base64 JSON of the FULL review plus dismissals — not just what is rendered — so a later `/dismiss` or `/undismiss` can re-render without re-running the engine (`render.ts:198-202`, `encodeState`/`parseReviewState` at `render.ts:493-513`). There is no side-channel database; the comment body is the state. Dismissed findings render in a collapsed `<details>` section, disjoint from the active list (a finding is never in both), keeping the main list clean while preserving a `by`/`reason` audit trail.
|
||||
|
||||
## Review Result Cache
|
||||
|
||||
The same durable comment state is the cache for automated CI reviews. `reviewInputHash` (`src/core/review-cache.ts`) hashes each scoped changed file's content from the materialized PR-head tree, a normalized patch (blob ids and hunk coordinates removed; deletion/mode information retained), and every stable input that can change the result: the engine package version, resolved prompts/config/models, PR title/body, selected agents and routing mode, and external context. It deliberately excludes the temporary trusted-config path and the base/head commit OIDs. A restack changes OIDs and may move hunk line numbers; if it leaves the actual review input byte-identical, CI reuses the complete `CoordinatorOutput` already embedded in the most recent author-verified reviewer comment instead of launching the agents again. Symlinks are hashed as link targets and never followed. A package release changes the key so upgraded review logic runs once. Legacy comments store one `inputHash`; aggregate comments store one per `ScopeReviewResult`, so an unchanged scope can hit independently while a changed sibling reruns.
|
||||
|
||||
Reuse is conservative. Manual `/review`/`--force` always reruns. Partial or failed reviews (`incomplete` or `couldNotComplete`) are never cacheable. Aggregate truncation removes the affected scope's hash because the embedded scope is no longer the full result. Stack-aware requalification and model-backed author-feedback seams stay fresh because their live GitHub inputs are not yet represented in the key. Cache lookup failures degrade to an ordinary fresh review. These gates make a false miss cost tokens, while preventing a false hit from presenting stale or incomplete coverage as current.
|
||||
|
||||
The hash is state, not display: it lives only inside the base64 state marker and survives `/dismiss`/`/undismiss` re-renders. The existing author-plus-marker ownership check remains the trust boundary for adopting prior state; a PR author cannot forge a look-alike comment and supply a cache entry.
|
||||
|
||||
**The coverage note only cries wolf when there is a wolf.** The partial-coverage warning renders only when `review.incomplete` is non-empty (`render.ts:166`) — the aggregate renderer additionally triggers it when unmatched files exist (`render.ts:373`) — a deliberate anti-noise choice so the warning is trustworthy when it appears (`render.test.ts:34`). The advisory footer ("never blocks a merge and never auto-approves") is duplicated verbatim across both renderers because it is a required trust-boundary claim, not incidental text.
|
||||
|
||||
## Truncation and Aggregate State
|
||||
|
||||
@@ -45,8 +45,6 @@ Flag exclusivity is rejected outright, never silently resolved, because silently
|
||||
|
||||
**It never fails the PR's checks.** A reviewer error degrades to a posted failure comment, not a non-zero process exit [observed: `AGENTS.md`; `src/commands/ci.ts:354-373` legacy catch posts a summary; `src/commands/ci.ts:571-576` per-scope catch builds a `failureReview` placeholder instead of aborting the fan-out]. PR-facing comment text uses `publicFailureReason(error)`, not the raw `errorMessage`, so the credential and internal detail never leak into a comment (LLP 0002) [observed: `src/commands/ci.ts:176,365,575`].
|
||||
|
||||
**Automated reruns reuse unchanged complete results.** Before launching agents, CI hashes the review job's scoped diff and stable review inputs and compares it with the hash hidden in the newest author-verified reviewer comment. An exact match re-renders the stored result (so links, dismissals, and live annotation-only feedback still update) without spending model budget. Routed CI does this independently per scope. Manual `/review`/`--force`, stack-aware runs, model-backed feedback runs, partial/failed results, and truncated aggregate state all bypass reuse and run fresh. The hash contract and durable-state rules are owned by LLP 0005's Review Result Cache section.
|
||||
|
||||
`--unsafe-config-from-head` is a deliberate escape hatch that reverses this model, loading config from the checkout. It is never scaffolded by `ecr init`, always prints a loud `⚠ SECURITY` warning, and is scheduled for removal on a minor version boundary [observed: `src/commands/ci.ts:85-91` `CI_USAGE`, `155-162` warning].
|
||||
|
||||
`--context-file` in CI takes the opposite read-error policy from `ecr review`, and the split is dictated by the never-fail-checks invariant. `ci` reads the file once before any fan-out and, on a missing/oversized/unreadable path, WARNs to stderr and continues with no context — it never fails, because a broken Atlantis-provided plan file must not turn the PR's check red [observed: `src/commands/ci.ts` `--context-file` read → stderr warn, no `exitCode`, `contextText` stays undefined]. The same reader (`readContextFile`, capped at `MAX_CONTEXT_FILE_BYTES`) backs both commands; only the catch policy differs — fatal locally, warn in CI. The byte-bounded raw text threads through the shared `CiRunOptions` into both `runLegacyCi` and `runRoutedCi`, and from there into every per-scope `runReview` as `contextText` (char-capped at prompt build) [observed: `src/commands/ci.ts` `CiRunOptions.contextText`; `runReview` calls in `runLegacyCi` and the fan-out loop].
|
||||
|
||||
@@ -91,11 +91,8 @@ settle. A reviewer of this feature named the case: a Terraform prompt citing
|
||||
"this is very likely going to drift" [observed] — not one of those has an extension, and
|
||||
`anthropic/claude-opus-5` is shaped identically while naming no file. Syntax cannot tell
|
||||
them apart, so an extensionless token counts as a citation exactly when it *names
|
||||
something real*: probed against the scope's own subtree first, then the repo root (a
|
||||
scoped prompt citing `alerts/` means its own tree even when the root has one too), brace
|
||||
lists cut back to their fixed prefix. A token carrying `*` resolves as a glob instead,
|
||||
scope-rebased when that is what matches, so the suggested `glob:` ref itself resolves.
|
||||
Model ids resolve to nothing and stay prose.
|
||||
something real*: probed against the repo root and the scope's own subtree, brace lists
|
||||
and wildcards cut back to their fixed prefix. Model ids resolve to nothing and stay prose.
|
||||
|
||||
The consequence is deliberate: an extensionless path that had already drifted before
|
||||
adoption is invisible (nothing distinguishes it from prose), while every one that is true
|
||||
|
||||
@@ -188,14 +188,8 @@ test("checkProviderAuth: FORBIDDEN + cross-provider guards still fire for an ant
|
||||
test("checkProviderAuth: a non-anthropic entry naming CLAUDE_CODE_OAUTH_TOKEN is refused", () => {
|
||||
// The long-lived Claude Max/Team subscription token would otherwise be forwarded to
|
||||
// a FOREIGN provider (here openai) as its bearer — it is neither in FORBIDDEN nor a
|
||||
// PROVIDER_KEY_ENV value, so only the anthropic-owned guard catches it. The
|
||||
// scaffolded default CLAUDE_CODE_REVIEW_SHARED_API_TOKEN is anthropic-owned the
|
||||
// same way: it always holds an Anthropic credential (templates/config.jsonc).
|
||||
for (const tokenEnv of [
|
||||
"CLAUDE_CODE_OAUTH_TOKEN",
|
||||
"ANTHROPIC_AUTH_TOKEN",
|
||||
"CLAUDE_CODE_REVIEW_SHARED_API_TOKEN",
|
||||
]) {
|
||||
// PROVIDER_KEY_ENV value, so only the anthropic-owned guard catches it.
|
||||
for (const tokenEnv of ["CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN"]) {
|
||||
const r = checkProviderAuth(cfg({ mode: "api-key", provider: "openai", tokenEnv }), {
|
||||
[tokenEnv]: `sk-ant-oat01-${"x".repeat(95)}`,
|
||||
});
|
||||
@@ -213,16 +207,6 @@ test("checkProviderAuth: an anthropic entry naming CLAUDE_CODE_OAUTH_TOKEN is st
|
||||
expect(r.ok).toBe(true);
|
||||
});
|
||||
|
||||
test("checkProviderAuth: the scaffolded default setup (anthropic + shared token env) passes", () => {
|
||||
// The exact shape `ecr init` scaffolds (templates/config.jsonc) — anthropic owns
|
||||
// the shared env, so its own entry must not trip the ownership guard.
|
||||
const r = checkProviderAuth(
|
||||
cfg({ provider: "anthropic", tokenEnv: "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }),
|
||||
{ CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: `sk-ant-oat01-${"x".repeat(95)}` },
|
||||
);
|
||||
expect(r.ok).toBe(true);
|
||||
});
|
||||
|
||||
test("checkProviderAuth: an upstream=anthropic alias may name CLAUDE_CODE_OAUTH_TOKEN", () => {
|
||||
// upstream is anthropic, so the owner matches the upstream and the token is not
|
||||
// being sent to a foreign provider.
|
||||
|
||||
@@ -69,8 +69,6 @@ test("isCodeCitation separates paths from prose tokens", () => {
|
||||
"src/entities/oauth/",
|
||||
".github/workflows/**",
|
||||
"*PrivacyPolicy.ts",
|
||||
"castai-spot.tf",
|
||||
"*/.terraform.lock.hcl",
|
||||
]) {
|
||||
expect(isCodeCitation(token)).toBe(true);
|
||||
}
|
||||
@@ -250,67 +248,6 @@ test("the ref the message suggests silences the citation it was suggested for",
|
||||
}
|
||||
});
|
||||
|
||||
test("a scope-relative citation WITH an extension is covered by its precise ref", async () => {
|
||||
const files = (pin: string): Record<string, string> => ({
|
||||
"infrastructure/pgbouncer/base/pdb.yaml": "kind: PodDisruptionBudget\n",
|
||||
"server/www/k8s/pdb.yaml": "kind: PodDisruptionBudget\n",
|
||||
"infrastructure/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infrastructure/.expo-code-review/agents/k8s.md": `${pin}\n\nSee \`pgbouncer/base/pdb.yaml\`.\n`,
|
||||
});
|
||||
|
||||
const unpinned = await checkConfigRefs({ root: await makeRepo(files("")) });
|
||||
expect(unpinned.problems).toHaveLength(1);
|
||||
// the suggestion is the precise path, never a basename glob
|
||||
expect(unpinned.problems[0]!.problem).toContain(
|
||||
`${REF} infrastructure/pgbouncer/base/pdb.yaml —`,
|
||||
);
|
||||
|
||||
const pinned = await checkConfigRefs({
|
||||
root: await makeRepo(
|
||||
files(`<!-- ${REF} infrastructure/pgbouncer/base/pdb.yaml — the PDB this rule is about -->`),
|
||||
),
|
||||
});
|
||||
expect(pinned.problems).toEqual([]);
|
||||
});
|
||||
|
||||
test("a scope-relative wildcard token gets a glob suggestion that itself resolves", async () => {
|
||||
const files = (pin: string): Record<string, string> => ({
|
||||
"infra/pgbouncer/production/patch.yaml": "x\n",
|
||||
"infra/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infra/.expo-code-review/agents/a.md": `${pin}\nSee \`pgbouncer/*/patch.yaml\`.\n`,
|
||||
});
|
||||
const report = await checkConfigRefs({ root: await makeRepo(files("")) });
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.problem).toContain(`${REF} glob:infra/pgbouncer/*/patch.yaml`);
|
||||
// …and that suggestion, once written, clears the citation and resolves
|
||||
const pinned = await checkConfigRefs({
|
||||
root: await makeRepo(files(`<!-- ${REF} glob:infra/pgbouncer/*/patch.yaml — the patches -->`)),
|
||||
});
|
||||
expect(pinned.problems).toEqual([]);
|
||||
});
|
||||
|
||||
test("scope wins over root when both could satisfy an extensionless citation", async () => {
|
||||
const root = await makeRepo({
|
||||
"alerts/root.txt": "x\n",
|
||||
"infra/alerts/scope.tf": "y\n",
|
||||
"infra/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infra/.expo-code-review/agents/a.md": "Watch `alerts/` closely.\n",
|
||||
});
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.problem).toContain(`${REF} infra/alerts/ —`);
|
||||
});
|
||||
|
||||
test("a token carrying its own wildcard keeps the glob suggestion", async () => {
|
||||
const root = await makeRepo({
|
||||
".github/workflows/deploy.yml": "on: push\n",
|
||||
".expo-code-review/config.jsonc": "{}\n",
|
||||
".expo-code-review/agents/sec.md": "Changes under `.github/workflows/**` are high risk.\n",
|
||||
});
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems[0]!.problem).toContain(`${REF} glob:.github/workflows/**`);
|
||||
});
|
||||
|
||||
test("a scope-relative annotated ref is broken, and names the root-relative fix", async () => {
|
||||
const root = await makeRepo({
|
||||
"infrastructure/general-central/module/main.tf": "resource {}\n",
|
||||
@@ -472,27 +409,6 @@ test("reviewSetupRefNotes advises on broken refs and on cited code the PR change
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
test("a ref to scrubbed ambient config is not review advice (the read root removes it)", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
".expo-code-review/shared.md":
|
||||
`<!-- ${REF} AGENTS.md — the conventions to judge against -->\n` +
|
||||
`<!-- ${REF} CLAUDE.md#conventions — same, with an anchor -->\n` +
|
||||
`<!-- ${REF} glob:**/.env.local — same, as a glob -->\n` +
|
||||
`<!-- ${REF} src/gone.ts — really moved -->\n`,
|
||||
}),
|
||||
);
|
||||
const setupDir = path.join(root, ".expo-code-review");
|
||||
const notes = await reviewSetupRefNotes({ root, setupDirs: [setupDir], changedFiles: [] });
|
||||
expect(notes).toHaveLength(1);
|
||||
expect(notes[0]).toContain("1 ref(s)");
|
||||
expect(notes[0]).not.toContain("shared.md:1");
|
||||
|
||||
// The standalone gate still reports all of them — there the tree is a real checkout.
|
||||
expect((await checkConfigRefs({ root })).problems).toHaveLength(4);
|
||||
});
|
||||
|
||||
test("setup problems are labelled by setup-dir position when the config lives outside the tree", async () => {
|
||||
const code = await makeRepo({ "src/auth.ts": "export const a = 1;\n" });
|
||||
const trustedBase = await makeRepo(
|
||||
|
||||
@@ -132,7 +132,7 @@ test("init --token-env refuses when review workflows already exist without --for
|
||||
path.join(root, ".github", "workflows", "expo-code-review.yml"),
|
||||
"utf8",
|
||||
);
|
||||
expect(text).toContain("CLAUDE_CODE_REVIEW_SHARED_API_TOKEN");
|
||||
expect(text).toContain("OPENAI_API_KEY");
|
||||
expect(text).not.toContain("CLAUDE_CODE_OAUTH_TOKEN");
|
||||
});
|
||||
|
||||
@@ -143,7 +143,7 @@ test("init --token-env --force rewrites existing review workflows", async () =>
|
||||
for (const file of ["expo-code-review.yml", "expo-code-review-command.yml"]) {
|
||||
const text = await readFile(path.join(root, ".github", "workflows", file), "utf8");
|
||||
expect(text).toContain("CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}");
|
||||
expect(text).not.toContain("CLAUDE_CODE_REVIEW_SHARED_API_TOKEN");
|
||||
expect(text).not.toContain("OPENAI_API_KEY");
|
||||
}
|
||||
});
|
||||
|
||||
@@ -210,7 +210,7 @@ test("init --force-workflows refuses when only the forwarded secret line is non-
|
||||
// The fallback alone reads as default, so the guard must also see the secret line.
|
||||
const wf = path.join(root, ".github", "workflows", "expo-code-review.yml");
|
||||
const patched = (await readFile(wf, "utf8")).replaceAll(
|
||||
"CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}",
|
||||
"OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}",
|
||||
"ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}",
|
||||
);
|
||||
await writeFile(wf, patched, "utf8");
|
||||
@@ -220,7 +220,7 @@ test("init --force-workflows refuses when only the forwarded secret line is non-
|
||||
process.exitCode = 0;
|
||||
expect(err).toContain("ANTHROPIC_API_KEY");
|
||||
expect(err).toContain("--token-env");
|
||||
// Untouched: no silent reversion to the default wiring.
|
||||
// Untouched: no silent reversion to the default OpenAI wiring.
|
||||
expect(await readFile(wf, "utf8")).toContain(
|
||||
"ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}",
|
||||
);
|
||||
@@ -229,7 +229,7 @@ test("init --force-workflows refuses when only the forwarded secret line is non-
|
||||
test("init --force-workflows without --token-env is fine on a default-credential repo", async () => {
|
||||
const root = await freshRepo();
|
||||
await initCommand([]);
|
||||
// Default credential wiring: nothing to revert, so no refusal.
|
||||
// Default OPENAI_API_KEY wiring: nothing to revert, so no refusal.
|
||||
await initCommand(["--force-workflows"]);
|
||||
expect(process.exitCode ?? 0).toBe(0);
|
||||
expect(
|
||||
@@ -240,8 +240,8 @@ test("init --force-workflows without --token-env is fine on a default-credential
|
||||
test("init --token-env prints the config.jsonc auth edit as a next step", async () => {
|
||||
await freshRepo();
|
||||
const out = await captureStdout(() => initCommand(["--token-env", "CLAUDE_CODE_OAUTH_TOKEN"]));
|
||||
// The scaffolded config.jsonc still declares the default tokenEnv, so without
|
||||
// this step CI's verify-config fails and the user has no pointer to why.
|
||||
// The scaffolded config.jsonc still declares OPENAI_API_KEY, so without this
|
||||
// step CI's verify-config fails and the user has no pointer to why.
|
||||
expect(out).toContain("config.jsonc at this credential");
|
||||
expect(out).toContain("refuses to review until the config names `CLAUDE_CODE_OAUTH_TOKEN`");
|
||||
});
|
||||
@@ -275,7 +275,7 @@ test("substituteTokenEnv fails loudly when the template markers drift", () => {
|
||||
});
|
||||
|
||||
test("parseTokenEnvs validates names and refuses well-known unrelated secrets", () => {
|
||||
expect(parseTokenEnvs(undefined)).toEqual(["CLAUDE_CODE_REVIEW_SHARED_API_TOKEN"]);
|
||||
expect(parseTokenEnvs(undefined)).toEqual(["OPENAI_API_KEY"]);
|
||||
expect(parseTokenEnvs("A_TOKEN, B_TOKEN")).toEqual(["A_TOKEN", "B_TOKEN"]);
|
||||
expect(() => parseTokenEnvs("lower_case")).toThrow(/UPPER_SNAKE_CASE/);
|
||||
expect(() => parseTokenEnvs("GH_TOKEN")).toThrow(/unrelated secret/);
|
||||
|
||||
@@ -87,17 +87,13 @@ test("aggregate: default scope uses plain fingerprints, non-default uses scoped
|
||||
|
||||
test("aggregate: state round-trips through parseReviewState (scopes intact); a v1 body still parses", () => {
|
||||
const results = [
|
||||
{
|
||||
...scope("default", true, { findings: [finding({ title: "D" })] }),
|
||||
inputHash: "a".repeat(64),
|
||||
},
|
||||
{ ...scope("api", false, { findings: [finding({ title: "W" })] }), inputHash: "b".repeat(64) },
|
||||
scope("default", true, { findings: [finding({ title: "D" })] }),
|
||||
scope("api", false, { findings: [finding({ title: "W" })] }),
|
||||
];
|
||||
const body = renderAggregateMarkdown(results, "tag", []);
|
||||
const state = parseReviewState(body, "tag");
|
||||
expect(state).not.toBeNull();
|
||||
expect(state!.scopes?.map((s) => s.scope)).toEqual(["default", "api"]);
|
||||
expect(state!.scopes?.map((s) => s.inputHash)).toEqual(["a".repeat(64), "b".repeat(64)]);
|
||||
|
||||
// A pre-routing v1 comment still parses (no scopes field).
|
||||
const v1 = renderMarkdown(review({ findings: [finding()] }), "tag");
|
||||
@@ -253,12 +249,7 @@ test("aggregate: the /undismiss pin set survives a render with no records and a
|
||||
}),
|
||||
);
|
||||
const big = renderAggregateMarkdown(
|
||||
[
|
||||
{
|
||||
...scope("api", false, { decision: "request_changes", findings: [pinned, ...many] }),
|
||||
inputHash: "c".repeat(64),
|
||||
},
|
||||
],
|
||||
[scope("api", false, { decision: "request_changes", findings: [pinned, ...many] })],
|
||||
"tag",
|
||||
[],
|
||||
undefined,
|
||||
@@ -268,5 +259,4 @@ test("aggregate: the /undismiss pin set survives a render with no records and a
|
||||
);
|
||||
expect(big.length).toBeLessThan(65_000);
|
||||
expect(parseReviewState(big, "tag")!.pins).toEqual(pins);
|
||||
expect(parseReviewState(big, "tag")!.scopes![0]!.inputHash).toBeUndefined();
|
||||
});
|
||||
@@ -70,14 +70,6 @@ test("review state (review + dismissals) round-trips via parseReviewState", () =
|
||||
expect(state!.review.findings.length).toBe(1);
|
||||
});
|
||||
|
||||
test("review input hash round-trips only in hidden comment state", () => {
|
||||
const inputHash = "a".repeat(64);
|
||||
const body = renderMarkdown(base, "tag", [], undefined, [], [], inputHash);
|
||||
expect(parseReviewState(body, "tag")!.inputHash).toBe(inputHash);
|
||||
expect(Buffer.from(inputHash).toString("base64")).not.toBe(inputHash);
|
||||
expect(body).not.toContain(inputHash);
|
||||
});
|
||||
|
||||
test("links a finding location to the PR diff line when the line is in the diff", () => {
|
||||
const out = renderMarkdown(
|
||||
{ ...base, findings: [finding({ file: "src/a.ts", line: 12 })] },
|
||||
|
||||
@@ -1,173 +0,0 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import type { LoadedConfig } from "../config/schema.js";
|
||||
import { reviewCanBeReused, reviewInputHash, reviewMatchesInput } from "../core/review-cache.js";
|
||||
import type { CoordinatorOutput, DiffEntry } from "../core/schema.js";
|
||||
|
||||
const config = (over: Partial<LoadedConfig> = {}): LoadedConfig => ({
|
||||
configDir: "/tmp/config-a",
|
||||
sharedPromptText: "shared rules",
|
||||
agents: [
|
||||
{
|
||||
id: "correctness",
|
||||
description: "bugs",
|
||||
alwaysRun: false,
|
||||
model: "anthropic/claude-sonnet-5",
|
||||
temperature: 0.1,
|
||||
tools: { read: true },
|
||||
promptText: "find bugs",
|
||||
},
|
||||
],
|
||||
coordinator: {
|
||||
model: "anthropic/claude-sonnet-5",
|
||||
temperature: 0,
|
||||
promptText: "dedupe",
|
||||
},
|
||||
policy: { includeSuggestions: false },
|
||||
chunk: { maxChangedLines: 1000, maxFiles: 20 },
|
||||
noise: { additionalIgnores: [], additionalMarkers: [] },
|
||||
breakGlassMarker: "/skip-review",
|
||||
commentTag: "review",
|
||||
auth: [{ provider: "anthropic", mode: "oauth", tokenEnv: "TOKEN" }],
|
||||
review: { trigger: "all", label: "ai-review", skipLabel: "ai-review:skip" },
|
||||
stack: {
|
||||
enabled: false,
|
||||
maxDepth: 4,
|
||||
maxPrs: 8,
|
||||
maxFilesPerPr: 100,
|
||||
requireSameAuthor: true,
|
||||
confirmWithPatch: false,
|
||||
maxConfirmations: 10,
|
||||
},
|
||||
feedback: {
|
||||
mode: "annotate",
|
||||
match: "both",
|
||||
dismiss: "never",
|
||||
protectedCategories: ["security", "secrets"],
|
||||
maxAdjudications: 10,
|
||||
},
|
||||
...over,
|
||||
});
|
||||
|
||||
const files: DiffEntry[] = [
|
||||
{
|
||||
path: "b.ts",
|
||||
patch: "diff --git a/b.ts b/b.ts\nindex 111..222 100644\n@@ -1 +1 @@\n-old\n+new",
|
||||
status: "M",
|
||||
},
|
||||
{ path: "a.ts", patch: "diff --git a/a.ts b/a.ts\n@@ -0,0 +1 @@\n+export {};", status: "A" },
|
||||
];
|
||||
|
||||
async function fixtureRoot(): Promise<string> {
|
||||
const root = await mkdtemp(path.join(tmpdir(), "ecr-review-cache-test-"));
|
||||
await mkdir(root, { recursive: true });
|
||||
await Promise.all([
|
||||
writeFile(path.join(root, "a.ts"), "export {};\n"),
|
||||
writeFile(path.join(root, "b.ts"), "new\n"),
|
||||
]);
|
||||
return root;
|
||||
}
|
||||
|
||||
const hash = (
|
||||
readRoot: string,
|
||||
over: Partial<Parameters<typeof reviewInputHash>[0]> = {},
|
||||
): Promise<string> =>
|
||||
reviewInputHash({
|
||||
files,
|
||||
config: config(),
|
||||
metadata: { title: "Change", body: "Why" },
|
||||
readRoot,
|
||||
agents: ["security", "correctness"],
|
||||
route: true,
|
||||
contextText: "plan",
|
||||
...over,
|
||||
});
|
||||
|
||||
test("review input hash ignores restack-only blob ids, hunk offsets, and ordering", async () => {
|
||||
const root = await fixtureRoot();
|
||||
try {
|
||||
const first = await hash(root);
|
||||
const restacked = files.map((file) =>
|
||||
file.path === "b.ts"
|
||||
? { ...file, patch: file.patch.replace("111..222", "aaa..bbb").replace("-1 +1", "-9 +9") }
|
||||
: file,
|
||||
);
|
||||
const second = await hash(root, {
|
||||
files: restacked.reverse(),
|
||||
config: config({ configDir: "/another/ephemeral/base-worktree" }),
|
||||
agents: ["correctness", "security"],
|
||||
});
|
||||
expect(first).toBe(second);
|
||||
expect(first).toMatch(/^[a-f0-9]{64}$/);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("review input hash changes with code, prompts, PR prose, selection, or context", async () => {
|
||||
const root = await fixtureRoot();
|
||||
try {
|
||||
const original = await hash(root);
|
||||
expect(
|
||||
await hash(root, { files: [{ ...files[0]!, patch: "+different" }, files[1]!] }),
|
||||
).not.toBe(original);
|
||||
await writeFile(path.join(root, "b.ts"), "different\n");
|
||||
expect(await hash(root)).not.toBe(original);
|
||||
await writeFile(path.join(root, "b.ts"), "new\n");
|
||||
expect(await hash(root, { config: config({ sharedPromptText: "new rules" }) })).not.toBe(
|
||||
original,
|
||||
);
|
||||
expect(await hash(root, { metadata: { title: "Renamed", body: "Why" } })).not.toBe(original);
|
||||
expect(await hash(root, { agents: ["correctness"] })).not.toBe(original);
|
||||
expect(await hash(root, { route: false })).not.toBe(original);
|
||||
expect(await hash(root, { contextText: "new plan" })).not.toBe(original);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
const review = (over: Partial<CoordinatorOutput> = {}): CoordinatorOutput => ({
|
||||
decision: "approve",
|
||||
findings: [],
|
||||
summary: "ok",
|
||||
incomplete: [],
|
||||
...over,
|
||||
});
|
||||
|
||||
test("only a complete review with the exact stored hash can be reused", async () => {
|
||||
const root = await fixtureRoot();
|
||||
try {
|
||||
const current = await hash(root);
|
||||
expect(reviewCanBeReused(review())).toBe(true);
|
||||
expect(reviewMatchesInput(review(), current, current)).toBe(true);
|
||||
expect(reviewMatchesInput(review(), "bad", current)).toBe(false);
|
||||
expect(reviewMatchesInput(review({ incomplete: ["timed out"] }), current, current)).toBe(false);
|
||||
expect(reviewMatchesInput(review({ couldNotComplete: true }), current, current)).toBe(false);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("file hashing refuses traversal and never follows a PR-controlled symlink", async () => {
|
||||
const root = await fixtureRoot();
|
||||
const outside = path.join(path.dirname(root), `${path.basename(root)}-secret`);
|
||||
try {
|
||||
await writeFile(outside, "secret one\n");
|
||||
await symlink(outside, path.join(root, "link"));
|
||||
const link: DiffEntry = { path: "link", patch: "diff --git a/link b/link", status: "A" };
|
||||
const first = await hash(root, { files: [link] });
|
||||
await writeFile(outside, "secret two\n");
|
||||
expect(await hash(root, { files: [link] })).toBe(first);
|
||||
await expect(
|
||||
hash(root, {
|
||||
files: [{ path: "../outside", patch: "diff --git a/x b/x", status: "A" }],
|
||||
}),
|
||||
).rejects.toThrow("escapes the PR tree");
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
await rm(outside, { force: true });
|
||||
}
|
||||
});
|
||||
+27
-157
@@ -23,13 +23,12 @@ import { repoRoot, resolveTrustedTool, run } from "../core/exec.js";
|
||||
import { errorMessage, publicFailureReason } from "../core/util.js";
|
||||
import { readContextFile } from "../core/context-file.js";
|
||||
import { buildDiffLineIndex } from "../core/render.js";
|
||||
import type { LinkContext, ReviewState, ScopeReviewResult } from "../core/render.js";
|
||||
import type { LinkContext, ScopeReviewResult } from "../core/render.js";
|
||||
import type { CoordinatorOutput, FeedbackPin, FeedbackRecord, Finding } from "../core/schema.js";
|
||||
import { applyPins, collectPins, scopedFingerprint } from "../core/schema.js";
|
||||
import { dropStaleVerdict, feedbackApplied, feedbackNeedsRunSeam } from "../core/adjudicate.js";
|
||||
import { runReview } from "../core/review.js";
|
||||
import type { ReviewRunOptions, ReviewRunResult } from "../core/review.js";
|
||||
import { reviewCanBeReused, reviewInputHash, reviewMatchesInput } from "../core/review-cache.js";
|
||||
import { GitHubPRSource } from "../sources/github-pr.js";
|
||||
import { memoizeSource, stackConfirmFromConfig, stackWalkFromConfig } from "../sources/source.js";
|
||||
import type { PreparedReadRoot, ReviewSource, StackWalkOptions } from "../sources/source.js";
|
||||
@@ -541,11 +540,9 @@ async function runLegacyCi(
|
||||
// unchanged (see mergeFeedback). Memoized on the source — no extra API call. A
|
||||
// metadata failure leaves it unknown, which re-judges replies instead of trusting a
|
||||
// verdict against source we cannot pin.
|
||||
let metadata: Awaited<ReturnType<ReviewSource["getMetadata"]>> | undefined;
|
||||
let headSha: string | undefined;
|
||||
try {
|
||||
metadata = await source.getMetadata();
|
||||
headSha = metadata.headOid;
|
||||
headSha = (await source.getMetadata()).headOid;
|
||||
} catch {
|
||||
// leave headSha unset → stored verdicts do not carry (fail safe, costs budget only)
|
||||
}
|
||||
@@ -572,76 +569,24 @@ async function runLegacyCi(
|
||||
);
|
||||
}
|
||||
|
||||
const stack = resolveStackWalk(config.stack, noStackAware);
|
||||
const stackConfirm = resolveStackConfirm(config.stack, noStackAware);
|
||||
const feedback = adjudicationSeam(config, reporter);
|
||||
// Dynamic stack context and model-backed reply adjudication have inputs outside
|
||||
// the scoped diff. Keep those paths fresh until their inputs join the cache key.
|
||||
// A maintainer's explicit /review is also always a real rerun.
|
||||
const cacheAllowed = !bypassTriggerGate && !stack && !feedback && metadata !== undefined;
|
||||
let inputHash: string | undefined;
|
||||
|
||||
try {
|
||||
if (cacheAllowed) {
|
||||
try {
|
||||
const prepared = await source.prepareReadRootAsync?.();
|
||||
try {
|
||||
inputHash = await reviewInputHash({
|
||||
files: await source.getChangedFiles(),
|
||||
config,
|
||||
metadata: metadata!,
|
||||
readRoot: prepared?.dir ?? cwd,
|
||||
agents,
|
||||
route,
|
||||
contextText,
|
||||
});
|
||||
} finally {
|
||||
await prepared?.cleanup();
|
||||
}
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not hash the review input (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
if (inputHash) {
|
||||
try {
|
||||
const prior = await reporter.readState();
|
||||
if (prior && reviewMatchesInput(prior.review, prior.inputHash, inputHash)) {
|
||||
await reporter.report(prior.review, undefined, inputHash);
|
||||
process.stderr.write(
|
||||
"CI reviewer: unchanged review input; reused the previous result.\n",
|
||||
);
|
||||
return;
|
||||
}
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not read the previous review cache (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const review = await runReview(source, {
|
||||
config,
|
||||
mode: "ci",
|
||||
agents,
|
||||
route,
|
||||
contextText,
|
||||
stack,
|
||||
stackConfirm,
|
||||
stack: resolveStackWalk(config.stack, noStackAware),
|
||||
stackConfirm: resolveStackConfirm(config.stack, noStackAware),
|
||||
runsDir: workspaceRunsDir(cwd),
|
||||
// Adjudicate mode judges the matched replies against the source before the
|
||||
// comment is rendered; annotate mode lets the reporter match them at report
|
||||
// time. Either way the feedback path is fail-open (runReview swallows its own
|
||||
// errors), so it never fails the PR's checks.
|
||||
feedback,
|
||||
feedback: adjudicationSeam(config, reporter),
|
||||
onProgress: (message) => process.stderr.write(`${message}\n`),
|
||||
});
|
||||
await reporter.report(
|
||||
review,
|
||||
review.feedback,
|
||||
inputHash && reviewCanBeReused(review) ? inputHash : undefined,
|
||||
);
|
||||
await reporter.report(review, review.feedback);
|
||||
process.stderr.write(`CI reviewer: posted review (${review.decision}).\n`);
|
||||
} catch (error) {
|
||||
// A reviewer failure must never fail the PR's checks — but it must also not be
|
||||
@@ -818,14 +763,13 @@ async function runRoutedCi(
|
||||
};
|
||||
// The reviewed head OID also binds each adjudication verdict to the source it judged
|
||||
// (see mergeFeedback); unresolved leaves it unknown, so verdicts are re-judged.
|
||||
let metadata: Awaited<ReturnType<ReviewSource["getMetadata"]>> | undefined;
|
||||
let headSha: string | undefined;
|
||||
try {
|
||||
metadata = await source.getMetadata();
|
||||
if (metadata.baseOid) {
|
||||
link.baseSha = metadata.baseOid;
|
||||
const { baseOid, headOid } = await source.getMetadata();
|
||||
if (baseOid) {
|
||||
link.baseSha = baseOid;
|
||||
}
|
||||
headSha = metadata.headOid;
|
||||
headSha = headOid;
|
||||
} catch {
|
||||
// leave baseSha unset → out-of-diff findings degrade to plain text
|
||||
}
|
||||
@@ -880,42 +824,11 @@ async function runRoutedCi(
|
||||
reporterFor(scopedCommentTag(rootTag, name), true),
|
||||
);
|
||||
|
||||
const cacheAllowed =
|
||||
!bypassTriggerGate &&
|
||||
!stackWalk &&
|
||||
!feedbackNeedsRunSeam(rootConfig.feedback) &&
|
||||
metadata !== undefined;
|
||||
let cacheReadRoot: string | undefined;
|
||||
if (cacheAllowed) {
|
||||
try {
|
||||
const prepared = await source.prepareReadRootAsync?.();
|
||||
cacheReadRoot = prepared?.dir ?? cwd;
|
||||
await prepared?.cleanup();
|
||||
} catch (error) {
|
||||
// The fresh run will retry the same materialization and apply CI's ordinary
|
||||
// fail-closed behavior. Cache setup itself never changes that outcome.
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not prepare the review cache input (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
let priorAggregateState: ReviewState | null = null;
|
||||
if (cacheReadRoot && mode === "single") {
|
||||
try {
|
||||
priorAggregateState = await singleModeReporter!.readState();
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not read the previous aggregate cache (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const results: ScopeReviewResult[] = [];
|
||||
for (const scope of active) {
|
||||
const scopeDef = manifest.scopes.find((entry) => entry.name === scope.name)!;
|
||||
const isDefault = scope.configDir === ".";
|
||||
let review: CoordinatorOutput;
|
||||
let inputHash: string | undefined;
|
||||
try {
|
||||
// A scope whose config dir doesn't exist at the TRUSTED base commit is a
|
||||
// scope this PR introduces: review it with the root config rather than
|
||||
@@ -945,66 +858,27 @@ async function runRoutedCi(
|
||||
: undefined,
|
||||
)
|
||||
: undefined;
|
||||
let cached: ScopeReviewResult | ReviewState | undefined;
|
||||
if (cacheReadRoot) {
|
||||
try {
|
||||
const included = new Set(scope.files);
|
||||
inputHash = await reviewInputHash({
|
||||
files: changed.filter((file) => included.has(file.path)),
|
||||
config,
|
||||
metadata: metadata!,
|
||||
readRoot: cacheReadRoot,
|
||||
agents,
|
||||
route,
|
||||
contextText,
|
||||
});
|
||||
if (mode === "single") {
|
||||
cached = priorAggregateState?.scopes?.find((entry) => entry.scope === scope.name);
|
||||
} else {
|
||||
cached = (await scopeReporter(scope.name).readState()) ?? undefined;
|
||||
}
|
||||
} catch (error) {
|
||||
inputHash = undefined;
|
||||
process.stderr.write(
|
||||
`CI reviewer: [${scope.name}] could not use the previous cache (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (cached && reviewMatchesInput(cached.review, cached.inputHash, inputHash!)) {
|
||||
review = cached.review;
|
||||
process.stderr.write(
|
||||
`CI reviewer: [${scope.name}] unchanged review input; reused the previous result.\n`,
|
||||
);
|
||||
} else {
|
||||
review = await runReview(source, {
|
||||
config,
|
||||
mode: "ci",
|
||||
agents,
|
||||
route,
|
||||
includePaths: scope.files,
|
||||
contextText,
|
||||
stack: stackWalk,
|
||||
stackConfirm,
|
||||
passesBudgetMs: budget,
|
||||
runsDir: workspaceRunsDir(cwd),
|
||||
feedback: feedbackSeam,
|
||||
onProgress: (message) => process.stderr.write(`[${scope.name}] ${message}\n`),
|
||||
});
|
||||
}
|
||||
review = await runReview(source, {
|
||||
config,
|
||||
mode: "ci",
|
||||
agents,
|
||||
route,
|
||||
includePaths: scope.files,
|
||||
contextText,
|
||||
stack: stackWalk,
|
||||
stackConfirm,
|
||||
passesBudgetMs: budget,
|
||||
runsDir: workspaceRunsDir(cwd),
|
||||
feedback: feedbackSeam,
|
||||
onProgress: (message) => process.stderr.write(`[${scope.name}] ${message}\n`),
|
||||
});
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: [${scope.name}] failed (non-blocking): ${errorMessage(error)}\n`,
|
||||
);
|
||||
review = failureReview(scope.name, publicFailureReason(error));
|
||||
inputHash = undefined;
|
||||
}
|
||||
results.push({
|
||||
scope: scope.name,
|
||||
isDefault,
|
||||
review,
|
||||
...(inputHash && reviewCanBeReused(review) ? { inputHash } : {}),
|
||||
});
|
||||
results.push({ scope: scope.name, isDefault, review });
|
||||
}
|
||||
|
||||
if (mode === "single") {
|
||||
@@ -1014,7 +888,7 @@ async function runRoutedCi(
|
||||
// A partial run (--scopes) is authoritative ONLY for the named scopes: merge
|
||||
// the other scopes' previous results out of the existing aggregate comment's
|
||||
// state so re-running one scope doesn't silently discard the rest.
|
||||
const prior = (priorAggregateState ?? (await aggregate.readState()))?.scopes ?? [];
|
||||
const prior = (await aggregate.readState())?.scopes ?? [];
|
||||
finalResults = mergePartialAggregate(
|
||||
results,
|
||||
prior,
|
||||
@@ -1073,11 +947,7 @@ async function runRoutedCi(
|
||||
const reporter = scopeReporter(scope.name);
|
||||
const result = results.find((entry) => entry.scope === scope.name);
|
||||
if (result) {
|
||||
await reporter.report(
|
||||
result.review,
|
||||
(result.review as ReviewRunResult).feedback,
|
||||
result.inputHash,
|
||||
);
|
||||
await reporter.report(result.review, (result.review as ReviewRunResult).feedback);
|
||||
} else {
|
||||
// A reconciled scope with zero matched files gets its stale comment deleted.
|
||||
await reporter.clear();
|
||||
|
||||
+13
-18
@@ -27,10 +27,9 @@ Options:
|
||||
--no-workflow Skip writing the CI workflows (review, command, and dismiss
|
||||
under .github/workflows/)
|
||||
--token-env <name[,name…]>
|
||||
Env var(s) holding the model credential (default
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN, e.g. OPENAI_API_KEY). The
|
||||
scaffolded workflows forward the matching repo secret(s) and
|
||||
expect this tokenEnv
|
||||
Env var(s) holding the model credential (default OPENAI_API_KEY,
|
||||
e.g. CLAUDE_CODE_OAUTH_TOKEN). The scaffolded workflows forward
|
||||
the matching repo secret(s) and expect this tokenEnv
|
||||
--force Overwrite existing files
|
||||
--force-workflows
|
||||
Overwrite only the CI workflow files, keeping your customized
|
||||
@@ -39,7 +38,7 @@ Options:
|
||||
-h, --help Show this help
|
||||
`;
|
||||
|
||||
const DEFAULT_TOKEN_ENV = "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN";
|
||||
const DEFAULT_TOKEN_ENV = "OPENAI_API_KEY";
|
||||
|
||||
export async function initCommand(argv: string[]): Promise<void> {
|
||||
if (argv.includes("-h") || argv.includes("--help")) {
|
||||
@@ -108,7 +107,7 @@ async function scaffold(argv: string[]): Promise<void> {
|
||||
|
||||
// The reverse of the guard above: --force-workflows (or --force) always rewrites
|
||||
// the review workflows from the pristine template, which forwards the default
|
||||
// Anthropic token env unless --token-env names the credential again. An adopter who
|
||||
// OPENAI_API_KEY unless --token-env names the credential again. An adopter who
|
||||
// scaffolded with a non-default tokenEnv and later refreshes the workflow YAML
|
||||
// without re-passing --token-env would silently lose the forwarded secret, and
|
||||
// CI would keep passing the auth lock but run with an empty credential. Refuse
|
||||
@@ -122,7 +121,7 @@ async function scaffold(argv: string[]): Promise<void> {
|
||||
`${baked}, but this run has no --token-env, so rewriting them would restore the ` +
|
||||
`default ${DEFAULT_TOKEN_ENV} and CI would run with an empty credential. Re-run with ` +
|
||||
`--token-env ${baked} to keep the current credential, or --token-env ${DEFAULT_TOKEN_ENV} ` +
|
||||
`to reset to the default on purpose.`,
|
||||
`to reset to OpenAI on purpose.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -227,15 +226,14 @@ async function scaffold(argv: string[]): Promise<void> {
|
||||
const steps = [
|
||||
`Customize ${CONFIG_DIRNAME}/agents/*.md (and shared.md, coordinator.md) for this repo.`,
|
||||
// --token-env only rewires the workflows; the scaffolded config.jsonc still
|
||||
// declares the default Anthropic tokenEnv, and CI's `ecr verify-config`
|
||||
// refuses to review until the config's tokenEnv set matches the workflow's
|
||||
// expected set.
|
||||
// declares OPENAI_API_KEY, and CI's `ecr verify-config` refuses to review
|
||||
// until the config's tokenEnv set matches the workflow's expected set.
|
||||
...(tokenEnvs.join(",") !== DEFAULT_TOKEN_ENV
|
||||
? [
|
||||
`Point ${CONFIG_DIRNAME}/config.jsonc at ${tokenEnvs.length > 1 ? "these credentials" : "this credential"}: set \`auth\` (and \`model\`) per the file's comments — CI's \`ecr verify-config\` refuses to review until the config names ${names}.`,
|
||||
]
|
||||
: []),
|
||||
"Log in with `claude` (or run `ecr setup-auth`) so local runs have a credential.",
|
||||
"Configure a model provider in OpenCode (or set REVIEWER_MODEL).",
|
||||
"Run `ecr doctor`, then `ecr review`.",
|
||||
withWorkflow
|
||||
? `Add the ${names} repo secret${tokenEnvs.length > 1 ? "s" : ""} referenced by the workflow, then add an \`ai-review\` label to a PR.`
|
||||
@@ -641,14 +639,11 @@ export function substituteTokenEnv(raw: string, tokenEnvs: string[]): string {
|
||||
return raw;
|
||||
}
|
||||
const expectedFallback = `vars.ECR_EXPECTED_TOKEN_ENV || '${DEFAULT_TOKEN_ENV}'`;
|
||||
// Byte-identical to the credential block in templates/workflow.yml AND
|
||||
// templates/command.yml (both flow through this function).
|
||||
const credentialBlock = [
|
||||
" # Anthropic review credential — the env var named by auth.tokenEnv in",
|
||||
" # config.jsonc. Store it as a repo secret: an `sk-ant-oat…` token minted",
|
||||
" # by `claude setup-token`, or an `sk-ant-api…` Console key (the Claude",
|
||||
" # Code CLI reads either).",
|
||||
" CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}",
|
||||
" # OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.",
|
||||
" # Store it as a repo secret; a project-scoped key restricted to model",
|
||||
" # inference (with a spend limit) is all the reviewer needs.",
|
||||
" OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}",
|
||||
].join("\n");
|
||||
if (!raw.includes(expectedFallback) || !raw.includes(credentialBlock)) {
|
||||
throw new Error("workflow template drifted: tokenEnv markers not found (report this bug)");
|
||||
|
||||
@@ -25,7 +25,7 @@ Exit 0 = every ref holds. Exit 1 = at least one is broken.
|
||||
|
||||
Options:
|
||||
--root <dir> Repository root to check (default: the current git repo).
|
||||
--json Emit {ok, problems:[{file, line, kind, problem, target?}]} on stdout.
|
||||
--json Emit {ok, problems:[{file, line, kind, problem}]} on stdout.
|
||||
`;
|
||||
|
||||
const KIND_LABEL: Record<RefProblem["kind"], string> = {
|
||||
|
||||
+1
-4
@@ -21,9 +21,7 @@ const PROVIDER_KEY_ENV: Record<string, string> = {
|
||||
* Provider-owned credential env vars BEYOND the x-api-key ones above: Anthropic's
|
||||
* OAuth/subscription bearer envs. CLAUDE_CODE_OAUTH_TOKEN holds the long-lived (1-year)
|
||||
* Claude Max/Team subscription token that `ecr setup-auth`/`claude setup-token` export,
|
||||
* ANTHROPIC_AUTH_TOKEN is Anthropic's documented bearer var, and
|
||||
* CLAUDE_CODE_REVIEW_SHARED_API_TOKEN is the SCAFFOLDED DEFAULT tokenEnv (see
|
||||
* templates/config.jsonc) — it always holds an Anthropic credential. They belong to
|
||||
* and ANTHROPIC_AUTH_TOKEN is Anthropic's documented bearer var. They belong to
|
||||
* anthropic, so the cross-provider guard below refuses a non-anthropic entry that names
|
||||
* one — without this, `{provider:"openai", tokenEnv:"CLAUDE_CODE_OAUTH_TOKEN"}` passes
|
||||
* (neither a FORBIDDEN secret nor a PROVIDER_KEY_ENV value) and prepareAuth forwards the
|
||||
@@ -33,7 +31,6 @@ const PROVIDER_KEY_ENV: Record<string, string> = {
|
||||
const ANTHROPIC_TOKEN_ENVS: Record<string, string> = {
|
||||
CLAUDE_CODE_OAUTH_TOKEN: "anthropic",
|
||||
ANTHROPIC_AUTH_TOKEN: "anthropic",
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: "anthropic",
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
+15
-51
@@ -8,7 +8,6 @@ import { CONFIG_DIRNAME, stripJsonComments, stripTrailingCommas } from "../confi
|
||||
import { ROUTING_FILENAME } from "../config/routing.js";
|
||||
import { git, pathInside } from "./exec.js";
|
||||
import { matchesIgnore } from "./noise.js";
|
||||
import { isAmbientRuntimeConfig } from "./scrub.js";
|
||||
|
||||
/**
|
||||
* Built by concatenation so this module's own regexes and doc comments are not
|
||||
@@ -47,7 +46,6 @@ const CITATION_EXTENSIONS = new Set([
|
||||
".css",
|
||||
".go",
|
||||
".graphql",
|
||||
".hcl",
|
||||
".h",
|
||||
".hpp",
|
||||
".html",
|
||||
@@ -71,8 +69,6 @@ const CITATION_EXTENSIONS = new Set([
|
||||
".sh",
|
||||
".sql",
|
||||
".swift",
|
||||
".tf",
|
||||
".tfvars",
|
||||
".toml",
|
||||
".ts",
|
||||
".tsx",
|
||||
@@ -99,8 +95,6 @@ export interface RefProblem {
|
||||
kind: RefProblemKind;
|
||||
/** One sentence, imperative where a fix is obvious. */
|
||||
problem: string;
|
||||
/** The ref target (or prose token) at fault. Absent for structural problems. */
|
||||
target?: string;
|
||||
}
|
||||
|
||||
export interface ParsedRef {
|
||||
@@ -699,11 +693,10 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
// Does an extensionless token name something real? Cached: prompts repeat the same
|
||||
// paths, and each miss would otherwise cost a stat per occurrence.
|
||||
const resolvable = new Map<string, string | null>();
|
||||
/** The root-relative path a token names, or null. Scope first: a scoped prompt
|
||||
* citing `alerts/` means its own tree even when the root has one too. */
|
||||
/** The root-relative path an extensionless token names, or null if it names nothing. */
|
||||
const namedPath = async (token: string, scopeRoot: string): Promise<string | null> => {
|
||||
for (const candidate of pathishCandidates(token)) {
|
||||
for (const base of [scopeRoot, root]) {
|
||||
for (const base of [root, scopeRoot]) {
|
||||
const absolute = path.resolve(base, candidate);
|
||||
let resolved = resolvable.get(absolute);
|
||||
if (resolved === undefined) {
|
||||
@@ -723,19 +716,6 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
return null;
|
||||
};
|
||||
|
||||
/** The glob a wildcard token names, rebased onto the scope when that is what
|
||||
* matches, so the suggested `glob:` ref actually resolves. */
|
||||
const namedGlob = (token: string, scopeRoot: string): string | null => {
|
||||
const scopeRel = path.relative(root, scopeRoot).split(path.sep).join("/");
|
||||
const candidates = [...(scopeRel && scopeRel !== "." ? [`${scopeRel}/${token}`] : []), token];
|
||||
for (const candidate of candidates) {
|
||||
if (index.files.some((file) => matchesIgnore(file, candidate))) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
for (const dir of dirs) {
|
||||
const scopeRoot = path.dirname(dir);
|
||||
for (const file of await setupFiles(dir)) {
|
||||
@@ -765,7 +745,6 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
line: ref.line,
|
||||
kind: problem.startsWith("cites a line number") ? "line-number-ref" : "broken-ref",
|
||||
problem,
|
||||
target: ref.target,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
@@ -783,15 +762,18 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
}
|
||||
|
||||
for (const { line, token } of findProseCitations(text)) {
|
||||
const isWild = token.includes("*");
|
||||
const named = isWild ? namedGlob(token, scopeRoot) : await namedPath(token, scopeRoot);
|
||||
// Shape alone makes a citation; otherwise it must name something real, or
|
||||
// `anthropic/claude-opus-5` would read as a path.
|
||||
// Extension or wildcard tail ⇒ a citation on shape alone. Otherwise it only
|
||||
// counts if it names something real: `eas-build-worker/terraform` and
|
||||
// `general-central/{module,production}` are paths, `anthropic/claude-opus-5`
|
||||
// is shaped identically and is not.
|
||||
const named = isCodeCitation(token) ? null : await namedPath(token, scopeRoot);
|
||||
if (!isCodeCitation(token) && !named) {
|
||||
continue;
|
||||
}
|
||||
// Coverage accepts the resolved path too, else the only ref that silences a
|
||||
// scope-relative citation is an over-broad `glob:**/<basename>`.
|
||||
// Coverage must accept the path the token RESOLVED to, not just the token as
|
||||
// written: a prompt says `cert-manager` and the ref that pins it is
|
||||
// `infrastructure/cert-manager/`. Without this, the fix the message suggests
|
||||
// does not silence the citation it was suggested for.
|
||||
const forms = [...citationForms(token), ...(named ? coveringForms(named) : [])];
|
||||
if (
|
||||
ignored.has(token) ||
|
||||
@@ -803,12 +785,9 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
continue;
|
||||
}
|
||||
const lineCitation = LINE_CITATION_RE.exec(token);
|
||||
// A wildcard token means the family; anything else gets the precise resolved path.
|
||||
const suggestion = isWild
|
||||
? named
|
||||
? `${GLOB_PREFIX}${named}`
|
||||
: suggestedRef(token)
|
||||
: (named ?? suggestedRef(token));
|
||||
// For an extensionless token the suggestion is the path that actually resolved,
|
||||
// which is also how a scope-relative citation learns its root-relative form.
|
||||
const suggestion = named ?? suggestedRef(token);
|
||||
problems.push({
|
||||
file: relative,
|
||||
line,
|
||||
@@ -816,7 +795,6 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
problem: lineCitation
|
||||
? `\`${token}\` pins a line number; cite the file or a \`#symbol\` instead, as \`${REF_MARK} ${suggestedRef(lineCitation[1]!)}\``
|
||||
: `\`${token}\` cites code without a ref; add \`${REF_MARK} ${suggestion} — why it matters\` (or \`${IGNORE_MARK} ${token}\` if it is not a path)`,
|
||||
target: token,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -843,13 +821,6 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
|
||||
};
|
||||
}
|
||||
|
||||
/** The file a target names, with any `glob:` prefix, `#anchor` or `:line` stripped. */
|
||||
function citedBasename(target: string): string {
|
||||
const withoutPrefix = target.startsWith(GLOB_PREFIX) ? target.slice(GLOB_PREFIX.length) : target;
|
||||
const withoutLine = LINE_CITATION_RE.exec(withoutPrefix)?.[1] ?? withoutPrefix;
|
||||
return path.basename(splitAnchor(withoutLine)[0]);
|
||||
}
|
||||
|
||||
/** How many examples a review-side note names before saying "and N more". */
|
||||
const NOTE_EXAMPLES = 5;
|
||||
|
||||
@@ -877,14 +848,7 @@ export async function reviewSetupRefNotes(options: {
|
||||
return []; // a check that cannot run must never degrade the review
|
||||
}
|
||||
const notes: string[] = [];
|
||||
// The read root is scrubbed of ambient runtime config (AGENTS.md, CLAUDE.md, .env…),
|
||||
// so a ref to one is missing by design here, not stale. The template itself cites
|
||||
// AGENTS.md, which would make this note cry wolf on every PR.
|
||||
const broken = report.problems.filter(
|
||||
(problem) =>
|
||||
problem.kind !== "unannotated-citation" &&
|
||||
!(problem.target && isAmbientRuntimeConfig(citedBasename(problem.target))),
|
||||
);
|
||||
const broken = report.problems.filter((problem) => problem.kind !== "unannotated-citation");
|
||||
if (broken.length > 0) {
|
||||
notes.push(
|
||||
`The reviewer setup cites code that no longer resolves (${broken.length} ref(s)): ` +
|
||||
|
||||
+2
-16
@@ -206,7 +206,6 @@ export function renderMarkdown(
|
||||
link?: LinkContext,
|
||||
feedback: FeedbackRecord[] = [],
|
||||
pins: FeedbackPin[] = [],
|
||||
inputHash?: string,
|
||||
): string {
|
||||
const dismissedByFp = new Map(dismissed.map((record) => [record.fp, record]));
|
||||
const withFp = review.findings.map((finding) => ({ finding, fp: fingerprintFinding(finding) }));
|
||||
@@ -284,9 +283,7 @@ export function renderMarkdown(
|
||||
const fingerprints = review.findings.map(fingerprintFinding);
|
||||
lines.push("", `<!-- ${tag}:fingerprints=${JSON.stringify(fingerprints)} -->`);
|
||||
lines.push(
|
||||
`<!-- ${tag}:state=${encodeState(
|
||||
reviewState({ review, dismissed, ...(inputHash ? { inputHash } : {}) }, feedbackByFp, pins),
|
||||
)} -->`,
|
||||
`<!-- ${tag}:state=${encodeState(reviewState({ review, dismissed }, feedbackByFp, pins))} -->`,
|
||||
);
|
||||
return lines.join("\n");
|
||||
}
|
||||
@@ -493,8 +490,6 @@ export interface ScopeReviewResult {
|
||||
/** config '.' → un-namespaced fingerprints (dismissal carry-over). */
|
||||
isDefault: boolean;
|
||||
review: CoordinatorOutput;
|
||||
/** Hash of the complete input that produced `review`; absent means do not reuse. */
|
||||
inputHash?: string;
|
||||
}
|
||||
|
||||
/** The machine-readable state embedded in the reviewer's comment. */
|
||||
@@ -502,8 +497,6 @@ export interface ReviewState {
|
||||
/** v1 field, kept: the aggregate stores the MERGED output here. */
|
||||
review: CoordinatorOutput;
|
||||
dismissed: DismissalRecord[];
|
||||
/** v5: legacy/single-scope review-result cache key. */
|
||||
inputHash?: string;
|
||||
/** v2: present only on aggregate comments (routing, comment:'single'). */
|
||||
scopes?: ScopeReviewResult[];
|
||||
/** v3: author replies matched to the findings shown in this comment. */
|
||||
@@ -737,7 +730,7 @@ export function renderAggregateMarkdown(
|
||||
// across re-renders. The per-scope data (`scopes`) plus a merged v1 `review`
|
||||
// keep both v2 and v1 consumers working.
|
||||
const stateScopes: ScopeReviewResult[] = rendered.map(
|
||||
({ result, shown, hidden, requalified, requalifiedHidden, dropped }) => {
|
||||
({ result, shown, requalified, dropped }) => {
|
||||
// @ref LLP 0011#suppression-is-never-silent — strip any per-scope `feedback`
|
||||
// a freshly-reviewed scope's ReviewRunResult carries: the top-level feedback
|
||||
// array (reviewState below) is the single source of truth. A stale per-scope
|
||||
@@ -749,13 +742,6 @@ export function renderAggregateMarkdown(
|
||||
return {
|
||||
scope: result.scope,
|
||||
isDefault: result.isDefault,
|
||||
// A truncated state is not the full result and therefore cannot be a cache
|
||||
// source. Omit its hash so the next run retries instead of reusing a subset.
|
||||
...(hidden === 0 && requalifiedHidden === 0
|
||||
? result.inputHash
|
||||
? { inputHash: result.inputHash }
|
||||
: {}
|
||||
: {}),
|
||||
// Requalified findings ride the embedded state (like dismissed ones) so a
|
||||
// re-render (/dismiss) round-trips them and the addressed section persists.
|
||||
// Under truncation they are trimmed exactly like `shown` — state bytes count
|
||||
|
||||
@@ -1,142 +0,0 @@
|
||||
// @ref LLP 0005#review-result-cache [implements] — automated CI reuses only a complete result whose full review input hashes identically
|
||||
import { createHash } from "node:crypto";
|
||||
import { createReadStream } from "node:fs";
|
||||
import { lstat, readlink } from "node:fs/promises";
|
||||
import { createRequire } from "node:module";
|
||||
import path from "node:path";
|
||||
|
||||
import type { LoadedConfig } from "../config/schema.js";
|
||||
import { pathInside } from "./exec.js";
|
||||
import type { CoordinatorOutput, DiffEntry, ReviewMetadata } from "./schema.js";
|
||||
|
||||
const packageVersion = (createRequire(import.meta.url)("../../package.json") as { version: string })
|
||||
.version;
|
||||
|
||||
/**
|
||||
* Version the cache-key contract independently of the embedded comment-state shape.
|
||||
* Bump this whenever review inputs gain a new source that is not represented below.
|
||||
*/
|
||||
const REVIEW_INPUT_HASH_VERSION = 1;
|
||||
|
||||
export interface ReviewInputHashOptions {
|
||||
files: DiffEntry[];
|
||||
config: LoadedConfig;
|
||||
metadata: Pick<ReviewMetadata, "title" | "body">;
|
||||
/** PR-head tree. Changed-file contents are hashed from here, never the base checkout. */
|
||||
readRoot: string;
|
||||
agents?: string[];
|
||||
route?: boolean;
|
||||
contextText?: string;
|
||||
}
|
||||
|
||||
/** Canonical JSON: object-key order never turns the same input into a cache miss. */
|
||||
function canonicalJson(value: unknown): string {
|
||||
if (value === null || typeof value !== "object") {
|
||||
return JSON.stringify(value) ?? "null";
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return `[${value.map(canonicalJson).join(",")}]`;
|
||||
}
|
||||
const record = value as Record<string, unknown>;
|
||||
return `{${Object.keys(record)
|
||||
.filter((key) => record[key] !== undefined)
|
||||
.sort()
|
||||
.map((key) => `${JSON.stringify(key)}:${canonicalJson(record[key])}`)
|
||||
.join(",")}}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hash every input that can change the code-review result while deliberately
|
||||
* excluding checkout-local paths and commit OIDs. A restack changes OIDs; when the
|
||||
* scoped changed-file contents, normalized patch, prompts/config and PR prose are
|
||||
* byte-identical, that is the cache hit this key is meant to recognize.
|
||||
*/
|
||||
function normalizedPatch(patch: string, binary: boolean): string {
|
||||
return (
|
||||
patch
|
||||
.split("\n")
|
||||
// Blob ids are volatile across a restack. A binary patch has no textual content
|
||||
// to hash, so retain its index line as the only content identity available.
|
||||
.filter((line) => binary || !line.startsWith("index "))
|
||||
// Unrelated lines landing earlier in the file move hunk coordinates without
|
||||
// changing the code under review. Preserve any trailing function heading.
|
||||
.map((line) => line.replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@(.*)$/, "@@ @@$1"))
|
||||
.join("\n")
|
||||
);
|
||||
}
|
||||
|
||||
async function fileContentIdentity(
|
||||
root: string,
|
||||
file: DiffEntry,
|
||||
): Promise<{ kind: string; digest?: string; mode?: number; target?: string }> {
|
||||
const abs = path.resolve(root, file.path);
|
||||
if (!pathInside(abs, root)) {
|
||||
throw new Error(`review cache path escapes the PR tree: ${file.path}`);
|
||||
}
|
||||
let stat;
|
||||
try {
|
||||
stat = await lstat(abs);
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
|
||||
return { kind: "missing" };
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
if (stat.isSymbolicLink()) {
|
||||
// Never follow a PR-controlled symlink out of the materialized tree.
|
||||
return { kind: "symlink", target: await readlink(abs), mode: stat.mode & 0o777 };
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
return { kind: "other", mode: stat.mode & 0o777 };
|
||||
}
|
||||
const hash = createHash("sha256");
|
||||
for await (const chunk of createReadStream(abs)) {
|
||||
hash.update(chunk as Buffer);
|
||||
}
|
||||
return { kind: "file", digest: hash.digest("hex"), mode: stat.mode & 0o777 };
|
||||
}
|
||||
|
||||
export async function reviewInputHash(options: ReviewInputHashOptions): Promise<string> {
|
||||
const { configDir: _configDir, ...portableConfig } = options.config;
|
||||
const sortedFiles = [...options.files].sort(
|
||||
(a, b) => a.path.localeCompare(b.path) || (a.status ?? "").localeCompare(b.status ?? ""),
|
||||
);
|
||||
const files = [];
|
||||
// Sequential reads avoid opening one descriptor per changed file on a very wide PR.
|
||||
for (const file of sortedFiles) {
|
||||
files.push({
|
||||
path: file.path,
|
||||
patch: normalizedPatch(file.patch, file.binary === true),
|
||||
status: file.status ?? "",
|
||||
binary: file.binary === true,
|
||||
content: await fileContentIdentity(options.readRoot, file),
|
||||
});
|
||||
}
|
||||
const input = {
|
||||
version: REVIEW_INPUT_HASH_VERSION,
|
||||
engineVersion: packageVersion,
|
||||
files,
|
||||
config: portableConfig,
|
||||
metadata: options.metadata,
|
||||
agents: options.agents ? [...options.agents].sort() : null,
|
||||
route: options.route === true,
|
||||
contextText: options.contextText ?? null,
|
||||
};
|
||||
return createHash("sha256").update(canonicalJson(input)).digest("hex");
|
||||
}
|
||||
|
||||
/** Partial/failed reviews must be retried, never made durable by a cache hit. */
|
||||
export function reviewCanBeReused(review: CoordinatorOutput): boolean {
|
||||
return review.couldNotComplete !== true && review.incomplete.length === 0;
|
||||
}
|
||||
|
||||
/** Exact hash match plus the completeness guard used by every CI comment shape. */
|
||||
export function reviewMatchesInput(
|
||||
review: CoordinatorOutput,
|
||||
storedHash: string | undefined,
|
||||
currentHash: string,
|
||||
): boolean {
|
||||
return (
|
||||
storedHash === currentHash && /^[a-f0-9]{64}$/.test(storedHash) && reviewCanBeReused(review)
|
||||
);
|
||||
}
|
||||
@@ -468,11 +468,7 @@ export class GitHubReporter implements Reporter {
|
||||
* (annotate mode). Either way the feedback path fails soft — it never blocks the
|
||||
* comment from being posted.
|
||||
*/
|
||||
async report(
|
||||
review: CoordinatorOutput,
|
||||
feedback?: FeedbackRecord[],
|
||||
inputHash?: string,
|
||||
): Promise<void> {
|
||||
async report(review: CoordinatorOutput, feedback?: FeedbackRecord[]): Promise<void> {
|
||||
// Carry forward any per-PR dismissals recorded in the existing comment so they
|
||||
// survive re-reviews (a dismissed finding stays in the collapsed section).
|
||||
const existing = await this.findExistingComment();
|
||||
@@ -491,7 +487,7 @@ export class GitHubReporter implements Reporter {
|
||||
: await this.computeFeedback(withFp, priorRecords, pinsIn);
|
||||
const link = await this.linkContextAsync();
|
||||
await this.upsertComment(
|
||||
renderMarkdown(review, this.options.commentTag, dismissed, link, records, pins, inputHash),
|
||||
renderMarkdown(review, this.options.commentTag, dismissed, link, records, pins),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -838,7 +834,6 @@ export class GitHubReporter implements Reporter {
|
||||
link,
|
||||
next.feedback,
|
||||
next.pins,
|
||||
state.inputHash,
|
||||
);
|
||||
await this.patchComment(existing.id, body);
|
||||
return {
|
||||
|
||||
@@ -3,11 +3,11 @@
|
||||
description: Security and secrets. Injection, credential or secret leakage, unsafe shell/child-process use, missing validation at trust boundaries.
|
||||
alwaysRun: true
|
||||
# Security is the highest-stakes agent and benefits most from stronger threat-model
|
||||
# reasoning, so it runs on the Opus tier even though the other specialists use the
|
||||
# reasoning, so it runs on the pro tier even though the other specialists use the
|
||||
# default model. Scoped to this one agent to limit the extra latency/rate-limit cost;
|
||||
# subdivide-on-timeout + the per-fetch deadline keep a slow Opus pass from hanging.
|
||||
# subdivide-on-timeout + the per-fetch deadline keep a slow pro pass from hanging.
|
||||
# @ref LLP 0009#config-and-prompt-templates [implements]
|
||||
model: anthropic/claude-opus-5
|
||||
model: openai/gpt-5.5-pro
|
||||
---
|
||||
|
||||
# Security & secrets
|
||||
|
||||
+5
-11
@@ -81,11 +81,6 @@ jobs:
|
||||
# disable setup-node's auto package-manager cache.
|
||||
package-manager-cache: false
|
||||
|
||||
# The anthropic/… models run through the Claude Code CLI (claude-code
|
||||
# engine). Pinned exactly — see workflow.yml (expo-code-review.yml).
|
||||
- name: Install Claude Code CLI
|
||||
run: npm install -g @anthropic-ai/[email protected]
|
||||
|
||||
# SECURITY: the base-ref checkout includes every .expo-code-review/ config,
|
||||
# whose auth.tokenEnv names the forwarded model credential. `ecr verify-config`
|
||||
# sweeps every config and refuses unless tokenEnv appears exactly once, in a
|
||||
@@ -94,7 +89,7 @@ jobs:
|
||||
# @ref LLP 0009#guard-step-ordering-and-job-budgets [implements] — same $ECR_VERSION feeds guard and review
|
||||
- name: Guard config tokenEnv (root + routing + all scopes)
|
||||
env:
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
run: npx --yes -p "@expo/code-review-cli@$ECR_VERSION" ecr verify-config
|
||||
|
||||
# Running via `issue_comment` makes this a manual /review, which the CLI detects
|
||||
@@ -106,11 +101,10 @@ jobs:
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
# Layer-1 auth lock: keep in sync with the guard's EXPECTED.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Anthropic review credential — the env var named by auth.tokenEnv in
|
||||
# config.jsonc (see workflow.yml for the accepted token shapes).
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
# OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
# Optional: override the model for every agent (uses your OpenCode login).
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
run: |
|
||||
# Build the flags as a bash array so the path never word-splits.
|
||||
|
||||
+7
-14
@@ -111,12 +111,6 @@ jobs:
|
||||
# to save an empty cache and error).
|
||||
package-manager-cache: false
|
||||
|
||||
# The anthropic/… models run through the Claude Code CLI (claude-code
|
||||
# engine). Pinned exactly — see workflow.yml (expo-code-review.yml).
|
||||
- name: Install Claude Code CLI
|
||||
if: steps.cmd.outputs.run == 'true'
|
||||
run: npm install -g @anthropic-ai/[email protected]
|
||||
|
||||
# SECURITY: the base-ref checkout above includes every .expo-code-review/
|
||||
# config.jsonc + routing.jsonc, whose auth.tokenEnv names the env var the CLI
|
||||
# forwards as the model credential. The canonical guard ships with the CLI:
|
||||
@@ -132,7 +126,7 @@ jobs:
|
||||
if: steps.cmd.outputs.run == 'true'
|
||||
env:
|
||||
# (Comma-separated set for a multi-credential auth.providers config.)
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
run: npx --yes -p "@expo/code-review-cli@$ECR_VERSION" ecr verify-config
|
||||
|
||||
- name: Run AI review
|
||||
@@ -142,13 +136,12 @@ jobs:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
# Layer-1 auth lock: the CLI refuses to run when the tokenEnv it would honor
|
||||
# differs from this. Keep it in sync with the guard's EXPECTED.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Anthropic review credential — the env var named by auth.tokenEnv in
|
||||
# config.jsonc. Store it as a repo secret: an `sk-ant-oat…` token minted
|
||||
# by `claude setup-token`, or an `sk-ant-api…` Console key (the Claude
|
||||
# Code CLI reads either).
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
# OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.
|
||||
# Store it as a repo secret; a project-scoped key restricted to model
|
||||
# inference (with a spend limit) is all the reviewer needs.
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
# Optional: override the model for every agent (uses your OpenCode login).
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
AGENTS: ${{ steps.cmd.outputs.agents }}
|
||||
ROUTE: ${{ steps.cmd.outputs.route }}
|
||||
|
||||
+39
-31
@@ -1,17 +1,15 @@
|
||||
// @ref LLP 0009#config-and-prompt-templates — root config: agent roster by filename, phase-1 defaults, auth
|
||||
{
|
||||
// Default model for every agent. Anthropic models run through the Claude Code
|
||||
// CLI engine (`claude -p`), inferred per agent from the model id — see `auth`
|
||||
// below for the credential. Override per-agent via frontmatter in the agent's
|
||||
// markdown, or at runtime with the REVIEWER_MODEL env var
|
||||
// (e.g. REVIEWER_MODEL=anthropic/claude-haiku-4-5).
|
||||
"model": "anthropic/claude-sonnet-5",
|
||||
// Default model for every agent. Override per-agent via frontmatter in the
|
||||
// agent's markdown, or at runtime with the REVIEWER_MODEL env var
|
||||
// (e.g. REVIEWER_MODEL=openai/gpt-5.4-mini).
|
||||
"model": "openai/gpt-5.5",
|
||||
|
||||
// Agents: every markdown file in agents/ is one reviewer (id = filename).
|
||||
// Add or remove files to change the roster — no list needed here.
|
||||
// shared.md (prepended to every agent + coordinator) and coordinator.md are
|
||||
// reserved filenames. Per-agent overrides go in each file's YAML frontmatter,
|
||||
// e.g. `---\nmodel: anthropic/claude-opus-5\n---`.
|
||||
// e.g. `---\nmodel: openai/gpt-5.5-pro\n---`.
|
||||
|
||||
// @ref LLP 0009#config-and-prompt-templates [implements] — suggestions off by default, not a schema limit
|
||||
"policy": {
|
||||
@@ -48,39 +46,49 @@
|
||||
// HTML marker used to find + update the single PR comment. Keep it stable.
|
||||
"commentTag": "expo-ai-code-reviewer",
|
||||
|
||||
// How model credentials are provided. Default: Anthropic via the Claude Code
|
||||
// CLI. Anthropic models always run through that CLI — the engine is inferred
|
||||
// per agent from the `anthropic/…` model id, so this entry only supplies the
|
||||
// credential. In CI the tokenEnv holds the review credential (an `sk-ant-oat…`
|
||||
// token minted by `claude setup-token`, or an `sk-ant-api…` Console key — the
|
||||
// CLI reads either); locally an active `claude` login is enough, and the entry
|
||||
// could even be omitted. ECR_EXPECTED_TOKEN_ENV (repo variable / workflow
|
||||
// fallback) must equal this tokenEnv. `ecr setup-auth` walks you through it.
|
||||
//
|
||||
// Other providers run through OpenCode:
|
||||
// How model credentials are provided. Default: an OpenAI API key.
|
||||
// "api-key": tokenEnv names the env var holding a provider API key. In CI,
|
||||
// store the key as a repo secret and pass it under that env var.
|
||||
// "auth": { "mode": "api-key", "provider": "openai", "tokenEnv": "OPENAI_API_KEY" }
|
||||
// store the key as a repo secret and pass it under that env var
|
||||
// (the scaffolded workflow does). If you omit `auth` entirely,
|
||||
// OpenCode's own login / ambient provider env vars are used.
|
||||
// For Anthropic/Claude, see the dedicated block below (it now runs through the
|
||||
// Claude Code CLI). For another provider, omit `auth` and set REVIEWER_MODEL
|
||||
// after an `opencode auth login` for that provider.
|
||||
//
|
||||
// ChatGPT/Codex subscription ("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.
|
||||
// MIXED setup (a ChatGPT/Codex subscription for the default models, plus a
|
||||
// metered API key for pro-tier models the subscription doesn't offer): use the
|
||||
// per-provider map instead, reference `openai-api/...` models in the frontmatter
|
||||
// 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_ACCESS_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 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.)
|
||||
//
|
||||
// Anthropic runs through the Claude Code CLI, inferred from the model — set your
|
||||
// models to "anthropic/…" (e.g. anthropic/claude-opus-5, anthropic/claude-sonnet-5)
|
||||
// and run `claude setup-token`, exporting the token as CLAUDE_CODE_OAUTH_TOKEN (CI)
|
||||
// or rely on your local `claude` login. An auth entry is OPTIONAL (tokenEnv just
|
||||
// names the credential env); `ecr setup-auth` walks you through it.
|
||||
// "auth": { "providers": {
|
||||
// "anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" }
|
||||
// } }
|
||||
//
|
||||
// MIXING engines is supported: the engine is inferred per agent from its model,
|
||||
// so this anthropic entry may coexist with an openai (or any other) OpenCode
|
||||
// provider — an `anthropic/…` agent runs through the Claude Code CLI while an
|
||||
// `openai/…` agent runs through OpenCode, in the SAME run:
|
||||
// MIXING engines is supported: the engine is inferred per agent from its model, so
|
||||
// an anthropic entry may coexist with an openai (or any other) OpenCode provider.
|
||||
// Each agent's `model` selects its engine — an `anthropic/…` agent runs through the
|
||||
// Claude Code CLI while an `openai/…` agent runs through OpenCode, in the SAME run.
|
||||
// "auth": { "providers": {
|
||||
// "anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" },
|
||||
// "anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" },
|
||||
// "openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" }
|
||||
// } }
|
||||
"auth": {
|
||||
"providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }
|
||||
}
|
||||
"mode": "api-key",
|
||||
"provider": "openai",
|
||||
"tokenEnv": "OPENAI_API_KEY"
|
||||
},
|
||||
|
||||
// Stack-aware requalification (ROOT-ONLY; off by default). When on, `ecr ci` walks
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
<!-- @ref LLP 0009#config-and-prompt-templates — pro tier pinned on purpose: consolidation quality over serial-tail latency -->
|
||||
---
|
||||
# The coordinator makes the final call — de-duping, re-judging severity, and
|
||||
# deciding — so it runs on the Opus tier: consolidation quality matters more here
|
||||
# deciding — so it runs on the pro tier: consolidation quality matters more here
|
||||
# than the small serial-tail latency it adds (no repo tools, one bounded pass).
|
||||
# Override with a cheaper model if you'd rather trade decision quality for latency.
|
||||
# @ref LLP 0009#config-and-prompt-templates [implements]
|
||||
model: anthropic/claude-opus-5
|
||||
model: openai/gpt-5.5-pro
|
||||
---
|
||||
|
||||
# Coordinator — consolidation & decision
|
||||
|
||||
+7
-15
@@ -67,13 +67,6 @@ jobs:
|
||||
# to save an empty cache and error).
|
||||
package-manager-cache: false
|
||||
|
||||
# The anthropic/… models in .expo-code-review/config.jsonc run through the
|
||||
# Claude Code CLI (the claude-code engine). Pinned exactly — an unpinned CLI
|
||||
# would silently drift on every CI run; bump it deliberately. Remove this
|
||||
# step if you switch every model to an OpenCode-served provider.
|
||||
- name: Install Claude Code CLI
|
||||
run: npm install -g @anthropic-ai/[email protected]
|
||||
|
||||
# SECURITY: the TRUSTED BASE checkout above includes every
|
||||
# .expo-code-review/config.jsonc + routing.jsonc, whose auth.tokenEnv names the
|
||||
# env var the CLI forwards as the model credential. The canonical guard ships
|
||||
@@ -94,7 +87,7 @@ jobs:
|
||||
- name: Guard config tokenEnv (root + routing + all scopes)
|
||||
env:
|
||||
# (Comma-separated set for a multi-credential auth.providers config.)
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
run: npx --yes -p "@expo/code-review-cli@$ECR_VERSION" ecr verify-config
|
||||
|
||||
- name: Run AI review
|
||||
@@ -108,13 +101,12 @@ jobs:
|
||||
# honor (root config.jsonc, or routing.jsonc defaults.auth) differs from
|
||||
# this — it catches what the guard step above can't. Keep it in sync
|
||||
# with the guard.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Anthropic review credential — the env var named by auth.tokenEnv in
|
||||
# config.jsonc. Store it as a repo secret: an `sk-ant-oat…` token minted
|
||||
# by `claude setup-token`, or an `sk-ant-api…` Console key (the Claude
|
||||
# Code CLI reads either).
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
# OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.
|
||||
# Store it as a repo secret; a project-scoped key restricted to model
|
||||
# inference (with a spend limit) is all the reviewer needs.
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
# Optional: override the model for every agent (uses your OpenCode login).
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
|
||||
# Observability: the per-run log (token/cache/cost totals + per-pass timing +
|
||||
|
||||
Reference in new issue
Block a user