mirror of
https://github.com/expo/code-review-cli.git
synced 2026-11-14 17:49:25 -10:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Philippe Loulidi
|
a283580869 | ||
|
Philippe Loulidi
|
c341f47418 |
No files matched your search
@@ -35,22 +35,10 @@ the same kind of thing, so the codebase stays uniform and predictable.
|
||||
|
||||
## This repo's conventions
|
||||
|
||||
<!-- @ref glob:src/commands/*.ts — every command follows the shape described here -->
|
||||
<!-- @ref src/core/util.ts#errorMessage — the required error path -->
|
||||
<!-- @ref src/cli.ts — where a new command must be registered -->
|
||||
<!-- @ref src/config/schema.ts — one of the three places a config option lands -->
|
||||
<!-- @ref src/config/load.ts — the second -->
|
||||
<!-- @ref templates/config.jsonc — the third; a missing option here is a finding -->
|
||||
<!-- @ref templates/ — the scaffolding source that must stay in sync with src/ -->
|
||||
<!-- @ref src/ — the engine side of that sync -->
|
||||
<!-- @ref src/reporters/reporter.ts — the reporter interface siblings implement -->
|
||||
<!-- @ref src/sources/source.ts — the source interface siblings implement -->
|
||||
<!-- @ref src/__tests__/ — where a new module's bun test file belongs -->
|
||||
|
||||
- **New CLI commands** (`src/commands/*.ts`): a `USAGE` string, `-h`/`--help`
|
||||
handled first, errors written to stderr via `errorMessage(error)` from
|
||||
`src/core/util.ts`, and `process.exitCode = 2` on failure — never
|
||||
`process.exit()`. Registered in `src/cli.ts`.
|
||||
`core/util.ts`, and `process.exitCode = 2` on failure — never `process.exit()`.
|
||||
Registered in `src/cli.ts`.
|
||||
- **New config options** land in three places together: the zod schema
|
||||
(`src/config/schema.ts`), loading/defaults (`src/config/load.ts`), and the
|
||||
commented example in `templates/config.jsonc`. An option missing from the
|
||||
|
||||
@@ -19,10 +19,6 @@ issues in the changed code.
|
||||
|
||||
## This repo's footguns
|
||||
|
||||
<!-- @ref src/core/verify.ts#matchEvidence — where model claims become trusted -->
|
||||
<!-- @ref-ignore .js -->
|
||||
|
||||
|
||||
- **ESM specifiers**: TypeScript with NodeNext resolution — relative imports need
|
||||
the `.js` suffix. A missing suffix can type-check yet break the built CLI at
|
||||
runtime.
|
||||
|
||||
@@ -23,14 +23,6 @@ average severity.
|
||||
|
||||
## This repo's sensitive surfaces
|
||||
|
||||
<!-- @ref src/core/auth.ts#FORBIDDEN_TOKEN_ENVS — the deny-list this section is about -->
|
||||
<!-- @ref src/core/exec.ts — the single child-process seam (execFile, no shell) -->
|
||||
<!-- @ref src/core/prompts.ts#sanitizeUntrusted — the injection boundary named below -->
|
||||
<!-- @ref src/core/render.ts — writes the state markers a PR must not forge -->
|
||||
<!-- @ref templates/workflow.yml — scaffolded into every adopting repo -->
|
||||
<!-- @ref templates/config.jsonc — the tokenEnv guard must stay in sync with it -->
|
||||
<!-- @ref-ignore .runs/ -->
|
||||
|
||||
This CLI's core job is forwarding a model-provider credential and executing
|
||||
PR-controlled config, so these areas are critical surface:
|
||||
|
||||
@@ -58,8 +50,6 @@ PR-controlled config, so these areas are critical surface:
|
||||
guard step affect all downstream users, and the guard must stay in sync with
|
||||
`templates/config.jsonc`.
|
||||
|
||||
<!-- @ref glob:.github/workflows/** — the workflows this section judges -->
|
||||
|
||||
## CI / workflow supply-chain (changes under `.github/workflows/**`)
|
||||
|
||||
Treat any changed workflow as high-risk and reason about the *trigger*, not just
|
||||
|
||||
@@ -37,14 +37,13 @@
|
||||
|
||||
// Anthropic models always run through the Claude Code CLI (`claude -p`) — the
|
||||
// engine is inferred per agent from its model, so this entry only supplies the
|
||||
// credential. In CI the tokenEnv holds the shared review credential (an
|
||||
// `sk-ant-oat…` token minted by `claude setup-token`, or an `sk-ant-api…`
|
||||
// Console key); locally an active `claude` login is enough — the entry could
|
||||
// even be omitted. ECR_EXPECTED_TOKEN_ENV (repo variable / workflow fallback)
|
||||
// must equal this tokenEnv.
|
||||
// credential. In CI the tokenEnv holds a token minted by `claude setup-token`
|
||||
// (an `sk-ant-oat…` subscription token; an `sk-ant-api…` Console key works too);
|
||||
// locally an active `claude` login is enough — the entry could even be omitted.
|
||||
// ECR_EXPECTED_TOKEN_ENV (repo variable) must equal this tokenEnv.
|
||||
"auth": {
|
||||
"providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" }
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -11,8 +11,6 @@ These rules apply to every reviewer and are concatenated onto your role prompt.
|
||||
- **Do not judge the diff in isolation.** Before reporting, read the surrounding
|
||||
source with your file/read/grep tools and trace the relevant execution path.
|
||||
If you cannot substantiate a concrete failure or exploit path, do not report it.
|
||||
<!-- @ref AGENTS.md — the conventions reviewers must judge against -->
|
||||
<!-- @ref-ignore CLAUDE.md -->
|
||||
- Ground your judgment in the repo's own conventions (`AGENTS.md` / `CLAUDE.md`
|
||||
at the repo root, and any per-directory guidance) rather than generic
|
||||
best-practices.
|
||||
|
||||
@@ -16,7 +16,7 @@ concurrency:
|
||||
|
||||
jobs:
|
||||
review:
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: expo-large
|
||||
# Trigger policy lives in .expo-code-review/config.jsonc (review.trigger); `ecr ci`
|
||||
# self-gates on it (and honors the ai-review:skip label). This coarse gate just
|
||||
# avoids spinning up a runner for a PR that explicitly opted out. Uses the array
|
||||
@@ -49,6 +49,29 @@ jobs:
|
||||
|
||||
- uses: oven-sh/setup-bun@v2
|
||||
|
||||
# `ecr ci` reads the whole PR through `gh`, which GitHub-hosted images ship and
|
||||
# ours do not. Pinned by version AND sha256: this binary runs with GH_TOKEN and
|
||||
# the model credential in its environment, so "whatever the image happens to
|
||||
# ship" is a worse answer than one we can name. Installs into $HOME, never the
|
||||
# workspace — resolveTrustedTool refuses a `gh` found inside the reviewed tree.
|
||||
- name: Install gh
|
||||
env:
|
||||
GH_VERSION: "2.97.0"
|
||||
GH_SHA256: "a2c9b8497e1f85b1ad0dfcb78b5a622e098801b8e461e459e88e1ee12f018112"
|
||||
run: |
|
||||
set -euo pipefail
|
||||
tarball="gh_${GH_VERSION}_linux_amd64.tar.gz"
|
||||
curl -fsSL --retry 3 -o "$RUNNER_TEMP/$tarball" \
|
||||
"https://github.com/cli/cli/releases/download/v${GH_VERSION}/${tarball}"
|
||||
echo "${GH_SHA256} $RUNNER_TEMP/$tarball" | sha256sum -c -
|
||||
mkdir -p "$HOME/.local/bin"
|
||||
tar -xzf "$RUNNER_TEMP/$tarball" -C "$RUNNER_TEMP"
|
||||
install -m 0755 "$RUNNER_TEMP/gh_${GH_VERSION}_linux_amd64/bin/gh" "$HOME/.local/bin/gh"
|
||||
echo "$HOME/.local/bin" >> "$GITHUB_PATH"
|
||||
|
||||
- name: Check gh is available
|
||||
run: gh --version
|
||||
|
||||
- name: Install dependencies
|
||||
run: bun install --frozen-lockfile
|
||||
|
||||
@@ -68,7 +91,7 @@ jobs:
|
||||
# source, so the guard runs from that same base source — same engine as `ci`.
|
||||
- name: Guard config tokenEnv
|
||||
env:
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_OAUTH_TOKEN' }}
|
||||
run: bun run src/cli.ts verify-config
|
||||
|
||||
- name: Run AI review
|
||||
@@ -83,10 +106,10 @@ jobs:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
# Layer-1 auth lock: `ecr ci` re-checks the honored tokenEnv at runtime,
|
||||
# catching what the guard step above can't. Keep in sync with the guard.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Shared Anthropic review credential. The claude-code engine forwards
|
||||
# it to the Claude Code CLI as the only credential (any ambient
|
||||
# ANTHROPIC_* key is stripped). A setup-token or Console key both work.
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_OAUTH_TOKEN' }}
|
||||
# Claude Max/Team subscription token minted by `claude setup-token`
|
||||
# (~1 year lifetime). The claude-code engine forwards it to the Claude
|
||||
# Code CLI as the only credential (any ANTHROPIC_* key is stripped).
|
||||
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
@@ -10,20 +10,16 @@ permissions:
|
||||
|
||||
jobs:
|
||||
ref-check:
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: expo-small
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5
|
||||
|
||||
# ref-check is a python3 script. GitHub-hosted images ship python3; a
|
||||
# self-hosted one is not guaranteed to, so ask for it explicitly.
|
||||
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Validate LLP references and metadata
|
||||
run: ./ref-check --verbose
|
||||
|
||||
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
|
||||
with:
|
||||
bun-version: latest
|
||||
|
||||
- run: bun install --frozen-lockfile
|
||||
|
||||
# The same grammar, applied to this repo's own review setup (see LLP 0012).
|
||||
- name: Validate review-setup refs
|
||||
run: bun run src/cli.ts ref-check
|
||||
@@ -12,7 +12,7 @@ permissions:
|
||||
|
||||
jobs:
|
||||
publish:
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: expo-small
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5
|
||||
|
||||
@@ -10,7 +10,7 @@ permissions:
|
||||
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: expo-small
|
||||
timeout-minutes: 10
|
||||
steps:
|
||||
- uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5
|
||||
|
||||
@@ -33,10 +33,6 @@ verifies their findings, and posts one updating PR comment.
|
||||
don't silently "fix" either side.
|
||||
- `./ref-check` (`bun run llp:check`) validates refs and metadata; CI runs it
|
||||
on every push and PR.
|
||||
- `ecr ref-check` validates the SAME `@ref` grammar inside a repo's
|
||||
`.expo-code-review/` setup (see LLP 0012). The two checkers share the grammar:
|
||||
a change to target kinds, the `<placeholder>` rule, or the glob dialect must
|
||||
land in `./ref-check` and `src/core/config-refs.ts` together.
|
||||
|
||||
## Conventions
|
||||
|
||||
|
||||
@@ -48,19 +48,21 @@ npx @expo/code-review-cli setup-auth
|
||||
npx @expo/code-review-cli doctor
|
||||
```
|
||||
|
||||
`setup-auth` reads the repo's config and walks through each credential it needs.
|
||||
The scaffolded default is **Anthropic via the Claude Code CLI**: locally your
|
||||
`claude` login is enough, and for CI it helps you mint a token with
|
||||
`claude setup-token`. It also handles the alternatives (an OpenAI **API key**,
|
||||
a **ChatGPT/Codex subscription** sign-in). `doctor` offers to run it whenever a
|
||||
credential is missing.
|
||||
`setup-auth` reads the repo's config and walks through each credential it needs:
|
||||
an OpenAI **API key** (the scaffolded default — it prints where to create the key
|
||||
and the exact restricted permissions to grant), and/or a **ChatGPT/Codex
|
||||
subscription** sign-in (it runs OpenCode's browser login and extracts the token
|
||||
for you). `doctor` offers to run it whenever a credential is missing.
|
||||
|
||||
In CI, store the credential as the repo secret the scaffolded workflow forwards
|
||||
(`CLAUDE_CODE_REVIEW_SHARED_API_TOKEN` by default — an `sk-ant-oat…` token from
|
||||
`claude setup-token`, or an `sk-ant-api…` Console key; the CLI reads either).
|
||||
In CI, store the same values as repo secrets (`OPENAI_API_KEY`; plus
|
||||
`CODEX_OAUTH_ACCESS_TOKEN` for the mixed setup) — the scaffolded workflow
|
||||
forwards them.
|
||||
|
||||
Prefer **OpenAI** (API key, or a ChatGPT/Codex subscription, or both mixed) or
|
||||
another provider? See [Other providers & auth modes](#other-providers) below.
|
||||
**Have a ChatGPT Plus/Pro (Codex) subscription? Use both.** The recommended
|
||||
production setup pairs the subscription (runs the default models at no marginal
|
||||
cost) with the usage-based key (covers only the pro-tier models the subscription
|
||||
excludes) — see [the mixed setup](#other-providers) below. Prefer
|
||||
**Anthropic/Claude** or another provider? Same section.
|
||||
|
||||
### Reviewing (already configured)
|
||||
|
||||
@@ -68,9 +70,9 @@ another provider? See [Other providers & auth modes](#other-providers) below.
|
||||
# Review working-tree changes; prints here, posts nothing
|
||||
ecr review
|
||||
# Review a GitHub PR by number (preview only)
|
||||
ecr review --pr 123
|
||||
ecr review --pr 4057
|
||||
# …and post it as the PR comment
|
||||
ecr review --pr 123 --post
|
||||
ecr review --pr 4057 --post
|
||||
```
|
||||
|
||||
Options (most to least common):
|
||||
@@ -114,7 +116,6 @@ is a ready example to adapt.
|
||||
| `ecr ci` | Review the current GitHub PR and post/update a comment. For GitHub Actions. |
|
||||
| `ecr doctor [--list-scopes]` | Check environment, config, credentials, and (with a manifest) scopes. |
|
||||
| `ecr feedback [--repo <owner/repo>]` | Report which findings PR authors pushed back on, across history. See below. |
|
||||
| `ecr ref-check [--json]` | Fail when the review setup cites code that moved or vanished. See below. |
|
||||
|
||||
Extra flags for monorepos: `review`/`ci` `--config-dir <dir>` (load config from an
|
||||
alternate dir; also `ECR_CONFIG_DIR`), `ci --scopes a,b` (limit the fan-out to
|
||||
@@ -126,48 +127,6 @@ 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):
|
||||
|
||||
```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.
|
||||
|
||||
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.
|
||||
|
||||
---
|
||||
|
||||
## Monorepos (routing manifest)
|
||||
|
||||
A monorepo can route different subtrees to different reviewer rosters from a single
|
||||
@@ -182,9 +141,9 @@ your-monorepo/
|
||||
routing.jsonc # the manifest — infra-owned, ordered scope list + locked defaults
|
||||
config.jsonc # the default/root scope; the ONLY place auth/tokenEnv lives
|
||||
shared.md coordinator.md agents/
|
||||
apps/
|
||||
api/.expo-code-review/{config.jsonc(NO auth),coordinator.md,agents/} # api team
|
||||
web/.expo-code-review/{config.jsonc(NO auth),coordinator.md,agents/} # web team
|
||||
server/
|
||||
www/.expo-code-review/{config.jsonc(NO auth),coordinator.md,agents/} # www team
|
||||
website/.expo-code-review/{config.jsonc(NO auth),coordinator.md,agents/} # website team
|
||||
.github/workflows/expo-code-review.yml # unchanged shape: one workflow, one `ecr ci`
|
||||
```
|
||||
|
||||
@@ -194,28 +153,28 @@ your-monorepo/
|
||||
// Central guardrails every scope inherits and CANNOT override.
|
||||
"defaults": {
|
||||
// The ONLY place auth/tokenEnv is honored (besides the root config.jsonc).
|
||||
"auth": { "providers": { "anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" } } },
|
||||
"auth": { "mode": "api-key", "provider": "openai", "tokenEnv": "OPENAI_API_KEY" },
|
||||
"enforceAgents": ["security"], // always runs on every scope, roster or not
|
||||
"commentTag": "expo-ai-code-reviewer" // per-scope markers derive from this
|
||||
},
|
||||
"comment": "single", // "single" = one aggregated comment (default) | "per-scope"
|
||||
// Ordered; the LAST matching scope wins per changed file (CODEOWNERS discipline).
|
||||
"scopes": [
|
||||
{ "name": "default", "paths": ["**/*"], "config": "." },
|
||||
{ "name": "apps-api", "paths": ["apps/api/**"], "config": "apps/api" },
|
||||
{ "name": "apps-web", "paths": ["apps/web/**"], "config": "apps/web" }
|
||||
{ "name": "default", "paths": ["**/*"], "config": "." },
|
||||
{ "name": "server-www", "paths": ["server/www/**"], "config": "server/www" },
|
||||
{ "name": "server-website", "paths": ["server/website/**"], "config": "server/website" }
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
```jsonc
|
||||
// apps/api/.expo-code-review/config.jsonc (the api team owns this)
|
||||
// server/www/.expo-code-review/config.jsonc (the www team owns this)
|
||||
{
|
||||
// NO "auth" block — locked centrally; a tokenEnv here is rejected by loader + CI guard.
|
||||
"model": "anthropic/claude-sonnet-5",
|
||||
"model": "openai/gpt-5.5",
|
||||
"policy": { "includeSuggestions": false },
|
||||
"noise": { "additionalIgnores": ["apps/api/**/__generated__/**"] }
|
||||
// shared.md, coordinator.md, agents/*.md live beside this file — the api team's roster.
|
||||
"noise": { "additionalIgnores": ["server/www/**/__generated__/**"] }
|
||||
// shared.md, coordinator.md, agents/*.md live beside this file — the www team's roster.
|
||||
}
|
||||
```
|
||||
|
||||
@@ -289,7 +248,7 @@ your-monorepo/
|
||||
security warning and will be removed on a minor boundary.
|
||||
|
||||
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`
|
||||
(the single authoritative router) and `/server/www/.expo-code-review/ @your-www-team`
|
||||
(each team owns only its own scope dir). Rerouting globs is gated behind infra review.
|
||||
|
||||
---
|
||||
@@ -314,11 +273,6 @@ Ownership is enforced with CODEOWNERS: `/.expo-code-review/routing.jsonc @your-i
|
||||
- **Reporter** — posts/updates a single fingerprinted PR comment (CI), or prints
|
||||
a grouped summary (local). Findings below the configured severity floor are
|
||||
suppressed.
|
||||
- **Whole-review reuse** — automated CI stores a hash of each review job's inputs
|
||||
in the hidden state of that comment. If a restack leaves a scope's files and
|
||||
review configuration unchanged, its complete prior result is reused. Manual
|
||||
`/review`, partial/failed reviews, stack-aware review, and model-adjudicated
|
||||
feedback always run fresh.
|
||||
|
||||
Built on the [OpenCode](https://opencode.ai) SDK, which spawns the model provider
|
||||
and applies the provider's prompt caching automatically.
|
||||
@@ -390,7 +344,7 @@ show writes (there is nothing to re-read within the run).
|
||||
---
|
||||
description: One line the router uses to decide relevance.
|
||||
alwaysRun: true # run even when the router would skip this agent
|
||||
model: anthropic/claude-opus-5 # override the default model
|
||||
model: openai/gpt-5.5-pro # override the default model
|
||||
temperature: 0.1
|
||||
---
|
||||
|
||||
@@ -399,14 +353,14 @@ temperature: 0.1
|
||||
|
||||
For a real-world example, see eas-cli's
|
||||
[`.expo-code-review/`](https://github.com/expo/eas-cli/tree/main/.expo-code-review)
|
||||
— correctness/security/consistency agents, a stronger model for security, and
|
||||
per-repo `noise.additionalIgnores`.
|
||||
— correctness/security/consistency agents, a stronger model for security + the
|
||||
coordinator, and per-repo `noise.additionalIgnores`.
|
||||
|
||||
`config.jsonc` (JSONC — comments + trailing commas supported):
|
||||
|
||||
```jsonc
|
||||
{
|
||||
"model": "anthropic/claude-sonnet-5", // default model for the specialists
|
||||
"model": "openai/gpt-5.5", // default model for the specialists
|
||||
"policy": { "includeSuggestions": false }, // suppress suggestion-severity findings
|
||||
"chunk": { "maxChangedLines": 1000, "maxFiles": 20 }, // concurrency defaults: 6 (API key) / 3 (subscription)
|
||||
"noise": { "additionalIgnores": ["packages/*/build/**"] },
|
||||
@@ -415,8 +369,8 @@ per-repo `noise.additionalIgnores`.
|
||||
"skipLabel": "ai-review:skip" }, // "label" (only labeled PRs)
|
||||
"breakGlass": { "marker": "/skip-review" }, // PR body marker that skips the review
|
||||
"commentTag": "expo-ai-code-reviewer", // hidden tag used to find/update the comment
|
||||
"auth": { "providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" } } }
|
||||
"auth": { "mode": "api-key", "provider": "openai",
|
||||
"tokenEnv": "OPENAI_API_KEY" }
|
||||
}
|
||||
```
|
||||
|
||||
@@ -430,17 +384,15 @@ Precedence: **`REVIEWER_MODEL` env** (global override) → per-file **frontmatte
|
||||
setup, and a developer can override everything locally.
|
||||
|
||||
- **Specialist agents** (correctness/security/consistency) benefit from a
|
||||
reasoning-tier model — **`anthropic/claude-sonnet-5`** is the quality/speed
|
||||
sweet spot (the scaffolded default). The **Opus tier** finds more but is slower
|
||||
and more expensive, so scope it to the highest-stakes agent: **security runs on
|
||||
`anthropic/claude-opus-5`** (set in `security.md` frontmatter), the rest on the
|
||||
default.
|
||||
reasoning-tier model — **`openai/gpt-5.5`** is the quality/speed sweet spot
|
||||
(the scaffolded default). The **pro tier** finds more but is slower and more
|
||||
expensive, so scope it to the highest-stakes agent: **security runs on
|
||||
`openai/gpt-5.5-pro`** (set in `security.md` frontmatter), the rest on the default.
|
||||
- **The coordinator** makes the final call (dedupe / re-judge / decide) — worth a
|
||||
strong model; the scaffold pins it to `anthropic/claude-opus-5` in
|
||||
`coordinator.md` frontmatter.
|
||||
strong model; set it in `coordinator.md` frontmatter.
|
||||
- If latency/timeouts dominate on big PRs, moving the specialists to a faster model
|
||||
(e.g. `anthropic/claude-haiku-4-5`) is the most direct lever (a real recall
|
||||
tradeoff — measure it).
|
||||
(e.g. `openai/gpt-5.4-mini`) is the most direct lever (a real recall tradeoff —
|
||||
measure it).
|
||||
- **Every run logs which model actually answered each pass** — in the job log
|
||||
(`Models used — …`), the Actions step summary table, and the run log's
|
||||
`agentModels` — and warns loudly if a pass ran on a different model than
|
||||
@@ -592,7 +544,7 @@ highest-value part — "repeat offenders": findings whose title recurred across
|
||||
PRs and drew a reply every single time.
|
||||
|
||||
```bash
|
||||
ecr feedback --repo your-org/your-repo --limit 100 --since 2026-06-01
|
||||
ecr feedback --repo expo/eas-cli --limit 100 --since 2026-06-01
|
||||
ecr feedback --as my-review-bot # if CI posts under a PAT/app identity
|
||||
ecr feedback --json # for scripting
|
||||
```
|
||||
@@ -655,34 +607,11 @@ visible even in CI (where the run log is ephemeral).
|
||||
<details>
|
||||
<summary><b>Other providers & auth modes</b></summary>
|
||||
|
||||
The scaffolded default is **Anthropic via the Claude Code CLI** — see First-time
|
||||
setup above and the Anthropic bullet below for how it authenticates and is
|
||||
sandboxed. Everything is set in `config.auth`; non-anthropic providers get their
|
||||
credentials through OpenCode.
|
||||
The recommended setup is an OpenAI API key — see Usage above. Alternatives, all
|
||||
set in `config.auth` (credentials come from OpenCode):
|
||||
|
||||
- **Anthropic / Claude (the default)** — use `anthropic/...` model ids and every
|
||||
anthropic pass runs through the **Claude Code CLI** (`claude -p
|
||||
--output-format json`), inferred from the model. The credential is (in order) a
|
||||
`tokenEnv` you name, an ambient `CLAUDE_CODE_OAUTH_TOKEN`, or your local
|
||||
`claude` login — an `auth` entry is entirely optional. Run `claude setup-token`
|
||||
for a Max/Team subscription token or point `tokenEnv` at an Anthropic Console
|
||||
API key (`sk-ant-api…`, forwarded as `ANTHROPIC_API_KEY`); the CLI reads
|
||||
either. `ecr setup-auth` walks you through it. Each pass is trust-isolated and
|
||||
read-only: it runs with `--safe-mode` (no `CLAUDE.md`/hooks/MCP/plugins),
|
||||
`--strict-mcp-config`, `--permission-mode dontAsk`, and only the
|
||||
`Read`/`Grep`/`Glob` tools — never `Bash`/`Edit`/`Write`/`WebFetch`/`WebSearch`.
|
||||
The child env is an allowlist that omits ambient `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`
|
||||
(only the configured credential is re-injected).
|
||||
```jsonc
|
||||
// The scaffolded default. No anthropic entry at all falls back to your
|
||||
// `claude` login.
|
||||
"auth": { "providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }
|
||||
} }
|
||||
```
|
||||
- **OpenAI: ChatGPT/Codex subscription (OAuth) + usage-based API key — the
|
||||
recommended mix if you review with OpenAI.** OpenAI permits subscription auth
|
||||
in third-party tools, and OpenCode
|
||||
- **ChatGPT/Codex subscription (OAuth) + usage-based API key — the recommended
|
||||
mix.** OpenAI permits subscription auth in third-party tools, and OpenCode
|
||||
ships the plugin for it — so the reviewer runs its default models on the
|
||||
subscription (zero marginal cost) and reserves the metered key for pro-tier
|
||||
models the subscription doesn't offer (`gpt-5.5-pro` is subscription-excluded).
|
||||
@@ -721,6 +650,25 @@ credentials through OpenCode.
|
||||
One caveat: OpenCode can't price alias models (they're config-declared), so
|
||||
pro passes report `$0` in the run log's cost column — token counts are
|
||||
correct, and the OpenAI project dashboard is the source of truth for spend.
|
||||
- **Anthropic / Claude** — use `anthropic/...` model ids and every anthropic pass
|
||||
runs through the **Claude Code CLI** (`claude -p --output-format json`), inferred
|
||||
from the model. The credential is (in order) a `tokenEnv` you name, an ambient
|
||||
`CLAUDE_CODE_OAUTH_TOKEN`, or your local `claude` login — an `auth` entry is
|
||||
entirely optional. Run `claude setup-token` for a Max/Team subscription token
|
||||
(forwarded as `CLAUDE_CODE_OAUTH_TOKEN`) or point `tokenEnv` at an Anthropic
|
||||
Console API key (`sk-ant-api…`, forwarded as `ANTHROPIC_API_KEY`); the CLI reads
|
||||
either. `ecr setup-auth` walks you through it. Each pass is trust-isolated and
|
||||
read-only: it runs with `--safe-mode` (no `CLAUDE.md`/hooks/MCP/plugins),
|
||||
`--strict-mcp-config`, `--permission-mode dontAsk`, and only the
|
||||
`Read`/`Grep`/`Glob` tools — never `Bash`/`Edit`/`Write`/`WebFetch`/`WebSearch`.
|
||||
The child env is an allowlist that omits ambient `ANTHROPIC_API_KEY`/`ANTHROPIC_AUTH_TOKEN`
|
||||
(only the configured credential is re-injected).
|
||||
```jsonc
|
||||
// Optional — no anthropic entry at all falls back to your `claude` login.
|
||||
"auth": { "providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" }
|
||||
} }
|
||||
```
|
||||
- **Another provider** — the current path is the `REVIEWER_MODEL`
|
||||
env override: `opencode auth login` once (pick the provider), then run with
|
||||
e.g. `REVIEWER_MODEL=google/gemini-3-pro`. It overrides every agent's model
|
||||
@@ -733,9 +681,8 @@ any other) OpenCode provider, and each agent's `model` selects its engine.
|
||||
`REVIEWER_MODEL` still overrides every agent's model (and therefore every agent's
|
||||
engine), converging the whole run onto one engine.
|
||||
|
||||
There is no shared fallback key; if a run fails for lack of credentials, log in
|
||||
with `claude` (the default) or authenticate a provider in OpenCode. `ecr doctor`
|
||||
diagnoses setup.
|
||||
There is no shared fallback key; if a run fails for lack of credentials, authenticate
|
||||
a provider in OpenCode. `ecr doctor` diagnoses setup.
|
||||
|
||||
**Setup errors fail fast, with the fix in the message.** A bad credential or model id
|
||||
would otherwise fail every pass identically — a run that spends its whole budget
|
||||
|
||||
@@ -61,14 +61,6 @@ The absolute constraint is the carve-out (`suppress.ts:39-44`): a `critical` or
|
||||
|
||||
**The comment is the durable store.** An embedded `<!-- tag:state=... -->` comment holds base64 JSON of the FULL review plus dismissals — not just what is rendered — so a later `/dismiss` or `/undismiss` can re-render without re-running the engine (`render.ts:198-202`, `encodeState`/`parseReviewState` at `render.ts:493-513`). There is no side-channel database; the comment body is the state. Dismissed findings render in a collapsed `<details>` section, disjoint from the active list (a finding is never in both), keeping the main list clean while preserving a `by`/`reason` audit trail.
|
||||
|
||||
## Review Result Cache
|
||||
|
||||
The same durable comment state is the cache for automated CI reviews. `reviewInputHash` (`src/core/review-cache.ts`) hashes each scoped changed file's content from the materialized PR-head tree, a normalized patch (blob ids and hunk coordinates removed; deletion/mode information retained), and every stable input that can change the result: the engine package version, resolved prompts/config/models, PR title/body, selected agents and routing mode, and external context. It deliberately excludes the temporary trusted-config path and the base/head commit OIDs. A restack changes OIDs and may move hunk line numbers; if it leaves the actual review input byte-identical, CI reuses the complete `CoordinatorOutput` already embedded in the most recent author-verified reviewer comment instead of launching the agents again. Symlinks are hashed as link targets and never followed. A package release changes the key so upgraded review logic runs once. Legacy comments store one `inputHash`; aggregate comments store one per `ScopeReviewResult`, so an unchanged scope can hit independently while a changed sibling reruns.
|
||||
|
||||
Reuse is conservative. Manual `/review`/`--force` always reruns. Partial or failed reviews (`incomplete` or `couldNotComplete`) are never cacheable. Aggregate truncation removes the affected scope's hash because the embedded scope is no longer the full result. Stack-aware requalification and model-backed author-feedback seams stay fresh because their live GitHub inputs are not yet represented in the key. Cache lookup failures degrade to an ordinary fresh review. These gates make a false miss cost tokens, while preventing a false hit from presenting stale or incomplete coverage as current.
|
||||
|
||||
The hash is state, not display: it lives only inside the base64 state marker and survives `/dismiss`/`/undismiss` re-renders. The existing author-plus-marker ownership check remains the trust boundary for adopting prior state; a PR author cannot forge a look-alike comment and supply a cache entry.
|
||||
|
||||
**The coverage note only cries wolf when there is a wolf.** The partial-coverage warning renders only when `review.incomplete` is non-empty (`render.ts:166`) — the aggregate renderer additionally triggers it when unmatched files exist (`render.ts:373`) — a deliberate anti-noise choice so the warning is trustworthy when it appears (`render.test.ts:34`). The advisory footer ("never blocks a merge and never auto-approves") is duplicated verbatim across both renderers because it is a required trust-boundary claim, not incidental text.
|
||||
|
||||
## Truncation and Aggregate State
|
||||
|
||||
@@ -45,8 +45,6 @@ Flag exclusivity is rejected outright, never silently resolved, because silently
|
||||
|
||||
**It never fails the PR's checks.** A reviewer error degrades to a posted failure comment, not a non-zero process exit [observed: `AGENTS.md`; `src/commands/ci.ts:354-373` legacy catch posts a summary; `src/commands/ci.ts:571-576` per-scope catch builds a `failureReview` placeholder instead of aborting the fan-out]. PR-facing comment text uses `publicFailureReason(error)`, not the raw `errorMessage`, so the credential and internal detail never leak into a comment (LLP 0002) [observed: `src/commands/ci.ts:176,365,575`].
|
||||
|
||||
**Automated reruns reuse unchanged complete results.** Before launching agents, CI hashes the review job's scoped diff and stable review inputs and compares it with the hash hidden in the newest author-verified reviewer comment. An exact match re-renders the stored result (so links, dismissals, and live annotation-only feedback still update) without spending model budget. Routed CI does this independently per scope. Manual `/review`/`--force`, stack-aware runs, model-backed feedback runs, partial/failed results, and truncated aggregate state all bypass reuse and run fresh. The hash contract and durable-state rules are owned by LLP 0005's Review Result Cache section.
|
||||
|
||||
`--unsafe-config-from-head` is a deliberate escape hatch that reverses this model, loading config from the checkout. It is never scaffolded by `ecr init`, always prints a loud `⚠ SECURITY` warning, and is scheduled for removal on a minor version boundary [observed: `src/commands/ci.ts:85-91` `CI_USAGE`, `155-162` warning].
|
||||
|
||||
`--context-file` in CI takes the opposite read-error policy from `ecr review`, and the split is dictated by the never-fail-checks invariant. `ci` reads the file once before any fan-out and, on a missing/oversized/unreadable path, WARNs to stderr and continues with no context — it never fails, because a broken Atlantis-provided plan file must not turn the PR's check red [observed: `src/commands/ci.ts` `--context-file` read → stderr warn, no `exitCode`, `contextText` stays undefined]. The same reader (`readContextFile`, capped at `MAX_CONTEXT_FILE_BYTES`) backs both commands; only the catch policy differs — fatal locally, warn in CI. The byte-bounded raw text threads through the shared `CiRunOptions` into both `runLegacyCi` and `runRoutedCi`, and from there into every per-scope `runReview` as `contextText` (char-capped at prompt build) [observed: `src/commands/ci.ts` `CiRunOptions.contextText`; `runReview` calls in `runLegacyCi` and the fan-out loop].
|
||||
|
||||
@@ -1,177 +0,0 @@
|
||||
# LLP 0012: Review-Setup Ref Integrity
|
||||
|
||||
**Type:** Explainer
|
||||
**Status:** Active
|
||||
**Systems:** Engine, CLI
|
||||
**Author:** Philippe Loulidi / Claude
|
||||
**Date:** 2026-08-03
|
||||
**Related:** [LLP 0006 Config Schema, Loading, and Routing](0006-config-schema-loading-routing.explainer.md), [LLP 0007 CLI Commands and CI](0007-cli-commands-and-ci.explainer.md), [LLP 0009 Adoption Templates and CI Workflows](0009-adoption-templates-and-ci-workflows.guide.md)
|
||||
|
||||
A repo's reviewer prompts are full of citations into its own code: "the only session
|
||||
entry point is `server/www/src/utils/session.ts`", "every webhook router must call
|
||||
`sanitizeSecretPatterns`". Those citations are load-bearing — they are how a prompt
|
||||
carries repo knowledge the model does not have — and nothing kept them true. Code
|
||||
moves, symbols get renamed, and the prompt keeps citing a path that no longer exists.
|
||||
The reviewer then reasons from a fiction, silently, on every PR.
|
||||
|
||||
`ecr ref-check` (`src/commands/ref-check.ts`, over `src/core/config-refs.ts`) closes
|
||||
that gap: every code citation in a `.expo-code-review/` setup must be a ref, and every
|
||||
ref must resolve against the checkout. It is deterministic — no model, no network —
|
||||
in the same family as `ecr verify-config` (see [LLP 0007][verify]).
|
||||
|
||||
Two measurements motivated the strict form. Swept against a real adopting monorepo
|
||||
(universe) the check found 165 citations, none of them annotated or verified by
|
||||
anything; swept against this repo's own setup it found 24 [observed]. Not one of those
|
||||
citations was checked before this existed.
|
||||
|
||||
## The `@ref` Grammar
|
||||
|
||||
One grammar, reused from the engine's own LLP annotations, so an author learns it once:
|
||||
|
||||
@ref <target> [relation] — why it matters
|
||||
|
||||
In markdown it lives in an HTML comment, in JSONC after `//`. Targets:
|
||||
|
||||
| Target | Resolves when |
|
||||
|---|---|
|
||||
| `path/to/file.ts` | the file exists |
|
||||
| `path/to/dir/` | the directory exists (trailing slash is required) |
|
||||
| `path/to/file.ts#symbolName` | the file exists and still contains that whole word |
|
||||
| `path/to/doc.md#heading` | the file exists and has that heading |
|
||||
| `glob:**/Handler.kt` | at least one file in the repo matches |
|
||||
| `https://…` | shape only, never fetched |
|
||||
|
||||
Refs are **always repo-root-relative**, including inside a scope's setup dir. A scope's
|
||||
prompts describe the same tree the reviewer sees in the diff, so a second base
|
||||
directory would only create ambiguity about which one a ref meant. Scoped prompts do
|
||||
naturally cite their own subtree (`general-central/module` for
|
||||
`infrastructure/general-central/module`), so that stays a broken ref — but the message
|
||||
resolves it against the scope root and names the root-relative form to use.
|
||||
|
||||
`glob:` exists for readability. A Kotlin path 90 characters deep gets abbreviated in
|
||||
prose (`.../config/MetricGroupCounter.kt`) and an abbreviation cannot be resolved, so
|
||||
the check suggests the suffix glob `glob:**/config/MetricGroupCounter.kt` — short in the
|
||||
prompt, still verified.
|
||||
|
||||
**Invariant:** a target is data, never a command. Resolution reads files inside the repo
|
||||
root only, via the same `pathInside` confinement the verifier uses ([LLP 0005][verify5]);
|
||||
absolute paths and `..` escapes are refused, not resolved.
|
||||
|
||||
## No Line Numbers, Symbol Anchors Instead
|
||||
|
||||
A ref may not pin a line. `src/auth.ts:42` is refused in an annotation *and* in prose,
|
||||
because a line number is the one form of citation that rots without any signal: insert
|
||||
a line above it and the ref still "resolves", now pointing at unrelated code. `#symbol`
|
||||
is the robust alternative — it survives the code moving inside its file, and breaks
|
||||
exactly when the thing cited is renamed or deleted, which is when a human should look.
|
||||
|
||||
For the same reason there is no content hash or pinned revision in the grammar. A digest
|
||||
would trip on every edit to the cited file, including edits that leave the prompt
|
||||
perfectly correct, and a check that cries wolf gets disabled. The drift signal lives on
|
||||
the review side instead (below).
|
||||
|
||||
## Unannotated Citations Are Broken Refs
|
||||
|
||||
Checking only what an author annotated would make the mechanism decorative: the stale
|
||||
citations are precisely the ones nobody thought about. So a backticked token in a setup
|
||||
file that *looks like* a repo path and is not a ref fails the check.
|
||||
|
||||
That rule needs a precise notion of "looks like a path", because prompts also backtick
|
||||
config keys, severities, commands and model ids. It comes in two tiers.
|
||||
|
||||
**Shape alone** (`isCodeCitation`): an allowlisted source extension, a trailing slash, or
|
||||
a wildcard tail inside a path — and never whitespace, `:`, `<`, `(` or quotes. So
|
||||
`session.ts`, `src/entities/oauth/` and `.github/workflows/**` are citations, while
|
||||
`ecr ci`, `label:<agent>`, `knex.raw()`, `critical` and a bare `.ts` are prose. Fenced
|
||||
code blocks are skipped whole: they are examples, not claims about the tree.
|
||||
|
||||
**Shape plus the filesystem** (`pathishCandidates`), for the citations shape cannot
|
||||
settle. A reviewer of this feature named the case: a Terraform prompt citing
|
||||
`eas-build-worker/terraform`, `general-central/{module,production}` and `finops`, with
|
||||
"this is very likely going to drift" [observed] — not one of those has an extension, and
|
||||
`anthropic/claude-opus-5` is shaped identically while naming no file. Syntax cannot tell
|
||||
them apart, so an extensionless token counts as a citation exactly when it *names
|
||||
something real*: probed against the scope's own subtree first, then the repo root (a
|
||||
scoped prompt citing `alerts/` means its own tree even when the root has one too), brace
|
||||
lists cut back to their fixed prefix. A token carrying `*` resolves as a glob instead,
|
||||
scope-rebased when that is what matches, so the suggested `glob:` ref itself resolves.
|
||||
Model ids resolve to nothing and stay prose.
|
||||
|
||||
The consequence is deliberate: an extensionless path that had already drifted before
|
||||
adoption is invisible (nothing distinguishes it from prose), while every one that is true
|
||||
today gets pinned and will be caught the moment it moves. This mechanism prevents future
|
||||
drift; it does not retroactively find old drift.
|
||||
|
||||
The escape hatch is explicit and per-token: `@ref-ignore knex.raw()` in a comment.
|
||||
Anything the heuristic misclassifies is one short line away from silence, which is why
|
||||
the rule can afford to be strict rather than advisory.
|
||||
|
||||
**Invariant:** the check never rewrites a prompt. It reports; a human decides whether the
|
||||
ref or the prompt was wrong. A stale citation often means the *guidance* is stale, not
|
||||
just the path.
|
||||
|
||||
## Structural Refs Need No Annotation
|
||||
|
||||
Some refs the config already declares, so they are checked without annotation:
|
||||
`enforceAgents` ids must exist as `agents/<id>.md` in the ROOT setup (that is where the
|
||||
loader resolves them, see [LLP 0006][routing]), every scope's `config` must point at a
|
||||
real `.expo-code-review/` directory inside the repo, and every scope glob must match at
|
||||
least one file — a scope whose paths match nothing ships prompts that review nothing.
|
||||
|
||||
## What Gets Scanned
|
||||
|
||||
Setup dirs are found by walking the tree, not by reading `routing.jsonc`: a scope dir the
|
||||
manifest forgot still ships prompts, and its stale refs are exactly what this exists to
|
||||
surface — the same reason `ecr verify-config` sweeps on disk instead of trusting the
|
||||
manifest ([LLP 0007][verify]). Inside a setup dir the scan covers prompts and configs
|
||||
(`.md`, `.json`, `.jsonc`, `.txt`) and skips dot entries, so `.runs/` artifacts are never
|
||||
scanned. Dot directories elsewhere are skipped too, which keeps `.claude/worktrees/`
|
||||
checkouts of the same repo from being swept as if they were scopes.
|
||||
|
||||
## Run Points: Command and Review
|
||||
|
||||
`ecr ref-check` exits 1 on any problem: that is the gate, for CI or a pre-commit hook.
|
||||
|
||||
The review path also runs it, and never fails a PR's checks with it — that invariant
|
||||
holds ([LLP 0002][pipeline]). Instead the review *advises*: broken refs in the setup
|
||||
become a note the reviewer comment carries, and `citedPathsTouchedBy` adds the drift
|
||||
signal a static check cannot give — "this PR changes code your reviewer prompts cite, so
|
||||
the guidance may need updating" — even when the ref still resolves. In CI the config
|
||||
comes from the trusted base ref while refs resolve against the PR head tree, so a PR that
|
||||
moves cited code is reported against the config that will actually review it.
|
||||
|
||||
## Two Checkers, One Grammar
|
||||
|
||||
The engine repo now has two ref checkers, and they scan overlapping files: `./ref-check`
|
||||
(the LLP corpus validator, which walks the whole tree) and `ecr ref-check` (setup dirs
|
||||
only). Running both surfaced two genuine conflicts, where one checker failed an
|
||||
annotation the other accepted [observed]:
|
||||
|
||||
- `glob:` targets were unknown to `./ref-check`, which read them as a literal path and
|
||||
reported "path not found".
|
||||
- Documenting the syntax broke both: a usage line containing the comment form with a
|
||||
`<target>` placeholder was collected as a real ref, and so were the README's fenced
|
||||
examples — the docs that teach refs were the first thing the checkers rejected.
|
||||
|
||||
All of it is fixed in the grammar rather than per-checker: `glob:` is understood by both,
|
||||
a target starting with `<` or a backtick is a documentation placeholder, and an annotation
|
||||
inside a fenced code block is an example that neither checker resolves.
|
||||
|
||||
Ownership stays split by target kind, not by file: an `LLP NNNN` target belongs to the
|
||||
design corpus and its own checker, so `ecr ref-check` neither resolves nor counts one —
|
||||
it owns exactly the refs that cite the reviewed repo's code. That split is why the
|
||||
templates can keep their engine annotations and still scaffold a repo that passes
|
||||
`ecr ref-check` on day one (asserted in `init-scaffold.test.ts`). An earlier attempt had
|
||||
`ecr init` strip those annotations while scaffolding; it mixed the two mechanisms and
|
||||
broke the "workflow templates land byte-identical" invariant [observed]. Parsing is what
|
||||
must be right: `LLP 0009#anchor` is one target containing a space, and taking the first
|
||||
whitespace-separated token left a bare `LLP` that read as a broken path.
|
||||
|
||||
**Invariant:** the grammar is shared. A change to target kinds, the placeholder rule, or
|
||||
the glob dialect must land in `./ref-check` and `src/core/config-refs.ts` together, or the
|
||||
same annotation will pass one checker and fail the other.
|
||||
|
||||
[verify]: 0007-cli-commands-and-ci.explainer.md#verify-config-the-config-guard
|
||||
[verify5]: 0005-verification-fingerprints-rendering.explainer.md#verifier-confinement-and-fail-open
|
||||
[routing]: 0006-config-schema-loading-routing.explainer.md#routing-manifest
|
||||
[pipeline]: 0002-review-engine-pipeline.explainer.md#pipeline-stages
|
||||
@@ -52,12 +52,6 @@ CODE_REF_RE = re.compile(re.escape(REF_MARK) + r"\s+(.+)")
|
||||
MD_REF_RE = re.compile(r"<!--\s*" + re.escape(REF_MARK) + r"\s+(.+?)-->", re.DOTALL)
|
||||
LLP_TARGET_RE = re.compile(r"^LLP\s+(\d{1,4})(?:#(\S+))?$")
|
||||
SHORTHAND_RE = re.compile(r"^[A-Z][A-Z0-9_]*(?:#\S+)?$")
|
||||
# Shared with `ecr ref-check`, which validates the SAME @ref grammar inside a repo's
|
||||
# .expo-code-review/ setup: `glob:<pattern>` targets a set of files, and a `<…>` target
|
||||
# documents the syntax rather than citing a file. Both checkers must agree on these or
|
||||
# the same annotation passes one and fails the other.
|
||||
GLOB_PREFIX = "glob:"
|
||||
PLACEHOLDER_RE = re.compile(r"^[<`]")
|
||||
URL_RE = re.compile(r"^https?://[^\s/]+\.[^\s/]+\S*$")
|
||||
HEADING_RE = re.compile(r"^(#{1,6})\s+(.+?)\s*$")
|
||||
LLP_FILE_RE = re.compile(r"^(\d{4})-[a-z0-9-]+\.([a-z0-9]+)\.md$")
|
||||
@@ -147,68 +141,17 @@ def parse_target(rest):
|
||||
token = rest.split()[0] if rest.split() else ""
|
||||
if token.startswith(("http://", "https://")):
|
||||
return "url", token, None
|
||||
if PLACEHOLDER_RE.match(token):
|
||||
return "placeholder", token, None
|
||||
if token.startswith(GLOB_PREFIX):
|
||||
return "glob", token[len(GLOB_PREFIX):], None
|
||||
if SHORTHAND_RE.match(token):
|
||||
return "shorthand", token, None
|
||||
path, _, anchor = token.partition("#")
|
||||
return "path", path, anchor or None
|
||||
|
||||
|
||||
def glob_to_regex(pattern):
|
||||
"""The mini-glob dialect shared with the CLI: `**` crosses `/`, `*` does not."""
|
||||
out, i = "", 0
|
||||
while i < len(pattern):
|
||||
ch = pattern[i]
|
||||
if ch == "*":
|
||||
if pattern[i + 1:i + 2] == "*":
|
||||
out += ".*"
|
||||
i += 1
|
||||
else:
|
||||
out += "[^/]*"
|
||||
elif ch in ".+^${}()|[]\\?":
|
||||
out += "\\" + ch
|
||||
else:
|
||||
out += ch
|
||||
i += 1
|
||||
return re.compile(f"^{out}$")
|
||||
|
||||
|
||||
def repo_files(root):
|
||||
"""Every file in the tree (SKIP_DIRS pruned), root-relative with / separators."""
|
||||
found = []
|
||||
for dirpath, dirnames, filenames in os.walk(root):
|
||||
dirnames[:] = [d for d in dirnames if d not in SKIP_DIRS]
|
||||
for name in filenames:
|
||||
rel = os.path.relpath(os.path.join(dirpath, name), root)
|
||||
found.append(rel.replace(os.sep, "/"))
|
||||
return found
|
||||
|
||||
|
||||
def fenced_lines(text):
|
||||
"""1-based line numbers inside a fenced code block (the fence lines included)."""
|
||||
fenced, open_fence = set(), False
|
||||
for i, line in enumerate(text.splitlines(), 1):
|
||||
if line.lstrip().startswith("```"):
|
||||
fenced.add(i)
|
||||
open_fence = not open_fence
|
||||
elif open_fence:
|
||||
fenced.add(i)
|
||||
return fenced
|
||||
|
||||
|
||||
def collect_refs(path, text):
|
||||
refs = [] # (lineno, rest)
|
||||
if path.endswith(".md"):
|
||||
# An annotation inside a fence documents the grammar; resolving it would make
|
||||
# every doc that explains refs fail. Same rule in ecr's config-refs.ts.
|
||||
fenced = fenced_lines(text)
|
||||
for m in MD_REF_RE.finditer(text):
|
||||
lineno = text.count("\n", 0, m.start()) + 1
|
||||
if lineno in fenced:
|
||||
continue
|
||||
refs.append((lineno, m.group(1)))
|
||||
else:
|
||||
for i, line in enumerate(text.splitlines(), 1):
|
||||
@@ -262,7 +205,6 @@ def check(root):
|
||||
|
||||
# --- refs: scan the tree ---
|
||||
n_files = n_refs = 0
|
||||
all_files = repo_files(root)
|
||||
exempt = (os.path.join("llp", "tombstones"), os.path.join("llp", "reviews"))
|
||||
for dirpath, dirnames, filenames in os.walk(root):
|
||||
dirnames[:] = [d for d in dirnames if d not in SKIP_DIRS]
|
||||
@@ -295,13 +237,6 @@ def check(root):
|
||||
elif kind == "url":
|
||||
if not URL_RE.match(target):
|
||||
rep.error(f"{where}: malformed URL target: {target}")
|
||||
elif kind == "placeholder":
|
||||
rep.info(f"{where}: documentation placeholder, not a citation: {target}")
|
||||
elif kind == "glob":
|
||||
if not target:
|
||||
rep.error(f"{where}: empty glob target")
|
||||
elif not any(glob_to_regex(target).match(f) for f in all_files):
|
||||
rep.error(f"{where}: broken ref: glob matches no file: {target}")
|
||||
elif kind == "shorthand":
|
||||
rep.info(f"{where}: unchecked shorthand: {target} (no mapping defined)")
|
||||
else: # path
|
||||
|
||||
@@ -188,14 +188,8 @@ test("checkProviderAuth: FORBIDDEN + cross-provider guards still fire for an ant
|
||||
test("checkProviderAuth: a non-anthropic entry naming CLAUDE_CODE_OAUTH_TOKEN is refused", () => {
|
||||
// The long-lived Claude Max/Team subscription token would otherwise be forwarded to
|
||||
// a FOREIGN provider (here openai) as its bearer — it is neither in FORBIDDEN nor a
|
||||
// PROVIDER_KEY_ENV value, so only the anthropic-owned guard catches it. The
|
||||
// scaffolded default CLAUDE_CODE_REVIEW_SHARED_API_TOKEN is anthropic-owned the
|
||||
// same way: it always holds an Anthropic credential (templates/config.jsonc).
|
||||
for (const tokenEnv of [
|
||||
"CLAUDE_CODE_OAUTH_TOKEN",
|
||||
"ANTHROPIC_AUTH_TOKEN",
|
||||
"CLAUDE_CODE_REVIEW_SHARED_API_TOKEN",
|
||||
]) {
|
||||
// PROVIDER_KEY_ENV value, so only the anthropic-owned guard catches it.
|
||||
for (const tokenEnv of ["CLAUDE_CODE_OAUTH_TOKEN", "ANTHROPIC_AUTH_TOKEN"]) {
|
||||
const r = checkProviderAuth(cfg({ mode: "api-key", provider: "openai", tokenEnv }), {
|
||||
[tokenEnv]: `sk-ant-oat01-${"x".repeat(95)}`,
|
||||
});
|
||||
@@ -213,16 +207,6 @@ test("checkProviderAuth: an anthropic entry naming CLAUDE_CODE_OAUTH_TOKEN is st
|
||||
expect(r.ok).toBe(true);
|
||||
});
|
||||
|
||||
test("checkProviderAuth: the scaffolded default setup (anthropic + shared token env) passes", () => {
|
||||
// The exact shape `ecr init` scaffolds (templates/config.jsonc) — anthropic owns
|
||||
// the shared env, so its own entry must not trip the ownership guard.
|
||||
const r = checkProviderAuth(
|
||||
cfg({ provider: "anthropic", tokenEnv: "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }),
|
||||
{ CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: `sk-ant-oat01-${"x".repeat(95)}` },
|
||||
);
|
||||
expect(r.ok).toBe(true);
|
||||
});
|
||||
|
||||
test("checkProviderAuth: an upstream=anthropic alias may name CLAUDE_CODE_OAUTH_TOKEN", () => {
|
||||
// upstream is anthropic, so the owner matches the upstream and the token is not
|
||||
// being sent to a foreign provider.
|
||||
|
||||
@@ -133,33 +133,33 @@ test("a carried-over scope (partial --scopes run) not in `results` keeps its pri
|
||||
});
|
||||
|
||||
test("a stale per-scope feedback copy on a carried-over scope never overrides a newer top-level record (undismiss revert)", () => {
|
||||
// Finding stale-scope-feedback: a full run cleared finding F on scope web, storing
|
||||
// R1 (applied:true) both at the top level AND inside web's per-scope review.feedback.
|
||||
// Finding stale-scope-feedback: a full run cleared finding F on scope www, storing
|
||||
// R1 (applied:true) both at the top level AND inside www's per-scope review.feedback.
|
||||
// A human /undismiss rewrote ONLY the top-level record to R2 (applied:false, pinned).
|
||||
// A later `--scopes api` run carries web over via mergePartialAggregate — its review
|
||||
// A later `--scopes api` run carries www over via mergePartialAggregate — its review
|
||||
// still holds the stale R1. The merge must keep R2 (from `prior`), never re-inject R1.
|
||||
const webFinding = finding({ file: "web.ts", title: "Web issue" });
|
||||
const webFp = scopedFingerprint("web", webFinding);
|
||||
const wwwFinding = finding({ file: "www.ts", title: "Www issue" });
|
||||
const wwwFp = scopedFingerprint("www", wwwFinding);
|
||||
// Top-level prior state = the human-overridden record.
|
||||
const r2 = record({
|
||||
fp: webFp,
|
||||
fp: wwwFp,
|
||||
by: "author1",
|
||||
applied: false,
|
||||
unclearedByHuman: true,
|
||||
maintainer: true,
|
||||
});
|
||||
const prior = [r2];
|
||||
// This run freshly reviews only "api"; web is carried over and still embeds the
|
||||
// This run freshly reviews only "api"; www is carried over and still embeds the
|
||||
// stale R1 (applied:true, no pin) inside its per-scope review.feedback.
|
||||
const apiFinding = finding({ file: "api.ts", title: "Api issue" });
|
||||
const results = [seamOkScope("api", [apiFinding], [])];
|
||||
const staleR1 = record({ fp: webFp, by: "author1", applied: true, maintainer: true });
|
||||
const staleR1 = record({ fp: wwwFp, by: "author1", applied: true, maintainer: true });
|
||||
const finalResults: ScopeReviewResult[] = [
|
||||
...results,
|
||||
seamOkScope("web", [webFinding], [staleR1]),
|
||||
seamOkScope("www", [wwwFinding], [staleR1]),
|
||||
];
|
||||
const merged = mergeAggregateFeedback(results, finalResults, prior, feedbackConfig, HEAD_SHA);
|
||||
const kept = merged.find((r) => r.fp === webFp);
|
||||
const kept = merged.find((r) => r.fp === wwwFp);
|
||||
expect(kept).toBeDefined();
|
||||
// The human override survives: pinned and un-applied, not re-hidden by the stale copy.
|
||||
expect(kept!.unclearedByHuman).toBe(true);
|
||||
|
||||
@@ -1,537 +0,0 @@
|
||||
import { test, expect } from "bun:test";
|
||||
import { mkdir, mkdtemp, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import {
|
||||
checkConfigRefs,
|
||||
citedPathsTouchedBy,
|
||||
discoverSetupDirs,
|
||||
findProseCitations,
|
||||
isCodeCitation,
|
||||
parseRefAnnotations,
|
||||
parseRefIgnores,
|
||||
reviewSetupRefNotes,
|
||||
slugifyHeading,
|
||||
suggestedRef,
|
||||
} from "../core/config-refs.js";
|
||||
import { refCheckCommand } from "../commands/ref-check.js";
|
||||
|
||||
const REF = "@ref";
|
||||
|
||||
async function makeRepo(files: Record<string, string>): Promise<string> {
|
||||
const root = await mkdtemp(path.join(tmpdir(), "ecr-refs-test-"));
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
const full = path.join(root, rel);
|
||||
await mkdir(path.dirname(full), { recursive: true });
|
||||
await writeFile(full, content, "utf8");
|
||||
}
|
||||
return root;
|
||||
}
|
||||
|
||||
/** A minimal setup dir: agents/ plus whatever the test adds. */
|
||||
function setup(extra: Record<string, string>): Record<string, string> {
|
||||
return {
|
||||
".expo-code-review/config.jsonc": "{}\n",
|
||||
".expo-code-review/agents/security.md": "# Security\n",
|
||||
...extra,
|
||||
};
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// parsing
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("parses refs from markdown comments and from // comments, with line numbers", () => {
|
||||
const md = `# Title\n\n<!-- ${REF} src/a.ts — the invariant -->\nprose\n<!-- ${REF} src/b.ts#foo [implements] -->\n`;
|
||||
expect(parseRefAnnotations(md, "agents/x.md")).toEqual([
|
||||
{ file: "agents/x.md", line: 3, target: "src/a.ts" },
|
||||
{ file: "agents/x.md", line: 5, target: "src/b.ts#foo" },
|
||||
]);
|
||||
|
||||
const jsonc = `{\n // ${REF} src/c.ts — why\n "a": 1\n}\n`;
|
||||
expect(parseRefAnnotations(jsonc, "config.jsonc")).toEqual([
|
||||
{ file: "config.jsonc", line: 2, target: "src/c.ts" },
|
||||
]);
|
||||
});
|
||||
|
||||
test("@ref-ignore lists tokens that are prose, not citations", () => {
|
||||
const text = `<!-- ${REF}-ignore knex.raw() session.ts -->\n`;
|
||||
expect([...parseRefIgnores(text)].sort()).toEqual(["knex.raw()", "session.ts"]);
|
||||
// the ignore marker must not be collected as a ref itself
|
||||
expect(parseRefAnnotations(text, "a.md")).toEqual([]);
|
||||
});
|
||||
|
||||
test("isCodeCitation separates paths from prose tokens", () => {
|
||||
for (const token of [
|
||||
"server/www/src/utils/session.ts",
|
||||
"session.ts",
|
||||
"src/entities/oauth/",
|
||||
".github/workflows/**",
|
||||
"*PrivacyPolicy.ts",
|
||||
"castai-spot.tf",
|
||||
"*/.terraform.lock.hcl",
|
||||
]) {
|
||||
expect(isCodeCitation(token)).toBe(true);
|
||||
}
|
||||
for (const token of [
|
||||
"ecr ci",
|
||||
"label:<agent>",
|
||||
"knex.raw()",
|
||||
"openai/gpt-5.5",
|
||||
"**",
|
||||
"/",
|
||||
"critical",
|
||||
'{ "findings": [] }',
|
||||
]) {
|
||||
expect(isCodeCitation(token)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
test("prose citations skip fenced code blocks", () => {
|
||||
const text = "`a/b.ts` here\n\n```\n`c/d.ts` example\n```\n\n`e/f.ts` again\n";
|
||||
expect(findProseCitations(text).map((c) => c.token)).toEqual(["a/b.ts", "e/f.ts"]);
|
||||
});
|
||||
|
||||
test("suggestedRef turns abbreviated and bare citations into suffix globs", () => {
|
||||
expect(suggestedRef("server/www/src/a.ts")).toBe("server/www/src/a.ts");
|
||||
expect(suggestedRef(".../config/MetricGroupCounter.kt")).toBe(
|
||||
"glob:**/config/MetricGroupCounter.kt",
|
||||
);
|
||||
expect(suggestedRef("session.ts")).toBe("glob:**/session.ts");
|
||||
expect(suggestedRef("*PrivacyPolicy.ts")).toBe("glob:**/*PrivacyPolicy.ts");
|
||||
});
|
||||
|
||||
test("slugifyHeading matches GitHub anchors", () => {
|
||||
expect(slugifyHeading("The `@ref` Grammar")).toBe("the-ref-grammar");
|
||||
expect(slugifyHeading("No Line Numbers, Symbol Anchors")).toBe("no-line-numbers-symbol-anchors");
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// resolution
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("a ref to an existing file, dir, symbol, heading, and glob resolves", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export function verifySession() {}\n",
|
||||
"src/entities/oauth/index.ts": "export const x = 1;\n",
|
||||
"docs/DESIGN.md": "# Design\n\n## Session Rules\n\ntext\n",
|
||||
".expo-code-review/agents/security.md":
|
||||
`<!-- ${REF} src/auth.ts — the only session entry point -->\n` +
|
||||
`<!-- ${REF} src/auth.ts#verifySession -->\n` +
|
||||
`<!-- ${REF} src/entities/oauth/ -->\n` +
|
||||
`<!-- ${REF} docs/DESIGN.md#session-rules -->\n` +
|
||||
`<!-- ${REF} glob:src/**/*.ts -->\n`,
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toEqual([]);
|
||||
expect(report.ok).toBe(true);
|
||||
expect(report.refs).toHaveLength(5);
|
||||
expect(report.citedPaths).toContain("src/auth.ts");
|
||||
});
|
||||
|
||||
test("a ref to a moved file, a renamed symbol, or a missing heading is broken", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export function verifySession() {}\n",
|
||||
"docs/DESIGN.md": "# Design\n",
|
||||
".expo-code-review/agents/security.md":
|
||||
`<!-- ${REF} src/moved.ts — gone -->\n` +
|
||||
`<!-- ${REF} src/auth.ts#checkSession -->\n` +
|
||||
`<!-- ${REF} docs/DESIGN.md#session-rules -->\n`,
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.ok).toBe(false);
|
||||
expect(report.problems.map((p) => [p.line, p.kind])).toEqual([
|
||||
[1, "broken-ref"],
|
||||
[2, "broken-ref"],
|
||||
[3, "broken-ref"],
|
||||
]);
|
||||
expect(report.problems[0]!.problem).toContain("no such path");
|
||||
expect(report.problems[1]!.problem).toContain("no longer contains");
|
||||
expect(report.problems[2]!.problem).toContain("no heading");
|
||||
});
|
||||
|
||||
test("line-number refs and citations are refused, in an annotation or in prose", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export function verifySession() {}\n",
|
||||
".expo-code-review/agents/security.md": `<!-- ${REF} src/auth.ts:42 — the check -->\n\nSee \`src/auth.ts:42-51\`.\n`,
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems.every((p) => p.kind === "line-number-ref")).toBe(true);
|
||||
expect(report.problems).toHaveLength(2);
|
||||
});
|
||||
|
||||
test("a ref may never escape the repository", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
".expo-code-review/agents/security.md": `<!-- ${REF} ../outside/secrets.ts -->\n<!-- ${REF} /etc/passwd -->\n`,
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toHaveLength(2);
|
||||
expect(report.problems[0]!.problem).toContain("escapes the repository");
|
||||
expect(report.problems[1]!.problem).toContain("absolute path");
|
||||
});
|
||||
|
||||
test("an unannotated path citation fails, and @ref-ignore or an annotation clears it", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
".expo-code-review/agents/security.md":
|
||||
"Read `src/auth.ts` and `knex.raw()` before flagging.\n",
|
||||
}),
|
||||
);
|
||||
const before = await checkConfigRefs({ root });
|
||||
expect(before.problems).toHaveLength(1);
|
||||
expect(before.problems[0]!.kind).toBe("unannotated-citation");
|
||||
expect(before.problems[0]!.problem).toContain(`${REF} src/auth.ts`);
|
||||
|
||||
const fixed = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
".expo-code-review/agents/security.md":
|
||||
`<!-- ${REF} src/auth.ts — session entry point -->\n` +
|
||||
`<!-- ${REF}-ignore knex.raw() -->\n` +
|
||||
"Read `src/auth.ts` and `knex.raw()` before flagging.\n",
|
||||
}),
|
||||
);
|
||||
expect((await checkConfigRefs({ root: fixed })).problems).toEqual([]);
|
||||
});
|
||||
|
||||
test("extensionless citations count only when they name something real", async () => {
|
||||
const root = await makeRepo({
|
||||
"infrastructure/eas-build-worker/terraform/main.tf": "resource {}\n",
|
||||
"infrastructure/general-central/module/main.tf": "resource {}\n",
|
||||
"infrastructure/finops/main.tf": "resource {}\n",
|
||||
"infrastructure/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infrastructure/.expo-code-review/agents/terraform-safety.md":
|
||||
"State reviewer for `general-central/{module,production}`, `finops`,\n" +
|
||||
"`eas-build-worker/terraform`, on `anthropic/claude-opus-5`.\n",
|
||||
});
|
||||
const report = await checkConfigRefs({ root });
|
||||
const tokens = report.problems.map((problem) => problem.problem);
|
||||
expect(tokens).toHaveLength(3);
|
||||
// the model id is shaped like a path and must never be flagged
|
||||
expect(tokens.some((problem) => problem.includes("claude-opus-5"))).toBe(false);
|
||||
// a scope-relative citation is taught its root-relative ref
|
||||
expect(
|
||||
tokens.some((problem) =>
|
||||
problem.includes(`${REF} infrastructure/eas-build-worker/terraform/ —`),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(tokens.some((problem) => problem.includes(`${REF} infrastructure/finops/ —`))).toBe(true);
|
||||
});
|
||||
|
||||
test("the ref the message suggests silences the citation it was suggested for", async () => {
|
||||
// A scoped prompt names an app by its directory basename; the ref that pins it is
|
||||
// the root-relative path. Coverage has to connect the two, or the suggested fix is
|
||||
// not a fix. Both the plain and the glob form of that path must count.
|
||||
const files = (pin: string): Record<string, string> => ({
|
||||
"infrastructure/cert-manager/kustomization.yaml": "resources: []\n",
|
||||
"infrastructure/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infrastructure/.expo-code-review/agents/k8s.md": `${pin}\n\nApps: \`cert-manager\`.\n`,
|
||||
});
|
||||
|
||||
const unpinned = await checkConfigRefs({ root: await makeRepo(files("")) });
|
||||
expect(unpinned.problems).toHaveLength(1);
|
||||
expect(unpinned.problems[0]!.problem).toContain(`${REF} infrastructure/cert-manager/ —`);
|
||||
|
||||
for (const pin of [
|
||||
`<!-- ${REF} infrastructure/cert-manager/ — the app -->`,
|
||||
`<!-- ${REF} glob:infrastructure/cert-manager/** — the app -->`,
|
||||
]) {
|
||||
expect((await checkConfigRefs({ root: await makeRepo(files(pin)) })).problems).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
test("a scope-relative citation WITH an extension is covered by its precise ref", async () => {
|
||||
const files = (pin: string): Record<string, string> => ({
|
||||
"infrastructure/pgbouncer/base/pdb.yaml": "kind: PodDisruptionBudget\n",
|
||||
"server/www/k8s/pdb.yaml": "kind: PodDisruptionBudget\n",
|
||||
"infrastructure/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infrastructure/.expo-code-review/agents/k8s.md": `${pin}\n\nSee \`pgbouncer/base/pdb.yaml\`.\n`,
|
||||
});
|
||||
|
||||
const unpinned = await checkConfigRefs({ root: await makeRepo(files("")) });
|
||||
expect(unpinned.problems).toHaveLength(1);
|
||||
// the suggestion is the precise path, never a basename glob
|
||||
expect(unpinned.problems[0]!.problem).toContain(
|
||||
`${REF} infrastructure/pgbouncer/base/pdb.yaml —`,
|
||||
);
|
||||
|
||||
const pinned = await checkConfigRefs({
|
||||
root: await makeRepo(
|
||||
files(`<!-- ${REF} infrastructure/pgbouncer/base/pdb.yaml — the PDB this rule is about -->`),
|
||||
),
|
||||
});
|
||||
expect(pinned.problems).toEqual([]);
|
||||
});
|
||||
|
||||
test("a scope-relative wildcard token gets a glob suggestion that itself resolves", async () => {
|
||||
const files = (pin: string): Record<string, string> => ({
|
||||
"infra/pgbouncer/production/patch.yaml": "x\n",
|
||||
"infra/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infra/.expo-code-review/agents/a.md": `${pin}\nSee \`pgbouncer/*/patch.yaml\`.\n`,
|
||||
});
|
||||
const report = await checkConfigRefs({ root: await makeRepo(files("")) });
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.problem).toContain(`${REF} glob:infra/pgbouncer/*/patch.yaml`);
|
||||
// …and that suggestion, once written, clears the citation and resolves
|
||||
const pinned = await checkConfigRefs({
|
||||
root: await makeRepo(files(`<!-- ${REF} glob:infra/pgbouncer/*/patch.yaml — the patches -->`)),
|
||||
});
|
||||
expect(pinned.problems).toEqual([]);
|
||||
});
|
||||
|
||||
test("scope wins over root when both could satisfy an extensionless citation", async () => {
|
||||
const root = await makeRepo({
|
||||
"alerts/root.txt": "x\n",
|
||||
"infra/alerts/scope.tf": "y\n",
|
||||
"infra/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infra/.expo-code-review/agents/a.md": "Watch `alerts/` closely.\n",
|
||||
});
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.problem).toContain(`${REF} infra/alerts/ —`);
|
||||
});
|
||||
|
||||
test("a token carrying its own wildcard keeps the glob suggestion", async () => {
|
||||
const root = await makeRepo({
|
||||
".github/workflows/deploy.yml": "on: push\n",
|
||||
".expo-code-review/config.jsonc": "{}\n",
|
||||
".expo-code-review/agents/sec.md": "Changes under `.github/workflows/**` are high risk.\n",
|
||||
});
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems[0]!.problem).toContain(`${REF} glob:.github/workflows/**`);
|
||||
});
|
||||
|
||||
test("a scope-relative annotated ref is broken, and names the root-relative fix", async () => {
|
||||
const root = await makeRepo({
|
||||
"infrastructure/general-central/module/main.tf": "resource {}\n",
|
||||
"infrastructure/.expo-code-review/config.jsonc": "{}\n",
|
||||
"infrastructure/.expo-code-review/agents/terraform-safety.md": `<!-- ${REF} general-central/module/ — the module tree -->\n`,
|
||||
});
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.kind).toBe("broken-ref");
|
||||
expect(report.problems[0]!.problem).toContain(
|
||||
"did you mean infrastructure/general-central/module",
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// structural refs
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("routing: a scope with no setup dir, an unknown enforced agent, and a dead glob all fail", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/a.ts": "export const a = 1;\n",
|
||||
".expo-code-review/routing.jsonc": JSON.stringify({
|
||||
defaults: { enforceAgents: ["security", "ghost"] },
|
||||
scopes: [
|
||||
{ name: "default", paths: ["**/*"], config: "." },
|
||||
{ name: "billing", paths: ["nope/**"], config: "packages/billing" },
|
||||
],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
const problems = report.problems.filter((p) => p.kind === "structural").map((p) => p.problem);
|
||||
expect(problems).toHaveLength(3);
|
||||
expect(problems.some((p) => p.includes('scope "billing" has no packages/billing/'))).toBe(true);
|
||||
expect(problems.some((p) => p.includes("agents/ghost.md"))).toBe(true);
|
||||
expect(problems.some((p) => p.includes("matches no file in the repo: nope/**"))).toBe(true);
|
||||
});
|
||||
|
||||
test("enforceAgents is judged against the manifest's own setup dir, not the code tree", async () => {
|
||||
// The `ecr ci` shape: setup from a trusted base-ref checkout, code from the PR head.
|
||||
const code = await makeRepo({ "src/a.ts": "export const a = 1;\n" });
|
||||
const trustedBase = await makeRepo({
|
||||
".expo-code-review/agents/security.md": "# Security\n",
|
||||
".expo-code-review/routing.jsonc": JSON.stringify({
|
||||
defaults: { enforceAgents: ["security"] },
|
||||
scopes: [{ name: "default", paths: ["**/*"], config: "." }],
|
||||
}),
|
||||
});
|
||||
const report = await checkConfigRefs({
|
||||
root: code,
|
||||
setupDirs: [path.join(trustedBase, ".expo-code-review")],
|
||||
});
|
||||
expect(report.problems.filter((problem) => problem.problem.includes("enforceAgents"))).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test("scope setup dirs are swept too, and dot dirs (other worktrees) are not", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"packages/billing/.expo-code-review/agents/money.md": `<!-- ${REF} src/gone.ts -->\n`,
|
||||
".claude/worktrees/other/.expo-code-review/agents/x.md": `<!-- ${REF} src/gone.ts -->\n`,
|
||||
}),
|
||||
);
|
||||
const dirs = await discoverSetupDirs(root);
|
||||
expect(dirs.map((d) => path.relative(root, d)).sort()).toEqual([
|
||||
".expo-code-review",
|
||||
path.join("packages", "billing", ".expo-code-review"),
|
||||
]);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.file).toBe(
|
||||
path.join("packages", "billing", ".expo-code-review", "agents", "money.md"),
|
||||
);
|
||||
});
|
||||
|
||||
test("one ref covers the prose form of the same citation", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/commands/review.ts": "export const a = 1;\n",
|
||||
"src/core/util.ts": "export function errorMessage() {}\n",
|
||||
"docs/deep/nested/Handler.kt": "class Handler\n",
|
||||
".expo-code-review/agents/security.md":
|
||||
`<!-- ${REF} glob:src/commands/*.ts — the command shape -->\n` +
|
||||
`<!-- ${REF} src/core/util.ts#errorMessage — the error path -->\n` +
|
||||
`<!-- ${REF} glob:**/Handler.kt — the handlers -->\n` +
|
||||
"Commands in `src/commands/*.ts` report via `src/core/util.ts`, like `.../nested/Handler.kt`.\n",
|
||||
}),
|
||||
);
|
||||
expect((await checkConfigRefs({ root })).problems).toEqual([]);
|
||||
});
|
||||
|
||||
test("annotations inside a fenced block document the grammar and are not resolved", () => {
|
||||
const text = `<!-- ${REF} src/real.ts -->\n\n\`\`\`md\n<!-- ${REF} src/example.ts — how to write one -->\n\`\`\`\n`;
|
||||
expect(parseRefAnnotations(text, "agents/x.md").map((ref) => ref.target)).toEqual([
|
||||
"src/real.ts",
|
||||
]);
|
||||
});
|
||||
|
||||
test("an LLP target belongs to another mechanism: not resolved, not counted", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
".expo-code-review/agents/security.md":
|
||||
`<!-- ${REF} LLP 0009#config-and-prompt-templates [implements] — engine corpus -->\n` +
|
||||
`<!-- ${REF} LLP 4 -->\n`,
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(report.problems).toEqual([]);
|
||||
expect(report.refs).toEqual([]);
|
||||
expect(report.citedPaths).toEqual([]);
|
||||
});
|
||||
|
||||
test("a target starting with < is a documented placeholder, not a citation", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
".expo-code-review/agents/security.md": `<!-- ${REF} <path/to/file.ts>#<symbol> — example -->\n`,
|
||||
}),
|
||||
);
|
||||
expect((await checkConfigRefs({ root })).problems).toEqual([]);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// review-side signal
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("reviewSetupRefNotes advises on broken refs and on cited code the PR changes", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
".expo-code-review/agents/security.md": `<!-- ${REF} src/auth.ts — entry point -->\n<!-- ${REF} src/gone.ts — moved -->\n`,
|
||||
}),
|
||||
);
|
||||
const setupDir = path.join(root, ".expo-code-review");
|
||||
const notes = await reviewSetupRefNotes({
|
||||
root,
|
||||
setupDirs: [setupDir],
|
||||
changedFiles: ["src/auth.ts"],
|
||||
});
|
||||
expect(notes).toHaveLength(2);
|
||||
expect(notes[0]).toContain("no longer resolves");
|
||||
expect(notes[0]).toContain("ecr ref-check");
|
||||
expect(notes[1]).toContain("src/auth.ts");
|
||||
|
||||
// An unannotated citation is a command-level failure, not review advice.
|
||||
const prose = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
".expo-code-review/agents/security.md": "Read `src/auth.ts`.\n",
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
await reviewSetupRefNotes({
|
||||
root: prose,
|
||||
setupDirs: [path.join(prose, ".expo-code-review")],
|
||||
changedFiles: [],
|
||||
}),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
test("a ref to scrubbed ambient config is not review advice (the read root removes it)", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
".expo-code-review/shared.md":
|
||||
`<!-- ${REF} AGENTS.md — the conventions to judge against -->\n` +
|
||||
`<!-- ${REF} CLAUDE.md#conventions — same, with an anchor -->\n` +
|
||||
`<!-- ${REF} glob:**/.env.local — same, as a glob -->\n` +
|
||||
`<!-- ${REF} src/gone.ts — really moved -->\n`,
|
||||
}),
|
||||
);
|
||||
const setupDir = path.join(root, ".expo-code-review");
|
||||
const notes = await reviewSetupRefNotes({ root, setupDirs: [setupDir], changedFiles: [] });
|
||||
expect(notes).toHaveLength(1);
|
||||
expect(notes[0]).toContain("1 ref(s)");
|
||||
expect(notes[0]).not.toContain("shared.md:1");
|
||||
|
||||
// The standalone gate still reports all of them — there the tree is a real checkout.
|
||||
expect((await checkConfigRefs({ root })).problems).toHaveLength(4);
|
||||
});
|
||||
|
||||
test("setup problems are labelled by setup-dir position when the config lives outside the tree", async () => {
|
||||
const code = await makeRepo({ "src/auth.ts": "export const a = 1;\n" });
|
||||
const trustedBase = await makeRepo(
|
||||
setup({ ".expo-code-review/agents/security.md": `<!-- ${REF} src/gone.ts -->\n` }),
|
||||
);
|
||||
const report = await checkConfigRefs({
|
||||
root: code,
|
||||
setupDirs: [path.join(trustedBase, ".expo-code-review")],
|
||||
});
|
||||
expect(report.problems).toHaveLength(1);
|
||||
expect(report.problems[0]!.file).toBe(path.join(".expo-code-review", "agents", "security.md"));
|
||||
});
|
||||
|
||||
test("citedPathsTouchedBy reports cited paths a PR changed, dirs included", async () => {
|
||||
const root = await makeRepo(
|
||||
setup({
|
||||
"src/auth.ts": "export const a = 1;\n",
|
||||
"src/entities/oauth/index.ts": "export const b = 2;\n",
|
||||
".expo-code-review/agents/security.md": `<!-- ${REF} src/auth.ts — entry point -->\n<!-- ${REF} src/entities/oauth/ -->\n`,
|
||||
}),
|
||||
);
|
||||
const report = await checkConfigRefs({ root });
|
||||
expect(citedPathsTouchedBy(report, ["src/auth.ts", "README.md"])).toEqual(["src/auth.ts"]);
|
||||
expect(citedPathsTouchedBy(report, ["src/entities/oauth/token.ts"])).toEqual([
|
||||
"src/entities/oauth",
|
||||
]);
|
||||
expect(citedPathsTouchedBy(report, ["README.md"])).toEqual([]);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// the command's argument parsing
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test("--root refuses a flag-shaped value instead of checking a nonexistent tree", async () => {
|
||||
await refCheckCommand(["--root", "--json"]);
|
||||
expect(process.exitCode).toBe(2);
|
||||
process.exitCode = 0;
|
||||
|
||||
await refCheckCommand(["--root"]);
|
||||
expect(process.exitCode).toBe(2);
|
||||
process.exitCode = 0;
|
||||
});
|
||||
@@ -47,7 +47,7 @@ test("resolveEngines: a scope selecting an anthropic model adds the Claude Code
|
||||
const root = await makeOpencodeRoot();
|
||||
// A nested scope pins an anthropic/… model — the scoped review needs the Claude
|
||||
// CLI/login even though the OpenCode-only root config never touches it.
|
||||
await writeConfigDir(path.join(root, "apps", "api", CONFIG_DIRNAME), {
|
||||
await writeConfigDir(path.join(root, "server", "www", CONFIG_DIRNAME), {
|
||||
config: '{ "model": "anthropic/claude-opus-5" }',
|
||||
agents: { style: agent("style") },
|
||||
});
|
||||
@@ -55,7 +55,7 @@ test("resolveEngines: a scope selecting an anthropic model adds the Claude Code
|
||||
path.join(root, CONFIG_DIRNAME, "routing.jsonc"),
|
||||
`{ "scopes": [
|
||||
{ "name": "default", "paths": ["**/*"], "config": "." },
|
||||
{ "name": "api", "paths": ["apps/api/**"], "config": "apps/api" }
|
||||
{ "name": "www", "paths": ["server/www/**"], "config": "server/www" }
|
||||
] }`,
|
||||
"utf8",
|
||||
);
|
||||
@@ -63,7 +63,7 @@ test("resolveEngines: a scope selecting an anthropic model adds the Claude Code
|
||||
const manifest = await loadRoutingManifest(root);
|
||||
const engines = await resolveEngines(root, rootConfig, manifest);
|
||||
// Without folding scopes in, this would be opencode-only and doctor would skip the
|
||||
// Claude CLI/login checks the api scope's review actually needs.
|
||||
// Claude CLI/login checks the www scope's review actually needs.
|
||||
expect(engines.has("claude-code")).toBe(true);
|
||||
expect(engines.has("opencode")).toBe(true);
|
||||
});
|
||||
|
||||
@@ -37,14 +37,14 @@ test("loadScopeConfig: nested scope inherits the ROOT feedback and stack config"
|
||||
}`,
|
||||
agents: { security: agent("security") },
|
||||
});
|
||||
await writeConfigDir(path.join(root, "apps", "api", CONFIG_DIRNAME), {
|
||||
await writeConfigDir(path.join(root, "server", "www", CONFIG_DIRNAME), {
|
||||
config: "{}",
|
||||
agents: { style: agent("style") },
|
||||
});
|
||||
const manifest = RoutingManifestSchema.parse({
|
||||
scopes: [
|
||||
{ name: "default", paths: ["**/*"], config: "." },
|
||||
{ name: "api", paths: ["apps/api/**"], config: "apps/api" },
|
||||
{ name: "www", paths: ["server/www/**"], config: "server/www" },
|
||||
],
|
||||
});
|
||||
const rootConfig = await loadReviewConfig(root);
|
||||
|
||||
@@ -5,7 +5,6 @@ import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
import { initCommand, parseTokenEnvs, substituteTokenEnv } from "../commands/init.js";
|
||||
import { checkConfigRefs } from "../core/config-refs.js";
|
||||
import { git } from "../core/exec.js";
|
||||
import { CONFIG_DIRNAME } from "../config/load.js";
|
||||
|
||||
@@ -132,7 +131,7 @@ test("init --token-env refuses when review workflows already exist without --for
|
||||
path.join(root, ".github", "workflows", "expo-code-review.yml"),
|
||||
"utf8",
|
||||
);
|
||||
expect(text).toContain("CLAUDE_CODE_REVIEW_SHARED_API_TOKEN");
|
||||
expect(text).toContain("OPENAI_API_KEY");
|
||||
expect(text).not.toContain("CLAUDE_CODE_OAUTH_TOKEN");
|
||||
});
|
||||
|
||||
@@ -143,7 +142,7 @@ test("init --token-env --force rewrites existing review workflows", async () =>
|
||||
for (const file of ["expo-code-review.yml", "expo-code-review-command.yml"]) {
|
||||
const text = await readFile(path.join(root, ".github", "workflows", file), "utf8");
|
||||
expect(text).toContain("CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}");
|
||||
expect(text).not.toContain("CLAUDE_CODE_REVIEW_SHARED_API_TOKEN");
|
||||
expect(text).not.toContain("OPENAI_API_KEY");
|
||||
}
|
||||
});
|
||||
|
||||
@@ -210,7 +209,7 @@ test("init --force-workflows refuses when only the forwarded secret line is non-
|
||||
// The fallback alone reads as default, so the guard must also see the secret line.
|
||||
const wf = path.join(root, ".github", "workflows", "expo-code-review.yml");
|
||||
const patched = (await readFile(wf, "utf8")).replaceAll(
|
||||
"CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}",
|
||||
"OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}",
|
||||
"ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}",
|
||||
);
|
||||
await writeFile(wf, patched, "utf8");
|
||||
@@ -220,7 +219,7 @@ test("init --force-workflows refuses when only the forwarded secret line is non-
|
||||
process.exitCode = 0;
|
||||
expect(err).toContain("ANTHROPIC_API_KEY");
|
||||
expect(err).toContain("--token-env");
|
||||
// Untouched: no silent reversion to the default wiring.
|
||||
// Untouched: no silent reversion to the default OpenAI wiring.
|
||||
expect(await readFile(wf, "utf8")).toContain(
|
||||
"ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}",
|
||||
);
|
||||
@@ -229,7 +228,7 @@ test("init --force-workflows refuses when only the forwarded secret line is non-
|
||||
test("init --force-workflows without --token-env is fine on a default-credential repo", async () => {
|
||||
const root = await freshRepo();
|
||||
await initCommand([]);
|
||||
// Default credential wiring: nothing to revert, so no refusal.
|
||||
// Default OPENAI_API_KEY wiring: nothing to revert, so no refusal.
|
||||
await initCommand(["--force-workflows"]);
|
||||
expect(process.exitCode ?? 0).toBe(0);
|
||||
expect(
|
||||
@@ -240,8 +239,8 @@ test("init --force-workflows without --token-env is fine on a default-credential
|
||||
test("init --token-env prints the config.jsonc auth edit as a next step", async () => {
|
||||
await freshRepo();
|
||||
const out = await captureStdout(() => initCommand(["--token-env", "CLAUDE_CODE_OAUTH_TOKEN"]));
|
||||
// The scaffolded config.jsonc still declares the default tokenEnv, so without
|
||||
// this step CI's verify-config fails and the user has no pointer to why.
|
||||
// The scaffolded config.jsonc still declares OPENAI_API_KEY, so without this
|
||||
// step CI's verify-config fails and the user has no pointer to why.
|
||||
expect(out).toContain("config.jsonc at this credential");
|
||||
expect(out).toContain("refuses to review until the config names `CLAUDE_CODE_OAUTH_TOKEN`");
|
||||
});
|
||||
@@ -275,7 +274,7 @@ test("substituteTokenEnv fails loudly when the template markers drift", () => {
|
||||
});
|
||||
|
||||
test("parseTokenEnvs validates names and refuses well-known unrelated secrets", () => {
|
||||
expect(parseTokenEnvs(undefined)).toEqual(["CLAUDE_CODE_REVIEW_SHARED_API_TOKEN"]);
|
||||
expect(parseTokenEnvs(undefined)).toEqual(["OPENAI_API_KEY"]);
|
||||
expect(parseTokenEnvs("A_TOKEN, B_TOKEN")).toEqual(["A_TOKEN", "B_TOKEN"]);
|
||||
expect(() => parseTokenEnvs("lower_case")).toThrow(/UPPER_SNAKE_CASE/);
|
||||
expect(() => parseTokenEnvs("GH_TOKEN")).toThrow(/unrelated secret/);
|
||||
@@ -303,13 +302,3 @@ test("every `uses:` in templates/*.yml is pinned by 40-hex commit SHA with a ver
|
||||
// Each template pins checkout + setup-node (+ upload-artifact in workflow.yml).
|
||||
expect(usesLines).toBeGreaterThanOrEqual(7);
|
||||
});
|
||||
|
||||
// @ref LLP 0012#what-gets-scanned [constrained-by] — a scaffold must pass `ecr ref-check` on day one
|
||||
test("a freshly scaffolded repo passes ref-check", async () => {
|
||||
const root = await freshRepo();
|
||||
await initCommand(["--monorepo"]);
|
||||
|
||||
// LLP annotations the templates carry belong to the engine's own corpus and are
|
||||
// skipped here, not reported: ecr only judges citations into the reviewed repo.
|
||||
expect((await checkConfigRefs({ root })).problems).toEqual([]);
|
||||
});
|
||||
@@ -52,7 +52,7 @@ test("worstDecision: picks the most severe across scopes", () => {
|
||||
test("aggregate: root marker first line, a table row + <details> per scope, worst decision at top", () => {
|
||||
const results = [
|
||||
scope("default", true, { decision: "approve", findings: [finding({ title: "D1" })] }),
|
||||
scope("api", false, {
|
||||
scope("www", false, {
|
||||
decision: "request_changes",
|
||||
findings: [finding({ title: "W1", severity: "critical", category: "security" })],
|
||||
}),
|
||||
@@ -62,22 +62,22 @@ test("aggregate: root marker first line, a table row + <details> per scope, wors
|
||||
expect(body).toContain("**Decision:** Request changes"); // worst across scopes
|
||||
expect(body).toContain("| Scope | Decision | Findings |");
|
||||
expect(body).toContain("| default | Approve | 1 |");
|
||||
expect(body).toContain("| api | Request changes | 1 |");
|
||||
expect(body).toContain("| www | Request changes | 1 |");
|
||||
// one details block per scope, in manifest order
|
||||
expect(body.indexOf("<summary>default")).toBeLessThan(body.indexOf("<summary>api"));
|
||||
expect((body.match(/<summary>(default|api) —/g) ?? []).length).toBe(2);
|
||||
expect(body.indexOf("<summary>default")).toBeLessThan(body.indexOf("<summary>www"));
|
||||
expect((body.match(/<summary>(default|www) —/g) ?? []).length).toBe(2);
|
||||
});
|
||||
|
||||
test("aggregate: default scope uses plain fingerprints, non-default uses scoped ids; embedded block is the union", () => {
|
||||
const dFinding = finding({ title: "D", evidence: "const defaultScopedThing = 1;" });
|
||||
const wFinding = finding({ title: "W", evidence: "const apiScopedThing = 2;" });
|
||||
const wFinding = finding({ title: "W", evidence: "const wwwScopedThing = 2;" });
|
||||
const results = [
|
||||
scope("default", true, { findings: [dFinding] }),
|
||||
scope("api", false, { findings: [wFinding] }),
|
||||
scope("www", false, { findings: [wFinding] }),
|
||||
];
|
||||
const body = renderAggregateMarkdown(results, "tag", []);
|
||||
const plainId = fingerprintFinding(dFinding);
|
||||
const scopedId = scopedFingerprint("api", wFinding);
|
||||
const scopedId = scopedFingerprint("www", wFinding);
|
||||
expect(body).toContain(`id:${plainId}`);
|
||||
expect(body).toContain(`id:${scopedId}`);
|
||||
const embedded = parseEmbeddedFingerprints(body, "tag");
|
||||
@@ -87,17 +87,13 @@ test("aggregate: default scope uses plain fingerprints, non-default uses scoped
|
||||
|
||||
test("aggregate: state round-trips through parseReviewState (scopes intact); a v1 body still parses", () => {
|
||||
const results = [
|
||||
{
|
||||
...scope("default", true, { findings: [finding({ title: "D" })] }),
|
||||
inputHash: "a".repeat(64),
|
||||
},
|
||||
{ ...scope("api", false, { findings: [finding({ title: "W" })] }), inputHash: "b".repeat(64) },
|
||||
scope("default", true, { findings: [finding({ title: "D" })] }),
|
||||
scope("www", false, { findings: [finding({ title: "W" })] }),
|
||||
];
|
||||
const body = renderAggregateMarkdown(results, "tag", []);
|
||||
const state = parseReviewState(body, "tag");
|
||||
expect(state).not.toBeNull();
|
||||
expect(state!.scopes?.map((s) => s.scope)).toEqual(["default", "api"]);
|
||||
expect(state!.scopes?.map((s) => s.inputHash)).toEqual(["a".repeat(64), "b".repeat(64)]);
|
||||
expect(state!.scopes?.map((s) => s.scope)).toEqual(["default", "www"]);
|
||||
|
||||
// A pre-routing v1 comment still parses (no scopes field).
|
||||
const v1 = renderMarkdown(review({ findings: [finding()] }), "tag");
|
||||
@@ -121,8 +117,8 @@ test("aggregate: a default-scope finding dismissed pre-routing (plain fp) lands
|
||||
test("aggregate: dismissed findings survive in embedded state so /undismiss can restore them", () => {
|
||||
const dFinding = finding({ title: "DismissedOne", evidence: "const dismissedThing = 1;" });
|
||||
const kFinding = finding({ title: "KeptOne", evidence: "const keptThing = 2;" });
|
||||
const results = [scope("api", false, { findings: [dFinding, kFinding] })];
|
||||
const fp = scopedFingerprint("api", dFinding);
|
||||
const results = [scope("www", false, { findings: [dFinding, kFinding] })];
|
||||
const fp = scopedFingerprint("www", dFinding);
|
||||
const body = renderAggregateMarkdown(results, "tag", [{ fp, by: "me", reason: "known" }]);
|
||||
expect(body).toContain("Dismissed on this PR (1)");
|
||||
|
||||
@@ -136,7 +132,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("| www | Approve | 2 |");
|
||||
expect(restored).not.toContain("Dismissed on this PR");
|
||||
});
|
||||
|
||||
@@ -147,11 +143,11 @@ test("aggregate: a freshly-reviewed scope's per-scope feedback array is stripped
|
||||
// top-level feedback array is the single source of truth, so the per-scope copy must
|
||||
// not survive into the embedded state.
|
||||
const f = finding({ title: "F", evidence: "const perScopeFeedbackStrip = 1;" });
|
||||
const fp = scopedFingerprint("api", f);
|
||||
const fp = scopedFingerprint("www", f);
|
||||
const staleRecord = { fp, by: "octocat", commentId: 9, maintainer: true, applied: false };
|
||||
const results: ScopeReviewResult[] = [
|
||||
{
|
||||
scope: "api",
|
||||
scope: "www",
|
||||
isDefault: false,
|
||||
review: {
|
||||
decision: "approve",
|
||||
@@ -164,8 +160,8 @@ test("aggregate: a freshly-reviewed scope's per-scope feedback array is stripped
|
||||
];
|
||||
const body = renderAggregateMarkdown(results, "tag", [], undefined, undefined, [staleRecord]);
|
||||
const state = parseReviewState(body, "tag")!;
|
||||
const apiScope = state.scopes!.find((s) => s.scope === "api")!;
|
||||
expect((apiScope.review as { feedback?: unknown }).feedback).toBeUndefined();
|
||||
const wwwScope = state.scopes!.find((s) => s.scope === "www")!;
|
||||
expect((wwwScope.review as { feedback?: unknown }).feedback).toBeUndefined();
|
||||
// The record still rides the top-level feedback array (the single source of truth).
|
||||
expect(state.feedback).toHaveLength(1);
|
||||
});
|
||||
@@ -175,9 +171,9 @@ test("aggregate: a scope whose reply cleared every finding opens its fold (visib
|
||||
// and requalifiedAll are both empty. The fold must still open so the audit note is
|
||||
// visible, matching renderMarkdown and LLP 0010's visible-suppression rule.
|
||||
const f = finding({ title: "Cleared by reply", evidence: "const replyClearedThing = 1;" });
|
||||
const fp = scopedFingerprint("api", f);
|
||||
const fp = scopedFingerprint("www", f);
|
||||
const body = renderAggregateMarkdown(
|
||||
[scope("api", false, { findings: [f] })],
|
||||
[scope("www", false, { findings: [f] })],
|
||||
"tag",
|
||||
[],
|
||||
undefined,
|
||||
@@ -194,9 +190,9 @@ 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>www — Approve (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).toMatch(/<details open>\n<summary>www —/);
|
||||
expect(body).toContain("have an author response");
|
||||
});
|
||||
|
||||
@@ -209,7 +205,7 @@ test('aggregate: oversized findings truncate with a "+N more" note and stay unde
|
||||
rationale: "x".repeat(300),
|
||||
}),
|
||||
);
|
||||
const results = [scope("api", false, { decision: "request_changes", findings: many })];
|
||||
const results = [scope("www", false, { decision: "request_changes", findings: many })];
|
||||
const body = renderAggregateMarkdown(results, "tag", []);
|
||||
expect(body.length).toBeLessThan(65_000);
|
||||
expect(body).toContain("more finding(s) — see the workflow log.");
|
||||
@@ -230,11 +226,11 @@ test("aggregate: unmatched files render as a coverage note", () => {
|
||||
// would silently release a restore no later run could recover.
|
||||
test("aggregate: the /undismiss pin set survives a render with no records and a truncating one", () => {
|
||||
const pinned = finding({ title: "Pinned", evidence: "const pinnedEvidence = 1;" });
|
||||
const pinnedFp = scopedFingerprint("api", pinned);
|
||||
const pinnedFp = scopedFingerprint("www", pinned);
|
||||
const pins = [{ fp: pinnedFp, commentId: 42 }];
|
||||
// No feedback record matched this run (the author edited the reply away).
|
||||
const plain = renderAggregateMarkdown(
|
||||
[scope("api", false, { findings: [pinned] })],
|
||||
[scope("www", false, { findings: [pinned] })],
|
||||
"tag",
|
||||
[],
|
||||
undefined,
|
||||
@@ -253,12 +249,7 @@ test("aggregate: the /undismiss pin set survives a render with no records and a
|
||||
}),
|
||||
);
|
||||
const big = renderAggregateMarkdown(
|
||||
[
|
||||
{
|
||||
...scope("api", false, { decision: "request_changes", findings: [pinned, ...many] }),
|
||||
inputHash: "c".repeat(64),
|
||||
},
|
||||
],
|
||||
[scope("www", false, { decision: "request_changes", findings: [pinned, ...many] })],
|
||||
"tag",
|
||||
[],
|
||||
undefined,
|
||||
@@ -268,5 +259,4 @@ test("aggregate: the /undismiss pin set survives a render with no records and a
|
||||
);
|
||||
expect(big.length).toBeLessThan(65_000);
|
||||
expect(parseReviewState(big, "tag")!.pins).toEqual(pins);
|
||||
expect(parseReviewState(big, "tag")!.scopes![0]!.inputHash).toBeUndefined();
|
||||
});
|
||||
@@ -31,18 +31,6 @@ test("parseEmbeddedFingerprints round-trips even with a regex-metachar comment t
|
||||
expect(parseEmbeddedFingerprints(body, tag).length).toBe(1);
|
||||
});
|
||||
|
||||
test("setup note renders only when there is ref advice, and never as a finding", () => {
|
||||
expect(renderMarkdown(base, "tag")).not.toContain("Review setup");
|
||||
const body = renderMarkdown(
|
||||
{ ...base, setupNotes: ["The reviewer setup cites code that no longer resolves (1 ref(s))."] },
|
||||
"tag",
|
||||
);
|
||||
expect(body).toContain("🔗 **Review setup:**");
|
||||
expect(body).toContain("no longer resolves");
|
||||
// advice does not change the decision or the findings list
|
||||
expect(body).toContain("No findings.");
|
||||
});
|
||||
|
||||
test("coverage note only renders when incomplete is non-empty (no more wolf-crying)", () => {
|
||||
expect(renderMarkdown(base, "tag")).not.toContain("Coverage note");
|
||||
expect(renderMarkdown({ ...base, incomplete: ["a pass timed out"] }, "tag")).toContain(
|
||||
@@ -70,14 +58,6 @@ test("review state (review + dismissals) round-trips via parseReviewState", () =
|
||||
expect(state!.review.findings.length).toBe(1);
|
||||
});
|
||||
|
||||
test("review input hash round-trips only in hidden comment state", () => {
|
||||
const inputHash = "a".repeat(64);
|
||||
const body = renderMarkdown(base, "tag", [], undefined, [], [], inputHash);
|
||||
expect(parseReviewState(body, "tag")!.inputHash).toBe(inputHash);
|
||||
expect(Buffer.from(inputHash).toString("base64")).not.toBe(inputHash);
|
||||
expect(body).not.toContain(inputHash);
|
||||
});
|
||||
|
||||
test("links a finding location to the PR diff line when the line is in the diff", () => {
|
||||
const out = renderMarkdown(
|
||||
{ ...base, findings: [finding({ file: "src/a.ts", line: 12 })] },
|
||||
|
||||
@@ -1,173 +0,0 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import type { LoadedConfig } from "../config/schema.js";
|
||||
import { reviewCanBeReused, reviewInputHash, reviewMatchesInput } from "../core/review-cache.js";
|
||||
import type { CoordinatorOutput, DiffEntry } from "../core/schema.js";
|
||||
|
||||
const config = (over: Partial<LoadedConfig> = {}): LoadedConfig => ({
|
||||
configDir: "/tmp/config-a",
|
||||
sharedPromptText: "shared rules",
|
||||
agents: [
|
||||
{
|
||||
id: "correctness",
|
||||
description: "bugs",
|
||||
alwaysRun: false,
|
||||
model: "anthropic/claude-sonnet-5",
|
||||
temperature: 0.1,
|
||||
tools: { read: true },
|
||||
promptText: "find bugs",
|
||||
},
|
||||
],
|
||||
coordinator: {
|
||||
model: "anthropic/claude-sonnet-5",
|
||||
temperature: 0,
|
||||
promptText: "dedupe",
|
||||
},
|
||||
policy: { includeSuggestions: false },
|
||||
chunk: { maxChangedLines: 1000, maxFiles: 20 },
|
||||
noise: { additionalIgnores: [], additionalMarkers: [] },
|
||||
breakGlassMarker: "/skip-review",
|
||||
commentTag: "review",
|
||||
auth: [{ provider: "anthropic", mode: "oauth", tokenEnv: "TOKEN" }],
|
||||
review: { trigger: "all", label: "ai-review", skipLabel: "ai-review:skip" },
|
||||
stack: {
|
||||
enabled: false,
|
||||
maxDepth: 4,
|
||||
maxPrs: 8,
|
||||
maxFilesPerPr: 100,
|
||||
requireSameAuthor: true,
|
||||
confirmWithPatch: false,
|
||||
maxConfirmations: 10,
|
||||
},
|
||||
feedback: {
|
||||
mode: "annotate",
|
||||
match: "both",
|
||||
dismiss: "never",
|
||||
protectedCategories: ["security", "secrets"],
|
||||
maxAdjudications: 10,
|
||||
},
|
||||
...over,
|
||||
});
|
||||
|
||||
const files: DiffEntry[] = [
|
||||
{
|
||||
path: "b.ts",
|
||||
patch: "diff --git a/b.ts b/b.ts\nindex 111..222 100644\n@@ -1 +1 @@\n-old\n+new",
|
||||
status: "M",
|
||||
},
|
||||
{ path: "a.ts", patch: "diff --git a/a.ts b/a.ts\n@@ -0,0 +1 @@\n+export {};", status: "A" },
|
||||
];
|
||||
|
||||
async function fixtureRoot(): Promise<string> {
|
||||
const root = await mkdtemp(path.join(tmpdir(), "ecr-review-cache-test-"));
|
||||
await mkdir(root, { recursive: true });
|
||||
await Promise.all([
|
||||
writeFile(path.join(root, "a.ts"), "export {};\n"),
|
||||
writeFile(path.join(root, "b.ts"), "new\n"),
|
||||
]);
|
||||
return root;
|
||||
}
|
||||
|
||||
const hash = (
|
||||
readRoot: string,
|
||||
over: Partial<Parameters<typeof reviewInputHash>[0]> = {},
|
||||
): Promise<string> =>
|
||||
reviewInputHash({
|
||||
files,
|
||||
config: config(),
|
||||
metadata: { title: "Change", body: "Why" },
|
||||
readRoot,
|
||||
agents: ["security", "correctness"],
|
||||
route: true,
|
||||
contextText: "plan",
|
||||
...over,
|
||||
});
|
||||
|
||||
test("review input hash ignores restack-only blob ids, hunk offsets, and ordering", async () => {
|
||||
const root = await fixtureRoot();
|
||||
try {
|
||||
const first = await hash(root);
|
||||
const restacked = files.map((file) =>
|
||||
file.path === "b.ts"
|
||||
? { ...file, patch: file.patch.replace("111..222", "aaa..bbb").replace("-1 +1", "-9 +9") }
|
||||
: file,
|
||||
);
|
||||
const second = await hash(root, {
|
||||
files: restacked.reverse(),
|
||||
config: config({ configDir: "/another/ephemeral/base-worktree" }),
|
||||
agents: ["correctness", "security"],
|
||||
});
|
||||
expect(first).toBe(second);
|
||||
expect(first).toMatch(/^[a-f0-9]{64}$/);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("review input hash changes with code, prompts, PR prose, selection, or context", async () => {
|
||||
const root = await fixtureRoot();
|
||||
try {
|
||||
const original = await hash(root);
|
||||
expect(
|
||||
await hash(root, { files: [{ ...files[0]!, patch: "+different" }, files[1]!] }),
|
||||
).not.toBe(original);
|
||||
await writeFile(path.join(root, "b.ts"), "different\n");
|
||||
expect(await hash(root)).not.toBe(original);
|
||||
await writeFile(path.join(root, "b.ts"), "new\n");
|
||||
expect(await hash(root, { config: config({ sharedPromptText: "new rules" }) })).not.toBe(
|
||||
original,
|
||||
);
|
||||
expect(await hash(root, { metadata: { title: "Renamed", body: "Why" } })).not.toBe(original);
|
||||
expect(await hash(root, { agents: ["correctness"] })).not.toBe(original);
|
||||
expect(await hash(root, { route: false })).not.toBe(original);
|
||||
expect(await hash(root, { contextText: "new plan" })).not.toBe(original);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
const review = (over: Partial<CoordinatorOutput> = {}): CoordinatorOutput => ({
|
||||
decision: "approve",
|
||||
findings: [],
|
||||
summary: "ok",
|
||||
incomplete: [],
|
||||
...over,
|
||||
});
|
||||
|
||||
test("only a complete review with the exact stored hash can be reused", async () => {
|
||||
const root = await fixtureRoot();
|
||||
try {
|
||||
const current = await hash(root);
|
||||
expect(reviewCanBeReused(review())).toBe(true);
|
||||
expect(reviewMatchesInput(review(), current, current)).toBe(true);
|
||||
expect(reviewMatchesInput(review(), "bad", current)).toBe(false);
|
||||
expect(reviewMatchesInput(review({ incomplete: ["timed out"] }), current, current)).toBe(false);
|
||||
expect(reviewMatchesInput(review({ couldNotComplete: true }), current, current)).toBe(false);
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("file hashing refuses traversal and never follows a PR-controlled symlink", async () => {
|
||||
const root = await fixtureRoot();
|
||||
const outside = path.join(path.dirname(root), `${path.basename(root)}-secret`);
|
||||
try {
|
||||
await writeFile(outside, "secret one\n");
|
||||
await symlink(outside, path.join(root, "link"));
|
||||
const link: DiffEntry = { path: "link", patch: "diff --git a/link b/link", status: "A" };
|
||||
const first = await hash(root, { files: [link] });
|
||||
await writeFile(outside, "secret two\n");
|
||||
expect(await hash(root, { files: [link] })).toBe(first);
|
||||
await expect(
|
||||
hash(root, {
|
||||
files: [{ path: "../outside", patch: "diff --git a/x b/x", status: "A" }],
|
||||
}),
|
||||
).rejects.toThrow("escapes the PR tree");
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
await rm(outside, { force: true });
|
||||
}
|
||||
});
|
||||
@@ -54,8 +54,8 @@ test("RoutingManifestSchema: rejects duplicate config dirs (after normalize)", (
|
||||
expect(() =>
|
||||
RoutingManifestSchema.parse({
|
||||
scopes: [
|
||||
{ name: "a", paths: ["x/**"], config: "apps/api" },
|
||||
{ name: "b", paths: ["y/**"], config: "apps/api/" },
|
||||
{ name: "a", paths: ["x/**"], config: "server/www" },
|
||||
{ name: "b", paths: ["y/**"], config: "server/www/" },
|
||||
],
|
||||
}),
|
||||
).toThrow(/duplicate scope config dir/);
|
||||
@@ -148,36 +148,36 @@ test("resolveScopes: single catch-all takes everything, unmatched empty", () =>
|
||||
test("resolveScopes: LAST matching scope wins", () => {
|
||||
const m = manifest([
|
||||
{ name: "default", paths: ["**/*"], config: "." },
|
||||
{ name: "api", paths: ["apps/api/**"], config: "apps/api" },
|
||||
{ name: "www", paths: ["server/www/**"], config: "server/www" },
|
||||
]);
|
||||
const r = resolveScopes(m, ["apps/api/app.ts", "other.ts"]);
|
||||
expect(r.active.find((s) => s.name === "api")!.files).toEqual(["apps/api/app.ts"]);
|
||||
const r = resolveScopes(m, ["server/www/app.ts", "other.ts"]);
|
||||
expect(r.active.find((s) => s.name === "www")!.files).toEqual(["server/www/app.ts"]);
|
||||
expect(r.active.find((s) => s.name === "default")!.files).toEqual(["other.ts"]);
|
||||
});
|
||||
|
||||
test("resolveScopes: reversing manifest order flips the winner", () => {
|
||||
const m = manifest([
|
||||
{ name: "api", paths: ["apps/api/**"], config: "apps/api" },
|
||||
{ name: "www", paths: ["server/www/**"], config: "server/www" },
|
||||
{ name: "default", paths: ["**/*"], config: "." },
|
||||
]);
|
||||
const r = resolveScopes(m, ["apps/api/app.ts"]);
|
||||
expect(r.active.find((s) => s.name === "default")!.files).toEqual(["apps/api/app.ts"]);
|
||||
expect(r.active.some((s) => s.name === "api")).toBe(false);
|
||||
const r = resolveScopes(m, ["server/www/app.ts"]);
|
||||
expect(r.active.find((s) => s.name === "default")!.files).toEqual(["server/www/app.ts"]);
|
||||
expect(r.active.some((s) => s.name === "www")).toBe(false);
|
||||
});
|
||||
|
||||
test("resolveScopes: reports overlaps with matched list + winner", () => {
|
||||
const m = manifest([
|
||||
{ name: "default", paths: ["**/*"], config: "." },
|
||||
{ name: "api", paths: ["apps/api/**"], config: "apps/api" },
|
||||
{ name: "www", paths: ["server/www/**"], config: "server/www" },
|
||||
]);
|
||||
const r = resolveScopes(m, ["apps/api/app.ts"]);
|
||||
const r = resolveScopes(m, ["server/www/app.ts"]);
|
||||
expect(r.overlaps).toEqual([
|
||||
{ file: "apps/api/app.ts", matched: ["default", "api"], winner: "api" },
|
||||
{ file: "server/www/app.ts", matched: ["default", "www"], winner: "www" },
|
||||
]);
|
||||
});
|
||||
|
||||
test("resolveScopes: no catch-all leaves unmatched files unrouted", () => {
|
||||
const m = manifest([{ name: "api", paths: ["apps/api/**"], config: "apps/api" }]);
|
||||
const m = manifest([{ name: "www", paths: ["server/www/**"], config: "server/www" }]);
|
||||
const r = resolveScopes(m, ["README.md"]);
|
||||
expect(r.active).toEqual([]);
|
||||
expect(r.unmatched).toEqual(["README.md"]);
|
||||
@@ -216,8 +216,8 @@ test("formatOwnerTable: one row per file, capped with a +N more tail", () => {
|
||||
|
||||
test("scopedCommentTag + marker isolation: root marker is NOT a substring of a scoped marker", () => {
|
||||
const root = "expo-ai-code-reviewer";
|
||||
const scoped = scopedCommentTag(root, "api");
|
||||
expect(scoped).toBe("expo-ai-code-reviewer:api");
|
||||
const scoped = scopedCommentTag(root, "www");
|
||||
expect(scoped).toBe("expo-ai-code-reviewer:www");
|
||||
expect(commentMarker(scoped).includes(commentMarker(root))).toBe(false);
|
||||
expect(commentMarker(root).includes(commentMarker(scoped))).toBe(false);
|
||||
});
|
||||
@@ -232,12 +232,12 @@ test("appendScopeEntry: inserts before the closing ] preserving comments", () =>
|
||||
]
|
||||
}`;
|
||||
const out = appendScopeEntry(raw, {
|
||||
name: "apps-api",
|
||||
paths: ["apps/api/**"],
|
||||
config: "apps/api",
|
||||
name: "server-www",
|
||||
paths: ["server/www/**"],
|
||||
config: "server/www",
|
||||
})!;
|
||||
expect(out).toContain("// keep me");
|
||||
expect(out).toContain('"name": "apps-api"');
|
||||
expect(out).toContain('"name": "server-www"');
|
||||
// Still valid JSON after stripping comments (comma inserted correctly).
|
||||
expect(() => RoutingManifestSchema.parse(JSON.parse(out.replace(/\/\/.*$/gm, "")))).not.toThrow();
|
||||
});
|
||||
@@ -249,9 +249,9 @@ test("appendScopeEntry: comma lands after the entry, not inside a trailing line
|
||||
]
|
||||
}`;
|
||||
const out = appendScopeEntry(raw, {
|
||||
name: "apps-api",
|
||||
paths: ["apps/api/**"],
|
||||
config: "apps/api",
|
||||
name: "server-www",
|
||||
paths: ["server/www/**"],
|
||||
config: "server/www",
|
||||
})!;
|
||||
expect(out).toContain('"config": "." }, // the default scope');
|
||||
// Still valid JSON after stripping comments (comma NOT swallowed by the comment).
|
||||
@@ -265,15 +265,15 @@ test("appendScopeEntry: handles a trailing comma plus block comment after the la
|
||||
{ "name": "default", "paths": ["**/*"], "config": "." }, /* keep */
|
||||
]
|
||||
}`;
|
||||
const out = appendScopeEntry(raw, { name: "api", paths: ["api/**"], config: "api" })!;
|
||||
const out = appendScopeEntry(raw, { name: "www", paths: ["www/**"], config: "www" })!;
|
||||
// No double comma; still parses after stripping both comment styles.
|
||||
const stripped = stripTrailingCommas(stripJsonComments(out));
|
||||
expect(() => RoutingManifestSchema.parse(JSON.parse(stripped))).not.toThrow();
|
||||
});
|
||||
|
||||
test("appendScopeEntry: idempotent on an already-present name", () => {
|
||||
const raw = `{ "scopes": [ { "name": "api", "paths": ["api/**"], "config": "api" } ] }`;
|
||||
expect(appendScopeEntry(raw, { name: "api", paths: ["api/**"], config: "api" })).toBe(raw);
|
||||
const raw = `{ "scopes": [ { "name": "www", "paths": ["www/**"], "config": "www" } ] }`;
|
||||
expect(appendScopeEntry(raw, { name: "www", paths: ["www/**"], config: "www" })).toBe(raw);
|
||||
});
|
||||
|
||||
test("appendScopeEntry: returns null when there is no scopes array", () => {
|
||||
@@ -303,8 +303,8 @@ test("scopedFingerprint: null scope === fingerprintFinding (BACKCOMPAT carry-ove
|
||||
test("scopedFingerprint: scoped ids are hex-only, same length, and differ per scope", () => {
|
||||
const f = finding();
|
||||
const plain = fingerprintFinding(f);
|
||||
const a = scopedFingerprint("api", f);
|
||||
const b = scopedFingerprint("web", f);
|
||||
const a = scopedFingerprint("www", f);
|
||||
const b = scopedFingerprint("website", f);
|
||||
expect(a).toMatch(/^[a-f0-9]+$/);
|
||||
expect(a.length).toBe(plain.length);
|
||||
expect(a).not.toBe(plain);
|
||||
|
||||
@@ -93,22 +93,22 @@ test("loadScopeConfig: config '.' reuses the root config, auth from loadAuthFrom
|
||||
|
||||
test("loadScopeConfig: nested scope reads its own roster/prompts; auth forced from root", async () => {
|
||||
const root = await makeRoot('{ "auth": { "mode": "oauth", "tokenEnv": "ROOT_TOKEN" } }');
|
||||
await writeConfigDir(path.join(root, "apps", "api", CONFIG_DIRNAME), {
|
||||
await writeConfigDir(path.join(root, "server", "www", CONFIG_DIRNAME), {
|
||||
// No auth here; scope-specific model + noise override the root.
|
||||
config:
|
||||
'{ "model": "anthropic/claude-opus-5", "noise": { "additionalIgnores": ["api/gen/**"] } }',
|
||||
'{ "model": "anthropic/claude-opus-5", "noise": { "additionalIgnores": ["www/gen/**"] } }',
|
||||
agents: { style: agent("style") },
|
||||
});
|
||||
const manifest = manifestOf({
|
||||
scopes: [
|
||||
{ name: "default", paths: ["**/*"], config: "." },
|
||||
{ name: "api", paths: ["apps/api/**"], config: "apps/api" },
|
||||
{ name: "www", paths: ["server/www/**"], config: "server/www" },
|
||||
],
|
||||
});
|
||||
const rootConfig = await loadReviewConfig(root);
|
||||
const scoped = await loadScopeConfig(root, manifest.scopes[1]!, manifest, rootConfig);
|
||||
expect(scoped.agents.some((a) => a.id === "style")).toBe(true);
|
||||
expect(scoped.noise.additionalIgnores).toEqual(["api/gen/**"]);
|
||||
expect(scoped.noise.additionalIgnores).toEqual(["www/gen/**"]);
|
||||
expect(scoped.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-5");
|
||||
// auth is forced from the root even though the scope config declares none.
|
||||
expect(scoped.auth[0]?.tokenEnv).toBe("ROOT_TOKEN");
|
||||
@@ -159,13 +159,13 @@ test("loadAuthFromRoot: manifest with defaults but no auth key keeps the root au
|
||||
|
||||
test("loadScopeConfig: enforceAgents injects the ROOT agent with alwaysRun", async () => {
|
||||
const root = await makeRoot();
|
||||
await writeConfigDir(path.join(root, "api", CONFIG_DIRNAME), {
|
||||
await writeConfigDir(path.join(root, "www", CONFIG_DIRNAME), {
|
||||
config: "{}",
|
||||
agents: { style: agent("style") }, // no security
|
||||
});
|
||||
const manifest = manifestOf({
|
||||
defaults: { enforceAgents: ["security"] },
|
||||
scopes: [{ name: "api", paths: ["api/**"], config: "api" }],
|
||||
scopes: [{ name: "www", paths: ["www/**"], config: "www" }],
|
||||
});
|
||||
const rootConfig = await loadReviewConfig(root);
|
||||
const scoped = await loadScopeConfig(root, manifest.scopes[0]!, manifest, rootConfig);
|
||||
@@ -177,14 +177,14 @@ test("loadScopeConfig: enforceAgents injects the ROOT agent with alwaysRun", asy
|
||||
|
||||
test("loadScopeConfig: a scope shadowing the enforced id gets the ROOT version (risk 11)", async () => {
|
||||
const root = await makeRoot();
|
||||
await writeConfigDir(path.join(root, "api", CONFIG_DIRNAME), {
|
||||
await writeConfigDir(path.join(root, "www", CONFIG_DIRNAME), {
|
||||
config: "{}",
|
||||
// The scope defines its OWN weaker security agent — must be overridden.
|
||||
agents: { security: agent("weak-security", ""), style: agent("style") },
|
||||
});
|
||||
const manifest = manifestOf({
|
||||
defaults: { enforceAgents: ["security"] },
|
||||
scopes: [{ name: "api", paths: ["api/**"], config: "api" }],
|
||||
scopes: [{ name: "www", paths: ["www/**"], config: "www" }],
|
||||
});
|
||||
const rootConfig = await loadReviewConfig(root);
|
||||
const scoped = await loadScopeConfig(root, manifest.scopes[0]!, manifest, rootConfig);
|
||||
@@ -231,12 +231,12 @@ test("config-dir override composes: root config + routing.jsonc from override, s
|
||||
path.join(root, "altroot", "routing.jsonc"),
|
||||
`{ "scopes": [
|
||||
{ "name": "default", "paths": ["**/*"], "config": "." },
|
||||
{ "name": "api", "paths": ["apps/api/**"], "config": "apps/api" }
|
||||
{ "name": "www", "paths": ["server/www/**"], "config": "server/www" }
|
||||
] }`,
|
||||
"utf8",
|
||||
);
|
||||
// The scope subtree lives at the REPO ROOT, not under the override dir.
|
||||
await writeConfigDir(path.join(root, "apps", "api", CONFIG_DIRNAME), {
|
||||
await writeConfigDir(path.join(root, "server", "www", CONFIG_DIRNAME), {
|
||||
config: '{ "model": "anthropic/claude-opus-5" }',
|
||||
agents: { style: agent("style") },
|
||||
});
|
||||
@@ -244,7 +244,7 @@ test("config-dir override composes: root config + routing.jsonc from override, s
|
||||
// routing.jsonc is read from the override dir (the default tree's is absent).
|
||||
const manifest = await loadRoutingManifest(root, { configDir: "altroot" });
|
||||
expect(manifest).not.toBeNull();
|
||||
expect(manifest!.scopes.map((s) => s.name)).toEqual(["default", "api"]);
|
||||
expect(manifest!.scopes.map((s) => s.name)).toEqual(["default", "www"]);
|
||||
|
||||
// The root config artifacts follow the override.
|
||||
const rootConfig = await loadReviewConfig(root, { configDir: "altroot" });
|
||||
@@ -254,11 +254,11 @@ test("config-dir override composes: root config + routing.jsonc from override, s
|
||||
const def = await loadScopeConfig(root, manifest!.scopes[0]!, manifest!, rootConfig);
|
||||
expect(def.commentTag).toBe("alt-root-tag");
|
||||
|
||||
// The nested scope is still resolved from the repo root (apps/api), NOT the
|
||||
// The nested scope is still resolved from the repo root (server/www), NOT the
|
||||
// override dir — the override must not relocate the scopes themselves.
|
||||
const api = await loadScopeConfig(root, manifest!.scopes[1]!, manifest!, rootConfig);
|
||||
expect(api.agents.some((a) => a.id === "style")).toBe(true);
|
||||
expect(api.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-5");
|
||||
const www = await loadScopeConfig(root, manifest!.scopes[1]!, manifest!, rootConfig);
|
||||
expect(www.agents.some((a) => a.id === "style")).toBe(true);
|
||||
expect(www.agents.find((a) => a.id === "style")!.model).toBe("anthropic/claude-opus-5");
|
||||
});
|
||||
|
||||
test("loadReviewConfig: ECR_CONFIG_DIR loads from the alternate dir; unset → .expo-code-review (BACKCOMPAT)", async () => {
|
||||
|
||||
@@ -43,7 +43,7 @@ test("refuses a scope config that declares auth", async () => {
|
||||
await put(root, `${CONFIG_DIRNAME}/config.jsonc`, `{ "auth": { "tokenEnv": "${EXPECTED}" } }`);
|
||||
await put(
|
||||
root,
|
||||
`apps/api/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`server/www/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`{ "model": "anthropic/claude-sonnet-5", "auth": { "mode": "api-key", "tokenEnv": "SNEAKY" } }`,
|
||||
);
|
||||
const result = await verifyConfig(root, { expected: EXPECTED });
|
||||
@@ -56,7 +56,7 @@ test("refuses a scope config that declares breakGlass", async () => {
|
||||
await put(root, `${CONFIG_DIRNAME}/config.jsonc`, `{ "auth": { "tokenEnv": "${EXPECTED}" } }`);
|
||||
await put(
|
||||
root,
|
||||
`apps/api/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`server/www/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`{ "model": "anthropic/claude-sonnet-5", "breakGlass": { "marker": "override-me" } }`,
|
||||
);
|
||||
const result = await verifyConfig(root, { expected: EXPECTED });
|
||||
@@ -69,7 +69,7 @@ test("refuses a scope config that declares commentTag", async () => {
|
||||
await put(root, `${CONFIG_DIRNAME}/config.jsonc`, `{ "auth": { "tokenEnv": "${EXPECTED}" } }`);
|
||||
await put(
|
||||
root,
|
||||
`apps/api/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`server/www/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`{ "model": "anthropic/claude-sonnet-5", "commentTag": "impostor-tag" }`,
|
||||
);
|
||||
const result = await verifyConfig(root, { expected: EXPECTED });
|
||||
@@ -156,7 +156,7 @@ test("refuses a tokenEnv declared only in a non-root file", async () => {
|
||||
await put(root, `${CONFIG_DIRNAME}/config.jsonc`, `{ "model": "anthropic/claude-sonnet-5" }`);
|
||||
await put(
|
||||
root,
|
||||
`apps/api/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`server/www/${CONFIG_DIRNAME}/config.jsonc`,
|
||||
`{ "auth": { "tokenEnv": "${EXPECTED}" } }`,
|
||||
);
|
||||
const result = await verifyConfig(root, { expected: EXPECTED });
|
||||
|
||||
@@ -5,7 +5,6 @@ import { dismissCommand } from "./commands/dismiss.js";
|
||||
import { doctorCommand } from "./commands/doctor.js";
|
||||
import { feedbackCommand } from "./commands/feedback.js";
|
||||
import { initCommand } from "./commands/init.js";
|
||||
import { refCheckCommand } from "./commands/ref-check.js";
|
||||
import { reviewCommand } from "./commands/review.js";
|
||||
import { setupAuthCommand } from "./commands/setup-auth.js";
|
||||
import { verifyConfigCommand } from "./commands/verify-config.js";
|
||||
@@ -22,7 +21,6 @@ Usage:
|
||||
ecr setup-auth [--yes] Walk through getting model credentials for local runs.
|
||||
ecr doctor [--list-scopes] Check environment, config, credentials, and scopes.
|
||||
ecr verify-config [--expected <env>] [--json] Refuse to run if a config could redirect the credential (CI guard).
|
||||
ecr ref-check [--root <dir>] [--json] Fail if the review setup cites code that moved or vanished.
|
||||
|
||||
Agents live in each repo under .expo-code-review/. This CLI is the engine.
|
||||
|
||||
@@ -72,9 +70,6 @@ async function main(): Promise<void> {
|
||||
case "verify-config":
|
||||
await verifyConfigCommand(rest);
|
||||
break;
|
||||
case "ref-check":
|
||||
await refCheckCommand(rest);
|
||||
break;
|
||||
default:
|
||||
process.stderr.write(`Unknown command: ${sub}\n\n${USAGE}`);
|
||||
process.exitCode = 2;
|
||||
|
||||
+27
-157
@@ -23,13 +23,12 @@ import { repoRoot, resolveTrustedTool, run } from "../core/exec.js";
|
||||
import { errorMessage, publicFailureReason } from "../core/util.js";
|
||||
import { readContextFile } from "../core/context-file.js";
|
||||
import { buildDiffLineIndex } from "../core/render.js";
|
||||
import type { LinkContext, ReviewState, ScopeReviewResult } from "../core/render.js";
|
||||
import type { LinkContext, ScopeReviewResult } from "../core/render.js";
|
||||
import type { CoordinatorOutput, FeedbackPin, FeedbackRecord, Finding } from "../core/schema.js";
|
||||
import { applyPins, collectPins, scopedFingerprint } from "../core/schema.js";
|
||||
import { dropStaleVerdict, feedbackApplied, feedbackNeedsRunSeam } from "../core/adjudicate.js";
|
||||
import { runReview } from "../core/review.js";
|
||||
import type { ReviewRunOptions, ReviewRunResult } from "../core/review.js";
|
||||
import { reviewCanBeReused, reviewInputHash, reviewMatchesInput } from "../core/review-cache.js";
|
||||
import { GitHubPRSource } from "../sources/github-pr.js";
|
||||
import { memoizeSource, stackConfirmFromConfig, stackWalkFromConfig } from "../sources/source.js";
|
||||
import type { PreparedReadRoot, ReviewSource, StackWalkOptions } from "../sources/source.js";
|
||||
@@ -541,11 +540,9 @@ async function runLegacyCi(
|
||||
// unchanged (see mergeFeedback). Memoized on the source — no extra API call. A
|
||||
// metadata failure leaves it unknown, which re-judges replies instead of trusting a
|
||||
// verdict against source we cannot pin.
|
||||
let metadata: Awaited<ReturnType<ReviewSource["getMetadata"]>> | undefined;
|
||||
let headSha: string | undefined;
|
||||
try {
|
||||
metadata = await source.getMetadata();
|
||||
headSha = metadata.headOid;
|
||||
headSha = (await source.getMetadata()).headOid;
|
||||
} catch {
|
||||
// leave headSha unset → stored verdicts do not carry (fail safe, costs budget only)
|
||||
}
|
||||
@@ -572,76 +569,24 @@ async function runLegacyCi(
|
||||
);
|
||||
}
|
||||
|
||||
const stack = resolveStackWalk(config.stack, noStackAware);
|
||||
const stackConfirm = resolveStackConfirm(config.stack, noStackAware);
|
||||
const feedback = adjudicationSeam(config, reporter);
|
||||
// Dynamic stack context and model-backed reply adjudication have inputs outside
|
||||
// the scoped diff. Keep those paths fresh until their inputs join the cache key.
|
||||
// A maintainer's explicit /review is also always a real rerun.
|
||||
const cacheAllowed = !bypassTriggerGate && !stack && !feedback && metadata !== undefined;
|
||||
let inputHash: string | undefined;
|
||||
|
||||
try {
|
||||
if (cacheAllowed) {
|
||||
try {
|
||||
const prepared = await source.prepareReadRootAsync?.();
|
||||
try {
|
||||
inputHash = await reviewInputHash({
|
||||
files: await source.getChangedFiles(),
|
||||
config,
|
||||
metadata: metadata!,
|
||||
readRoot: prepared?.dir ?? cwd,
|
||||
agents,
|
||||
route,
|
||||
contextText,
|
||||
});
|
||||
} finally {
|
||||
await prepared?.cleanup();
|
||||
}
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not hash the review input (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
if (inputHash) {
|
||||
try {
|
||||
const prior = await reporter.readState();
|
||||
if (prior && reviewMatchesInput(prior.review, prior.inputHash, inputHash)) {
|
||||
await reporter.report(prior.review, undefined, inputHash);
|
||||
process.stderr.write(
|
||||
"CI reviewer: unchanged review input; reused the previous result.\n",
|
||||
);
|
||||
return;
|
||||
}
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not read the previous review cache (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const review = await runReview(source, {
|
||||
config,
|
||||
mode: "ci",
|
||||
agents,
|
||||
route,
|
||||
contextText,
|
||||
stack,
|
||||
stackConfirm,
|
||||
stack: resolveStackWalk(config.stack, noStackAware),
|
||||
stackConfirm: resolveStackConfirm(config.stack, noStackAware),
|
||||
runsDir: workspaceRunsDir(cwd),
|
||||
// Adjudicate mode judges the matched replies against the source before the
|
||||
// comment is rendered; annotate mode lets the reporter match them at report
|
||||
// time. Either way the feedback path is fail-open (runReview swallows its own
|
||||
// errors), so it never fails the PR's checks.
|
||||
feedback,
|
||||
feedback: adjudicationSeam(config, reporter),
|
||||
onProgress: (message) => process.stderr.write(`${message}\n`),
|
||||
});
|
||||
await reporter.report(
|
||||
review,
|
||||
review.feedback,
|
||||
inputHash && reviewCanBeReused(review) ? inputHash : undefined,
|
||||
);
|
||||
await reporter.report(review, review.feedback);
|
||||
process.stderr.write(`CI reviewer: posted review (${review.decision}).\n`);
|
||||
} catch (error) {
|
||||
// A reviewer failure must never fail the PR's checks — but it must also not be
|
||||
@@ -818,14 +763,13 @@ async function runRoutedCi(
|
||||
};
|
||||
// The reviewed head OID also binds each adjudication verdict to the source it judged
|
||||
// (see mergeFeedback); unresolved leaves it unknown, so verdicts are re-judged.
|
||||
let metadata: Awaited<ReturnType<ReviewSource["getMetadata"]>> | undefined;
|
||||
let headSha: string | undefined;
|
||||
try {
|
||||
metadata = await source.getMetadata();
|
||||
if (metadata.baseOid) {
|
||||
link.baseSha = metadata.baseOid;
|
||||
const { baseOid, headOid } = await source.getMetadata();
|
||||
if (baseOid) {
|
||||
link.baseSha = baseOid;
|
||||
}
|
||||
headSha = metadata.headOid;
|
||||
headSha = headOid;
|
||||
} catch {
|
||||
// leave baseSha unset → out-of-diff findings degrade to plain text
|
||||
}
|
||||
@@ -880,42 +824,11 @@ async function runRoutedCi(
|
||||
reporterFor(scopedCommentTag(rootTag, name), true),
|
||||
);
|
||||
|
||||
const cacheAllowed =
|
||||
!bypassTriggerGate &&
|
||||
!stackWalk &&
|
||||
!feedbackNeedsRunSeam(rootConfig.feedback) &&
|
||||
metadata !== undefined;
|
||||
let cacheReadRoot: string | undefined;
|
||||
if (cacheAllowed) {
|
||||
try {
|
||||
const prepared = await source.prepareReadRootAsync?.();
|
||||
cacheReadRoot = prepared?.dir ?? cwd;
|
||||
await prepared?.cleanup();
|
||||
} catch (error) {
|
||||
// The fresh run will retry the same materialization and apply CI's ordinary
|
||||
// fail-closed behavior. Cache setup itself never changes that outcome.
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not prepare the review cache input (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
let priorAggregateState: ReviewState | null = null;
|
||||
if (cacheReadRoot && mode === "single") {
|
||||
try {
|
||||
priorAggregateState = await singleModeReporter!.readState();
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: could not read the previous aggregate cache (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const results: ScopeReviewResult[] = [];
|
||||
for (const scope of active) {
|
||||
const scopeDef = manifest.scopes.find((entry) => entry.name === scope.name)!;
|
||||
const isDefault = scope.configDir === ".";
|
||||
let review: CoordinatorOutput;
|
||||
let inputHash: string | undefined;
|
||||
try {
|
||||
// A scope whose config dir doesn't exist at the TRUSTED base commit is a
|
||||
// scope this PR introduces: review it with the root config rather than
|
||||
@@ -945,66 +858,27 @@ async function runRoutedCi(
|
||||
: undefined,
|
||||
)
|
||||
: undefined;
|
||||
let cached: ScopeReviewResult | ReviewState | undefined;
|
||||
if (cacheReadRoot) {
|
||||
try {
|
||||
const included = new Set(scope.files);
|
||||
inputHash = await reviewInputHash({
|
||||
files: changed.filter((file) => included.has(file.path)),
|
||||
config,
|
||||
metadata: metadata!,
|
||||
readRoot: cacheReadRoot,
|
||||
agents,
|
||||
route,
|
||||
contextText,
|
||||
});
|
||||
if (mode === "single") {
|
||||
cached = priorAggregateState?.scopes?.find((entry) => entry.scope === scope.name);
|
||||
} else {
|
||||
cached = (await scopeReporter(scope.name).readState()) ?? undefined;
|
||||
}
|
||||
} catch (error) {
|
||||
inputHash = undefined;
|
||||
process.stderr.write(
|
||||
`CI reviewer: [${scope.name}] could not use the previous cache (continuing fresh): ${errorMessage(error)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (cached && reviewMatchesInput(cached.review, cached.inputHash, inputHash!)) {
|
||||
review = cached.review;
|
||||
process.stderr.write(
|
||||
`CI reviewer: [${scope.name}] unchanged review input; reused the previous result.\n`,
|
||||
);
|
||||
} else {
|
||||
review = await runReview(source, {
|
||||
config,
|
||||
mode: "ci",
|
||||
agents,
|
||||
route,
|
||||
includePaths: scope.files,
|
||||
contextText,
|
||||
stack: stackWalk,
|
||||
stackConfirm,
|
||||
passesBudgetMs: budget,
|
||||
runsDir: workspaceRunsDir(cwd),
|
||||
feedback: feedbackSeam,
|
||||
onProgress: (message) => process.stderr.write(`[${scope.name}] ${message}\n`),
|
||||
});
|
||||
}
|
||||
review = await runReview(source, {
|
||||
config,
|
||||
mode: "ci",
|
||||
agents,
|
||||
route,
|
||||
includePaths: scope.files,
|
||||
contextText,
|
||||
stack: stackWalk,
|
||||
stackConfirm,
|
||||
passesBudgetMs: budget,
|
||||
runsDir: workspaceRunsDir(cwd),
|
||||
feedback: feedbackSeam,
|
||||
onProgress: (message) => process.stderr.write(`[${scope.name}] ${message}\n`),
|
||||
});
|
||||
} catch (error) {
|
||||
process.stderr.write(
|
||||
`CI reviewer: [${scope.name}] failed (non-blocking): ${errorMessage(error)}\n`,
|
||||
);
|
||||
review = failureReview(scope.name, publicFailureReason(error));
|
||||
inputHash = undefined;
|
||||
}
|
||||
results.push({
|
||||
scope: scope.name,
|
||||
isDefault,
|
||||
review,
|
||||
...(inputHash && reviewCanBeReused(review) ? { inputHash } : {}),
|
||||
});
|
||||
results.push({ scope: scope.name, isDefault, review });
|
||||
}
|
||||
|
||||
if (mode === "single") {
|
||||
@@ -1014,7 +888,7 @@ async function runRoutedCi(
|
||||
// A partial run (--scopes) is authoritative ONLY for the named scopes: merge
|
||||
// the other scopes' previous results out of the existing aggregate comment's
|
||||
// state so re-running one scope doesn't silently discard the rest.
|
||||
const prior = (priorAggregateState ?? (await aggregate.readState()))?.scopes ?? [];
|
||||
const prior = (await aggregate.readState())?.scopes ?? [];
|
||||
finalResults = mergePartialAggregate(
|
||||
results,
|
||||
prior,
|
||||
@@ -1073,11 +947,7 @@ async function runRoutedCi(
|
||||
const reporter = scopeReporter(scope.name);
|
||||
const result = results.find((entry) => entry.scope === scope.name);
|
||||
if (result) {
|
||||
await reporter.report(
|
||||
result.review,
|
||||
(result.review as ReviewRunResult).feedback,
|
||||
result.inputHash,
|
||||
);
|
||||
await reporter.report(result.review, (result.review as ReviewRunResult).feedback);
|
||||
} else {
|
||||
// A reconciled scope with zero matched files gets its stale comment deleted.
|
||||
await reporter.clear();
|
||||
|
||||
+13
-18
@@ -27,10 +27,9 @@ Options:
|
||||
--no-workflow Skip writing the CI workflows (review, command, and dismiss
|
||||
under .github/workflows/)
|
||||
--token-env <name[,name…]>
|
||||
Env var(s) holding the model credential (default
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN, e.g. OPENAI_API_KEY). The
|
||||
scaffolded workflows forward the matching repo secret(s) and
|
||||
expect this tokenEnv
|
||||
Env var(s) holding the model credential (default OPENAI_API_KEY,
|
||||
e.g. CLAUDE_CODE_OAUTH_TOKEN). The scaffolded workflows forward
|
||||
the matching repo secret(s) and expect this tokenEnv
|
||||
--force Overwrite existing files
|
||||
--force-workflows
|
||||
Overwrite only the CI workflow files, keeping your customized
|
||||
@@ -39,7 +38,7 @@ Options:
|
||||
-h, --help Show this help
|
||||
`;
|
||||
|
||||
const DEFAULT_TOKEN_ENV = "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN";
|
||||
const DEFAULT_TOKEN_ENV = "OPENAI_API_KEY";
|
||||
|
||||
export async function initCommand(argv: string[]): Promise<void> {
|
||||
if (argv.includes("-h") || argv.includes("--help")) {
|
||||
@@ -108,7 +107,7 @@ async function scaffold(argv: string[]): Promise<void> {
|
||||
|
||||
// The reverse of the guard above: --force-workflows (or --force) always rewrites
|
||||
// the review workflows from the pristine template, which forwards the default
|
||||
// Anthropic token env unless --token-env names the credential again. An adopter who
|
||||
// OPENAI_API_KEY unless --token-env names the credential again. An adopter who
|
||||
// scaffolded with a non-default tokenEnv and later refreshes the workflow YAML
|
||||
// without re-passing --token-env would silently lose the forwarded secret, and
|
||||
// CI would keep passing the auth lock but run with an empty credential. Refuse
|
||||
@@ -122,7 +121,7 @@ async function scaffold(argv: string[]): Promise<void> {
|
||||
`${baked}, but this run has no --token-env, so rewriting them would restore the ` +
|
||||
`default ${DEFAULT_TOKEN_ENV} and CI would run with an empty credential. Re-run with ` +
|
||||
`--token-env ${baked} to keep the current credential, or --token-env ${DEFAULT_TOKEN_ENV} ` +
|
||||
`to reset to the default on purpose.`,
|
||||
`to reset to OpenAI on purpose.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -227,15 +226,14 @@ async function scaffold(argv: string[]): Promise<void> {
|
||||
const steps = [
|
||||
`Customize ${CONFIG_DIRNAME}/agents/*.md (and shared.md, coordinator.md) for this repo.`,
|
||||
// --token-env only rewires the workflows; the scaffolded config.jsonc still
|
||||
// declares the default Anthropic tokenEnv, and CI's `ecr verify-config`
|
||||
// refuses to review until the config's tokenEnv set matches the workflow's
|
||||
// expected set.
|
||||
// declares OPENAI_API_KEY, and CI's `ecr verify-config` refuses to review
|
||||
// until the config's tokenEnv set matches the workflow's expected set.
|
||||
...(tokenEnvs.join(",") !== DEFAULT_TOKEN_ENV
|
||||
? [
|
||||
`Point ${CONFIG_DIRNAME}/config.jsonc at ${tokenEnvs.length > 1 ? "these credentials" : "this credential"}: set \`auth\` (and \`model\`) per the file's comments — CI's \`ecr verify-config\` refuses to review until the config names ${names}.`,
|
||||
]
|
||||
: []),
|
||||
"Log in with `claude` (or run `ecr setup-auth`) so local runs have a credential.",
|
||||
"Configure a model provider in OpenCode (or set REVIEWER_MODEL).",
|
||||
"Run `ecr doctor`, then `ecr review`.",
|
||||
withWorkflow
|
||||
? `Add the ${names} repo secret${tokenEnvs.length > 1 ? "s" : ""} referenced by the workflow, then add an \`ai-review\` label to a PR.`
|
||||
@@ -641,14 +639,11 @@ export function substituteTokenEnv(raw: string, tokenEnvs: string[]): string {
|
||||
return raw;
|
||||
}
|
||||
const expectedFallback = `vars.ECR_EXPECTED_TOKEN_ENV || '${DEFAULT_TOKEN_ENV}'`;
|
||||
// Byte-identical to the credential block in templates/workflow.yml AND
|
||||
// templates/command.yml (both flow through this function).
|
||||
const credentialBlock = [
|
||||
" # Anthropic review credential — the env var named by auth.tokenEnv in",
|
||||
" # config.jsonc. Store it as a repo secret: an `sk-ant-oat…` token minted",
|
||||
" # by `claude setup-token`, or an `sk-ant-api…` Console key (the Claude",
|
||||
" # Code CLI reads either).",
|
||||
" CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}",
|
||||
" # OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.",
|
||||
" # Store it as a repo secret; a project-scoped key restricted to model",
|
||||
" # inference (with a spend limit) is all the reviewer needs.",
|
||||
" OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}",
|
||||
].join("\n");
|
||||
if (!raw.includes(expectedFallback) || !raw.includes(credentialBlock)) {
|
||||
throw new Error("workflow template drifted: tokenEnv markers not found (report this bug)");
|
||||
|
||||
@@ -1,93 +0,0 @@
|
||||
// @ref LLP 0012#run-points-command-and-review — the gating run point: exit 1 on any broken ref
|
||||
import path from "node:path";
|
||||
|
||||
import { checkConfigRefs } from "../core/config-refs.js";
|
||||
import type { RefProblem } from "../core/config-refs.js";
|
||||
import { repoRoot } from "../core/exec.js";
|
||||
import { errorMessage } from "../core/util.js";
|
||||
|
||||
const USAGE = `ecr ref-check — fail when the review setup cites code that moved or vanished
|
||||
|
||||
Usage:
|
||||
ecr ref-check [--root <dir>] [--json]
|
||||
|
||||
Sweeps every .expo-code-review/ directory in the repo (root and scopes) and checks
|
||||
that each code citation still resolves against this checkout:
|
||||
• \`@ref <target>\` annotations in prompts and configs. A target is a file, a
|
||||
directory (trailing slash), \`glob:<pattern>\`, a \`file#symbol\`, or a
|
||||
\`doc.md#heading\`. Never a line number — lines rot silently.
|
||||
• Unannotated citations: a backticked token that looks like a repo path must be a
|
||||
ref, so nothing cites code without being checked. Use \`@ref-ignore <token>\`
|
||||
for a token that is not a path.
|
||||
• Structural refs the config already declares: enforceAgents ids, scope config
|
||||
directories, and scope path globs.
|
||||
Exit 0 = every ref holds. Exit 1 = at least one is broken.
|
||||
|
||||
Options:
|
||||
--root <dir> Repository root to check (default: the current git repo).
|
||||
--json Emit {ok, problems:[{file, line, kind, problem, target?}]} on stdout.
|
||||
`;
|
||||
|
||||
const KIND_LABEL: Record<RefProblem["kind"], string> = {
|
||||
"broken-ref": "broken ref",
|
||||
"line-number-ref": "line-number ref",
|
||||
"unannotated-citation": "unannotated citation",
|
||||
structural: "structural ref",
|
||||
};
|
||||
|
||||
export async function refCheckCommand(argv: string[]): Promise<void> {
|
||||
if (argv.includes("-h") || argv.includes("--help")) {
|
||||
process.stdout.write(USAGE);
|
||||
return;
|
||||
}
|
||||
|
||||
let root: string | undefined;
|
||||
let json = false;
|
||||
for (let i = 0; i < argv.length; i++) {
|
||||
const arg = argv[i]!;
|
||||
if (arg === "--json") {
|
||||
json = true;
|
||||
} else if (arg === "--root") {
|
||||
root = argv[++i];
|
||||
// A flag-shaped value means the directory was forgotten: taking it would check
|
||||
// some nonexistent path and report "all resolve" while swallowing the real flag.
|
||||
if (!root || root.startsWith("-")) {
|
||||
process.stderr.write("ecr ref-check: --root needs a directory\n");
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
process.stderr.write(`ecr ref-check: unknown argument ${arg}\n${USAGE}`);
|
||||
process.exitCode = 2;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const resolvedRoot = root ? path.resolve(root) : ((await repoRoot()) ?? process.cwd());
|
||||
const report = await checkConfigRefs({ root: resolvedRoot });
|
||||
|
||||
if (json) {
|
||||
process.stdout.write(`${JSON.stringify({ ok: report.ok, problems: report.problems })}\n`);
|
||||
} else if (report.ok) {
|
||||
process.stdout.write(
|
||||
`ref-check: ${report.refs.length} ref(s) across ${report.scannedFiles.length} setup file(s) — all resolve\n`,
|
||||
);
|
||||
} else {
|
||||
for (const problem of report.problems) {
|
||||
process.stderr.write(
|
||||
`${problem.file}:${problem.line}: ${KIND_LABEL[problem.kind]}: ${problem.problem}\n`,
|
||||
);
|
||||
}
|
||||
process.stderr.write(
|
||||
`\nref-check: ${report.problems.length} problem(s). Update the ref or the prompt that cites it.\n`,
|
||||
);
|
||||
}
|
||||
if (!report.ok) {
|
||||
process.exitCode = 1;
|
||||
}
|
||||
} catch (error) {
|
||||
process.stderr.write(`ecr ref-check: ${errorMessage(error)}\n`);
|
||||
process.exitCode = 2;
|
||||
}
|
||||
}
|
||||
+1
-4
@@ -21,9 +21,7 @@ const PROVIDER_KEY_ENV: Record<string, string> = {
|
||||
* Provider-owned credential env vars BEYOND the x-api-key ones above: Anthropic's
|
||||
* OAuth/subscription bearer envs. CLAUDE_CODE_OAUTH_TOKEN holds the long-lived (1-year)
|
||||
* Claude Max/Team subscription token that `ecr setup-auth`/`claude setup-token` export,
|
||||
* ANTHROPIC_AUTH_TOKEN is Anthropic's documented bearer var, and
|
||||
* CLAUDE_CODE_REVIEW_SHARED_API_TOKEN is the SCAFFOLDED DEFAULT tokenEnv (see
|
||||
* templates/config.jsonc) — it always holds an Anthropic credential. They belong to
|
||||
* and ANTHROPIC_AUTH_TOKEN is Anthropic's documented bearer var. They belong to
|
||||
* anthropic, so the cross-provider guard below refuses a non-anthropic entry that names
|
||||
* one — without this, `{provider:"openai", tokenEnv:"CLAUDE_CODE_OAUTH_TOKEN"}` passes
|
||||
* (neither a FORBIDDEN secret nor a PROVIDER_KEY_ENV value) and prepareAuth forwards the
|
||||
@@ -33,7 +31,6 @@ const PROVIDER_KEY_ENV: Record<string, string> = {
|
||||
const ANTHROPIC_TOKEN_ENVS: Record<string, string> = {
|
||||
CLAUDE_CODE_OAUTH_TOKEN: "anthropic",
|
||||
ANTHROPIC_AUTH_TOKEN: "anthropic",
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: "anthropic",
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,919 +0,0 @@
|
||||
// @ref LLP 0012#the-ref-grammar — one grammar for every code citation in a review setup
|
||||
// @ref LLP 0012#no-line-numbers-symbol-anchors-instead — targets are files, dirs, globs, symbols; never line numbers
|
||||
// @ref LLP 0012#unannotated-citations-are-broken-refs — a path-like backtick token that is not a ref fails the check
|
||||
import { readdir, readFile, stat } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
import { CONFIG_DIRNAME, stripJsonComments, stripTrailingCommas } from "../config/load.js";
|
||||
import { ROUTING_FILENAME } from "../config/routing.js";
|
||||
import { git, pathInside } from "./exec.js";
|
||||
import { matchesIgnore } from "./noise.js";
|
||||
import { isAmbientRuntimeConfig } from "./scrub.js";
|
||||
|
||||
/**
|
||||
* Built by concatenation so this module's own regexes and doc comments are not
|
||||
* themselves collected as refs by a scanner (the repo's `ref-check` does the same).
|
||||
*/
|
||||
const REF_MARK = "@" + "ref";
|
||||
const IGNORE_MARK = REF_MARK + "-ignore";
|
||||
|
||||
/** `@ref <target> [relation] — gloss` inside a `//`-style comment. */
|
||||
const LINE_REF_RE = new RegExp(String.raw`${REF_MARK}\s+(.+)`);
|
||||
/** `<!-- @ref <target> — gloss -->`, possibly spanning lines. */
|
||||
const MD_REF_RE = new RegExp(String.raw`<!--\s*${REF_MARK}\s+([\s\S]+?)-->`, "g");
|
||||
const IGNORE_RE = new RegExp(String.raw`${IGNORE_MARK}\s+(.+)`);
|
||||
|
||||
/** LLP-corpus targets (`LLP 0004#anchor`) belong to the engine repo, not an adopting one. */
|
||||
const LLP_TARGET_RE = /^LLP\s+\d{1,4}(?:#\S+)?$/;
|
||||
const URL_RE = /^https?:\/\/\S+$/;
|
||||
/** `<path>`, `<agent-id>`: documenting the grammar, not citing a file. */
|
||||
const PLACEHOLDER_RE = /^[<`]/;
|
||||
const GLOB_PREFIX = "glob:";
|
||||
|
||||
/** A citation that pins a line (`src/a.ts:42`, `src/a.ts:42-51`) — always refused. */
|
||||
const LINE_CITATION_RE = /^(.+?):(\d+)(?:-\d+)?$/;
|
||||
|
||||
/**
|
||||
* Extensions that make a backticked token a *code citation* rather than prose.
|
||||
* Deliberately an allowlist: `openai/gpt-5.5` and `label:<agent>` must not read as
|
||||
* paths, while `session.ts` and `.github/workflows/**` must.
|
||||
*/
|
||||
const CITATION_EXTENSIONS = new Set([
|
||||
".c",
|
||||
".cc",
|
||||
".cjs",
|
||||
".cpp",
|
||||
".cs",
|
||||
".css",
|
||||
".go",
|
||||
".graphql",
|
||||
".hcl",
|
||||
".h",
|
||||
".hpp",
|
||||
".html",
|
||||
".java",
|
||||
".js",
|
||||
".json",
|
||||
".jsonc",
|
||||
".jsx",
|
||||
".kt",
|
||||
".lock",
|
||||
".md",
|
||||
".mjs",
|
||||
".mts",
|
||||
".php",
|
||||
".prisma",
|
||||
".proto",
|
||||
".py",
|
||||
".rb",
|
||||
".rs",
|
||||
".scss",
|
||||
".sh",
|
||||
".sql",
|
||||
".swift",
|
||||
".tf",
|
||||
".tfvars",
|
||||
".toml",
|
||||
".ts",
|
||||
".tsx",
|
||||
".txt",
|
||||
".vue",
|
||||
".yaml",
|
||||
".yml",
|
||||
".zsh",
|
||||
]);
|
||||
|
||||
/** Files inside a review-setup dir that are prompts or config (everything else is skipped). */
|
||||
const SCANNED_EXTENSIONS = new Set([".md", ".markdown", ".json", ".jsonc", ".txt"]);
|
||||
|
||||
export type RefProblemKind =
|
||||
| "broken-ref"
|
||||
| "line-number-ref"
|
||||
| "unannotated-citation"
|
||||
| "structural";
|
||||
|
||||
export interface RefProblem {
|
||||
/** Repo-relative file the problem was found in. */
|
||||
file: string;
|
||||
line: number;
|
||||
kind: RefProblemKind;
|
||||
/** One sentence, imperative where a fix is obvious. */
|
||||
problem: string;
|
||||
/** The ref target (or prose token) at fault. Absent for structural problems. */
|
||||
target?: string;
|
||||
}
|
||||
|
||||
export interface ParsedRef {
|
||||
file: string;
|
||||
line: number;
|
||||
/** The target as written, gloss and relation stripped (`src/a.ts#foo`, `glob:**\/*.ts`). */
|
||||
target: string;
|
||||
}
|
||||
|
||||
export interface RefCheckReport {
|
||||
ok: boolean;
|
||||
problems: RefProblem[];
|
||||
/** Every resolvable ref found, so callers can tell when a PR touches cited code. */
|
||||
refs: ParsedRef[];
|
||||
/** Repo-relative review-setup files that were scanned. */
|
||||
scannedFiles: string[];
|
||||
/** Repo-relative paths (files and dirs) that some ref cites. */
|
||||
citedPaths: string[];
|
||||
}
|
||||
|
||||
interface RepoIndex {
|
||||
root: string;
|
||||
/** Tracked files, repo-relative with `/` separators. Empty when git is unavailable. */
|
||||
files: string[];
|
||||
}
|
||||
|
||||
function escapeRegExp(text: string): string {
|
||||
return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||
}
|
||||
|
||||
/** GitHub-style heading slug: lowercase, spaces to hyphens, drop other punctuation. */
|
||||
export function slugifyHeading(text: string): string {
|
||||
return [...text.replace(/`/g, "").trim().toLowerCase()]
|
||||
.map((ch) => (/[a-z0-9\-_]/.test(ch) ? ch : /\s/.test(ch) ? "-" : ""))
|
||||
.join("");
|
||||
}
|
||||
|
||||
/** Heading texts outside fenced code blocks. */
|
||||
function markdownHeadings(text: string): string[] {
|
||||
const headings: string[] = [];
|
||||
let fenced = false;
|
||||
for (const line of text.split(/\r?\n/)) {
|
||||
if (line.trimStart().startsWith("```")) {
|
||||
fenced = !fenced;
|
||||
continue;
|
||||
}
|
||||
const match = fenced ? null : /^#{1,6}\s+(.+?)\s*$/.exec(line);
|
||||
if (match) {
|
||||
headings.push(match[1]!);
|
||||
}
|
||||
}
|
||||
return headings;
|
||||
}
|
||||
|
||||
/** Strip the trailing `— gloss` and `[relation]` so only the target remains. */
|
||||
function refTarget(body: string): string {
|
||||
const withoutGloss = body.split(/\s+(?:[—–]|--)(?:\s+|$)/)[0]!.trim();
|
||||
const withoutRelation = withoutGloss.replace(/\s*\[[a-z-]+\]\s*$/, "").trim();
|
||||
// `LLP 0009#anchor` is ONE target that contains a space; taking the first
|
||||
// whitespace-separated token would leave a bare `LLP` and read as a broken path.
|
||||
const llp = /^LLP\s+\d{1,4}(?:#\S+)?/.exec(withoutRelation);
|
||||
return llp ? llp[0] : (withoutRelation.split(/\s+/)[0] ?? "");
|
||||
}
|
||||
|
||||
/** Line numbers (1-based) that sit inside a fenced code block. */
|
||||
function fencedLines(text: string): Set<number> {
|
||||
const fenced = new Set<number>();
|
||||
let open = false;
|
||||
text.split(/\r?\n/).forEach((line, index) => {
|
||||
if (line.trimStart().startsWith("```")) {
|
||||
fenced.add(index + 1); // the fence line itself is never a ref site
|
||||
open = !open;
|
||||
return;
|
||||
}
|
||||
if (open) {
|
||||
fenced.add(index + 1);
|
||||
}
|
||||
});
|
||||
return fenced;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every `@ref` annotation in a setup file, markdown comments included. Fenced code
|
||||
* blocks are skipped: an annotation shown inside one is documenting the grammar, and
|
||||
* resolving it would make every doc that explains refs fail the check.
|
||||
*/
|
||||
export function parseRefAnnotations(text: string, file: string): ParsedRef[] {
|
||||
const refs: ParsedRef[] = [];
|
||||
if (file.endsWith(".md") || file.endsWith(".markdown")) {
|
||||
const fenced = fencedLines(text);
|
||||
for (const match of text.matchAll(MD_REF_RE)) {
|
||||
const line = text.slice(0, match.index).split("\n").length;
|
||||
if (fenced.has(line)) {
|
||||
continue;
|
||||
}
|
||||
refs.push({ file, line, target: refTarget(match[1]!) });
|
||||
}
|
||||
return refs;
|
||||
}
|
||||
text.split(/\r?\n/).forEach((lineText, index) => {
|
||||
const match = LINE_REF_RE.exec(lineText);
|
||||
if (match) {
|
||||
refs.push({ file, line: index + 1, target: refTarget(match[1]!) });
|
||||
}
|
||||
});
|
||||
return refs;
|
||||
}
|
||||
|
||||
/** Tokens an author declared as prose, not citations (`@ref-ignore knex.raw()`). */
|
||||
export function parseRefIgnores(text: string): Set<string> {
|
||||
const ignored = new Set<string>();
|
||||
for (const lineText of text.split(/\r?\n/)) {
|
||||
const match = IGNORE_RE.exec(lineText);
|
||||
if (!match) {
|
||||
continue;
|
||||
}
|
||||
for (const token of match[1]!.replace(/-->\s*$/, "").split(/[\s,]+/)) {
|
||||
const cleaned = token.replace(/^`|`$/g, "").trim();
|
||||
if (cleaned) {
|
||||
ignored.add(cleaned);
|
||||
}
|
||||
}
|
||||
}
|
||||
return ignored;
|
||||
}
|
||||
|
||||
/**
|
||||
* Does this backticked token cite code? True for `a/b/c.ts`, `session.ts`,
|
||||
* `src/entities/oauth/` and `.github/workflows/**`; false for `ecr ci`,
|
||||
* `label:<agent>`, `knex.raw()`, `openai/gpt-5.5` and bare `**`.
|
||||
*/
|
||||
export function isCodeCitation(token: string): boolean {
|
||||
if (!token || /[\s<>(){}"'|=,;]/.test(token) || token.length > 200) {
|
||||
return false;
|
||||
}
|
||||
const withoutLine = LINE_CITATION_RE.exec(token)?.[1] ?? token;
|
||||
if (withoutLine.includes(":")) {
|
||||
return false;
|
||||
}
|
||||
if (withoutLine.endsWith("/")) {
|
||||
return withoutLine.replace(/[/*]/g, "").length > 0;
|
||||
}
|
||||
const base = withoutLine.split("/").pop() ?? "";
|
||||
// A wildcard tail inside a path (`.github/workflows/**`) is a citation with no extension.
|
||||
if (base.includes("*") && withoutLine.includes("/")) {
|
||||
return withoutLine.replace(/[/*]/g, "").length > 0;
|
||||
}
|
||||
// A bare extension (`.ts`, `.js`) is prose about a suffix, not a file.
|
||||
if (CITATION_EXTENSIONS.has(base.toLowerCase())) {
|
||||
return false;
|
||||
}
|
||||
const dot = base.lastIndexOf(".");
|
||||
if (dot <= 0 && !base.startsWith(".")) {
|
||||
return false;
|
||||
}
|
||||
const extension = base.slice(base.lastIndexOf("."));
|
||||
return CITATION_EXTENSIONS.has(extension.toLowerCase()) && base.replace(/[*.]/g, "").length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* The ref an unannotated citation should have become. Abbreviated paths (`.../a/B.kt`),
|
||||
* wildcards (`*Policy.ts`) and bare filenames (`session.ts`) become suffix globs, so a
|
||||
* prompt keeps its short readable form and still gets checked.
|
||||
*/
|
||||
export function suggestedRef(token: string): string {
|
||||
const trimmed = token.replace(/^\.\.\.\/?/, "").replace(/^\/+/, "");
|
||||
if (token.startsWith("...") || !trimmed.includes("/")) {
|
||||
return `${GLOB_PREFIX}**/${trimmed}`;
|
||||
}
|
||||
return trimmed.includes("*") ? `${GLOB_PREFIX}${trimmed}` : trimmed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Every form under which an annotated target can cover a prose citation, so one ref
|
||||
* silences the token it is about: `glob:src/commands/*.ts` covers `src/commands/*.ts`,
|
||||
* and `src/core/util.ts#errorMessage` covers `src/core/util.ts`.
|
||||
*/
|
||||
function coveringForms(target: string): string[] {
|
||||
const bare = target.startsWith(GLOB_PREFIX) ? target.slice(GLOB_PREFIX.length) : target;
|
||||
const withoutAnchor = splitAnchor(bare)[0];
|
||||
return [target, bare, withoutAnchor].flatMap((form) => [
|
||||
form,
|
||||
form.replace(/\/$/, ""),
|
||||
`${form}/`,
|
||||
]);
|
||||
}
|
||||
|
||||
/** The forms a prose citation could have been annotated as. */
|
||||
function citationForms(token: string): string[] {
|
||||
const suggested = suggestedRef(token);
|
||||
return [
|
||||
token,
|
||||
token.replace(/\/$/, ""),
|
||||
suggested,
|
||||
suggested.startsWith(GLOB_PREFIX) ? suggested.slice(GLOB_PREFIX.length) : suggested,
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Paths to test when a token has no extension to give it away — `eas-build-worker/terraform`,
|
||||
* `general-central/{module,production}`, a bare `finops`. These are only citations if they
|
||||
* actually resolve, since `anthropic/claude-opus-5` is shaped exactly the same and is not
|
||||
* a path. Returns the candidate paths to probe, or [] when the token cannot be one.
|
||||
*/
|
||||
export function pathishCandidates(token: string): string[] {
|
||||
if (!token || token.length > 200) {
|
||||
return [];
|
||||
}
|
||||
// Cut a brace list or wildcard tail off FIRST: the part before it is the path to
|
||||
// probe, and only that part has to look like one (`a/{b,c}` carries a comma).
|
||||
const cut = Math.min(
|
||||
...[token.indexOf("{"), token.indexOf("*")].filter((index) => index >= 0),
|
||||
token.length,
|
||||
);
|
||||
const prefix = token.slice(0, cut).replace(/\/$/, "");
|
||||
if (
|
||||
!prefix ||
|
||||
!/[a-zA-Z]/.test(prefix) ||
|
||||
/[\s<>()"'|=,;:]/.test(prefix) ||
|
||||
prefix.startsWith("-")
|
||||
) {
|
||||
return [];
|
||||
}
|
||||
return prefix === token ? [token] : [prefix];
|
||||
}
|
||||
|
||||
/** Backticked code citations, outside fenced code blocks, with their line numbers. */
|
||||
export function findProseCitations(text: string): Array<{ line: number; token: string }> {
|
||||
const found: Array<{ line: number; token: string }> = [];
|
||||
let fenced = false;
|
||||
text.split(/\r?\n/).forEach((lineText, index) => {
|
||||
if (lineText.trimStart().startsWith("```")) {
|
||||
fenced = !fenced;
|
||||
return;
|
||||
}
|
||||
if (fenced) {
|
||||
return;
|
||||
}
|
||||
for (const match of lineText.matchAll(/`([^`]+)`/g)) {
|
||||
found.push({ line: index + 1, token: match[1]! });
|
||||
}
|
||||
});
|
||||
return found;
|
||||
}
|
||||
|
||||
async function pathKind(absolute: string): Promise<"file" | "dir" | "missing"> {
|
||||
try {
|
||||
const stats = await stat(absolute);
|
||||
return stats.isDirectory() ? "dir" : "file";
|
||||
} catch {
|
||||
return "missing";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve one ref target against the repo. Returns the problem text, or null when
|
||||
* the ref holds. Refs are ALWAYS repo-root-relative — a scope's prompts cite the
|
||||
* same paths the reviewer sees in the diff, so there is no per-directory base.
|
||||
*/
|
||||
async function resolveTarget(
|
||||
target: string,
|
||||
index: RepoIndex,
|
||||
/** A scope's own subtree, used only to say "did you mean" on a scope-relative path. */
|
||||
scopeRoot?: string,
|
||||
): Promise<string | null> {
|
||||
if (!target) {
|
||||
return `empty ${REF_MARK} target`;
|
||||
}
|
||||
if (URL_RE.test(target) || LLP_TARGET_RE.test(target) || PLACEHOLDER_RE.test(target)) {
|
||||
// URLs are shape-only (never fetched); LLP numbers belong to the engine's own
|
||||
// corpus (`./ref-check`); `<placeholder>` targets are documentation of the syntax.
|
||||
return null;
|
||||
}
|
||||
if (target.startsWith(GLOB_PREFIX)) {
|
||||
const pattern = target.slice(GLOB_PREFIX.length);
|
||||
if (!pattern) {
|
||||
return `empty glob in ${REF_MARK} target`;
|
||||
}
|
||||
if (index.files.length === 0) {
|
||||
return null; // no file list to match against (not a git checkout) — uncheckable
|
||||
}
|
||||
return index.files.some((file) => matchesIgnore(file, pattern))
|
||||
? null
|
||||
: `glob matches no file in the repo: ${pattern}`;
|
||||
}
|
||||
const lineCitation = LINE_CITATION_RE.exec(target);
|
||||
if (lineCitation) {
|
||||
return `cites a line number (${target}); refs pin a file, dir, glob, or \`#symbol\` — line numbers rot silently`;
|
||||
}
|
||||
|
||||
const [rawPath, anchor] = splitAnchor(target);
|
||||
if (path.isAbsolute(rawPath) || rawPath.startsWith("~")) {
|
||||
return `absolute path (${rawPath}); refs are repo-root-relative`;
|
||||
}
|
||||
const absolute = path.resolve(index.root, rawPath);
|
||||
if (!pathInside(absolute, index.root)) {
|
||||
return `escapes the repository (${rawPath})`;
|
||||
}
|
||||
const kind = await pathKind(absolute);
|
||||
if (kind === "missing") {
|
||||
// A scope's prompts naturally say `general-central/module` for what is really
|
||||
// `infrastructure/general-central/module`. That is still a broken ref (one base, no
|
||||
// ambiguity), but the fix is named instead of left as a puzzle.
|
||||
const scoped = scopeRoot ? path.resolve(scopeRoot, rawPath) : null;
|
||||
if (scoped && pathInside(scoped, index.root) && (await pathKind(scoped)) !== "missing") {
|
||||
const suggestion = path.relative(index.root, scoped).split(path.sep).join("/");
|
||||
return `no such path in the repo: ${rawPath} — refs are repo-root-relative; did you mean ${suggestion}?`;
|
||||
}
|
||||
return `no such path in the repo: ${rawPath}`;
|
||||
}
|
||||
if (rawPath.endsWith("/") && kind !== "dir") {
|
||||
return `${rawPath} is a file, not a directory (drop the trailing slash)`;
|
||||
}
|
||||
if (!anchor) {
|
||||
return null;
|
||||
}
|
||||
if (kind === "dir") {
|
||||
return `${rawPath} is a directory, so \`#${anchor}\` cannot resolve`;
|
||||
}
|
||||
let content: string;
|
||||
try {
|
||||
content = await readFile(absolute, "utf8");
|
||||
} catch {
|
||||
return null; // unreadable (binary, permissions) — uncheckable, never a failure
|
||||
}
|
||||
if (rawPath.endsWith(".md") || rawPath.endsWith(".markdown")) {
|
||||
const slugs = new Set(markdownHeadings(content).map(slugifyHeading));
|
||||
return slugs.has(slugifyHeading(anchor)) ? null : `${rawPath} has no heading #${anchor}`;
|
||||
}
|
||||
return new RegExp(String.raw`\b${escapeRegExp(anchor)}\b`).test(content)
|
||||
? null
|
||||
: `${rawPath} no longer contains \`${anchor}\``;
|
||||
}
|
||||
|
||||
/**
|
||||
* How a setup file is named in a problem. Normally repo-relative, but in CI the setup
|
||||
* is materialized from the trusted base ref OUTSIDE the code tree, where a relative
|
||||
* path would read as `../../tmp/…`; there, name it by its position in the setup dir.
|
||||
*/
|
||||
function fileLabel(root: string, setupDir: string, file: string): string {
|
||||
const relative = path.relative(root, file);
|
||||
if (!relative.startsWith("..")) {
|
||||
return relative;
|
||||
}
|
||||
return path.join(CONFIG_DIRNAME, path.relative(setupDir, file));
|
||||
}
|
||||
|
||||
function splitAnchor(target: string): [string, string | undefined] {
|
||||
const hash = target.indexOf("#");
|
||||
return hash === -1 ? [target, undefined] : [target.slice(0, hash), target.slice(hash + 1)];
|
||||
}
|
||||
|
||||
/**
|
||||
* The repo's files, repo-relative with `/` separators — what `glob:` targets and scope
|
||||
* globs match against. `git ls-files` when possible (fast, and "tracked" is the right
|
||||
* notion for reviewable code), else a filesystem walk so the check still works in a
|
||||
* plain directory. Empty only for an unreadable root.
|
||||
*/
|
||||
async function listRepoFiles(root: string): Promise<string[]> {
|
||||
try {
|
||||
// Tracked AND untracked-but-not-ignored: a plain path ref resolves with `stat` and
|
||||
// therefore sees a file the moment it exists, so a glob must too — otherwise a
|
||||
// freshly scaffolded (uncommitted) tree reports globs as matching nothing.
|
||||
const tracked = (await git(["ls-files", "--cached", "--others", "--exclude-standard"], root))
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.filter(Boolean);
|
||||
if (tracked.length > 0) {
|
||||
return tracked;
|
||||
}
|
||||
} catch {
|
||||
// not a git checkout — fall through to the walk
|
||||
}
|
||||
const found: string[] = [];
|
||||
async function walk(dir: string): Promise<void> {
|
||||
let entries;
|
||||
try {
|
||||
entries = await readdir(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
if (entry.name === "node_modules" || (entry.name.startsWith(".") && entry.isDirectory())) {
|
||||
continue;
|
||||
}
|
||||
const child = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
await walk(child);
|
||||
} else if (entry.isFile()) {
|
||||
found.push(path.relative(root, child).split(path.sep).join("/"));
|
||||
}
|
||||
}
|
||||
}
|
||||
await walk(root);
|
||||
return found.sort();
|
||||
}
|
||||
|
||||
// @ref LLP 0012#what-gets-scanned [implements] — on-disk sweep of every setup dir, .runs/ excluded
|
||||
/**
|
||||
* Every review-setup directory in the repo, found by walking the tree (not the
|
||||
* routing manifest): a scope dir the manifest forgot still ships prompts to nobody,
|
||||
* and its stale refs are exactly what this check exists to surface.
|
||||
*/
|
||||
export async function discoverSetupDirs(root: string): Promise<string[]> {
|
||||
const found: string[] = [];
|
||||
async function walk(dir: string): Promise<void> {
|
||||
let entries;
|
||||
try {
|
||||
entries = await readdir(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory()) {
|
||||
continue;
|
||||
}
|
||||
const child = path.join(dir, entry.name);
|
||||
if (entry.name === CONFIG_DIRNAME) {
|
||||
found.push(child);
|
||||
continue; // a setup dir never nests another
|
||||
}
|
||||
// Other dot dirs are skipped wholesale: `.claude/worktrees/` holds checkouts of
|
||||
// this same repo, whose setup dirs would otherwise be swept as if they were scopes.
|
||||
if (entry.name === "node_modules" || entry.name.startsWith(".")) {
|
||||
continue;
|
||||
}
|
||||
await walk(child);
|
||||
}
|
||||
}
|
||||
await walk(root);
|
||||
return found.sort();
|
||||
}
|
||||
|
||||
/** Prompt and config files inside a setup dir. Skips `.runs/` and other dot entries. */
|
||||
async function setupFiles(dir: string): Promise<string[]> {
|
||||
const found: string[] = [];
|
||||
async function walk(current: string): Promise<void> {
|
||||
let entries;
|
||||
try {
|
||||
entries = await readdir(current, { withFileTypes: true });
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
if (entry.name.startsWith(".")) {
|
||||
continue;
|
||||
}
|
||||
const child = path.join(current, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
await walk(child);
|
||||
} else if (SCANNED_EXTENSIONS.has(path.extname(entry.name).toLowerCase())) {
|
||||
found.push(child);
|
||||
}
|
||||
}
|
||||
}
|
||||
await walk(dir);
|
||||
return found.sort();
|
||||
}
|
||||
|
||||
function parseJsonc(text: string): Record<string, unknown> | null {
|
||||
try {
|
||||
const value = JSON.parse(stripTrailingCommas(stripJsonComments(text))) as unknown;
|
||||
return value && typeof value === "object" && !Array.isArray(value)
|
||||
? (value as Record<string, unknown>)
|
||||
: null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function stringArray(value: unknown): string[] {
|
||||
return Array.isArray(value)
|
||||
? value.filter((item): item is string => typeof item === "string")
|
||||
: [];
|
||||
}
|
||||
|
||||
// @ref LLP 0012#structural-refs-need-no-annotation [implements] — ids and scope dirs are refs the config already declares
|
||||
/**
|
||||
* Refs the config declares structurally, so they are checked without an annotation:
|
||||
* every `enforceAgents` id must have a root `agents/<id>.md`, every scope must point
|
||||
* at a real setup dir, and every scope glob must match at least one tracked file (a
|
||||
* glob matching nothing means those prompts review nothing).
|
||||
*/
|
||||
async function checkRoutingManifest(
|
||||
file: string,
|
||||
index: RepoIndex,
|
||||
rootAgentIds: Set<string>,
|
||||
): Promise<RefProblem[]> {
|
||||
const problems: RefProblem[] = [];
|
||||
const relative = fileLabel(index.root, path.dirname(file), file);
|
||||
let parsed: Record<string, unknown> | null;
|
||||
try {
|
||||
parsed = parseJsonc(await readFile(file, "utf8"));
|
||||
} catch {
|
||||
return problems;
|
||||
}
|
||||
if (!parsed) {
|
||||
return [
|
||||
{ file: relative, line: 1, kind: "structural", problem: "could not be parsed as JSONC" },
|
||||
];
|
||||
}
|
||||
|
||||
const defaults = (parsed.defaults ?? {}) as Record<string, unknown>;
|
||||
const enforced = new Set(stringArray(defaults.enforceAgents));
|
||||
const scopes = Array.isArray(parsed.scopes) ? parsed.scopes : [];
|
||||
for (const scope of scopes) {
|
||||
if (!scope || typeof scope !== "object") {
|
||||
continue;
|
||||
}
|
||||
const entry = scope as Record<string, unknown>;
|
||||
const name = typeof entry.name === "string" ? entry.name : "(unnamed)";
|
||||
for (const id of stringArray(entry.enforceAgents)) {
|
||||
enforced.add(id);
|
||||
}
|
||||
const configDir = typeof entry.config === "string" ? entry.config : null;
|
||||
if (configDir) {
|
||||
const setupDir = path.resolve(index.root, configDir, CONFIG_DIRNAME);
|
||||
if (!pathInside(setupDir, index.root)) {
|
||||
problems.push({
|
||||
file: relative,
|
||||
line: 1,
|
||||
kind: "structural",
|
||||
problem: `scope "${name}" points outside the repository (config: ${configDir})`,
|
||||
});
|
||||
} else if ((await pathKind(setupDir)) !== "dir") {
|
||||
problems.push({
|
||||
file: relative,
|
||||
line: 1,
|
||||
kind: "structural",
|
||||
problem: `scope "${name}" has no ${configDir}/${CONFIG_DIRNAME}/ directory`,
|
||||
});
|
||||
}
|
||||
}
|
||||
if (index.files.length > 0) {
|
||||
for (const pattern of stringArray(entry.paths)) {
|
||||
const variants = [pattern, pattern.replace(/\*\*\//g, "")];
|
||||
if (
|
||||
!index.files.some((repoFile) => variants.some((v) => v && matchesIgnore(repoFile, v)))
|
||||
) {
|
||||
problems.push({
|
||||
file: relative,
|
||||
line: 1,
|
||||
kind: "structural",
|
||||
problem: `scope "${name}" glob matches no file in the repo: ${pattern}`,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (const id of enforced) {
|
||||
if (!rootAgentIds.has(id)) {
|
||||
problems.push({
|
||||
file: relative,
|
||||
line: 1,
|
||||
kind: "structural",
|
||||
problem: `enforceAgents names "${id}", but the root setup has no agents/${id}.md`,
|
||||
});
|
||||
}
|
||||
}
|
||||
return problems;
|
||||
}
|
||||
|
||||
/** Agent ids in a setup dir (id = filename without `.md`, per the loader). */
|
||||
async function agentIds(setupDir: string): Promise<Set<string>> {
|
||||
try {
|
||||
const entries = await readdir(path.join(setupDir, "agents"));
|
||||
return new Set(entries.filter((name) => name.endsWith(".md")).map((name) => name.slice(0, -3)));
|
||||
} catch {
|
||||
return new Set();
|
||||
}
|
||||
}
|
||||
|
||||
export interface CheckConfigRefsOptions {
|
||||
/** Repo root. Refs resolve against this tree — in CI, the PR head checkout. */
|
||||
root: string;
|
||||
/** Restrict the sweep to these setup dirs (absolute). Default: every one in the repo. */
|
||||
setupDirs?: string[];
|
||||
}
|
||||
|
||||
// @ref LLP 0012#run-points-command-and-review [implements] — one pure-ish entry point both the command and the review call
|
||||
/**
|
||||
* Check every code citation in the repo's review setup. Deterministic, no model, no
|
||||
* network: refs either resolve against the tree or they do not.
|
||||
*/
|
||||
export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<RefCheckReport> {
|
||||
const root = path.resolve(options.root);
|
||||
const index: RepoIndex = { root, files: await listRepoFiles(root) };
|
||||
const dirs = options.setupDirs ?? (await discoverSetupDirs(root));
|
||||
|
||||
const problems: RefProblem[] = [];
|
||||
const refs: ParsedRef[] = [];
|
||||
const scannedFiles: string[] = [];
|
||||
const citedPaths = new Set<string>();
|
||||
|
||||
// Does an extensionless token name something real? Cached: prompts repeat the same
|
||||
// paths, and each miss would otherwise cost a stat per occurrence.
|
||||
const resolvable = new Map<string, string | null>();
|
||||
/** The root-relative path a token names, or null. Scope first: a scoped prompt
|
||||
* citing `alerts/` means its own tree even when the root has one too. */
|
||||
const namedPath = async (token: string, scopeRoot: string): Promise<string | null> => {
|
||||
for (const candidate of pathishCandidates(token)) {
|
||||
for (const base of [scopeRoot, root]) {
|
||||
const absolute = path.resolve(base, candidate);
|
||||
let resolved = resolvable.get(absolute);
|
||||
if (resolved === undefined) {
|
||||
const kind = pathInside(absolute, root) ? await pathKind(absolute) : "missing";
|
||||
resolved =
|
||||
kind === "missing"
|
||||
? null
|
||||
: path.relative(root, absolute).split(path.sep).join("/") +
|
||||
(kind === "dir" ? "/" : "");
|
||||
resolvable.set(absolute, resolved);
|
||||
}
|
||||
if (resolved) {
|
||||
return resolved;
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
/** The glob a wildcard token names, rebased onto the scope when that is what
|
||||
* matches, so the suggested `glob:` ref actually resolves. */
|
||||
const namedGlob = (token: string, scopeRoot: string): string | null => {
|
||||
const scopeRel = path.relative(root, scopeRoot).split(path.sep).join("/");
|
||||
const candidates = [...(scopeRel && scopeRel !== "." ? [`${scopeRel}/${token}`] : []), token];
|
||||
for (const candidate of candidates) {
|
||||
if (index.files.some((file) => matchesIgnore(file, candidate))) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
for (const dir of dirs) {
|
||||
const scopeRoot = path.dirname(dir);
|
||||
for (const file of await setupFiles(dir)) {
|
||||
const relative = fileLabel(root, dir, file);
|
||||
let text: string;
|
||||
try {
|
||||
text = await readFile(file, "utf8");
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
scannedFiles.push(relative);
|
||||
|
||||
const annotated = parseRefAnnotations(text, relative);
|
||||
const ignored = parseRefIgnores(text);
|
||||
const covered = new Set(annotated.flatMap((ref) => coveringForms(ref.target)));
|
||||
// A glob ref also covers any citation the glob itself matches, so a prompt can
|
||||
// cite `.../nested/Handler.kt` and pin it once with `glob:**/Handler.kt`.
|
||||
const coveringGlobs = annotated
|
||||
.filter((ref) => ref.target.startsWith(GLOB_PREFIX))
|
||||
.map((ref) => ref.target.slice(GLOB_PREFIX.length));
|
||||
|
||||
for (const ref of annotated) {
|
||||
const problem = await resolveTarget(ref.target, index, scopeRoot);
|
||||
if (problem) {
|
||||
problems.push({
|
||||
file: relative,
|
||||
line: ref.line,
|
||||
kind: problem.startsWith("cites a line number") ? "line-number-ref" : "broken-ref",
|
||||
problem,
|
||||
target: ref.target,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
// An `LLP NNNN` target belongs to a different mechanism (a design corpus, owned
|
||||
// by that repo's own checker). ecr neither resolves nor counts it: the refs it
|
||||
// owns are the ones citing the reviewed code.
|
||||
if (LLP_TARGET_RE.test(ref.target)) {
|
||||
continue;
|
||||
}
|
||||
refs.push(ref);
|
||||
const cited = splitAnchor(ref.target)[0];
|
||||
if (cited && !cited.startsWith(GLOB_PREFIX) && !URL_RE.test(cited)) {
|
||||
citedPaths.add(cited.replace(/\/$/, ""));
|
||||
}
|
||||
}
|
||||
|
||||
for (const { line, token } of findProseCitations(text)) {
|
||||
const isWild = token.includes("*");
|
||||
const named = isWild ? namedGlob(token, scopeRoot) : await namedPath(token, scopeRoot);
|
||||
// Shape alone makes a citation; otherwise it must name something real, or
|
||||
// `anthropic/claude-opus-5` would read as a path.
|
||||
if (!isCodeCitation(token) && !named) {
|
||||
continue;
|
||||
}
|
||||
// Coverage accepts the resolved path too, else the only ref that silences a
|
||||
// scope-relative citation is an over-broad `glob:**/<basename>`.
|
||||
const forms = [...citationForms(token), ...(named ? coveringForms(named) : [])];
|
||||
if (
|
||||
ignored.has(token) ||
|
||||
forms.some((form) => covered.has(form)) ||
|
||||
coveringGlobs.some((pattern) =>
|
||||
forms.some((form) => matchesIgnore(form.replace(/^\.\.\.\//, ""), pattern)),
|
||||
)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
const lineCitation = LINE_CITATION_RE.exec(token);
|
||||
// A wildcard token means the family; anything else gets the precise resolved path.
|
||||
const suggestion = isWild
|
||||
? named
|
||||
? `${GLOB_PREFIX}${named}`
|
||||
: suggestedRef(token)
|
||||
: (named ?? suggestedRef(token));
|
||||
problems.push({
|
||||
file: relative,
|
||||
line,
|
||||
kind: lineCitation ? "line-number-ref" : "unannotated-citation",
|
||||
problem: lineCitation
|
||||
? `\`${token}\` pins a line number; cite the file or a \`#symbol\` instead, as \`${REF_MARK} ${suggestedRef(lineCitation[1]!)}\``
|
||||
: `\`${token}\` cites code without a ref; add \`${REF_MARK} ${suggestion} — why it matters\` (or \`${IGNORE_MARK} ${token}\` if it is not a path)`,
|
||||
target: token,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const routing = path.join(dir, ROUTING_FILENAME);
|
||||
if ((await pathKind(routing)) === "file") {
|
||||
// The roster comes from the dir that OWNS the manifest, never from `root`:
|
||||
// under `ecr ci` the setup is a trusted base-ref checkout while `root` is the PR
|
||||
// head tree, so reading `root/.expo-code-review/agents` would judge enforceAgents
|
||||
// against a different (or absent) roster than the one actually loaded.
|
||||
problems.push(...(await checkRoutingManifest(routing, index, await agentIds(dir))));
|
||||
}
|
||||
}
|
||||
|
||||
problems.sort(
|
||||
(a, b) => a.file.localeCompare(b.file) || a.line - b.line || a.problem.localeCompare(b.problem),
|
||||
);
|
||||
return {
|
||||
ok: problems.length === 0,
|
||||
problems,
|
||||
refs,
|
||||
scannedFiles,
|
||||
citedPaths: [...citedPaths].sort(),
|
||||
};
|
||||
}
|
||||
|
||||
/** The file a target names, with any `glob:` prefix, `#anchor` or `:line` stripped. */
|
||||
function citedBasename(target: string): string {
|
||||
const withoutPrefix = target.startsWith(GLOB_PREFIX) ? target.slice(GLOB_PREFIX.length) : target;
|
||||
const withoutLine = LINE_CITATION_RE.exec(withoutPrefix)?.[1] ?? withoutPrefix;
|
||||
return path.basename(splitAnchor(withoutLine)[0]);
|
||||
}
|
||||
|
||||
/** How many examples a review-side note names before saying "and N more". */
|
||||
const NOTE_EXAMPLES = 5;
|
||||
|
||||
function andMore(items: string[]): string {
|
||||
const shown = items.slice(0, NOTE_EXAMPLES).join(", ");
|
||||
const rest = items.length - NOTE_EXAMPLES;
|
||||
return rest > 0 ? `${shown}, and ${rest} more` : shown;
|
||||
}
|
||||
|
||||
// @ref LLP 0012#run-points-command-and-review [constrained-by] — advises, never fails the review
|
||||
/**
|
||||
* The advice a review gives about its own setup: refs that no longer resolve, and cited
|
||||
* code this PR changes (where the ref still resolves but the guidance may not). Returns
|
||||
* an empty array when the setup is clean, so a healthy run stays silent.
|
||||
*/
|
||||
export async function reviewSetupRefNotes(options: {
|
||||
root: string;
|
||||
setupDirs: string[];
|
||||
changedFiles: string[];
|
||||
}): Promise<string[]> {
|
||||
let report: RefCheckReport;
|
||||
try {
|
||||
report = await checkConfigRefs({ root: options.root, setupDirs: options.setupDirs });
|
||||
} catch {
|
||||
return []; // a check that cannot run must never degrade the review
|
||||
}
|
||||
const notes: string[] = [];
|
||||
// The read root is scrubbed of ambient runtime config (AGENTS.md, CLAUDE.md, .env…),
|
||||
// so a ref to one is missing by design here, not stale. The template itself cites
|
||||
// AGENTS.md, which would make this note cry wolf on every PR.
|
||||
const broken = report.problems.filter(
|
||||
(problem) =>
|
||||
problem.kind !== "unannotated-citation" &&
|
||||
!(problem.target && isAmbientRuntimeConfig(citedBasename(problem.target))),
|
||||
);
|
||||
if (broken.length > 0) {
|
||||
notes.push(
|
||||
`The reviewer setup cites code that no longer resolves (${broken.length} ref(s)): ` +
|
||||
`${andMore(broken.map((problem) => `${problem.file}:${problem.line}`))}. ` +
|
||||
"Run `ecr ref-check` — the prompts may be reviewing against code that moved.",
|
||||
);
|
||||
}
|
||||
const touched = citedPathsTouchedBy(report, options.changedFiles);
|
||||
if (touched.length > 0) {
|
||||
notes.push(
|
||||
`This PR changes code the reviewer prompts cite (${andMore(touched)}). ` +
|
||||
"Check that the guidance quoting it is still correct.",
|
||||
);
|
||||
}
|
||||
return notes;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which of this PR's changed files are cited by a ref. The review uses this to say
|
||||
* "you moved code the reviewer prompts point at" even when the ref still resolves.
|
||||
*/
|
||||
export function citedPathsTouchedBy(report: RefCheckReport, changedFiles: string[]): string[] {
|
||||
const touched = new Set<string>();
|
||||
for (const changed of changedFiles) {
|
||||
for (const cited of report.citedPaths) {
|
||||
if (changed === cited || changed.startsWith(`${cited}/`)) {
|
||||
touched.add(cited);
|
||||
}
|
||||
}
|
||||
}
|
||||
return [...touched].sort();
|
||||
}
|
||||
+2
-35
@@ -206,7 +206,6 @@ export function renderMarkdown(
|
||||
link?: LinkContext,
|
||||
feedback: FeedbackRecord[] = [],
|
||||
pins: FeedbackPin[] = [],
|
||||
inputHash?: string,
|
||||
): string {
|
||||
const dismissedByFp = new Map(dismissed.map((record) => [record.fp, record]));
|
||||
const withFp = review.findings.map((finding) => ({ finding, fp: fingerprintFinding(finding) }));
|
||||
@@ -239,7 +238,6 @@ export function renderMarkdown(
|
||||
);
|
||||
}
|
||||
|
||||
lines.push(...setupNote(review.setupNotes));
|
||||
lines.push(...requalificationAuditNote(requalified.map((entry) => entry.finding)));
|
||||
lines.push(...feedbackAuditNote([...feedbackByFp.values()]));
|
||||
|
||||
@@ -284,9 +282,7 @@ export function renderMarkdown(
|
||||
const fingerprints = review.findings.map(fingerprintFinding);
|
||||
lines.push("", `<!-- ${tag}:fingerprints=${JSON.stringify(fingerprints)} -->`);
|
||||
lines.push(
|
||||
`<!-- ${tag}:state=${encodeState(
|
||||
reviewState({ review, dismissed, ...(inputHash ? { inputHash } : {}) }, feedbackByFp, pins),
|
||||
)} -->`,
|
||||
`<!-- ${tag}:state=${encodeState(reviewState({ review, dismissed }, feedbackByFp, pins))} -->`,
|
||||
);
|
||||
return lines.join("\n");
|
||||
}
|
||||
@@ -356,15 +352,6 @@ function renderFindingLines(
|
||||
return out;
|
||||
}
|
||||
|
||||
// @ref LLP 0012#run-points-command-and-review [implements] — setup advice renders outside the findings list, so it never blocks
|
||||
/** Advice about the reviewer's own setup (stale refs, cited code this PR moves). */
|
||||
function setupNote(notes: string[] = []): string[] {
|
||||
if (notes.length === 0) {
|
||||
return [];
|
||||
}
|
||||
return ["> 🔗 **Review setup:**", ...notes.map((note) => `> - ${stripStateMarkers(note)}`), ""];
|
||||
}
|
||||
|
||||
// @ref LLP 0010#rendering-in-all-three-paths [implements] — the visible audit count is mandatory: requalification's only effect on a real finding is moving it out of the blocking set, so it must never be silent
|
||||
/**
|
||||
* The visible one-line audit note in the OPEN body, naming the addressing PRs. This
|
||||
@@ -493,8 +480,6 @@ export interface ScopeReviewResult {
|
||||
/** config '.' → un-namespaced fingerprints (dismissal carry-over). */
|
||||
isDefault: boolean;
|
||||
review: CoordinatorOutput;
|
||||
/** Hash of the complete input that produced `review`; absent means do not reuse. */
|
||||
inputHash?: string;
|
||||
}
|
||||
|
||||
/** The machine-readable state embedded in the reviewer's comment. */
|
||||
@@ -502,8 +487,6 @@ export interface ReviewState {
|
||||
/** v1 field, kept: the aggregate stores the MERGED output here. */
|
||||
review: CoordinatorOutput;
|
||||
dismissed: DismissalRecord[];
|
||||
/** v5: legacy/single-scope review-result cache key. */
|
||||
inputHash?: string;
|
||||
/** v2: present only on aggregate comments (routing, comment:'single'). */
|
||||
scopes?: ScopeReviewResult[];
|
||||
/** v3: author replies matched to the findings shown in this comment. */
|
||||
@@ -627,15 +610,6 @@ export function renderAggregateMarkdown(
|
||||
lines.push("");
|
||||
}
|
||||
|
||||
const setupLines = results.flatMap((result) =>
|
||||
(result.review.setupNotes ?? []).map(
|
||||
(note) => `> - [${stripStateMarkers(result.scope)}] ${stripStateMarkers(note)}`,
|
||||
),
|
||||
);
|
||||
if (setupLines.length > 0) {
|
||||
lines.push("> 🔗 **Review setup:**", ...setupLines, "");
|
||||
}
|
||||
|
||||
// Shown = the most-severe N kept findings per scope (N = limitPerScope). The
|
||||
// embedded state trims KEPT findings to the same set so a truncated comment
|
||||
// still fits GitHub's body limit (the hidden findings are noted, not silently
|
||||
@@ -737,7 +711,7 @@ export function renderAggregateMarkdown(
|
||||
// across re-renders. The per-scope data (`scopes`) plus a merged v1 `review`
|
||||
// keep both v2 and v1 consumers working.
|
||||
const stateScopes: ScopeReviewResult[] = rendered.map(
|
||||
({ result, shown, hidden, requalified, requalifiedHidden, dropped }) => {
|
||||
({ result, shown, requalified, dropped }) => {
|
||||
// @ref LLP 0011#suppression-is-never-silent — strip any per-scope `feedback`
|
||||
// a freshly-reviewed scope's ReviewRunResult carries: the top-level feedback
|
||||
// array (reviewState below) is the single source of truth. A stale per-scope
|
||||
@@ -749,13 +723,6 @@ export function renderAggregateMarkdown(
|
||||
return {
|
||||
scope: result.scope,
|
||||
isDefault: result.isDefault,
|
||||
// A truncated state is not the full result and therefore cannot be a cache
|
||||
// source. Omit its hash so the next run retries instead of reusing a subset.
|
||||
...(hidden === 0 && requalifiedHidden === 0
|
||||
? result.inputHash
|
||||
? { inputHash: result.inputHash }
|
||||
: {}
|
||||
: {}),
|
||||
// Requalified findings ride the embedded state (like dismissed ones) so a
|
||||
// re-render (/dismiss) round-trips them and the addressed section persists.
|
||||
// Under truncation they are trimmed exactly like `shown` — state bytes count
|
||||
|
||||
@@ -1,142 +0,0 @@
|
||||
// @ref LLP 0005#review-result-cache [implements] — automated CI reuses only a complete result whose full review input hashes identically
|
||||
import { createHash } from "node:crypto";
|
||||
import { createReadStream } from "node:fs";
|
||||
import { lstat, readlink } from "node:fs/promises";
|
||||
import { createRequire } from "node:module";
|
||||
import path from "node:path";
|
||||
|
||||
import type { LoadedConfig } from "../config/schema.js";
|
||||
import { pathInside } from "./exec.js";
|
||||
import type { CoordinatorOutput, DiffEntry, ReviewMetadata } from "./schema.js";
|
||||
|
||||
const packageVersion = (createRequire(import.meta.url)("../../package.json") as { version: string })
|
||||
.version;
|
||||
|
||||
/**
|
||||
* Version the cache-key contract independently of the embedded comment-state shape.
|
||||
* Bump this whenever review inputs gain a new source that is not represented below.
|
||||
*/
|
||||
const REVIEW_INPUT_HASH_VERSION = 1;
|
||||
|
||||
export interface ReviewInputHashOptions {
|
||||
files: DiffEntry[];
|
||||
config: LoadedConfig;
|
||||
metadata: Pick<ReviewMetadata, "title" | "body">;
|
||||
/** PR-head tree. Changed-file contents are hashed from here, never the base checkout. */
|
||||
readRoot: string;
|
||||
agents?: string[];
|
||||
route?: boolean;
|
||||
contextText?: string;
|
||||
}
|
||||
|
||||
/** Canonical JSON: object-key order never turns the same input into a cache miss. */
|
||||
function canonicalJson(value: unknown): string {
|
||||
if (value === null || typeof value !== "object") {
|
||||
return JSON.stringify(value) ?? "null";
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
return `[${value.map(canonicalJson).join(",")}]`;
|
||||
}
|
||||
const record = value as Record<string, unknown>;
|
||||
return `{${Object.keys(record)
|
||||
.filter((key) => record[key] !== undefined)
|
||||
.sort()
|
||||
.map((key) => `${JSON.stringify(key)}:${canonicalJson(record[key])}`)
|
||||
.join(",")}}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hash every input that can change the code-review result while deliberately
|
||||
* excluding checkout-local paths and commit OIDs. A restack changes OIDs; when the
|
||||
* scoped changed-file contents, normalized patch, prompts/config and PR prose are
|
||||
* byte-identical, that is the cache hit this key is meant to recognize.
|
||||
*/
|
||||
function normalizedPatch(patch: string, binary: boolean): string {
|
||||
return (
|
||||
patch
|
||||
.split("\n")
|
||||
// Blob ids are volatile across a restack. A binary patch has no textual content
|
||||
// to hash, so retain its index line as the only content identity available.
|
||||
.filter((line) => binary || !line.startsWith("index "))
|
||||
// Unrelated lines landing earlier in the file move hunk coordinates without
|
||||
// changing the code under review. Preserve any trailing function heading.
|
||||
.map((line) => line.replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@(.*)$/, "@@ @@$1"))
|
||||
.join("\n")
|
||||
);
|
||||
}
|
||||
|
||||
async function fileContentIdentity(
|
||||
root: string,
|
||||
file: DiffEntry,
|
||||
): Promise<{ kind: string; digest?: string; mode?: number; target?: string }> {
|
||||
const abs = path.resolve(root, file.path);
|
||||
if (!pathInside(abs, root)) {
|
||||
throw new Error(`review cache path escapes the PR tree: ${file.path}`);
|
||||
}
|
||||
let stat;
|
||||
try {
|
||||
stat = await lstat(abs);
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
|
||||
return { kind: "missing" };
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
if (stat.isSymbolicLink()) {
|
||||
// Never follow a PR-controlled symlink out of the materialized tree.
|
||||
return { kind: "symlink", target: await readlink(abs), mode: stat.mode & 0o777 };
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
return { kind: "other", mode: stat.mode & 0o777 };
|
||||
}
|
||||
const hash = createHash("sha256");
|
||||
for await (const chunk of createReadStream(abs)) {
|
||||
hash.update(chunk as Buffer);
|
||||
}
|
||||
return { kind: "file", digest: hash.digest("hex"), mode: stat.mode & 0o777 };
|
||||
}
|
||||
|
||||
export async function reviewInputHash(options: ReviewInputHashOptions): Promise<string> {
|
||||
const { configDir: _configDir, ...portableConfig } = options.config;
|
||||
const sortedFiles = [...options.files].sort(
|
||||
(a, b) => a.path.localeCompare(b.path) || (a.status ?? "").localeCompare(b.status ?? ""),
|
||||
);
|
||||
const files = [];
|
||||
// Sequential reads avoid opening one descriptor per changed file on a very wide PR.
|
||||
for (const file of sortedFiles) {
|
||||
files.push({
|
||||
path: file.path,
|
||||
patch: normalizedPatch(file.patch, file.binary === true),
|
||||
status: file.status ?? "",
|
||||
binary: file.binary === true,
|
||||
content: await fileContentIdentity(options.readRoot, file),
|
||||
});
|
||||
}
|
||||
const input = {
|
||||
version: REVIEW_INPUT_HASH_VERSION,
|
||||
engineVersion: packageVersion,
|
||||
files,
|
||||
config: portableConfig,
|
||||
metadata: options.metadata,
|
||||
agents: options.agents ? [...options.agents].sort() : null,
|
||||
route: options.route === true,
|
||||
contextText: options.contextText ?? null,
|
||||
};
|
||||
return createHash("sha256").update(canonicalJson(input)).digest("hex");
|
||||
}
|
||||
|
||||
/** Partial/failed reviews must be retried, never made durable by a cache hit. */
|
||||
export function reviewCanBeReused(review: CoordinatorOutput): boolean {
|
||||
return review.couldNotComplete !== true && review.incomplete.length === 0;
|
||||
}
|
||||
|
||||
/** Exact hash match plus the completeness guard used by every CI comment shape. */
|
||||
export function reviewMatchesInput(
|
||||
review: CoordinatorOutput,
|
||||
storedHash: string | undefined,
|
||||
currentHash: string,
|
||||
): boolean {
|
||||
return (
|
||||
storedHash === currentHash && /^[a-f0-9]{64}$/.test(storedHash) && reviewCanBeReused(review)
|
||||
);
|
||||
}
|
||||
+1
-19
@@ -50,7 +50,6 @@ import { confirmStackRequalifications, patchConfirmer } from "./stack-confirm.js
|
||||
import { sortFindings } from "./render.js";
|
||||
import { appendStepSummary } from "./step-summary.js";
|
||||
import { errorMessage, sleep } from "./util.js";
|
||||
import { reviewSetupRefNotes } from "./config-refs.js";
|
||||
import { verifyFindings } from "./verify.js";
|
||||
import { applyInlineIgnores } from "./suppress.js";
|
||||
|
||||
@@ -298,20 +297,6 @@ export async function runReview(
|
||||
process.chdir(readRoot.dir);
|
||||
}
|
||||
|
||||
// Ref integrity of the setup that is about to review this PR. Resolved against the
|
||||
// tree the reviewers read (PR head when materialized), while the setup itself may
|
||||
// come from the trusted base ref — so a PR that moves cited code is reported against
|
||||
// the prompts that will actually judge it.
|
||||
// @ref LLP 0012#run-points-command-and-review [implements] — every review checks its own refs; advice only, never a gate
|
||||
const setupNotes = await reviewSetupRefNotes({
|
||||
root: readRoot?.dir ?? originalCwd,
|
||||
setupDirs: [config.configDir],
|
||||
changedFiles: kept.map((entry) => entry.path),
|
||||
});
|
||||
for (const note of setupNotes) {
|
||||
progress(` setup: ${note}`);
|
||||
}
|
||||
|
||||
const starting = [
|
||||
usesClaude ? "Claude Code engine" : null,
|
||||
usesOpencode ? "OpenCode server" : null,
|
||||
@@ -1182,10 +1167,7 @@ export async function runReview(
|
||||
summary: output.summary,
|
||||
});
|
||||
|
||||
// Engine-owned: overwrite whatever the coordinator may have emitted under this key,
|
||||
// so setup advice is always the checker's, never model text.
|
||||
const reviewed = { ...output, setupNotes };
|
||||
return feedbackRecords ? { ...reviewed, feedback: feedbackRecords } : reviewed;
|
||||
return feedbackRecords ? { ...output, feedback: feedbackRecords } : output;
|
||||
} catch (error) {
|
||||
await safeLog(logPath, {
|
||||
...baseRecord,
|
||||
|
||||
@@ -170,16 +170,6 @@ export const CoordinatorOutputSchema = z.object({
|
||||
* overrides the presentation instead).
|
||||
*/
|
||||
couldNotComplete: z.boolean().optional(),
|
||||
/**
|
||||
* Advice about the review's OWN setup: refs in `.expo-code-review/` that no longer
|
||||
* resolve, and cited code this PR changes. Engine-set (never the model, never a
|
||||
* finding) and never blocking — a stale prompt is a maintenance signal, not a defect
|
||||
* in the PR.
|
||||
*/
|
||||
// @ref LLP 0012#run-points-command-and-review [implements] — the review advises about stale refs instead of failing on them
|
||||
// Optional (like couldNotComplete) so every internal CoordinatorOutput literal stays
|
||||
// valid without restating an engine-owned field.
|
||||
setupNotes: z.array(z.string()).optional(),
|
||||
});
|
||||
export type CoordinatorOutput = z.infer<typeof CoordinatorOutputSchema>;
|
||||
|
||||
|
||||
@@ -468,11 +468,7 @@ export class GitHubReporter implements Reporter {
|
||||
* (annotate mode). Either way the feedback path fails soft — it never blocks the
|
||||
* comment from being posted.
|
||||
*/
|
||||
async report(
|
||||
review: CoordinatorOutput,
|
||||
feedback?: FeedbackRecord[],
|
||||
inputHash?: string,
|
||||
): Promise<void> {
|
||||
async report(review: CoordinatorOutput, feedback?: FeedbackRecord[]): Promise<void> {
|
||||
// Carry forward any per-PR dismissals recorded in the existing comment so they
|
||||
// survive re-reviews (a dismissed finding stays in the collapsed section).
|
||||
const existing = await this.findExistingComment();
|
||||
@@ -491,7 +487,7 @@ export class GitHubReporter implements Reporter {
|
||||
: await this.computeFeedback(withFp, priorRecords, pinsIn);
|
||||
const link = await this.linkContextAsync();
|
||||
await this.upsertComment(
|
||||
renderMarkdown(review, this.options.commentTag, dismissed, link, records, pins, inputHash),
|
||||
renderMarkdown(review, this.options.commentTag, dismissed, link, records, pins),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -838,7 +834,6 @@ export class GitHubReporter implements Reporter {
|
||||
link,
|
||||
next.feedback,
|
||||
next.pins,
|
||||
state.inputHash,
|
||||
);
|
||||
await this.patchComment(existing.id, body);
|
||||
return {
|
||||
|
||||
@@ -65,15 +65,6 @@ export class TerminalReporter implements Reporter {
|
||||
out.push("");
|
||||
}
|
||||
|
||||
const setupNotes = review.setupNotes ?? [];
|
||||
if (setupNotes.length > 0) {
|
||||
out.push(this.paint(BOLD, "🔗 Review setup:"));
|
||||
for (const note of setupNotes) {
|
||||
out.push(this.paint(DIM, ` - ${note}`));
|
||||
}
|
||||
out.push("");
|
||||
}
|
||||
|
||||
if (review.findings.length === 0) {
|
||||
out.push(this.paint(DIM, "No findings."), "");
|
||||
} else {
|
||||
|
||||
@@ -39,10 +39,7 @@ must support `--non-interactive` the way sibling commands do (a non-interactive
|
||||
path with no prompts, erroring clearly when a required value is missing), and it
|
||||
must expose flags to supply every prompted value so the command stays scriptable.
|
||||
|
||||
<!-- TODO: replace the example above with this repo's most important conventions.
|
||||
Pin each file, directory, or symbol you cite with a ref on its own line —
|
||||
@ref <path/to/file.ts>#<symbol> — why this matters
|
||||
so `ecr ref-check` fails when the cited code moves. Never a line number. -->
|
||||
<!-- TODO: replace the example above with this repo's most important conventions. -->
|
||||
|
||||
## What NOT to flag
|
||||
|
||||
|
||||
@@ -19,13 +19,7 @@ issues in the changed code.
|
||||
concrete trigger.
|
||||
|
||||
<!-- TODO: customize for this repo — add project-specific correctness rules,
|
||||
e.g. framework conventions, required flag handling, API compatibility.
|
||||
|
||||
Cite real code, and pin every citation with a ref so `ecr ref-check` fails when
|
||||
it moves. In a comment of its own, on one line:
|
||||
@ref <path/to/file.ts>#<symbol> — why this matters
|
||||
Targets: a file, a `<dir>/`, `glob:<pattern>`, `<file>#<symbol>`, `<doc>.md#<heading>`.
|
||||
Never a line number. Not a path? `@ref-ignore <token>`. -->
|
||||
e.g. framework conventions, required flag handling, API compatibility. -->
|
||||
|
||||
## What NOT to flag
|
||||
|
||||
|
||||
@@ -3,11 +3,11 @@
|
||||
description: Security and secrets. Injection, credential or secret leakage, unsafe shell/child-process use, missing validation at trust boundaries.
|
||||
alwaysRun: true
|
||||
# Security is the highest-stakes agent and benefits most from stronger threat-model
|
||||
# reasoning, so it runs on the Opus tier even though the other specialists use the
|
||||
# reasoning, so it runs on the pro tier even though the other specialists use the
|
||||
# default model. Scoped to this one agent to limit the extra latency/rate-limit cost;
|
||||
# subdivide-on-timeout + the per-fetch deadline keep a slow Opus pass from hanging.
|
||||
# subdivide-on-timeout + the per-fetch deadline keep a slow pro pass from hanging.
|
||||
# @ref LLP 0009#config-and-prompt-templates [implements]
|
||||
model: anthropic/claude-opus-5
|
||||
model: openai/gpt-5.5-pro
|
||||
---
|
||||
|
||||
# Security & secrets
|
||||
@@ -26,14 +26,7 @@ average severity.
|
||||
- Insecure file permissions, or writing secrets to world-readable paths.
|
||||
|
||||
<!-- TODO: customize for this repo — name the sensitive surfaces specific to this
|
||||
codebase (credential stores, tokens, arbitrary-command features, etc.).
|
||||
|
||||
Pin every path or symbol you cite with a ref, on a line of its own:
|
||||
@ref <path/to/file.ts>#<symbol> — why this matters
|
||||
`ecr ref-check` then fails when that code moves, so this prompt never sends a
|
||||
reviewer after a file that no longer exists. Never cite a line number. -->
|
||||
|
||||
<!-- @ref glob:.github/workflows/** — the workflows this section judges -->
|
||||
codebase (credential stores, tokens, arbitrary-command features, etc.). -->
|
||||
|
||||
## CI / workflow supply-chain (changes under `.github/workflows/**`)
|
||||
|
||||
|
||||
+5
-11
@@ -81,11 +81,6 @@ jobs:
|
||||
# disable setup-node's auto package-manager cache.
|
||||
package-manager-cache: false
|
||||
|
||||
# The anthropic/… models run through the Claude Code CLI (claude-code
|
||||
# engine). Pinned exactly — see workflow.yml (expo-code-review.yml).
|
||||
- name: Install Claude Code CLI
|
||||
run: npm install -g @anthropic-ai/[email protected]
|
||||
|
||||
# SECURITY: the base-ref checkout includes every .expo-code-review/ config,
|
||||
# whose auth.tokenEnv names the forwarded model credential. `ecr verify-config`
|
||||
# sweeps every config and refuses unless tokenEnv appears exactly once, in a
|
||||
@@ -94,7 +89,7 @@ jobs:
|
||||
# @ref LLP 0009#guard-step-ordering-and-job-budgets [implements] — same $ECR_VERSION feeds guard and review
|
||||
- name: Guard config tokenEnv (root + routing + all scopes)
|
||||
env:
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
run: npx --yes -p "@expo/code-review-cli@$ECR_VERSION" ecr verify-config
|
||||
|
||||
# Running via `issue_comment` makes this a manual /review, which the CLI detects
|
||||
@@ -106,11 +101,10 @@ jobs:
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
# Layer-1 auth lock: keep in sync with the guard's EXPECTED.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Anthropic review credential — the env var named by auth.tokenEnv in
|
||||
# config.jsonc (see workflow.yml for the accepted token shapes).
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
# OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
# Optional: override the model for every agent (uses your OpenCode login).
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
run: |
|
||||
# Build the flags as a bash array so the path never word-splits.
|
||||
|
||||
+7
-14
@@ -111,12 +111,6 @@ jobs:
|
||||
# to save an empty cache and error).
|
||||
package-manager-cache: false
|
||||
|
||||
# The anthropic/… models run through the Claude Code CLI (claude-code
|
||||
# engine). Pinned exactly — see workflow.yml (expo-code-review.yml).
|
||||
- name: Install Claude Code CLI
|
||||
if: steps.cmd.outputs.run == 'true'
|
||||
run: npm install -g @anthropic-ai/[email protected]
|
||||
|
||||
# SECURITY: the base-ref checkout above includes every .expo-code-review/
|
||||
# config.jsonc + routing.jsonc, whose auth.tokenEnv names the env var the CLI
|
||||
# forwards as the model credential. The canonical guard ships with the CLI:
|
||||
@@ -132,7 +126,7 @@ jobs:
|
||||
if: steps.cmd.outputs.run == 'true'
|
||||
env:
|
||||
# (Comma-separated set for a multi-credential auth.providers config.)
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
run: npx --yes -p "@expo/code-review-cli@$ECR_VERSION" ecr verify-config
|
||||
|
||||
- name: Run AI review
|
||||
@@ -142,13 +136,12 @@ jobs:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
# Layer-1 auth lock: the CLI refuses to run when the tokenEnv it would honor
|
||||
# differs from this. Keep it in sync with the guard's EXPECTED.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Anthropic review credential — the env var named by auth.tokenEnv in
|
||||
# config.jsonc. Store it as a repo secret: an `sk-ant-oat…` token minted
|
||||
# by `claude setup-token`, or an `sk-ant-api…` Console key (the Claude
|
||||
# Code CLI reads either).
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
# OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.
|
||||
# Store it as a repo secret; a project-scoped key restricted to model
|
||||
# inference (with a spend limit) is all the reviewer needs.
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
# Optional: override the model for every agent (uses your OpenCode login).
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
AGENTS: ${{ steps.cmd.outputs.agents }}
|
||||
ROUTE: ${{ steps.cmd.outputs.route }}
|
||||
|
||||
+39
-31
@@ -1,17 +1,15 @@
|
||||
// @ref LLP 0009#config-and-prompt-templates — root config: agent roster by filename, phase-1 defaults, auth
|
||||
{
|
||||
// Default model for every agent. Anthropic models run through the Claude Code
|
||||
// CLI engine (`claude -p`), inferred per agent from the model id — see `auth`
|
||||
// below for the credential. Override per-agent via frontmatter in the agent's
|
||||
// markdown, or at runtime with the REVIEWER_MODEL env var
|
||||
// (e.g. REVIEWER_MODEL=anthropic/claude-haiku-4-5).
|
||||
"model": "anthropic/claude-sonnet-5",
|
||||
// Default model for every agent. Override per-agent via frontmatter in the
|
||||
// agent's markdown, or at runtime with the REVIEWER_MODEL env var
|
||||
// (e.g. REVIEWER_MODEL=openai/gpt-5.4-mini).
|
||||
"model": "openai/gpt-5.5",
|
||||
|
||||
// Agents: every markdown file in agents/ is one reviewer (id = filename).
|
||||
// Add or remove files to change the roster — no list needed here.
|
||||
// shared.md (prepended to every agent + coordinator) and coordinator.md are
|
||||
// reserved filenames. Per-agent overrides go in each file's YAML frontmatter,
|
||||
// e.g. `---\nmodel: anthropic/claude-opus-5\n---`.
|
||||
// e.g. `---\nmodel: openai/gpt-5.5-pro\n---`.
|
||||
|
||||
// @ref LLP 0009#config-and-prompt-templates [implements] — suggestions off by default, not a schema limit
|
||||
"policy": {
|
||||
@@ -48,39 +46,49 @@
|
||||
// HTML marker used to find + update the single PR comment. Keep it stable.
|
||||
"commentTag": "expo-ai-code-reviewer",
|
||||
|
||||
// How model credentials are provided. Default: Anthropic via the Claude Code
|
||||
// CLI. Anthropic models always run through that CLI — the engine is inferred
|
||||
// per agent from the `anthropic/…` model id, so this entry only supplies the
|
||||
// credential. In CI the tokenEnv holds the review credential (an `sk-ant-oat…`
|
||||
// token minted by `claude setup-token`, or an `sk-ant-api…` Console key — the
|
||||
// CLI reads either); locally an active `claude` login is enough, and the entry
|
||||
// could even be omitted. ECR_EXPECTED_TOKEN_ENV (repo variable / workflow
|
||||
// fallback) must equal this tokenEnv. `ecr setup-auth` walks you through it.
|
||||
//
|
||||
// Other providers run through OpenCode:
|
||||
// How model credentials are provided. Default: an OpenAI API key.
|
||||
// "api-key": tokenEnv names the env var holding a provider API key. In CI,
|
||||
// store the key as a repo secret and pass it under that env var.
|
||||
// "auth": { "mode": "api-key", "provider": "openai", "tokenEnv": "OPENAI_API_KEY" }
|
||||
// store the key as a repo secret and pass it under that env var
|
||||
// (the scaffolded workflow does). If you omit `auth` entirely,
|
||||
// OpenCode's own login / ambient provider env vars are used.
|
||||
// For Anthropic/Claude, see the dedicated block below (it now runs through the
|
||||
// Claude Code CLI). For another provider, omit `auth` and set REVIEWER_MODEL
|
||||
// after an `opencode auth login` for that provider.
|
||||
//
|
||||
// ChatGPT/Codex subscription ("oauth"): tokenEnv holds the ACCESS token from
|
||||
// an `opencode auth login` ChatGPT sign-in — `ecr setup-auth` extracts it.
|
||||
// NEVER share the refresh token: it is single-use and dies on first rotation.
|
||||
// MIXED setup (a ChatGPT/Codex subscription for the default models, plus a
|
||||
// metered API key for pro-tier models the subscription doesn't offer): use the
|
||||
// per-provider map instead, reference `openai-api/...` models in the frontmatter
|
||||
// of the agents that need the pro tier, and set ECR_EXPECTED_TOKEN_ENV in the
|
||||
// workflow to the comma-separated set of both env names.
|
||||
// "auth": { "providers": {
|
||||
// "openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" }
|
||||
// "openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" },
|
||||
// "openai-api": { "mode": "api-key", "tokenEnv": "OPENAI_API_KEY", "upstream": "openai" }
|
||||
// } }
|
||||
// (openai oauth: tokenEnv holds the ACCESS token from an `opencode auth login`
|
||||
// ChatGPT sign-in — `ecr setup-auth` extracts it. NEVER share the refresh
|
||||
// token: it is single-use and dies on first rotation.)
|
||||
//
|
||||
// Anthropic runs through the Claude Code CLI, inferred from the model — set your
|
||||
// models to "anthropic/…" (e.g. anthropic/claude-opus-5, anthropic/claude-sonnet-5)
|
||||
// and run `claude setup-token`, exporting the token as CLAUDE_CODE_OAUTH_TOKEN (CI)
|
||||
// or rely on your local `claude` login. An auth entry is OPTIONAL (tokenEnv just
|
||||
// names the credential env); `ecr setup-auth` walks you through it.
|
||||
// "auth": { "providers": {
|
||||
// "anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" }
|
||||
// } }
|
||||
//
|
||||
// MIXING engines is supported: the engine is inferred per agent from its model,
|
||||
// so this anthropic entry may coexist with an openai (or any other) OpenCode
|
||||
// provider — an `anthropic/…` agent runs through the Claude Code CLI while an
|
||||
// `openai/…` agent runs through OpenCode, in the SAME run:
|
||||
// MIXING engines is supported: the engine is inferred per agent from its model, so
|
||||
// an anthropic entry may coexist with an openai (or any other) OpenCode provider.
|
||||
// Each agent's `model` selects its engine — an `anthropic/…` agent runs through the
|
||||
// Claude Code CLI while an `openai/…` agent runs through OpenCode, in the SAME run.
|
||||
// "auth": { "providers": {
|
||||
// "anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" },
|
||||
// "anthropic": { "tokenEnv": "CLAUDE_CODE_OAUTH_TOKEN" },
|
||||
// "openai": { "mode": "oauth", "tokenEnv": "CODEX_OAUTH_ACCESS_TOKEN" }
|
||||
// } }
|
||||
"auth": {
|
||||
"providers": {
|
||||
"anthropic": { "tokenEnv": "CLAUDE_CODE_REVIEW_SHARED_API_TOKEN" }
|
||||
}
|
||||
"mode": "api-key",
|
||||
"provider": "openai",
|
||||
"tokenEnv": "OPENAI_API_KEY"
|
||||
},
|
||||
|
||||
// Stack-aware requalification (ROOT-ONLY; off by default). When on, `ecr ci` walks
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
<!-- @ref LLP 0009#config-and-prompt-templates — pro tier pinned on purpose: consolidation quality over serial-tail latency -->
|
||||
---
|
||||
# The coordinator makes the final call — de-duping, re-judging severity, and
|
||||
# deciding — so it runs on the Opus tier: consolidation quality matters more here
|
||||
# deciding — so it runs on the pro tier: consolidation quality matters more here
|
||||
# than the small serial-tail latency it adds (no repo tools, one bounded pass).
|
||||
# Override with a cheaper model if you'd rather trade decision quality for latency.
|
||||
# @ref LLP 0009#config-and-prompt-templates [implements]
|
||||
model: anthropic/claude-opus-5
|
||||
model: openai/gpt-5.5-pro
|
||||
---
|
||||
|
||||
# Coordinator — consolidation & decision
|
||||
|
||||
@@ -12,9 +12,6 @@ These rules apply to every reviewer and are concatenated onto your role prompt.
|
||||
- **Do not judge the diff in isolation.** Before reporting, read the surrounding
|
||||
source with your file/read/grep tools and trace the relevant execution path.
|
||||
If you cannot substantiate a concrete failure or exploit path, do not report it.
|
||||
<!-- Generic guidance, not a claim that either file exists. Once this repo has one,
|
||||
replace this with a real ref: @ref AGENTS.md — the conventions to judge against -->
|
||||
<!-- @ref-ignore AGENTS.md CLAUDE.md -->
|
||||
- Ground your judgment in the repo's own conventions (`AGENTS.md` / `CLAUDE.md`
|
||||
at the repo root, and any per-directory guidance) rather than generic
|
||||
best-practices.
|
||||
|
||||
+7
-15
@@ -67,13 +67,6 @@ jobs:
|
||||
# to save an empty cache and error).
|
||||
package-manager-cache: false
|
||||
|
||||
# The anthropic/… models in .expo-code-review/config.jsonc run through the
|
||||
# Claude Code CLI (the claude-code engine). Pinned exactly — an unpinned CLI
|
||||
# would silently drift on every CI run; bump it deliberately. Remove this
|
||||
# step if you switch every model to an OpenCode-served provider.
|
||||
- name: Install Claude Code CLI
|
||||
run: npm install -g @anthropic-ai/[email protected]
|
||||
|
||||
# SECURITY: the TRUSTED BASE checkout above includes every
|
||||
# .expo-code-review/config.jsonc + routing.jsonc, whose auth.tokenEnv names the
|
||||
# env var the CLI forwards as the model credential. The canonical guard ships
|
||||
@@ -94,7 +87,7 @@ jobs:
|
||||
- name: Guard config tokenEnv (root + routing + all scopes)
|
||||
env:
|
||||
# (Comma-separated set for a multi-credential auth.providers config.)
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
run: npx --yes -p "@expo/code-review-cli@$ECR_VERSION" ecr verify-config
|
||||
|
||||
- name: Run AI review
|
||||
@@ -108,13 +101,12 @@ jobs:
|
||||
# honor (root config.jsonc, or routing.jsonc defaults.auth) differs from
|
||||
# this — it catches what the guard step above can't. Keep it in sync
|
||||
# with the guard.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'CLAUDE_CODE_REVIEW_SHARED_API_TOKEN' }}
|
||||
# Anthropic review credential — the env var named by auth.tokenEnv in
|
||||
# config.jsonc. Store it as a repo secret: an `sk-ant-oat…` token minted
|
||||
# by `claude setup-token`, or an `sk-ant-api…` Console key (the Claude
|
||||
# Code CLI reads either).
|
||||
CLAUDE_CODE_REVIEW_SHARED_API_TOKEN: ${{ secrets.CLAUDE_CODE_REVIEW_SHARED_API_TOKEN }}
|
||||
# Optional: override the model for every agent.
|
||||
ECR_EXPECTED_TOKEN_ENV: ${{ vars.ECR_EXPECTED_TOKEN_ENV || 'OPENAI_API_KEY' }}
|
||||
# OpenAI API key — the env var named by auth.tokenEnv in config.jsonc.
|
||||
# Store it as a repo secret; a project-scoped key restricted to model
|
||||
# inference (with a spend limit) is all the reviewer needs.
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
# Optional: override the model for every agent (uses your OpenCode login).
|
||||
REVIEWER_MODEL: ${{ vars.REVIEWER_MODEL }}
|
||||
|
||||
# Observability: the per-run log (token/cache/cost totals + per-pass timing +
|
||||
|
||||
Reference in new issue
Block a user