- Local diffs also pass --submodule=short and --ignore-submodules=dirty, so
diff.submodule=log|diff, diff.ignoreSubmodules=all and ignore = all in
.gitmodules can't hide or rewrite a submodule pointer change, and edits
inside a submodule whose pointer didn't move aren't sent for review.
- The tests remove every GIT_* variable and point HOME and XDG_CONFIG_HOME
at an empty directory, so a hook's GIT_DIR/GIT_INDEX_FILE or the user's
global git files can't touch another repo or fail the fixtures.
- New tests: deletions under diff.noprefix and diff.mnemonicPrefix (the only
case --src-prefix protects), and the submodule settings above.
- LLP 0004 and 0008 line references updated.
A textconv driver showed converted text instead of the real file, or hid a change
when both sides converted alike, and `diff.renames=copies` turned a copied file
into a hunk-less entry whose contents went unreviewed. `--no-textconv` and
`--find-renames` join the shared flags.
The tests now fail when any one flag is removed: the fixture has a `b/` path that
`diff.noprefix` breaks, and new cases cover `diff.mnemonicPrefix`, textconv and copy
detection. Fixture commits ignore global signing and hooks, and LLP 0008's line
references are current again.
`git diff` follows the user's git config. An external diff tool
(`diff.external`, for example difftastic) replaces the unified diff, forced
color puts escape codes before `diff --git`, and `diff.noprefix` or
`diff.mnemonicPrefix` drop or change the `a/` and `b/` prefixes. With any of
them, a local `ecr review` parsed no files and reported "No reviewable
changes".
Every `git diff` that LocalGitSource parses now passes `--no-ext-diff
--no-color --src-prefix=a/ --dst-prefix=b/`. CI is unaffected: it reads
the PR diff from GitHub's API.
The Claude Code engine scopes every read tool to the PR-head tree, but in CI the patch workspace is written under the base checkout's `.runs/`, outside that tree. Every patch read the prompts ask for was denied: a reviewer's context-only files and the cross-file pass's files past its inline budget. The run's `patches/` directory is now a second read scope; the rest of `.runs/` stays unreadable.
Each `**/` in a routing or agent `paths` glob now matches zero or more directories on its own, and ref-check uses the same matcher. The diff parser decodes git-quoted paths (non-ASCII names), keeping a path quoted when decoding would be lossy or unsafe (invalid UTF-8, control/format/line-separator characters). Header lines are read only before the first hunk, so an added `++ b/x` line can no longer rename an entry, and a pure rename takes its path from `rename to`.
`--permission-mode dontAsk` rejects only tools that ask for permission. Against
Claude Code 2.1.280, a review pass could still run EnterWorktree (it created a
git worktree), RemoteTrigger (it listed the logged-in account's cloud
routines), SendMessage, and Skill, because none of them is on the deny list and
none asks for permission.
buildClaudeArgs now passes `--tools` with exactly the granted read tools
(`Read,Grep,Glob` or fewer, and "" for the coordinator). The CLI loads only
those built-ins, so a tool a future CLI version adds is never loaded. MCP tools
and the StructuredOutput tool used by --json-schema stay loaded. The scoped
allow rules, dontAsk, and ALWAYS_DENIED_TOOLS stay as inner layers.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
The scaffolded default model moves from anthropic/claude-sonnet-5 to
anthropic/claude-opus-5-5, and the security and coordinator pins move from
claude-opus-5 to claude-opus-5-5. Claude Code 2.1.212 rejects Opus 5.5
("version 2.1.280 or newer is required"), so every workflow template installs
2.1.280.
The CLI bump re-checked the deny list: 2.1.280 adds ListAgents, which is now
in ALWAYS_DENIED_TOOLS; every other denied name stays hidden.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
One .expo-agents/ directory now holds every Expo agent tool's per-repo
files (this tool's and @expo/verify's) instead of one root dot directory
per tool. Every lookup — root, scopes, the config-file sweep, the reference
index, the .runs log — takes the new name first and falls back to the
legacy .expo-code-review/, and ecr init reuses an existing legacy directory
rather than doubling it. The CI templates upload reviews.jsonl from either
location. This repo's own setup dir moved.
kill(pid, 0) succeeds while the killed grandchild is still a zombie awaiting
init's reap — the direct sh dies first, the orphaned sleep reparents, and
reaping is asynchronous. Fast machines won that race; loaded release runners
lost it twice (v0.14.1, v0.14.2). Poll until the pid is gone or its ps state
is Z, bounded at 5s — a sleep 30 that genuinely survived the group kill still
fails the assertion.
Findings anchored to a line in the PR diff are posted as inline review
comments on that line (full rationale/sources/suggestion there); the main
comment renders them short — title, links, one-line rationale — with a link
to the thread. The main comment stays the single durable state store.
- Root-only config: inline { enabled: false, maxComments: 20 }
- Per-finding byte-0 marker + author = thread identity; threads patch in
place, sticky cap never evicts a live thread, retired threads with human
replies are stubbed (neutral text), bare ones deleted
- Teardown only when the target set is provably complete: failure reviews
now set couldNotComplete, --scopes partial runs and /dismiss re-renders
are additive-only (truncated aggregate state cannot prove a finding gone)
- Inline replies feed author feedback via threadFp (matches in every match
mode; clearing still requires the unquoted id: token); review-comment ids
are stored negated to stay collision-free vs issue-comment ids, with
magnitude-within-stream recency and a cross-stream pin-lift refusal
- hasUnquotedInlineMarker backstop keeps our own inline bodies (which carry
an unquoted id: token) from ever matching as replies; quoted copies still
count as genuine quote-replies
- Creates verify the live PR head, mutations run sequentially, 403/429
aborts the sync, every sync writes a step-summary audit line
Design + adversarial review record: PLAN-inline-comments.md
Prompt policy: tell reviewers that every search, fetch, and context
expansion spends one slot of the shared review budget; that a rejected
query means the shape was unsafe, not that the tool is down; and that a
finding standing on an ungroundable external contract must record the
gap in trace.uncertainties and cap its Confidence at Medium.
Visibility: count final findings that assert external platform behavior
(version/API-level claims, documented-lifecycle wording) without a
grounded citation, and show the count in the usefulness line and the
Actions summary. Advisory only — the heuristic never gates a decision.
Cleanup: delete the prefetch-era dead code the prefetch removal left
behind (platformResearchSection, formatResearchEvidence, the always-empty
ResearchRun.promptText) so only the reviewer-driven MCP path remains.
Defaults: raise research.timeoutMs from 15s to 30s. One search can spend
~10s on discovery plus sequential bounded page fetches of ~10s each, so
the old default cut off healthy slow searches on the OpenCode engine
while the Claude engine has no equivalent per-call cap.
* Give the verifier the cited documentation it judges
A finding that materially depends on external platform behavior was
verified by an agent with read/grep only and no view of the citation:
the verifier could drop a documented true positive from model memory,
and an on-topic-URL/off-topic-claim citation was never checked at all.
Escalate every finding with grounded citations to the LLM verify pass
and inline the audited passages behind those citations into the task as
fenced untrusted reference data. The verifier separately returns
citationSupported; an explicit false strips the citation (including its
fingerprint-carried copy, so the post-coordination merge cannot restore
it) while the finding itself still stands or falls on the ordinary
verdict. Ambiguity fails open toward keeping the citation.
Also make citation grounding fragment-insensitive: models frequently
normalize a copied URL by dropping its #fragment, which silently
discarded an otherwise exact citation (Expo search results carry
fragments). The audited URL, never the model's variant, is restored.
* Strip forged DOC_PASSAGE boundary lines from cited verifier passages
The AI review on this PR found that sanitizeUntrusted does not remove a
bare DOC_PASSAGE line, unlike the boundary strips the other fences have.
A cited page an outsider can edit — a YouTrack issue description is the
concrete path — could close the fence early and address the verifier as
engine prose, flipping verified or citationSupported.
Strip forged boundary lines from the sanitized passage before fencing,
mirroring CONTEXT_FILE_BOUNDARY, and cover it with a test that a forged
opener/closer pair inside a passage never survives as a boundary line.
* Keep the decision unchanged when only a citation is stripped
The AI review on this PR caught a decision regression in the widened
verify gate: a citation strip keeps every finding, but re-deriving the
decision ran decisionAfterVerification over the unchanged set, which
downgrades a criticals-free request_changes to approve_with_comments.
A blocking review silently stopped blocking (exit code 1 → 0) because a
link was removed.
Assign the kept findings whenever either list is non-empty, but
re-derive the decision only when findings were actually dropped.
* Run-log stripped citations with their reason
The AI review on this PR noted that citationStripped skipped the run-log
audit trail its siblings use: verifierDropped and requalificationStrips
both persist finding-plus-reason records, so the reason a citation was
removed was lost after the run.
Carry a reason on each citationStripped entry, add citationStrips to
RunLogRecord, and write it in the review run log next to its siblings.
Three fixes to the documentation MCP surfaced by review:
Query recall: the sanitizer required a CamelCase-style API anchor, which
rejected legitimate queries such as `expo-camera barcode scanner`,
`gradle configuration cache`, and `coroutine cancellation cooperative`
— exactly the guide, release-note, and Expo topics that have no symbol.
Accept hyphenated package tokens as anchors and accept short multi-word
lowercase concept phrases. The length, entropy, token-count, and secret
checks are unchanged, and a single generic word still fails closed.
Honest limits: the search tool advertised limit 1-10 (default 5) while
the server silently clipped results to resultsPerQuery (default 2), and
the same bound accidentally capped fetch_platform_doc's focused passage
count. Derive the advertised schema maximum and default from the
enforced cap, and give the focused passage count its own independent
bound.
Rejection visibility: sanitizer rejections, refused direct-fetch URLs,
and budget-exhausted attempts left no audit trace, so operators could
not see unmet research demand. Audit each refusal by reason class only
(query-rejected, url-rejected, budget-exhausted) — never the refused
input, which may be exactly the material the sanitizer refused to send —
and surface the counts in progress output and the step summary. Also
warn instead of returning silently empty when requested source kinds
exclude the Expo search, and record the platform on fetch audit inputs
so failed fetches stop mislabeling as react-native.
The evaluation scripts spawned the MCP as a subprocess and made live
Brave, Algolia, and page requests, so they could not run in CI and had
drifted: one fixture query no longer passes the current sanitizer, and
both scripts exceed the per-server call budget added in v0.12.2.
Run the documentation server in-process over an InMemoryTransport, one
server per case, and replay recorded fixtures through the existing
fetchImplementation seam. Each case carries its own fixture; the replay
synthesizes the exact wire formats (Brave hits, Algolia hits, the OkHttp
static index, DocC JSON, HTML, Markdown, YouTrack JSON) keyed by the
request URLs the production provider logic computes, so the sanitizer,
routing, allowlists, redirect and content-type checks, extraction, and
ranking all run unmodified. RESEARCH_EVAL_LIVE=1 keeps a live mode for
true retrieval-quality runs.
Thread fetchImplementation through the Expo Algolia search so the
recorded mode covers the expo provider, replace the sanitizer-broken
kotlin-coroutines query with a symbol-anchored one, and run both
evaluations in CI.
The test spawned a node child that installed a SIGTERM handler and relied on
a 250ms deadline to fire after it. The deadline timer starts at spawn, so on a
loaded runner node had not yet reached `process.on('SIGTERM')` when the signal
landed: the default disposition killed it outright, the 5s grace-timer
escalation never ran, and the elapsed-time assertion saw 462ms. That failed
the v0.10.0 release job before the publish step.
Use `sh -c 'trap "" TERM; exec sleep 30'` instead. The shell boots in ~1ms
rather than a node runtime's tens of ms, and an ignored signal survives exec
where an installed handler would not, so the sleep holding the group ignores
SIGTERM too. Widen the deadline to 1s as well, so the window the child has to
get there is 4x larger.
* ref-check: cover scope-relative citations that carry an extension
A scoped prompt writes `pgbouncer/base/pdb.yaml` for what is really
`infrastructure/pgbouncer/base/pdb.yaml`. Only extensionless tokens were resolved
before, so the precise ref did not silence the citation and the only thing that
did was `glob:**/pdb.yaml` — which resolves while pointing at every same-named
file in the repo. Six agents pinning a real setup all reached for that glob.
Suggestions now name the resolved path; a token with its own wildcard keeps the
glob form.
* ref-check: don't report scrubbed ambient config as a stale ref
The read root is scrubbed of AGENTS.md, CLAUDE.md, .env and friends, so a ref to
one is missing there by design. The template's own shared.md cites AGENTS.md, so
the review note cried wolf on every PR — including this repo's, which is how it
was found. The standalone gate is unchanged: there the tree is a real checkout.
* ref-check: strip anchor, line and glob prefix before the ambient-config test
`AGENTS.md#conventions` and `glob:**/.env.local` name scrubbed files too. Found by
the review on this PR.
* ref-check: see Terraform citations, resolve wildcards as globs, probe scope first
Re-review findings, each reproduced before fixing:
- .tf/.tfvars/.hcl were missing from the citation allowlist, so `backend.tf` or
`*/.terraform.lock.hcl` in a prompt was never checked — 18 unchecked citations
in the infra setup that motivated this feature.
- A wildcard token now resolves as a glob, scope-rebased when that is what
matches; the suggested `glob:` ref for `pgbouncer/*/patch.yaml` used to be a
ref that itself failed the check.
- namedPath probes the scope before the root: a scoped prompt citing `alerts/`
means its own tree even when the root has one too.
- USAGE documents the `target` field --json now emits.
A scoped prompt names a directory by basename (`cert-manager`); the ref that pins
it is root-relative (`infrastructure/cert-manager/`). Coverage only compared the
token as written, so adding the exact ref the message suggested left the same
problem reported. Coverage now also accepts the path the token resolved to, in
plain and glob form.
* refs: check the review setup's own code citations on every run
Reviewer prompts cite real paths and symbols, and nothing kept those true. When
the code moves the prompt keeps citing it, so the reviewer reasons from a fiction
on every PR.
`ecr ref-check` makes each citation checkable: pin it with `@ref <target>`, where
a target is a file, a dir, a glob, a `file#symbol` or a `doc.md#heading`, never a
line number. A backticked token that looks like a repo path must be a ref, so the
citations nobody thought about are covered too; extensionless tokens count when
they name something real, which keeps model ids out. `ecr review`/`ecr ci` run the
same check and only advise, since a stale prompt is not a defect in the PR.
The engine's own `./ref-check` now shares the grammar (glob targets, `<placeholder>`
targets, fenced examples), and this repo's setup is annotated to pass both.
* refs: format config-refs with oxfmt
* refs: read the enforceAgents roster from the manifest's own setup dir
Under `ecr ci` the setup comes from a trusted base-ref checkout while the code
tree is the PR head, so building the roster from the code tree judged
enforceAgents against a different (or missing) agents/ dir. Found by ecr
reviewing this PR.
* refs: leave LLP targets to their own checker, and see uncommitted files
`LLP NNNN#anchor` is a design-corpus target owned by a different mechanism. ecr
now parses it correctly (it is one target containing a space, so the old parse
left a bare `LLP` that read as a broken path) and then skips it: not resolved,
not counted. So a scaffolded repo passes `ecr ref-check` on day one while the
templates keep their engine annotations.
Glob targets now list tracked AND untracked-not-ignored files, matching what a
plain path ref already saw via stat — a freshly scaffolded tree reported its own
workflows as matching nothing.
Also fixes the three template spots the checker caught on a fresh scaffold.
* ref-check: refuse a flag-shaped --root value
`--root --json` silently checked a nonexistent directory and reported "all
resolve" while swallowing --json. Matches the verify-config convention.
* author feedback: match PR replies to findings, mine them with ecr feedback, adjudicate dark
* review fixes: author-only adjudicated clears, preserve feedback records on mode off and seam failure, restorable /undismiss, resilient crawl, shared single-mode reporter
* review fixes round 2: never re-inject stale per-scope feedback copies, open reply-suppressed aggregate folds, warn on ecr feedback repo/config mismatch
* review fixes round 3: bind adjudication verdicts to the reviewed head, make the noHandle test guard actually fail
* review fixes round 4: pin /undismiss to the finding, share the PR-author lookup across reporters, apply the verdict staleness rule on the aggregate path
* review fixes round 5: store the /undismiss pin in comment state, cache only successful author lookups, recompute applied on the dismiss path
* review fixes round 6: require a cited finding id to clear, strip model-supplied agent at the parse boundary, pin the hard-floor set
Opus 5 is out at the same price as Opus 4.8 ($5/$25 per MTok); update the
config example comments and the test fixtures that model the current
Anthropic catalog. Historical incident comments and the ROADMAP changelog
keep their claude-opus-4-8 mentions — they describe what happened at the
time.
* stack-aware review: requalify findings addressed in stacked PRs
Add root-only, off-by-default stack-aware requalification: walk the
open PRs stacked on top of the current one, inject a paths-only
manifest into the coordinator, and let it mark absence-style findings
as addressed by a later PR. Requalified findings stay visible and
counted but stop blocking. Grounding strips forged, hallucinated, or
protected (critical/secrets/security) requalifications deterministically,
after verify and before suppress.
* stack-aware review: confirm upstack requalifications against the real patch
Brent Vatne