Compare commits

...
33 changed files with 695 additions and 516 deletions

No files matched your search

+320 -487
View File
@@ -11,6 +11,10 @@ is the **engine** — each repo supplies its own agents and settings under
Inspired in part by Cloudflare's [_How we built our AI code review bot_](https://blog.cloudflare.com/ai-code-review/).
This README is the scannable overview. Every subsystem has a detailed design doc
under [`llp/`](./llp/) — start with
[LLP 0000](./llp/0000-expo-code-review-cli.explainer.md), the system map.
```mermaid
flowchart TD
SRC["Source<br/>local git · GitHub PR (gh)"] --> FILTER["Noise filter<br/>drop lockfiles · generated · binary"]
@@ -23,6 +27,70 @@ flowchart TD
VERIFY --> REPORT["Reporter<br/>one PR comment (CI) · terminal (local)"]
```
## Why not just `claude -p "review this diff"`?
A one-shot prompt in a workflow works — until the failure modes show up. The
engine exists for those:
- **Everything the reviewer reads is attacker-controlled.** The diff, the PR
description, and the repo files are all input an outside contributor wrote —
and a bare CLI run obeys instruction files from that same checkout
(`CLAUDE.md`, `.mcp.json`, plugins) while holding the model credential and
comment token. Here, configuration comes only from the trusted base commit,
those files are scrubbed before the engine starts, and every pass is read-only
([details below](#the-security-gap-specifically)).
- **Confident-but-wrong findings post as-is.** Raw model output is the review.
Here, every finding is quote-grounded against the real file and criticals are
adversarially verified before anything is posted.
- **Failures read as silence or approval.** A one-shot call that stalls, gets
rate-limited, or returns garbage either fails the job or posts nothing. Here,
each failure class has its own bounded retry, a failed run never renders as a
clean result, and CI always posts a terminal comment.
- **Big diffs blow the context window.** Large PRs are chunked with a separate
cross-cutting pass for multi-file issues; specialist agents run in parallel; the
prompt layout is cache-stable, and every run reports tokens, cost, and cache
hit rate.
- **Noise wastes the model's attention.** Lockfiles, generated bundles, and
binaries are filtered before the model sees them — recorded, never silently
dropped.
- **Comment spam.** One fingerprinted comment updated in place, with `/dismiss`,
severity floors, inline ignores, and author-reply tracking — not a new wall of
text per push.
- **Review prompts rot.** `ecr ref-check` fails when a prompt cites code that
moved or vanished, and every run warns loudly if a provider substituted a
different model than configured.
### The security gap, specifically
A review bot is a process holding a model credential and a GitHub write token
that reads attacker-controlled input for a living. A bare `claude -p` workflow
typically checks out the PR head and hands the model broad tools — so a PR that
says the right words can run code, read secrets, or post as you. The trust model
here ([LLP 0001](./llp/0001-trust-model.principles.md)) is built around that:
- **No PR-controlled code is ever built or executed.** The engine runs as the
published npm package via `npx`; the scaffolded workflows check out only the
base commit, with `persist-credentials: false`.
- **The model has no write tools and no ambient web.** `Read`/`Grep`/`Glob` only
— never `Bash`, `Edit`, or `WebFetch`. An injected instruction has nothing to
act with; the worst it can do is produce a wrong finding, which then has to
survive verification.
- **Review policy is not PR-editable.** Config, prompts, rosters, and the auth
mapping load from the PR's immutable base commit; `tokenEnv` is honored in
exactly one root-owned place, enforced at the schema level and again by an
independent CI guard step.
- **Credentials are compartmentalized.** Child environments are allowlists that
omit ambient keys; the research MCP never sees a model credential, the model
process never sees the search key, and outbound research queries fail closed
on credential-shaped input.
- **Untrusted text can't impersonate the reviewer.** PR prose travels inside
sanitized boundary markers, and every `<!--` in it is escaped so a forged
state marker can't hijack the dismissal list. `critical`/`secrets` findings
can't be dismissed or cleared by replies — enforced in code, not the prompt.
- **The blast radius is capped by design.** The review is advisory and
comment-only: even a fully fooled model can't approve, merge, or block
anything.
## Usage
Run via `npx @expo/code-review-cli <command>` (or the `ecr` / `expo-code-review`
@@ -52,15 +120,16 @@ npx @expo/code-review-cli doctor
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.
a **ChatGPT/Codex subscription** sign-in, or a Meta Model API key for **Muse
Spark**). `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).
Prefer **OpenAI** (API key, or a ChatGPT/Codex subscription, or both mixed) or
another provider? See [Other providers & auth modes](#other-providers) below.
Prefer **Muse Spark**, **OpenAI** (API key, or a ChatGPT/Codex subscription, or
both mixed), or another provider? See [Other providers & auth modes](#other-providers)
below.
### Reviewing (already configured)
@@ -133,219 +202,92 @@ untrusted external context; see below).
## Keeping prompts true (`ecr ref-check`)
Good reviewer prompts cite real code: "the only session entry point is
`server/src/session.ts`", "every webhook router must call `sanitizeSecrets`". Then the
code moves and the prompt keeps citing a path that no longer exists — the reviewer
reasons from a fiction, on every PR, with nothing to warn you.
`ecr ref-check` makes those citations checkable. Pin each one with a ref, in a comment
of its own (`<!-- … -->` in Markdown, `//` in JSONC):
Good reviewer prompts cite real code ("every webhook router must call
`sanitizeSecrets`"). Then the code moves and the prompt keeps citing a path that no
longer exists — the reviewer reasons from a fiction on every PR. `ecr ref-check`
makes those citations checkable: pin each one with a `@ref` comment (`<!-- … -->` in
Markdown, `//` in JSONC):
```md
<!-- @ref server/src/session.ts#createSession — the only place a session is minted -->
<!-- @ref server/src/entities/oauth/ — every provider lives here -->
<!-- @ref glob:**/*WebhookRouter.ts — the routers this rule is about -->
```
A target is a file, a `dir/`, `glob:<pattern>`, `file#symbol`, or `doc.md#heading` —
never a line number, since a line number rots without any signal. Symbol anchors are
checked by whole-word match, so code moving inside its file is fine and a rename is not.
The check is strict on purpose: a backticked token in `.expo-code-review/` that looks
like a repo path **must** be a ref, because the stale citations are exactly the ones
nobody thought to annotate. A token with no extension (`eas-build-worker/terraform`,
`general-central/{module,production}`, `finops`) counts when it names something that
exists — so those get pinned too, while `anthropic/claude-opus-5`, shaped the same way,
stays prose. For a token that only looks like a path, say so once:
`<!-- @ref-ignore knex.raw() -->`. It also checks what your config already declares —
`enforceAgents` ids, scope `config` directories, scope path globs.
Refs are repo-root-relative, including in a scope's own setup dir. A scope prompt that
cites `general-central/module` for `infrastructure/general-central/module` gets told the
root-relative form to use.
never a line number (line numbers rot without any signal). The check is strict on
purpose: any backticked token in `.expo-code-review/` that names something that
exists in the repo **must** be a ref, because stale citations are exactly the ones
nobody annotated. Mark the rare false positive once with
`<!-- @ref-ignore knex.raw() -->`. Refs are always repo-root-relative, including in
a scope's own setup dir, and config declarations (`enforceAgents` ids, scope
directories and globs) are checked too.
Two run points:
- `ecr ref-check` exits 1 on any problem. Run it in CI or a pre-commit hook.
- `ecr review` / `ecr ci` run it too, and never fail a PR's checks with it. The comment
carries a **Review setup** note instead: refs that no longer resolve, plus cited code
*this PR* changes, where the ref still resolves but the guidance may not.
- `ecr review` / `ecr ci` run it too but never fail a PR's checks with it — broken
refs (and cited code *this PR* changes) surface as a **Review setup** note in the
comment.
Full detail: [LLP 0012](./llp/0012-config-ref-integrity.explainer.md).
---
## Providing context and research capabilities
## Platform research (bundled MCP)
`@expo/code-review-cli` includes `review-research-mcp` and exposes that one bundled,
local MCP directly to reviewer and cross-file passes. The agent decides whether an
external API contract needs research and can either search for an exact symbol or
fetch an exact supported documentation URL already present in the review context.
Coordinator, verifier, stack-verifier, and time-critical no-tools passes never receive
the MCP.
The package bundles `review-research-mcp`, a local MCP exposed only to the reviewer
and cross-file passes (never the coordinator, verifier, or no-tools passes). When a
judgment depends on an externally owned API contract, an agent can search a fixed
catalog of official documentation providers — Apple/Android platform APIs, Expo,
React Native, OkHttp, Kotlin coroutines, Gradle/AGP, Swift evolution, and more — or
fetch one exact supported documentation URL. Queries are short exact symbols
(`CameraView barcodeScannerSettings` is useful; a source snippet or a question is
not), and an empty result stays empty rather than becoming a loose guess. There is
no offline index: every passage is fetched live during that review.
Enable it only in the root config, which CI loads from the PR's trusted base:
Enable it only in the root config (CI loads it from the PR's trusted base) and add a
`BRAVE_SEARCH_API_KEY` Actions secret (Expo and OkHttp use their own official search
and consume no Brave quota):
```jsonc
{
"research": {
"enabled": true,
"maxQueries": 8,
"resultsPerQuery": 2,
"timeoutMs": 30000
}
"research": { "enabled": true, "maxQueries": 8, "resultsPerQuery": 2, "timeoutMs": 30000 }
}
```
Add a repository Actions secret named `BRAVE_SEARCH_API_KEY`. The generated
automatic and command-triggered workflows pass only that search credential to the
MCP. Expo uses its public documentation search and OkHttp uses its official static
search index; neither consumes Brave quota.
The boundary, in brief:
ECR resolves the MCP entry point inside its own installed package and starts it with
the current absolute Node executable, so a PR-owned `PATH` entry cannot replace
either component. Each review gets an owner-only temporary MCP config and append-only
audit. Claude receives that explicit config under `--strict-mcp-config`, with project
settings and slash commands disabled; OpenCode receives the same fixed local command.
- **Bounded egress, not a confidentiality boundary.** Queries are normalized and
sanitized (credential-shaped or secret-labeled input fails closed); direct URLs
must be plain HTTPS on a fixed provider host/path allowlist; redirects, response
sizes, content types, and per-call deadlines are enforced server-side. The model
still chooses the query terms, so enable research only where repository-derived
terms may be shared with Brave and the documentation providers.
- **The server never sees a model credential.** ECR starts the MCP from its own
installed package through a wrapper that rebuilds the child environment from an
explicit allowlist; the Brave key never enters the model process either.
- **Audited and citable.** Every outbound query and returned result is audited (job
log, Actions step summary, `.runs/reviews.jsonl`). A finding can cite only URLs
actually retrieved during that review, and the verifier strips citations whose
passage does not support the claim.
- **`maxQueries` bounds MCP calls, not HTTP requests** — one search can fan out to
many discovery and page fetches, so each call reports its own request ledger and
the run reports totals.
- **Root-only in routed monorepos** (it starts a host process); scope configs
cannot alter it. Result-cache reuse is disabled while research is enabled,
because web content can change without a config change.
That command is a wrapper, not the server. Both engines merge a configured MCP `env`
onto their own environment instead of replacing it, so the declared block alone cannot
bound the child — the engine's model credential reaches it, and OpenCode additionally
passes down the runner's whole ambient environment. The wrapper therefore rebuilds the
environment from an explicit allowlist before starting the server, and loads no parser
and opens no socket of its own. The server process sees the search key, the call
bounds, and locale/proxy settings; it never sees a model credential. The Brave
credential travels the same way and is never added to the model process env.
The MCP bounds the shape, host, and volume of outbound requests. It is not a
confidentiality boundary: the reviewing model chooses the query terms and URLs, and a
low-entropy identifier can carry repository-derived data past every check below.
Enable research only where repository-derived terms may be shared with Brave and the
documentation providers. Search queries are normalized before
logging or networking: quoted literals, URLs, email addresses, paths, prose stop
words, overlong/high-entropy tokens, and unsupported punctuation are removed;
credential-shaped or secret-labeled input fails closed. The remaining query must be at
most eight short tokens and either contain an API-like symbol or be a short multi-word
lowercase concept phrase. Direct URLs must use plain
HTTPS with no credentials, port, query string, or fragment; suspicious/high-entropy
path segments fail closed. The fixed provider host/path allowlist and redirect,
response-size, content-type, and timeout checks still apply after that first gate.
These deterministic checks greatly reduce accidental exfiltration; they are not a
proof that every low-entropy string is harmless, so reviewer prompts also forbid
sending repository text and the review-wide MCP budget defaults to eight calls.
`maxQueries` bounds MCP calls, not network requests. One search selects up to four
providers, and each issues its own discovery request plus a page fetch per candidate,
so eight calls can mean roughly thirty discovery requests and over a hundred page
downloads. Every call therefore reports its own ledger — discovery requests, page
fetches, redirect hops, total HTTP requests, and elapsed time — and the review log and
Actions summary report the totals. `timeoutMs` is the MCP's own end-to-end deadline for
one call, enforced by the server across discovery, redirects, retrieval, and
extraction; a call that hits it returns what it already has rather than failing. It has
to live there because OpenCode's `timeout` bounds only tool discovery and Claude
provides no per-call timeout at all.
For non-Expo providers, discovery sends a fixed, provider-owned `site:` scope plus
the bounded query to Brave's fixed Web Search endpoint. Search snippets and titles
are never treated as evidence. ECR independently rejects off-allowlist result URLs,
manually validates every redirect, fetches a few official pages, verifies content
types and response sizes, extracts visible documentation text, and returns locally
ranked bounded passages. Sparse search-engine coverage therefore produces an honest
empty result rather than a loose guess.
Research is root-only in routed monorepos because it starts a host process; scope
configs cannot alter its network behavior or limits. Result-cache reuse remains
disabled while research is enabled because web results and documentation can change
without a config change.
The fixed provider catalog covers Apple/Android APIs plus SDWebImage, Media3, Glide, OkHttp,
Kotlin coroutines, Gradle/AGP, Swift concurrency/evolution, platform release/API
availability, Expo, React Native, Reanimated, Gesture Handler, Screens, and Worklets.
Queries are short exact symbols plus at most one useful member or behavior term. For
example, `CameraView barcodeScannerSettings` is useful; a source snippet, import path,
or natural-language question is not. The MCP publishes the same guidance in its tool
metadata. An empty result stays empty; it is not replaced with a loose semantic guess.
The tool metadata also includes an explicit provider map, so the reviewing model can
distinguish core platform APIs from release notes, dependency-owned documentation,
build-tool references, and issue-tracker context before choosing a corpus.
Reviewer instructions require grounding whenever a judgment depends on an externally
owned API contract, whether the evidence confirms a finding or dismisses a candidate
as safe; model memory alone is not treated as sufficient for those decisions.
Native source keeps its platform context: Apple or Android documents the OS contract,
while an explicit dependency provider documents library-owned behavior. Providers are
additive when both contracts matter. A path under `packages/expo-*` does not by itself
route Swift or Kotlin code to Expo's JavaScript documentation.
Direct clients can also call `fetch_platform_doc` with an exact documentation URL.
The tool infers the narrowest matching provider (or accepts an explicit provider
hint), then applies the same fixed HTTPS host/path allowlist, manual redirect checks,
10-second timeout, 5 MB response limit, content-type validation, extraction, and
passage bounds as search-discovered pages. It returns normalized extracted text, never
raw HTML or DocC JSON. An optional `query` selects context only within that one page;
it never broadens discovery. Context expands progressively:
- `focused` returns the best passage plus adjacent passages.
- `section` (the default) returns a contiguous window of at most 12,000 characters
around the best passage.
- `document` returns at most 20,000 characters of extracted page text and should be
used only when the contract is spread across the page.
The response reports returned and original character counts, whether it was truncated,
the anchor passage id, and bounded available passage ids. Search results also carry
neighboring passage ids so an agent can recognize when more local context exists. For example,
`https://developer.apple.com/documentation/swiftui/view/menustyle(_:)` is resolved to
Apple's DocC JSON and returned with the canonical page URL and API availability.
Every research-enabled review audits each sanitized outbound query and each returned result's
title, provider, provenance class, and canonical URL. GitHub Actions receives the
same audit trail in the step summary, while `.runs/reviews.jsonl` keeps the queries
plus bounded returned passages for short-lived operational inspection. Reviewers
are instructed to attach `sources` only when documentation materially supports a
finding. ECR accepts only exact URLs returned during that review, restores canonical
titles, carries citations through coordination, and renders them below the finding.
A citation to a URL this review never retrieved is dropped outright. Relatedness is a
separate, weaker guarantee: a cited finding is escalated to the verifier with the
audited passage inline, which judges whether that passage actually supports the claim
and strips the citation when it does not.
Reviewers also emit a bounded `researchDecisions` record only when documentation
materially confirms a finding candidate or proves one safe. ECR grounds those records
against the exact MCP audit and discards ungrounded claims. After verification and
suppression, the log and Actions summary report final findings with citations,
supported and dismissed candidates, and unique audited results materially used versus
unused. Counts use canonical URLs rather than passage count, so repeated hits do not
inflate usefulness.
For a query routed to the `expo` provider, `serve` POSTs the already-sanitized query
directly to Expo's public Algolia search endpoint and returns canonical
`docs.expo.dev` hits. The endpoint, application id, and browser-visible search-only
key are fixed in the package; redirects are rejected; response size, timeout, hit
count, and returned URL host are bounded.
OkHttp's newly migrated documentation is still sparse in Brave, so its provider
downloads the fixed official `lysine.dev` static search index, validates and indexes
it in memory once per MCP process, and rejects any entry outside the existing OkHttp
allowlist. Brave remains a fallback if that official index is unavailable.
There is no offline index. Every passage a review sees is fetched live from the
provider allowlist during that review, so evidence is never served from a local
artifact whose contents ECR cannot vouch for. The crawler, its seed catalog, and
`research.indexPath` were removed once live discovery replaced them; a config still
naming `indexPath` fails to parse rather than silently ignoring it.
Installation-specific provider configuration is intentionally
deferred: when added, it should follow the trusted root-config model used for agents
without permitting PR-controlled URLs, commands, or executable parsers. Expo skills
are complementary, not another search corpus: their pinned procedural guidance can
later be supplied to review agents as separately labeled trusted context, while
documentation search continues to return citable API evidence. Dynamic skills or
instructions retrieved from documentation must never become executable reviewer
instructions.
Full detail — providers, query grammar, `fetch_platform_doc` modes, provenance and
citation grounding: [LLP 0013](./llp/0013-platform-research.explainer.md).
## Monorepos (routing manifest)
A monorepo can route different subtrees to different reviewer rosters from a single
infra-owned manifest. There is still **one workflow, one `ecr ci` process** per PR:
it reads the changed files once, assigns each to exactly one scope, reviews each
active scope over only its files, and renders one comment. Because it is a single
writer in a single process there is no comment/check race and no locking.
it reads the changed files once, assigns each to exactly one scope (scopes are
ordered, the **last** match wins — CODEOWNERS discipline), reviews each active scope
over only its files, and renders one comment — a single writer, so no comment race
and no locking.
```
your-monorepo/
@@ -390,79 +332,56 @@ your-monorepo/
}
```
- **Path glob dialect** — `**` crosses `/`, `*` matches within a segment. Keep a
`**/*` catch-all scope so no changed file goes unreviewed (`ecr doctor` flags a
coverage gap otherwise). Scopes are ordered and the **last** match wins, so put
broad scopes first and specific ones after (CODEOWNERS/Renovate discipline).
- **Comment modes** — `single` posts one aggregated comment (a scope summary table
+ a collapsed `<details>` per scope) under the existing marker; `per-scope` posts
one cleanly-namespaced comment per scope. A scope with zero matched files gets its
stale comment deleted.
- **Scoped flags** — `ecr ci --scopes a,b` limits the fan-out; `ecr ci --comment
single|per-scope` overrides the manifest; `ecr review --scope <name>` runs one
scope locally; `ecr review`/`ecr ci --config-dir <dir>` (or `ECR_CONFIG_DIR`)
load config from an alternate directory; `ecr doctor --list-scopes` prints the
scope table. **`--config-dir` designates an alternate ROOT config dir: both
`config.jsonc` and `routing.jsonc` are read from it, so the root config and its
manifest always travel together. Scope `config` paths stay repo-root-relative —
the override swaps the root config/manifest against the *real* scope tree, it
does not relocate the scopes themselves.**
- **Passes budget** — `defaults`-level `budget` bounds total review time:
`totalPassesMinutes` (default 55) is split across active scopes (which run
sequentially in one `ecr ci`), clamped up to `minScopeMinutes` (default 5) so a
single scope still gets a workable window. When enough scopes are active that the
floor would overshoot the total, `ecr ci` keeps the floor but warns, and `ecr
doctor` flags the worst case (`scopes × floor` vs total) — raise the workflow
`timeout-minutes` or trim scopes.
- **Adoption is incremental** — with no `routing.jsonc`, behavior is exactly as
before (single config). Add the manifest with just a default scope → still one
comment, identical behavior. Land per-team scope dirs one at a time; everything
else keeps hitting the default scope.
- **Keep a `**/*` catch-all scope** so no changed file goes unreviewed (`ecr
doctor` flags a coverage gap otherwise); broad scopes first, specific ones after.
- **Comment modes** — `single` posts one aggregated comment; `per-scope` posts one
namespaced comment per scope. A scope with zero matched files gets its stale
comment deleted.
- **Passes budget** — a `defaults`-level `budget` splits `totalPassesMinutes`
(default 55) across the active scopes, floored at `minScopeMinutes` (default 5)
per scope; `ecr ci` and `ecr doctor` warn when the floor can overshoot the total.
- **Scoped flags** — `ecr ci --scopes a,b`, `ecr ci --comment single|per-scope`,
`ecr review --scope <name>`, `--config-dir <dir>` / `ECR_CONFIG_DIR` (alternate
ROOT config+manifest dir — scope `config` paths stay repo-root-relative), and
`ecr doctor --list-scopes`.
- **Adoption is incremental** — no `routing.jsonc` means exactly the old
single-config behavior; a manifest with just a default scope is identical; land
per-team scope dirs one at a time.
### Security
- **auth and research are locked to the root.** `tokenEnv` (which env var becomes the model
credential) is honored in exactly one place: the root `config.jsonc` or
`routing.jsonc` `defaults.auth`. A scope config declaring `auth`/`breakGlass`/`research`
**fails to parse** (Zod-level rejection). A scope declaring `research` also fails,
so PR-controlled routing cannot select a different host index. The CI guard step independently
sweeps every `.expo-code-review/config.jsonc`/`routing.jsonc` repo-wide and refuses
to run unless `tokenEnv` appears exactly once, in a root-owned file, equal to
`ECR_EXPECTED_TOKEN_ENV`. A routing manifest can never widen exposure — globs only
choose *which roster* reviews a file, never *which secret* is sent.
- **enforceAgents can't be weakened.** Agents listed in `defaults.enforceAgents`
(e.g. `security`) are injected into every scope with `alwaysRun`, taken from the
root roster — a scope defining a same-id agent gets the root one, so a team can't
shadow the enforced reviewer with a weaker version on its own subtree.
- **Configuration comes from the PR's trusted base commit.** In `ecr ci`, review
policy and reviewer configuration — `config.jsonc`, `routing.jsonc`, prompts,
models, and the auth mapping — load from the PR's immutable **base** commit,
materialized via the GitHub API. The PR head is untrusted data: it is
materialized separately (pinned to its immutable OID) purely as source content
to read and verify against. A PR editing rosters, prompts, or routing is
reviewed under the **previous** config; its changes activate after merge. If
the base commit can't be materialized, the run fails closed (one terminal
comment) — it never falls back to the checkout. A scope config that is new in
a PR is reviewed with the root config until it merges.
- **The model runtime never sees PR-owned ambient config.** The head worktree the
agents read from is scrubbed of runtime configuration before the OpenCode
server starts: `opencode.json{,c}`, `.opencode/` (plugins), `AGENTS.md`,
`CLAUDE.md`, `.claude/`, `.mcp.json`, `.cursor*`, and `.env*` at every depth.
A PR can't install a plugin, MCP server, instruction file, or `.env` into the
process that holds the model credential and the comment token. (Changes to
those files are still reviewed — their diffs are inlined in the prompt — but a
finding citing one can't be re-read during verification; that's the tradeoff.)
- **The scaffolded workflows check out only the base commit** with
`persist-credentials: false`; the CLI's own git fetches authenticate through
`gh` from `GH_TOKEN`, so the token never lands in `.git/config` or argv. The
CLI enforces the trust model itself, so a custom workflow that checks out the
PR head still gets base-commit configuration. The temporary escape hatch
`ecr ci --unsafe-config-from-head` restores the old behavior with a loud
security warning and will be removed on a minor boundary.
Enforced in code and by an independent CI guard step, not by convention:
Ownership is enforced with CODEOWNERS: `/.expo-code-review/routing.jsonc @your-infra`
(the single authoritative router) and `/apps/api/.expo-code-review/ @your-api-team`
(each team owns only its own scope dir). Rerouting globs is gated behind infra review.
- **auth and research are locked to the root.** `tokenEnv` is honored only in the
root `config.jsonc` / `routing.jsonc` `defaults.auth`; a scope config declaring
`auth`/`breakGlass`/`research` fails to parse, and the CI guard sweeps every
config file repo-wide and refuses to run unless `tokenEnv` appears exactly once,
root-owned, equal to `ECR_EXPECTED_TOKEN_ENV`. Routing globs choose *which
roster* reviews a file, never *which secret* is sent.
- **enforceAgents can't be weakened.** Enforced agents (e.g. `security`) are
injected into every scope from the ROOT roster with `alwaysRun`; a scope defining
a same-id agent gets the root one.
- **Configuration comes from the PR's trusted base commit.** In `ecr ci`, all
review configuration loads from the PR's immutable base; the head is untrusted
content, materialized separately only to read and verify against. A PR editing
rosters, prompts, or routing is reviewed under the **previous** config, and a
missing base fails closed — never a fallback to the checkout. (Temporary escape
hatch: `ecr ci --unsafe-config-from-head`, with a loud warning; to be removed.)
- **The model runtime never sees PR-owned ambient config.** The head worktree is
scrubbed of `opencode.json{,c}`, `.opencode/`, `AGENTS.md`, `CLAUDE.md`,
`.claude/`, `.mcp.json`, `.cursor*`, and `.env*` before the engine starts, so a
PR can't install a plugin, MCP server, instruction file, or `.env` into the
process holding the model credential and comment token.
- **The scaffolded workflows check out only the base commit** with
`persist-credentials: false`; git fetches authenticate through `gh`, so the
token never lands in `.git/config` or argv. The CLI enforces the trust model
itself, so even a custom workflow that checks out the head still gets
base-commit configuration.
Enforce ownership with CODEOWNERS: `/.expo-code-review/routing.jsonc @your-infra`,
`/apps/api/.expo-code-review/ @your-api-team`. Full detail:
[LLP 0006](./llp/0006-config-schema-loading-routing.explainer.md) (routing) and
[LLP 0001](./llp/0001-trust-model.principles.md) (trust model).
---
@@ -493,62 +412,42 @@ Ownership is enforced with CODEOWNERS: `/.expo-code-review/routing.jsonc @your-i
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.
and applies the provider's prompt caching automatically. Full detail:
[LLP 0002](./llp/0002-review-engine-pipeline.explainer.md) (pipeline) and
[LLP 0005](./llp/0005-verification-fingerprints-rendering.explainer.md)
(verification and rendering).
</details>
<details>
<summary><b>Tokens, cost &amp; prompt caching</b></summary>
Every run reports what it spent and how much of it was served from the prompt
cache, in three places:
Every run reports what it spent and how much was served from the prompt cache, in
three places: one `Token usage — …` line in the job log, a per-pass table + cache
hit rate in the GitHub Actions step summary (which also preserves each run's posted
comment, since the PR comment is updated in place), and one JSON line per run in
`.expo-code-review/.runs/reviews.jsonl` (uploaded as a CI artifact) with per-pass
tokens, raw per-agent findings, bounded reviewer traces, and coverage notes.
- **Job log (stderr)** — one line at the end of the run:
`Token usage — input …, output …, cache read …, cache write … (cost $…)`.
- **GitHub Actions step summary** — a per-pass table (one row per agent, plus the
cross-cutting pass, coordinator, and verifier), the run's cache hit rate, and
the exact comment that was posted. The PR comment is updated in place on every
run, so the step summary is where past runs' comments remain readable.
- **`.expo-code-review/.runs/reviews.jsonl`** — one JSON line per run (uploaded as
a CI artifact) with the same totals plus per-pass `agentTokens`, the raw
per-agent findings, bounded reviewer traces, coverage notes, and what the verifier
dropped.
Each reviewer can also return a compact trace (up to three concrete checks and two
unresolved questions). It is stored only inside the hidden base64 comment marker as
`review.reviewTrace` — never rendered — declared
`trust: "unverified-model-diagnostics"`, and capped at 6 KB so it can't crowd
visible findings out of GitHub's comment-size limit.
Each reviewer can also return a compact trace with up to three concrete checks and
two unresolved questions. The reporter stores it only inside the existing base64
`<!-- <commentTag>:state=… -->` comment marker as `review.reviewTrace`; it does not
render in the visible review. Agents and other machine consumers can decode that
state to see what a clean review covered. The payload declares
`trust: "unverified-model-diagnostics"`: it contains bounded conclusions, never a
raw transcript or chain-of-thought, and must not be treated as a verified finding.
The engine sorts agent ids and caps the complete decoded trace at 6 KB so this hidden
diagnostic cannot crowd visible findings out of GitHub's comment-size limit.
**How the caching works.** Provider prompt caching is a *prefix match*: any byte
change in the prefix invalidates everything after it. The reviewer keeps the prefix
stable — the system prompt (`shared.md` + the agent's own `.md`) is byte-identical
for every chunk an agent reviews, while the volatile parts (diff, file lists, PR
metadata) travel after it. OpenAI caches automatically at a steep read discount;
Anthropic charges ~1.25× input to **write** and ~0.1× to **read**. Hit rate =
`cache read / (cache read + input)`; multi-chunk reviews should show a high rate,
single-chunk reviews mostly show writes.
**How the caching works.** Provider prompt caching is a *prefix match*: the
provider caches the rendered prompt up to a point, and any byte change anywhere
in that prefix invalidates everything after it. The reviewer is laid out so the
prefix is stable — the system prompt (`shared.md` + the agent's own `.md`) is
byte-identical for every chunk an agent reviews, while the volatile parts (the
diff, file lists, PR metadata) travel in the user message *after* the prefix and
never touch it. OpenAI caches automatically (no write premium; cached input is
billed at a steep discount and shows up as `cache read`); Anthropic charges a
small premium to **write** the cache (~1.25× input) and ~0.1× input to **read**
it. Entries live minutes, refreshed on use — comfortably covering a run's
concurrent calls.
**Reading the numbers.** Hit rate = `cache read / (cache read + input)` — the
share of prompt tokens served from cache instead of being reprocessed at full
price. Multi-chunk reviews should show a high rate; single-chunk reviews mostly
show writes (there is nothing to re-read within the run).
**Keeping hits high:**
- Keep `shared.md` and `agents/*.md` stable. Any edit writes a new prefix — one
extra cache write per agent on the next run, then it is warm again. Never put
varying text (dates, PR numbers) into prompt files.
- Very short prompts may show `cache read 0`: prompts below the model's minimum
cacheable size (~1–4K tokens depending on the model) are silently not cached.
That is expected, not a bug.
To keep hits high: keep `shared.md` and `agents/*.md` stable (an edit costs one
cache write per agent on the next run, then it's warm again), and never put varying
text (dates, PR numbers) in prompt files. Prompts below the model's minimum
cacheable size (~1–4K tokens) show `cache read 0` — expected, not a bug.
</details>
@@ -637,60 +536,32 @@ change which model reviewed your code. Use an explicit override instead.
<details>
<summary><b>Reliability</b> — never hangs, never silently drops work</summary>
- **Per-task time caps** — chunk passes 15 min; coordinator 10 min. A global passes
budget (55 min) bounds all passes incl. the subdivision waves, fitting inside the
CI job's `timeout-minutes` (90).
- **The cross-file pass is elastic** — it gets whatever is left of the passes budget
rather than a fixed cap, because it's the one pass whose scope can't be traded for
convergence: halving its file set deletes exactly the coverage it exists for. Chunk
passes run alongside it under their own caps, so a long cross-file pass doesn't
starve them.
- **Tool-call cap** — a pass that makes too many `read`/`grep` calls without
finishing is *wandering*, not converging; hitting the cap trips the soft landing.
The cross-file ceiling scales with the diff's file count (its diffs are inlined, so
tool calls go to *tracing*, not fetching).
- **Stall detection** — a pass whose reply stops changing entirely (no new tool call,
no streamed text or reasoning, no token growth) has a wedged model request, not a
hard problem. After 4 min of silence it's abandoned and retried once from a clean
session, inside the same budget — instead of spending the whole cap on a dead
request. Progress lines say how long a reply has been silent, so this is legible in
the CI log.
- **Rate limits are detected and waited out, not fought.** The reviewer watches the
OpenCode server's own log for provider 429s (hard evidence, per run). A stall
*with* recent 429 evidence is throttling, not a wedge — the pass waits in 90s
beats (without consuming its one retry) instead of re-sending its whole context
into a limited account; explicit 429 errors retry on a slow 15s/45s/90s schedule.
Subscription (oauth) runs also default to `concurrency` 3 instead of 6, since one
account may be serving several PRs' reviews at once. Rate-limit events are
reported in the job log and the run log (`rateLimitEvents`), so throttling is a
visible fact about a run, never a mystery slowdown.
- **Soft landing on timeout** — at either cap, the run is interrupted and the agent
is asked to return the findings it already has, rather than discarding its work.
Tools are disabled for that request, so the salvage step can't resume investigating
instead of answering.
- **Subdivide-on-timeout** — a reviewer pass that times out with nothing to show has
its chunk split in half and the halves re-reviewed (recursively, down to a single
file), then a fast **no-tools fallback** over the inlined diff (the cross-file pass
skips straight to the fallback, which still sees the whole diff). Only a genuinely
un-reducible pass reports a coverage gap — and it is always reported, never silent.
- **Parse failures are retried** (same session, then once in a bounded fresh
session) — separate from the timeout path.
- **Transient API errors are retried** (bounded backoff on 429/5xx/network) —
distinct from both the timeout path (abandon) and the parse path; a one-off blip
no longer drops an entire pass.
- **Auth failures surface once, and fail fast** — `ecr` checks the configured
provider's credential at startup and stops with one clear message if it's missing
(rather than failing every pass); a credential rejected mid-run (401/403)
collapses into a single actionable coverage note pointing at
`auth.tokenEnv`/`REVIEWER_MODEL`.
- **A failed run never reads as "Approve"** — all passes fail → "could not
complete"; some fail → never a clean approve, and coverage-reduced.
- **The coordinator can't sink the run** — if consolidation fails, findings are
merged deterministically and still posted.
- **Coverage notes** — passes that timed out/failed are listed (routine noise
filtering is *not* flagged — it's expected and stays in the run log).
- **CI always gets a terminal state** — on any failure the PR gets a "didn't run"
comment, not a stuck reaction and silence.
- **Time caps everywhere** — chunk passes 15 min, coordinator 10 min, and a global
55-min passes budget that fits inside the CI job's `timeout-minutes`. The
cross-file pass is elastic (it gets whatever budget is left) because halving its
file set would delete exactly the coverage it exists for.
- **Wandering and wedged passes are cut short** — a tool-call cap catches a pass
that reads without converging; a 4-minute stall detector abandons a wedged model
request and retries once from a clean session, inside the same budget.
- **Rate limits are waited out, not fought** — provider 429s (observed in the
OpenCode server log) turn a stall into 90s waits instead of re-sends; explicit
429s retry on a slow schedule; subscription (oauth) runs default to lower
concurrency. Rate-limit events are reported in the job and run logs, so
throttling is never a mystery slowdown.
- **Soft landing, then subdivision** — at a cap, the agent is asked to return the
findings it already has (tools disabled, so it can't keep investigating). A pass
with nothing to show has its chunk split and re-reviewed recursively, down to a
no-tools fallback over the inlined diff. Only a genuinely un-reducible pass
reports a coverage gap — always reported, never silent.
- **Bounded retries per failure class** — parse failures, transient API errors
(429/5xx/network), and timeouts each have their own separate retry path; a
one-off blip never drops a whole pass.
- **Failure can't read as success** — auth problems fail fast with one actionable
message; all-passes-failed reads "could not complete"; a partial run is never a
clean pass; a failed coordinator falls back to deterministic merging; and CI
always posts a terminal comment, never silence.
Full detail: [LLP 0002](./llp/0002-review-engine-pipeline.explainer.md).
</details>
@@ -718,19 +589,19 @@ line: **comments = one-shot actions, labels = persistent configuration.**
These workflows are comment-only (they never fail the PR's checks). The engine runs
as the published package via `npx`, so no PR-controlled code is built.
Full detail: [LLP 0009](./llp/0009-adoption-templates-and-ci-workflows.guide.md)
(workflows) and [LLP 0007](./llp/0007-cli-commands-and-ci.explainer.md) (commands).
</details>
<details>
<summary><b>Author feedback (replies to findings)</b></summary>
A PR author's reply to a finding is matched to it deterministically — by quoting
the finding's title back, or by citing its short `` `id:…` `` token (only the id can
clear a finding, see below) — and recorded
in the comment's embedded state, no model involved in the matching itself. A
matched finding shows `💬 @login replied` (linked to the comment) and a visible
count above the fold; the reply's own text is never stored or rendered, only the
login, the comment link, and (optionally) an enum-valued verdict. Controlled by
the root-only `feedback` block in `config.jsonc`:
the finding's title back, or by citing its short `` `id:…` `` token — with no model
involved in the matching. A matched finding shows `💬 @login replied` (linked to
the comment); the reply's own text is never stored or rendered. Controlled by the
root-only `feedback` block in `config.jsonc`:
```jsonc
"feedback": {
@@ -743,36 +614,26 @@ the root-only `feedback` block in `config.jsonc`:
```
- **`annotate`** (the default) matches and shows "author replied" with zero effect
on the decision — safe and useful even if you never touch this block.
- **Clearing a finding always needs the `` `id:…` `` token**, in the replier's own words
(an id inside a `>` quote does not count). Quoting the title is enough to *annotate*,
never to clear: GitHub's "Quote reply" copies the PR author's text verbatim, so a
maintainer clicking it would otherwise dismiss a finding on words the author wrote —
by accident, or because they were led to.
- **`adjudicate`** additionally has a model re-check the reply against the actual
source (distrust-by-default, like the finding verifier) and record a verdict.
Whether that verdict can actually clear a finding is a separate, still-off-by-
default choice — `mode` and `dismiss` are independent axes: `dismiss:
"maintainers"` lets a maintainer's own reply dismiss with no model involved
(it works under plain `annotate` too); `dismiss: "adjudicated"` additionally
accepts an author reply the model confirmed (which does need `mode:
"adjudicate"` for verdicts to exist). Either way the reply has to cite the finding's
`` `id:…` ``. A `critical` finding, or one categorized `secrets`/
`security`, can never be cleared this way, whatever the config — that floor is
enforced in code, not the prompt.
- **`/undismiss <id>` wins over a reply.** Running it on a finding a reply cleared
puts the finding back in the active list and keeps it there: another reply from
the PR author can't clear it again. The restore is recorded against the FINDING
in the comment state, not against the reply, so editing or deleting the reply
doesn't drop it either. Only a maintainer lifts that — either `/dismiss <id>` on
the same finding, or a maintainer's own reply to it.
on the decision — safe even if you never touch this block.
- **Clearing a finding always needs the `` `id:…` `` token** in the replier's own
words (an id inside a `>` quote does not count). Quoting the title *annotates*,
never clears — otherwise GitHub's "Quote reply" could dismiss a finding on words
the PR author wrote.
- **`mode` and `dismiss` are independent axes.** `adjudicate` has a model re-check
the reply against the actual source and record a verdict; `dismiss:
"maintainers"` lets a maintainer's own reply dismiss with no model involved;
`dismiss: "adjudicated"` additionally accepts an author reply the model
confirmed. A `critical`, `secrets`, or `security` finding can never be cleared
this way, whatever the config — that floor is enforced in code, not the prompt.
- **`/undismiss <id>` wins over a reply** — it restores the finding and pins the
restore to the FINDING, so another author reply (or editing/deleting the old
one) can't clear it again; only a maintainer lifts that.
`ecr feedback` mines this substrate retroactively, with no model call and no
re-review: it crawls a repo's PRs, reads each one's existing reviewer comment
(which already embeds its findings), matches non-bot replies against it, and
reports totals, a reply-rate, breakdowns by category/severity/agent, and — the
highest-value part — "repeat offenders": findings whose title recurred across 2+
PRs and drew a reply every single time.
re-review: it crawls a repo's PRs, matches non-bot replies against each existing
reviewer comment, and reports totals, a reply-rate, breakdowns by
category/severity/agent, and "repeat offenders" — findings whose title recurred
across 2+ PRs and drew a reply every time.
```bash
ecr feedback --repo your-org/your-repo --limit 100 --since 2026-06-01
@@ -780,19 +641,13 @@ ecr feedback --as my-review-bot # if CI posts under a PAT/app identity
ecr feedback --json # for scripting
```
The crawl matches the reviewer's comments by author. CI posts them as
`github-actions[bot]` (the default), so a locally-run crawl uses that identity —
pass `--as <login>` when your workflow posts under something else.
The crawl matches the reviewer's comments by author (`github-actions[bot]` by
default — pass `--as <login>` when your workflow posts under something else), and
always reads config from the LOCAL checkout, warning when `commentTag` may not
match a different `--repo`.
`ecr feedback` always reads `.expo-code-review/config.jsonc` from the LOCAL
checkout, even with `--repo`. If `--repo` points at a different repo, it warns
that `commentTag` may not match, so a zero-findings result there is not read as
zero pushback. It also warns when every scanned PR had no bot comment at all,
instead of leaving that as an easy-to-miss "0 with a bot comment" in the totals.
See [LLP 0011](./llp/0011-author-feedback.explainer.md) for why matching is
deterministic, why reply text is never echoed into the comment, and why the
defaults are asymmetric.
Full detail — why matching is deterministic, why reply text is never echoed, why
the defaults are asymmetric: [LLP 0011](./llp/0011-author-feedback.explainer.md).
</details>
@@ -845,39 +700,54 @@ head commit, or local comment-policy fingerprint no longer matches.
<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.
Everything is set in `config.auth`. Engines are inferred **per agent** from that
agent's resolved model: an `anthropic/…` agent runs through the Claude Code CLI,
any other provider runs through OpenCode — in the SAME run. `REVIEWER_MODEL`
overrides every agent's model (and therefore engine) at once. There is no shared
fallback key; `ecr doctor` diagnoses setup.
- **Anthropic / Claude (the scaffolded default).** 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. `claude setup-token`
mints a Max/Team subscription token for CI, or point `tokenEnv` at a Console API
key (`sk-ant-api…`); the CLI reads either. Each pass is trust-isolated and
read-only: `--safe-mode` (no `CLAUDE.md`/hooks/MCP/plugins),
`--strict-mcp-config`, only `Read`/`Grep`/`Glob`, and an allowlisted child env
that omits ambient Anthropic credentials.
- **Anthropic / Claude (the default)** — use `anthropic/...` model ids and every
anthropic pass runs through the **Claude Code CLI** (`claude -p
--output-format stream-json --verbose`), 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.
// The scaffolded default. No anthropic entry at all falls back to `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
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).
Use the per-provider map form:
- **Meta / Muse Spark 1.2 (public Model API).** Set the default model and any
model-pinned agent frontmatter to `meta/muse-spark-1.2`. ECR supplies the fixed
public Responses endpoint and the `@ai-sdk/openai` adapter; the generic
Chat-Completions compatibility adapter is intentionally not used because it
loses Muse's reasoning continuity across tool turns. Standard and Contributor
model ids are supported.
```jsonc
"model": "meta/muse-spark-1.2",
"auth": { "providers": {
"meta": { "mode": "api-key", "tokenEnv": "META_API_KEY" }
} }
```
Create the key in the [Meta AI developer portal](https://developer.meta.com/ai/).
Locally, export it as `META_API_KEY`. In each review-running workflow, replace
the Anthropic credential line with
`META_API_KEY: ${{ secrets.META_API_KEY }}`, set the repo variable
`ECR_EXPECTED_TOKEN_ENV=META_API_KEY`, and remove the Claude CLI install step
if no configured agent uses an `anthropic/…` model. Muse runs with high reasoning.
- **OpenAI: ChatGPT/Codex subscription (OAuth) + usage-based API key** — the
recommended mix if you review with OpenAI. Default models run on the
subscription (zero marginal cost); a metered key covers subscription-excluded
pro models via a synthesized `openai-api` alias (agents reference
`openai-api/gpt-5.5-pro` in frontmatter):
```jsonc
"auth": { "providers": {
@@ -886,63 +756,26 @@ credentials through OpenCode.
} }
```
`openai-api` is an alias the reviewer synthesizes in the OpenCode config
(`upstream` names the SDK it's backed by): agents reference `openai-api/gpt-5.5-pro`
in frontmatter while everything else stays on `openai/gpt-5.5`. Notes:
The oauth `tokenEnv` holds the ACCESS token from an `opencode auth login`
ChatGPT sign-in (`ecr setup-auth` extracts it) — never the single-use refresh
token. Access tokens expire (~10 days observed), so CI secrets need periodic
re-minting; `doctor` and the run preflight warn before expiry. The API key
needs only *Responses → Request* and *Chat completions → Request*, in a
dedicated budget-capped project. In CI, set `ECR_EXPECTED_TOKEN_ENV` to both
env names, comma-separated. Every pass logs which provider/model answered it,
so the subscription/API split is visible per run (alias-model passes report
`$0` cost — OpenCode can't price config-declared aliases; the OpenAI dashboard
is the source of truth for spend).
- **The oauth `tokenEnv` holds the ACCESS token** from an `opencode auth login`
ChatGPT sign-in (`ecr setup-auth` extracts it) — a plain bearer, valid for
days, with no rotation involvement. Do **not** use the refresh token as a
shared secret: refresh tokens are single-use (rotation), so a static copy is
spent by its first use and the sign-in dies with it. Access tokens expire
(~10 days observed), so CI secrets need periodic re-minting — see the
token-rotator item in the [roadmap](./ROADMAP.md); `doctor` and the run
preflight warn before expiry.
- **The API key needs exactly two permissions** — a *Restricted* key with
*Model capabilities*: **Responses → Request** and **Chat completions →
Request**; everything else (including *List models*) stays None. Create it
in a dedicated, budget-capped project. (`ecr setup-auth` prints these
instructions too.)
- **In CI**, set the `ECR_EXPECTED_TOKEN_ENV` repo variable to the
comma-separated set of both env names
(`CODEX_OAUTH_ACCESS_TOKEN,OPENAI_API_KEY`) and pass both secrets in the
workflow.
- **Auditability**: every pass logs which provider/model answered it (job log,
step summary, run log), so the subscription/API split is visible per run.
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.
- **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
and uses your OpenCode login, so no `auth` block is needed.
- **Another provider** — `opencode auth login` once, then run with e.g.
`REVIEWER_MODEL=google/gemini-3-pro`. No `auth` block needed.
Engines are inferred **per agent** from that agent's resolved model alone: an
`anthropic/…` agent runs through the Claude Code CLI while other agents run through
OpenCode — in the SAME run. So an anthropic model may coexist with an `openai` (or
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.
Setup errors fail fast, with the fix in the message, instead of failing every pass
identically: the credential's shape is checked by name before any pass runs,
configured OpenCode model ids are validated against the running server (with close
matches suggested; `anthropic/…` ids are validated per-request by Claude instead),
and `ecr doctor` reports the `opencode` version actually in use.
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.
**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
rediscovering one fixable thing, then reports N coverage gaps. So before any pass runs:
- **The credential's shape is checked.** OpenCode refuses a malformed credential by
dropping the provider entirely, which then surfaces as "model not found" for every
model, with nothing pointing at the credential. A truncated value, surrounding
whitespace, or a token that can't work for the configured `auth.mode` is rejected
by name.
- **Configured model ids for OpenCode-routed providers are checked against the running
server**, so a typo or an id the provider doesn't have is reported once, up front,
with the close matches. `anthropic/…` (Claude Code) model ids aren't checked up
front — Claude validates them per-request, so a typo there surfaces as a per-pass
error instead.
- **`ecr doctor` reports the `opencode` version actually in use** and warns when a
different one is first on your `PATH` — runs use the version this package pins.
Full detail: [LLP 0003](./llp/0003-model-runtimes-and-credentials.explainer.md).
</details>
+2 -2
View File
@@ -222,7 +222,7 @@ PRs** (do these first, in this order):
`gh pr checkout`s the PR head; it builds/runs only the trusted base ref. See
"Reliability & security" below for the residual work.
- **Failure-path hardening (audit 2026-07-22)** — a failed/timed-out run never
renders as a clean "Approve"; the coordinator has its own 5-min cap +
renders as a clean "Ready for human review"; the coordinator has its own 5-min cap +
soft-landing and a deterministic local-merge fallback (a coordinator hiccup no
longer discards all findings); CI always posts a terminal state on failure;
coverage notes now include filtered (binary/generated/ignored) files; the
@@ -342,7 +342,7 @@ Guarantees (priority order; ✅ = shipped):
4. ✅ **Always post a result** — a failed run reports "could not complete"; CI
posts a terminal state on any failure; the coordinator has a deterministic
fallback so its failure can't discard findings; and a failed/timed-out run
never renders as a clean "Approve".
never renders as a clean "Ready for human review".
5. ✅ **Global time budget.** *(Shipped 2026-07-22; retuned 2026-07-26.)*
`PASSES_BUDGET_MS` (55m) is a hard wall-clock ceiling for all passes incl.
subdivision/fallback waves: past it, a timed-out pass is reported as a gap rather
+1 -1
View File
@@ -37,7 +37,7 @@ These hold repo-wide. Each is owned in detail by one doc; break any of them and
1. **Trusted base-commit config.** Review policy and executable reviewer config (`config.jsonc`, `routing.jsonc`, prompts, models, auth/`tokenEnv` mapping) load only from the PR's immutable base commit; the PR head is untrusted data used only for diff/source content and finding verification [observed: commit `601b19a` "Trusted base-commit configuration by default; isolate the runtime from PR-head config"; PLAN-trusted-base-config.md]. Owned by [LLP 0001](0001-trust-model.principles.md) / [LLP 0006](0006-config-schema-loading-routing.explainer.md).
2. **Single-source `tokenEnv`.** `auth.tokenEnv` is honored in exactly one place (root config or `routing.jsonc` `defaults.auth`), enforced twice: schema-level (scope configs reject `auth`/`breakGlass`) and CI-guard level (`verify-config` sweeps every config file) [observed: AGENTS.md:51-53; src/core/auth.ts:45,206 `FORBIDDEN_TOKEN_ENVS`]. Owned by [LLP 0006](0006-config-schema-loading-routing.explainer.md).
3. **A failed run never reads as Approve, and `ecr ci` never fails checks.** A failed, partial, or timed-out run must never render as a clean "Approve"; `ci` is comment-only and non-blocking [observed: src/commands/ci.ts:60 "non-blocking (a reviewer failure never fails the PR's checks)"; AGENTS.md:50]. Owned by [LLP 0007](0007-cli-commands-and-ci.explainer.md).
3. **A failed run never reads as Approve, and `ecr ci` never fails checks.** A failed, partial, or timed-out run must never render as a clean "Ready for human review"; `ci` is comment-only and non-blocking [observed: src/commands/ci.ts:60 "non-blocking (a reviewer failure never fails the PR's checks)"; AGENTS.md:50]. Owned by [LLP 0007](0007-cli-commands-and-ci.explainer.md).
4. **Critical/secrets findings are never silently suppressed.** A critical- or secrets-severity finding can never be silently dropped by any mechanism except the explicit `/skip-review` marker; all other suppression is a display filter, never a review skip [observed: src/core/suppress.ts:21-23,39-43 (critical/secrets findings are never dropped by the inline-ignore backstop)]. Owned by [LLP 0005](0005-verification-fingerprints-rendering.explainer.md).
5. **Read-only agents.** Reviewer agents run with read-only tools only (read/grep/glob/list for OpenCode; Read/Grep/Glob for Claude Code passes) — never Bash/Edit/Write/WebFetch/WebSearch [observed: src/core/claude-code.ts:80-85 `READ_TOOL_MAP`/`ALL_READ_TOOLS`]. Owned by [LLP 0003](0003-model-runtimes-and-credentials.explainer.md).
6. **Stateless runs.** Prior comments, findings, and dismissal state are never fed back into review agents; every run is stateless over (diff + repo + PR title/body) [observed: src/core/prompts.ts:132-174 (the reviewer task is built only from the current diff, not prior findings); src/commands/dismiss.ts:1 (dismissal is a display-only mutation that never re-runs the review); src/core/render.ts:144]. Owned by [LLP 0002](0002-review-engine-pipeline.explainer.md).
+1 -1
View File
@@ -59,7 +59,7 @@ The degraded paths keep a failed run honest:
- **Coordinator failure never discards findings.** If `coordinate()` throws, `fallbackConsolidation()` merges and dedupes the agents' findings by fingerprint and picks a conservative decision — `request_changes` on any critical, else `approve_with_comments`, never a clean approve when findings exist [observed] (`review.ts:800-809`, `978-1009`).
- **`applyReviewPolicy` runs identically on both paths** — the coordinator's normal output and the local fallback both pass through the same policy (drop suggestions unless opted in, sort by severity, cap by `maxFindings`, downgrade `approve_with_comments`→`approve` when nothing remains), so enforcement can't diverge between the happy and degraded paths [observed] (`review.ts:792` and `998-1008`).
- **Any failed/timed-out pass forces the decision off a clean approve** — downgraded to `approve_with_comments` at minimum, independent of what the coordinator decided [observed] (`review.ts:810-815`).
- **An all-failed run sets `couldNotComplete`.** When every pass fails or times out, the run is never rendered as a normal decision string; the flag overrides presentation so the comment header can't read "Approve with comments" over a review that reviewed nothing [observed] (`review.ts:764-777`; commit `188f852`, euxy#8).
- **An all-failed run sets `couldNotComplete`.** When every pass fails or times out, the run is never rendered as a normal decision string; the flag overrides presentation so the comment header can't read "Ready for human review (with comments)" over a review that reviewed nothing [observed] (`review.ts:764-777`; commit `188f852`, euxy#8).
## Post-Coordination Order
@@ -96,6 +96,26 @@ POSIX, where `execvp` never searches cwd. This is a knowingly accepted residual,
oversight, and must not be silently reintroduced as a "fix" ([observed]
`opencode.ts:305-314`).
### Muse Spark via Meta Model API
Muse Spark uses the existing OpenCode engine, but OpenCode `1.18.4` does not ship a
resolvable `meta` provider. `buildOpencodeConfig` therefore synthesizes one only for an
in-use `meta` API-key entry without an `upstream`; an entry with `upstream` remains an
alias and never redirects that upstream's credential to Meta. The native provider's
destination is fixed to `https://api.meta.ai/v1`, its credential is an environment
interpolation (never the key value), and its adapter is `@ai-sdk/openai` so requests use
the Responses API and preserve encrypted reasoning across tool turns. The generic
`@ai-sdk/openai-compatible` adapter is deliberately not used because its Chat
Completions path loses that continuity [observed:
`opencode.ts` `META_MODEL_API_BASE_URL` / `META_MUSE_MODELS` / `buildOpencodeConfig`].
The declared model catalog is an allowlist (`muse-spark-1.2` and
`muse-spark-1.2-contributor`), not a reflection of arbitrary config strings. This keeps
the existing preflight honest: a misspelled or not-yet-supported Meta model remains
absent from the server's provider model list and fails once before any pass. Both models
declare their public context/output limits, multimodal inputs, high reasoning, and
encrypted reasoning inclusion [observed: `opencode.ts` `META_MUSE_MODELS`].
## Claude Code CLI Containment
The Claude Code engine runs each review pass as a single stateless
@@ -137,6 +137,7 @@ doctor resolves auth the way real reviews do: a `routing.jsonc` `defaults.auth`
- It hands out only ACCESS tokens; the refresh token never leaves OpenCode's store, because OpenCode's refresh tokens are SINGLE-USE (rotation) and OpenCode is their sole legitimate consumer — a copy in a shell config or CI secret dies on the next rotation and can take the whole sign-in down with it [observed: `src/commands/setup-auth.ts:87-93`].
- The exported shell line single-quotes the token value so a token containing shell metacharacters (`$HOME`, `!`) is never expanded when sourced [observed: `src/commands/setup-auth.ts:127-131`; test in `src/__tests__/setup-auth.test.ts`].
- `anthropic` auth is always satisfied by the Claude Code CLI subscription-login flow regardless of the declared `mode` (oauth vs api-key) — mode is irrelevant for that provider [observed: `src/commands/setup-auth.ts:50-59`].
- A `meta` API-key entry is a manual-key plan: setup-auth points to the Meta AI developer portal and prints the configured `META_API_KEY` export, while doctor uses the same generic provider readiness check as the real review [observed: `src/commands/setup-auth.ts` `planFromAuth` / manual-key guidance; `src/core/auth.ts` `PROVIDER_KEY_ENV`].
- The two interactive logins run via `spawnSync` with `stdio: "inherit"` and `cwd: os.tmpdir()` — the tmpdir cwd keeps a possibly-untrusted checkout from becoming the login subprocess's working directory, and the `inherit` stdio is why these two calls cannot be converted to `exec.ts`'s capturing `run()` [observed: `src/commands/setup-auth.ts:200-203,256-259`].
## init and dismiss
@@ -55,6 +55,15 @@ The three workflows share a posture built entirely around one fact: a reviewer j
**Concurrency defaults by auth mode.** The commented `chunk.concurrency` default is 6 for an API key and 3 for a subscription (OAuth) credential, "one account handles many parallel streams poorly, and several PRs may review on the same credential at once" [observed: `templates/config.jsonc:28-30`].
**Muse Spark is opt-in without weakening the default secret surface.** The root config
documents `meta/muse-spark-1.2` plus a `META_API_KEY` auth entry, and each
review-running workflow shows the corresponding secret line commented out. It is not
forwarded by the default scaffold: adopters replace the Anthropic line and set
`ECR_EXPECTED_TOKEN_ENV=META_API_KEY`, so the model process receives only the
credential actually selected by trusted config. The default remains Anthropic and
`dismiss.yml` still receives no model credential [observed: `templates/config.jsonc`;
`templates/workflow.yml`, `command.yml`, and `atlantis.yml`].
**Keep the `**/*` catch-all first in `routing.jsonc`.** Scopes match last-match-wins, so a `**/*` catch-all must come first and more specific scopes come after to override it [observed: `templates/routing.jsonc:27-31`; `AGENTS.md:17-18`]. Nothing validates the order — reversing it silently changes which config applies to already-routed files [inferred: no code checks that the `**/*` catch-all comes first, so reversing it silently changes routing].
**`scope-config.jsonc` deliberately omits `auth` and `commentTag`.** It is a *different* template from the root `config.jsonc`, not a copy. A scope config carries no `auth` block — credentials are root-only, and the loader plus the CI guard reject a scope config that declares one [observed: `templates/scope-config.jsonc:1-3`; `AGENTS.md:51`]. It also omits `commentTag`, because a scope's PR-comment marker is always derived as `<rootTag>:<scope-name>` so `ecr ci` and `ecr review --scope --post` target the same comment; declaring one is rejected by the scope schema [observed: `templates/scope-config.jsonc:23-25`]. Reusing the root `config.jsonc` template for a scope — a tempting "simplification" — would break the trust boundary (LLP 0006).
+19
View File
@@ -270,6 +270,25 @@ test("checkProviderAuth: an entry for an IN-USE provider is still enforced", ()
expect(r.detail).toContain("openai");
});
test("checkProviderAuth: Meta uses META_API_KEY and rejects cross-provider reuse", () => {
const meta = cfgWithModels(["meta/muse-spark-1.2"], {
mode: "api-key",
provider: "meta",
tokenEnv: "META_API_KEY",
});
expect(checkProviderAuth(meta, {}).detail).toContain("META_API_KEY");
expect(checkProviderAuth(meta, { META_API_KEY: "meta-review-key" }).ok).toBe(true);
const wrongOwner = cfgWithModels(["openai/gpt-5.5"], {
mode: "api-key",
provider: "openai",
tokenEnv: "META_API_KEY",
});
const result = checkProviderAuth(wrongOwner, { META_API_KEY: "meta-review-key" });
expect(result.ok).toBe(false);
expect(result.detail).toContain("meta's well-known credential env");
});
test("prepareAuth: does not forward a dead unused-provider api-key into its key env", async () => {
// An entry for a provider no model uses must not have its tokenEnv copied into the
// provider key env — checkProviderAuth skips its guard, so forwarding would
+46
View File
@@ -285,6 +285,52 @@ test("parseClaudeResult: success maps text/cost/tokens/model", () => {
expect(r.structuredOutputFailure).toBe(false);
});
test("parseClaudeResult: records thinking tokens as reasoning, without inflating output", () => {
const stdout = JSON.stringify({
type: "result",
subtype: "success",
is_error: false,
result: "the findings",
total_cost_usd: 0.5,
usage: {
input_tokens: 10,
output_tokens: 200,
output_tokens_details: { thinking_tokens: 170 },
cache_creation_input_tokens: 0,
cache_read_input_tokens: 0,
},
});
const r = parseClaudeResult(stdout);
expect(r.tokens.reasoning).toBe(170);
expect(r.tokens.output).toBe(200);
});
test("parseClaudeResult: omits reasoning when the CLI reports no breakdown", () => {
const stdout = JSON.stringify({
type: "result",
subtype: "success",
is_error: false,
result: "the findings",
total_cost_usd: 0.5,
usage: { input_tokens: 10, output_tokens: 200 },
});
expect(parseClaudeResult(stdout).tokens.reasoning).toBeUndefined();
});
test("parseClaudeResult: survives a null/!object output_tokens_details", () => {
for (const details of [null, "nope", 7]) {
const stdout = JSON.stringify({
type: "result",
subtype: "success",
is_error: false,
result: "x",
total_cost_usd: 0,
usage: { input_tokens: 1, output_tokens: 2, output_tokens_details: details },
});
expect(parseClaudeResult(stdout).tokens.reasoning).toBeUndefined();
}
});
test("parseClaudeResult: reads the final result from stream-json JSONL", () => {
const stdout = [
JSON.stringify({ type: "system", subtype: "init", model: "claude-opus-5" }),
+80
View File
@@ -13,6 +13,7 @@ import {
resolveEngineDispatch,
resolveOpencodeCli,
STACK_VERIFIER_AGENT,
META_MODEL_API_BASE_URL,
} from "../core/opencode.js";
import type { OpencodeHandle } from "../core/opencode.js";
import type { ClaudeCodeHandle } from "../core/claude-code.js";
@@ -341,6 +342,85 @@ test("an upstream-alias auth entry synthesizes a provider block with exactly the
expect(opencode.provider?.openai).toBeUndefined();
});
test("Meta auth synthesizes the public Muse Responses provider without embedding its key", () => {
const config = configWith({
agents: [{ id: "correctness", model: "meta/muse-spark-1.2" }],
coordinatorModel: "meta/muse-spark-1.2",
auth: [{ mode: "api-key", provider: "meta", tokenEnv: "META_API_KEY" }],
});
const opencode = buildOpencodeConfig(config) as {
provider?: Record<
string,
{
npm: string;
options: { baseURL: string; apiKey: string };
models: Record<string, { reasoning?: boolean; options?: Record<string, unknown> }>;
}
>;
};
const meta = opencode.provider?.meta;
expect(meta).toBeDefined();
expect(meta!.npm).toBe("@ai-sdk/openai");
expect(meta!.options).toEqual({
baseURL: META_MODEL_API_BASE_URL,
apiKey: "{env:META_API_KEY}",
});
expect(Object.keys(meta!.models)).toEqual(["muse-spark-1.2"]);
expect(meta!.models["muse-spark-1.2"]?.reasoning).toBe(true);
expect(meta!.models["muse-spark-1.2"]?.options).toMatchObject({
reasoningEffort: "high",
include: ["reasoning.encrypted_content"],
});
});
test("a Meta-named upstream alias does not redirect the upstream credential to Meta", () => {
const config = configWith({
agents: [{ id: "correctness", model: "meta/muse-spark-1.2" }],
coordinatorModel: "meta/muse-spark-1.2",
auth: [
{
mode: "api-key",
provider: "meta",
tokenEnv: "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN",
upstream: "anthropic",
},
],
});
const opencode = buildOpencodeConfig(config) as {
provider?: Record<
string,
{ npm: string; options: { apiKey: string; baseURL?: string }; models: object }
>;
};
const alias = opencode.provider?.meta;
expect(alias).toBeDefined();
expect(alias!.npm).toBe("@ai-sdk/anthropic");
expect(alias!.options).toEqual({ apiKey: "{env:CLAUDE_CODE_REVIEW_SHARED_API_TOKEN}" });
expect(alias!.options.baseURL).toBeUndefined();
expect(Object.keys(alias!.models)).toEqual(["muse-spark-1.2"]);
});
test("Meta provider does not register an unknown Muse id, so preflight can reject it", () => {
const config = configWith({
agents: [{ id: "correctness", model: "meta/muse-spark-typo" }],
coordinatorModel: "meta/muse-spark-typo",
auth: [{ mode: "api-key", provider: "meta", tokenEnv: "META_API_KEY" }],
});
const opencode = buildOpencodeConfig(config) as {
provider?: Record<string, { models: Record<string, unknown> }>;
};
expect(opencode.provider?.meta?.models).toEqual({});
expect(
findUnknownModels(["meta/muse-spark-typo"], { meta: ["muse-spark-1.2"] }, ["meta"]),
).toEqual([
{
model: "meta/muse-spark-typo",
reason: "model",
suggestions: ["muse-spark-1.2"],
},
]);
});
test("buildOpencodeConfig registers the no-tools stack verifier (empty tool list)", () => {
const opencode = buildOpencodeConfig(configWith({})) as {
agent: Record<string, { tools: Record<string, boolean> }>;
+4 -4
View File
@@ -61,7 +61,7 @@ test("aggregate: root marker first line, a table row + <details> per scope, wors
expect(body.split("\n")[0]).toBe(commentMarker("tag"));
expect(body).toContain("**Decision:** Request changes"); // worst across scopes
expect(body).toContain("| Scope | Decision | Findings |");
expect(body).toContain("| default | Approve | 1 |");
expect(body).toContain("| default | Ready for human review | 1 |");
expect(body).toContain("| api | Request changes | 1 |");
// one details block per scope, in manifest order
expect(body.indexOf("<summary>default")).toBeLessThan(body.indexOf("<summary>api"));
@@ -141,7 +141,7 @@ test("aggregate: a default-scope finding dismissed pre-routing (plain fp) lands
]);
expect(body).toContain("Dismissed on this PR (1)");
expect(body).toContain(`id:${plainFp}`);
expect(body).toContain("| default | Approve | 0 |"); // moved out of the active count
expect(body).toContain("| default | Ready for human review | 0 |"); // moved out of the active count
});
test("aggregate: dismissed findings survive in embedded state so /undismiss can restore them", () => {
@@ -162,7 +162,7 @@ test("aggregate: dismissed findings survive in embedded state so /undismiss can
// (what /undismiss does) restores the finding to the active list.
const restored = renderAggregateMarkdown(state.scopes!, "tag", []);
expect(restored).toContain(`id:${fp}`);
expect(restored).toContain("| api | Approve | 2 |");
expect(restored).toContain("| api | Ready for human review | 2 |");
expect(restored).not.toContain("Dismissed on this PR");
});
@@ -220,7 +220,7 @@ test("aggregate: a scope whose reply cleared every finding opens its fold (visib
],
);
// The finding is suppressed (moved to Dismissed); the scope shows no active finding.
expect(body).toContain("<summary>api — Approve (0)</summary>");
expect(body).toContain("<summary>api — Ready for human review (0)</summary>");
// ...but the fold OPENS so the audit note is above-the-fold, not collapsed away.
expect(body).toMatch(/<details open>\n<summary>api —/);
expect(body).toContain("have an author response");
+1
View File
@@ -39,6 +39,7 @@ test("stdio MCP advertises the read-only documentation tools and their enforced
);
expect(searchTool?.description ?? "").toContain("Native source retains platform context");
expect(searchTool?.description ?? "").toContain("react-native-worklets=Worklets");
expect(searchTool?.description ?? "").toContain("metro=Metro bundler");
expect(fetchTool?.annotations?.readOnlyHint).toBe(true);
expect(fetchTool?.description ?? "").toMatch(/every redirect/);
const inputSchema = searchTool?.inputSchema as {
@@ -7,6 +7,7 @@ import {
expoProvider,
glideProvider,
jetbrainsIssuesProvider,
metroProvider,
okHttpProvider,
reactNativeGestureHandlerProvider,
reactNativeProvider,
@@ -180,6 +181,7 @@ test("React Native ecosystem providers stay on their exact documentation hosts a
reactNativeWorkletsProvider,
"https://docs.swmansion.com/react-native-worklets/docs/fundamentals/getting-started/",
],
[metroProvider, "https://metrobundler.dev/docs/configuration"],
] as const) {
expect(resolveAllowedUrl(provider, url).hostname).toBe(new URL(url).hostname);
}
@@ -193,4 +195,7 @@ test("React Native ecosystem providers stay on their exact documentation hosts a
expect(() => resolveAllowedUrl(expoProvider, "https://expo.dev/accounts")).toThrow(
/outside the expo documentation allowlist/,
);
expect(() =>
resolveAllowedUrl(metroProvider, "https://metrobundler.dev.evil.example/docs/configuration"),
).toThrow(/outside the metro documentation allowlist/);
});
+3
View File
@@ -52,6 +52,7 @@ test("the wrapper hands the MCP server a constructed environment, not an inherit
PATH: process.env.PATH ?? "",
HOME: "/root",
ANTHROPIC_API_KEY: "must-not-reach-the-server",
META_API_KEY: "must-not-reach-the-server",
CLAUDE_CODE_OAUTH_TOKEN: "must-not-reach-the-server",
AWS_SECRET_ACCESS_KEY: "must-not-reach-the-server",
GITHUB_TOKEN: "must-not-reach-the-server",
@@ -93,6 +94,7 @@ test("research child environment forwards only locale, proxy variables, and the
no_proxy: "localhost",
BRAVE_SEARCH_API_KEY: "search-only",
SECRET_TOKEN: "must-not-be-forwarded",
META_API_KEY: "must-not-be-forwarded",
PATH: "/private/bin",
}),
).toMatchObject({
@@ -103,6 +105,7 @@ test("research child environment forwards only locale, proxy variables, and the
BRAVE_SEARCH_API_KEY: "search-only",
});
expect(researchChildEnvironment({ SECRET_TOKEN: "nope" })).not.toHaveProperty("SECRET_TOKEN");
expect(researchChildEnvironment({ META_API_KEY: "nope" })).not.toHaveProperty("META_API_KEY");
expect(researchChildEnvironment({ PATH: "/private/bin" })).not.toHaveProperty("PATH");
});
+28 -1
View File
@@ -4,7 +4,12 @@ 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 {
reviewCacheAllowed,
reviewCanBeReused,
reviewInputHash,
reviewMatchesInput,
} from "../core/review-cache.js";
import type { CoordinatorOutput, DiffEntry } from "../core/schema.js";
const config = (over: Partial<LoadedConfig> = {}): LoadedConfig => ({
@@ -171,3 +176,25 @@ test("file hashing refuses traversal and never follows a PR-controlled symlink",
await rm(outside, { force: true });
}
});
test("reviewCacheAllowed is the one policy both CI paths share", () => {
const base = {
bypassTriggerGate: false,
stack: false,
feedback: false,
hasMetadata: true,
};
expect(reviewCacheAllowed(base)).toBe(true);
// Each flag independently means "this run has an input the key can't represent".
expect(reviewCacheAllowed({ ...base, bypassTriggerGate: true })).toBe(false);
expect(reviewCacheAllowed({ ...base, stack: true })).toBe(false);
expect(reviewCacheAllowed({ ...base, feedback: true })).toBe(false);
expect(reviewCacheAllowed({ ...base, hasMetadata: false })).toBe(false);
// Research is deliberately NOT a gate. It was one only because the offline index
// was an artifact the key could not represent; with the index gone, a researched
// review fetches everything live during the run. The routed path kept this gate
// after the legacy path dropped it, which is the drift this function prevents.
expect(reviewCacheAllowed(base)).toBe(true);
});
+8
View File
@@ -14,6 +14,14 @@ test("plan: the mixed setup needs one ChatGPT login and one manual key", () => {
expect(plan.unsupported).toEqual([]);
});
test("plan: Meta Model API is a manual API key setup", () => {
const plan = planFromAuth([{ provider: "meta", mode: "api-key", tokenEnv: "META_API_KEY" }]);
expect(plan.manualKeys).toEqual([
{ provider: "meta", tokenEnv: "META_API_KEY", upstream: undefined },
]);
expect(plan.unsupported).toEqual([]);
});
test("plan: an anthropic entry (any mode) → claudeLogin; api-key without tokenEnv needs nothing", () => {
const plan = planFromAuth([
// Mode is irrelevant for anthropic — it is always served by the Claude Code CLI.
+2 -2
View File
@@ -82,9 +82,9 @@ test("renderAggregateMarkdown: per-scope addressed bucket, count excludes requal
const out = renderAggregateMarkdown(results, "tag", []);
// Worst decision across scopes is approve_with_comments (the requalified-only scope
// contributes approve, not request_changes).
expect(out).toContain("**Decision:** Approve with comments");
expect(out).toContain("**Decision:** Ready for human review (with comments)");
// The api scope's blocking count is 0 (the requalified finding is not counted).
expect(out).toMatch(/\| api \| Approve \| 0 \|/);
expect(out).toMatch(/\| api \| Ready for human review \| 0 \|/);
expect(out).toContain("Addressed in stacked PRs (1)");
expect(out).toContain("marked addressed by stacked PR(s) (#42)");
});
+18 -14
View File
@@ -30,7 +30,12 @@ import { summarizePriorReview } from "../core/prior-review.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 {
reviewCacheAllowed,
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";
@@ -576,13 +581,12 @@ 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.
// Research no longer forces a fresh run: with the local index gone, every result
// is fetched live from an allowlisted host during the run, so there is no mounted
// artifact whose contents could drift out from under the cache key.
const cacheAllowed = !bypassTriggerGate && !stack && !feedback && metadata !== undefined;
const cacheAllowed = reviewCacheAllowed({
bypassTriggerGate,
stack: Boolean(stack),
feedback: Boolean(feedback),
hasMetadata: metadata !== undefined,
});
let inputHash: string | undefined;
// The previous review's embedded comment state, read ONCE: the cache check below
@@ -905,12 +909,12 @@ async function runRoutedCi(
reporterFor(scopedCommentTag(rootTag, name), true),
);
const cacheAllowed =
!bypassTriggerGate &&
!stackWalk &&
!feedbackNeedsRunSeam(rootConfig.feedback) &&
!rootConfig.research.enabled &&
metadata !== undefined;
const cacheAllowed = reviewCacheAllowed({
bypassTriggerGate,
stack: Boolean(stackWalk),
feedback: feedbackNeedsRunSeam(rootConfig.feedback),
hasMetadata: metadata !== undefined,
});
let cacheReadRoot: string | undefined;
if (cacheAllowed) {
try {
+6 -1
View File
@@ -25,7 +25,8 @@ getting each credential:
opens your browser) and prints the \`export <tokenEnv>=…\` line for
CI/headless runs.
• an API key (api-key entries): prints where to create the key, the exact
permissions it needs, and the export line to fill in.
permissions it needs, and the export line to fill in. Meta Model API
(\`meta/muse-spark-…\`) uses META_API_KEY from the Meta AI developer portal.
Without a repo config, it offers the recommended ChatGPT/Codex subscription flow
with the default env name.
@@ -300,6 +301,10 @@ export async function setupAuthCommand(argv: string[] = []): Promise<void> {
} else if (upstream === "anthropic") {
err(" https://console.anthropic.com/settings/keys — a workspace-scoped key");
err(" with a spend limit is all the reviewer needs.");
} else if (upstream === "meta") {
// @ref LLP 0007#doctor-and-setup-auth [implements] — Meta uses a manual API key
err(" https://developer.meta.com/ai/ — create a Model API key with access to");
err(" Muse Spark, then store it as a dedicated review secret.");
} else {
err(` mint a key for the "${upstream}" provider.`);
}
+1
View File
@@ -15,6 +15,7 @@ const PROVIDER_KEY_ENV: Record<string, string> = {
openai: "OPENAI_API_KEY",
google: "GOOGLE_GENERATIVE_AI_API_KEY",
openrouter: "OPENROUTER_API_KEY",
meta: "META_API_KEY",
};
/**
+3
View File
@@ -582,6 +582,9 @@ export function parseClaudeResult(stdout: string): ClaudeResult {
const tokens: TokenUsage = {
input: num(usage.input_tokens),
output: num(usage.output_tokens),
reasoning: num(
(usage.output_tokens_details as Record<string, unknown> | null | undefined)?.thinking_tokens,
),
cache: {
write: num(usage.cache_creation_input_tokens),
read: num(usage.cache_read_input_tokens),
+60
View File
@@ -19,6 +19,44 @@ import type { ResearchMcpRuntime } from "./research.js";
/** Discriminant for the Claude Code CLI engine (see core/claude-code.ts). */
export const CLAUDE_CODE_ENGINE = "claude-code" as const;
/** Fixed so repo config cannot redirect the Meta credential. */
// @ref LLP 0003#muse-spark-via-meta-model-api [implements] — fixed endpoint and model allowlist
export const META_MODEL_API_BASE_URL = "https://api.meta.ai/v1";
/** Supported public Muse models. Unknown ids remain visible to preflight. */
export const META_MUSE_MODELS: Record<string, Record<string, unknown>> = {
"muse-spark-1.2": {
name: "Muse Spark 1.2",
reasoning: true,
limit: { context: 1_048_576, output: 131_072 },
modalities: {
input: ["text", "image", "pdf", "video"],
output: ["text"],
},
options: {
store: false,
reasoningEffort: "high",
reasoningSummary: "auto",
include: ["reasoning.encrypted_content"],
},
},
"muse-spark-1.2-contributor": {
name: "Muse Spark 1.2 Contributor",
reasoning: true,
limit: { context: 1_048_576, output: 131_072 },
modalities: {
input: ["text", "image", "pdf", "video"],
output: ["text"],
},
options: {
store: false,
reasoningEffort: "high",
reasoningSummary: "auto",
include: ["reasoning.encrypted_content"],
},
},
};
/**
* Resolve which engine an agent's pass dispatches to, and (when claude) which
* claude handle to run it against. The per-agent router (engineOf) wins; absent it
@@ -221,6 +259,28 @@ export function buildOpencodeConfig(
config.coordinator.model,
];
for (const entry of config.auth) {
if (
entry.provider === "meta" &&
entry.mode === "api-key" &&
entry.tokenEnv &&
!entry.upstream
) {
// Muse needs the Responses adapter; openai-compatible uses Chat Completions.
provider.meta = {
npm: "@ai-sdk/openai",
name: "Meta Model API",
options: {
baseURL: META_MODEL_API_BASE_URL,
apiKey: `{env:${entry.tokenEnv}}`,
},
models: Object.fromEntries(
Object.entries(META_MUSE_MODELS).filter(([model]) =>
referencedModels.includes(`meta/${model}`),
),
),
};
continue;
}
if (!entry.upstream || !entry.tokenEnv) {
continue;
}
+5 -2
View File
@@ -86,9 +86,12 @@ export function buildDiffLineIndex(
return index;
}
// User-facing labels only — the wire/schema enum stays `approve` etc. The review
// is advisory and never approves anything, so the labels must not read as an
// approval either: a clean result is "ready for a human", not "approved".
const DECISION_LABEL: Record<Decision, string> = {
approve: "Approve",
approve_with_comments: "Approve with comments",
approve: "Ready for human review",
approve_with_comments: "Ready for human review (with comments)",
request_changes: "Request changes",
};
+26
View File
@@ -137,6 +137,32 @@ export async function reviewInputHash(options: ReviewInputHashOptions): Promise<
return createHash("sha256").update(canonicalJson(input)).digest("hex");
}
/**
* Whether a run may reuse a cached result at all — the single definition of that
* policy, shared by the legacy and routed CI paths.
*
* It lives here because it was previously written out twice, once per path, and
* the copies drifted: when the offline index was removed, only the legacy copy
* dropped its research gate, so every routed repo silently kept running fresh
* reviews for a reason that no longer existed. Two expressions of one policy is
* the bug; one function that both paths call is the fix.
*
* Each flag means "this run has an input the cache key does not represent":
* dynamic stack context and model-backed reply adjudication both reach outside
* the scoped diff, and a maintainer's explicit /review is always a real rerun.
*/
export function reviewCacheAllowed(run: {
bypassTriggerGate: boolean;
/** Stack walking is on for this run. */
stack: boolean;
/** A feedback seam runs this pass (adjudication), not merely annotation. */
feedback: boolean;
/** PR metadata resolved; without it there is nothing stable to key on. */
hasMetadata: boolean;
}): boolean {
return !run.bypassTriggerGate && !run.stack && !run.feedback && run.hasMetadata;
}
/** 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;
+2
View File
@@ -33,6 +33,7 @@ const DIRECT_PROVIDER_ORDER: readonly ProviderId[] = [
"react-native-screens",
"react-native-worklets",
"react-native",
"metro",
"expo",
];
@@ -56,6 +57,7 @@ const DIRECT_SOURCE_KIND: Record<ProviderId, SourceKind> = {
"react-native-gesture-handler": "official-guide",
"react-native-screens": "official-guide",
"react-native-worklets": "official-guide",
metro: "official-guide",
};
export interface DirectDocumentationTarget {
+5
View File
@@ -431,6 +431,10 @@ export const reactNativeWorkletsProvider = htmlProvider(
true,
);
export const metroProvider = htmlProvider("metro", "react-native", "Metro bundler documentation", [
{ hostname: "metrobundler.dev", prefixes: [""] },
]);
const providers: Record<ProviderId, DocumentationProvider> = {
apple: appleProvider,
"apple-releases": appleReleasesProvider,
@@ -451,6 +455,7 @@ const providers: Record<ProviderId, DocumentationProvider> = {
"react-native-gesture-handler": reactNativeGestureHandlerProvider,
"react-native-screens": reactNativeScreensProvider,
"react-native-worklets": reactNativeWorkletsProvider,
metro: metroProvider,
};
export function getProvider(provider: ProviderId): DocumentationProvider {
+4
View File
@@ -91,6 +91,10 @@ const providerSearchDefinitions: Record<Exclude<ProviderId, "expo">, ProviderSea
scopes: ["docs.swmansion.com/react-native-worklets/docs"],
sourceKind: "official-api",
},
metro: {
scopes: ["metrobundler.dev/docs"],
sourceKind: "official-guide",
},
};
export interface RemoteSearchOptions {
+1 -1
View File
@@ -25,7 +25,7 @@ const queryGuidance =
"Formulate short documentation queries from exact API symbols plus one behavior or constraint term. Good: `CameraView barcodeScannerSettings`, `NWPathMonitor pathUpdateHandler`, `GestureDetector simultaneous gestures`. Avoid questions, prose, package/import names, code snippets, literals, paths, credentials, and other sensitive context. If the first result is broad, retry with a narrower symbol or member name.";
const providerGuidance =
"Provider map: apple=Apple SDK APIs and Human Interface Guidelines; apple-releases=Xcode and Apple platform release notes; swift-evolution=Swift Evolution proposals; sdwebimage=SDWebImage APIs and caching/loading behavior; android=Android, Jetpack, Compose, and Google Play services APIs; android-releases=Android platform releases and behavior changes; media3=Jetpack Media3; glide=Glide; okhttp=OkHttp; kotlin-coroutines=Kotlin coroutines; gradle=Gradle; agp=Android Gradle Plugin; jetbrains-issues=JetBrains YouTrack context; expo=Expo documentation; react-native=React Native core; react-native-reanimated=Reanimated; react-native-gesture-handler=Gesture Handler; react-native-screens=Screens; react-native-worklets=Worklets. Native source retains platform context: use apple/android for OS contracts and add the dependency provider for dependency-owned behavior; an Expo package path does not make a native API an Expo-docs query. Issue-tracker results are context, not API contracts.";
"Provider map: apple=Apple SDK APIs and Human Interface Guidelines; apple-releases=Xcode and Apple platform release notes; swift-evolution=Swift Evolution proposals; sdwebimage=SDWebImage APIs and caching/loading behavior; android=Android, Jetpack, Compose, and Google Play services APIs; android-releases=Android platform releases and behavior changes; media3=Jetpack Media3; glide=Glide; okhttp=OkHttp; kotlin-coroutines=Kotlin coroutines; gradle=Gradle; agp=Android Gradle Plugin; jetbrains-issues=JetBrains YouTrack context; expo=Expo documentation; react-native=React Native core; react-native-reanimated=Reanimated; react-native-gesture-handler=Gesture Handler; react-native-screens=Screens; react-native-worklets=Worklets; metro=Metro bundler. Native source retains platform context: use apple/android for OS contracts and add the dependency provider for dependency-owned behavior; an Expo package path does not make a native API an Expo-docs query. Issue-tracker results are context, not API contracts.";
export interface DocumentationServerOptions {
braveApiKey?: string;
+1
View File
@@ -23,6 +23,7 @@ export const PROVIDERS = [
"react-native-gesture-handler",
"react-native-screens",
"react-native-worklets",
"metro",
] as const;
export type ProviderId = (typeof PROVIDERS)[number];
+2
View File
@@ -110,6 +110,8 @@ jobs:
# 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 }}
# Muse alternative (also set ECR_EXPECTED_TOKEN_ENV=META_API_KEY):
# META_API_KEY: ${{ secrets.META_API_KEY }}
# Optional search-only credential for trusted platform documentation research.
BRAVE_SEARCH_API_KEY: ${{ secrets.BRAVE_SEARCH_API_KEY }}
# Optional: override the model for every agent.
+2
View File
@@ -148,6 +148,8 @@ jobs:
# 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 }}
# Muse alternative (also set ECR_EXPECTED_TOKEN_ENV=META_API_KEY):
# META_API_KEY: ${{ secrets.META_API_KEY }}
# Optional search-only credential for trusted platform documentation research.
BRAVE_SEARCH_API_KEY: ${{ secrets.BRAVE_SEARCH_API_KEY }}
# Optional: override the model for every agent.
+7
View File
@@ -86,6 +86,13 @@
// store the key as a repo secret and pass it under that env var.
// "auth": { "mode": "api-key", "provider": "openai", "tokenEnv": "OPENAI_API_KEY" }
//
// Muse Spark via the public Meta Model API Responses endpoint:
// set models to `meta/muse-spark-1.2` and configure:
// "auth": { "providers": {
// "meta": { "mode": "api-key", "tokenEnv": "META_API_KEY" }
// } }
// Map META_API_KEY in the workflows and set ECR_EXPECTED_TOKEN_ENV to match.
//
// 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.
+2
View File
@@ -114,6 +114,8 @@ jobs:
# 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 }}
# Muse alternative (also set ECR_EXPECTED_TOKEN_ENV=META_API_KEY):
# META_API_KEY: ${{ secrets.META_API_KEY }}
# Optional search-only credential for trusted platform documentation research.
BRAVE_SEARCH_API_KEY: ${{ secrets.BRAVE_SEARCH_API_KEY }}
# Optional: override the model for every agent.