* feat: strip hidden text from PR titles and bodies
Remove HTML comments, markdown comments, and zero-width or bidi characters before the model reads PR text, because GitHub does not show them to people. Tell the coordinator when the text hid something, but not for comments that the repository's PR template also has.
* fix: strip Unicode tag characters and other invisible format characters
Tag characters spell invisible ASCII, a known carrier for hidden prompts. The old list covered only zero-width and bidi characters.
* style: format the tag character test
* fix: keep the joiners, soft hyphens and flag tags that normal text needs
Stripping every Cf character counted emoji, Persian words and the England flag
as hidden text, so the coordinator would say a normal PR body hid content.
* feat: let reviewers read docs for SQLite, Git, Node.js and other tools
Add ten documentation providers, and search a provider the caller names on any platform. Before, an iOS reviewer could not ask about SQLite.
* fix: keep forum, source and blog pages out of the SQLite and pnpm docs
The verifier treats a cited documentation page as evidence. A forum post or a blog post is not the tool's contract.
* fix: keep the issue-tracker provider on its own platform
A named documentation provider works from any platform, but YouTrack pages are
user-written text. Searching them should stay a choice for Android reviews.
* feat: pin a CI review to one head commit, and add --expect-head
* fix: keep doc comments next to the declarations they describe
* fix: check the head after break-glass, keep the routed aggregate, and reject malformed --expect-head
* fix: diff the PR's fixed base and head commits, so a push during the run cannot swap the diff
gh pr diff takes no commit, and reading the head again after it missed a push
followed by a force-push back to the same commit. getChangedFiles now fetches
the compare of the base and head OIDs the metadata read, the same commits the
trees are checked out at, and falls back to gh pr diff only when an OID is
unknown. That removes the second head read and HeadMovedError.
--expect-head's note now states what was seen: the PR's head and the commit the
review was requested for, or that the head is unknown.
* fix: reject a repeated --expect-head, and say that older releases ignore it
parseValueFlag keeps the first occurrence, so a second --expect-head was
ignored without a word. Releases from before the flag ignore it like any
unknown argument, so the README and LLP 0007 say to pin a release that has it.
---------
Co-authored-by: Brent Vatne <[email protected]>
The label said "No review — every pass failed", but the same flag also marks
CI runs that failed or stopped before any pass ran, so the reason was often
wrong. The summary under it already says what happened.
- resolveOwnLogin asked `gh api user`, which an installation token can't read,
then assumed github-actions[bot] under Actions. A GitHub App token was taken
for github-actions[bot], so it never recognised its own review comment or
removed its own ready label. It now asks GraphQL `viewer` next, accepts only a
Bot, and adds the `[bot]` suffix REST uses; the github-actions[bot] fallback
stays for when both fail.
- The ready label kept saying "ready" after a push until the next review
posted, or for good if that run was cancelled or killed. `ecr ci` now removes
the bot's label before reviewing a `synchronize` event; the review adds it
back if the new head is ready.
* feat: stop repeating inline findings in the main comment
* fix: keep inlined finding ids in the main comment, and cover dismissals and truncation
* fix: trim inlined findings first when the aggregate comment is too large
* fix: keep 3 inlined findings per scope, and restore threads before the /undismiss render
* fix: keep the size floor at 3 per scope, and link a thread only once it shows the finding
- The aggregate size cap kept 3 inlined plus 3 listed findings per scope, twice
the old floor, so a comment that fit before could exceed GitHub's limit. Now
every inlined finding is trimmed before a listed one, and a scope keeps at
least 3 in total. The trim note says the trimmed ids can't be dismissed.
- syncInline linked an existing thread before patching it. A stub whose restore
failed or was skipped after a rate limit was linked, so the finding was shown
nowhere. Now a thread is linked only when it already shows the finding or its
update succeeded.
- The inline row shows each finding's location: the fingerprint leaves out the
line, so a moved finding keeps its thread on the old line.
- The thread footer no longer says the main comment has the full review.
* fix: give back room the size cap over-trimmed, and link only threads that show the finding
- The inlined limit is shared by every scope, so one scope's overflow trimmed
scopes that fit and dropped their input hash. Once the body fits, both limits
are raised again by binary search, inlined first.
- When 3 large findings still don't fit, the cap goes below the floor instead of
returning a body GitHub rejects.
- An outdated thread (GitHub returns line null once its line left the diff) is
not linked, so its finding is listed in full.
- /dismiss and /undismiss link a thread only when it shows the stored finding as
it renders now, the rule syncInline uses, and pick the newest of duplicates.
- LLP 0008 is split into shorter sections without the history, and LLP 0005,
the plan, README and template describe the cap and the live-thread rule.
---------
Co-authored-by: Brent Vatne <[email protected]>
* feat: add a label when the review is ready for a human
* fix: respect a person's label changes, and remove the label when the review goes away
* fix: match label names case-insensitively, and give no label on partial coverage
- GitHub label names are unique regardless of case. A config spelled differently
from the repo's label never saw it as present, so it was never removed, and a
person's removal was not matched. The presence check, the event match and the
config check now ignore case, and DELETE uses the PR's spelling.
- The aggregate was labelled ready with changed files no scope matched, or on a
--scopes run with no result for the other scopes. reportAggregate now takes
coverageComplete from ci.ts and checks unmatched files.
- README, LLP 0008 and the template describe what a GitHub App token actually
does, and that /skip-review removes the label only under ecr ci.
* fix: give partial runs and runs no reviewer covered no label
- An explicit agent subset (`--agents`, `/review <agents>`), a `--scopes` run,
and a review saved with `--agents` and posted by `ecr post-review` never add
the label; they can still remove the bot's own. This replaces the
coverageComplete check, which only a --scopes run could fail.
- A review where no agent's paths matched approves without a coverage note; it
is now not ready. A noise-only review stays ready.
- When the event history reaches the page cap, the label is left alone, since
the newest event may not have been read.
- The README holds the contract, including the login caveats, instead of "a
person's add or removal always wins"; the schema and template point to it.
---------
Co-authored-by: Brent Vatne <[email protected]>
* feat: show each reviewer's own summary in the review comment
Each reviewer pass may return a short `summary` of what it reviewed and concluded. The
engine keeps one row per roster agent (its summary, or why it did not run: `paths` or
the router) in an engine-owned `agentSummaries` field, and the comment lists them in a
collapsed section under the overall summary. Summaries are model text, so the reporter
flattens them to one line, escapes HTML, and breaks `@` mentions.
* feat: let an agent mention configured people when a change needs a human review
An agent's front matter can list GitHub logins or teams under `mention`. When the agent
sets `humanReview` with a reason, the review comment shows a visible line that mentions
those people. Only the trusted config names people; the model writes only the reason,
and the reporter breaks every `@` in it. An invalid `mention` is a setup warning.
* fix: close mention bypasses in agent summaries, and keep the review when the comment is too big
- plainModelLine let `_@login_` (rendered as `<em>@login</em>`) and
character references (`@login`, `@login`, `@octocat`)
through, and GitHub decodes references before it looks for mentions.
Escape every reference's `&`, and break each `@` that doesn't follow an
ASCII letter or digit, which keeps email addresses whole.
- Only the aggregate comment checked MAX_COMMENT_CHARS. Escaping and the
base64 state copy make summaries several times their byte budget, and a
single comment over GitHub's limit failed to post, so the finished review
was replaced by the failure notice. Over the limit, leave the summaries out
with the same note the aggregate uses.
- Summary parts lived in a plain object, so an agent named `constructor` or
`toString` crashed the whole run; they are now a Map. Parts are kept in
diff order rather than pass-completion order, and a `cross-cutting` agent
is listed once.
- Clipping no longer leaves half an emoji, and an unknown "not run" reason
from an old or newer engine's state reads "not run".
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix: escape only the character references that spell `@`
Escaping every reference made model text like `<T>` or `&` show
up literally. Only a reference that decodes to `@` itself can start a
mention: one after the `@` spells a login character, but that `@` is broken
like any other.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix: only configured mentions notify, once, and a request is never folded away
- Titles, rationales, suggestions, source titles, the summary and the
requalification reason went through stripStateMarkers only, so model text
could still notify anyone (a title citing `@param` or `@types/node` pings a
user or team). They now go through the same mention breaking as the
summaries, in the single, aggregate and inline bodies.
- In the aggregate comment, a scope with no findings kept its fold closed, so
the person a request notified landed on a collapsed section. A mention line
now opens it.
- humanReview is recomputed every run, and GitHub notifies when an edit adds
a mention, so a request that dropped out and came back notified again. The
comment state now remembers who was notified; a returning handle renders
without notifying.
- MENTION_TARGET_RE now follows GitHub's rules: single inner hyphens, at most
39 characters, an optional EMU `_shortcode` suffix, and team slugs. One
invalid entry no longer drops the agent's whole `mention` list: it is left
out with a warning naming it.
- Document that team mentions posted with GITHUB_TOKEN are untested.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix: break every `@` of a run like `@@login`
The capture in `(^|[^A-Za-z0-9])@` consumed the first `@` of `@@login`, so
the second never matched and `@@login` still notified. A lookbehind
breaks each one. Found by the repo's own AI review.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---------
Co-authored-by: Brent Vatne <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
* feat: show each reviewer's own summary in the review comment
Each reviewer pass may return a short `summary` of what it reviewed and concluded. The
engine keeps one row per roster agent (its summary, or why it did not run: `paths` or
the router) in an engine-owned `agentSummaries` field, and the comment lists them in a
collapsed section under the overall summary. Summaries are model text, so the reporter
flattens them to one line, escapes HTML, and breaks `@` mentions.
* fix: close mention bypasses in agent summaries, and keep the review when the comment is too big
- plainModelLine let `_@login_` (rendered as `<em>@login</em>`) and
character references (`@login`, `@login`, `@octocat`)
through, and GitHub decodes references before it looks for mentions.
Escape every reference's `&`, and break each `@` that doesn't follow an
ASCII letter or digit, which keeps email addresses whole.
- Only the aggregate comment checked MAX_COMMENT_CHARS. Escaping and the
base64 state copy make summaries several times their byte budget, and a
single comment over GitHub's limit failed to post, so the finished review
was replaced by the failure notice. Over the limit, leave the summaries out
with the same note the aggregate uses.
- Summary parts lived in a plain object, so an agent named `constructor` or
`toString` crashed the whole run; they are now a Map. Parts are kept in
diff order rather than pass-completion order, and a `cross-cutting` agent
is listed once.
- Clipping no longer leaves half an emoji, and an unknown "not run" reason
from an old or newer engine's state reads "not run".
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix: escape only the character references that spell `@`
Escaping every reference made model text like `<T>` or `&` show
up literally. Only a reference that decodes to `@` itself can start a
mention: one after the `@` spells a login character, but that `@` is broken
like any other.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix: break every `@` of a run like `@@login`
The capture in `(^|[^A-Za-z0-9])@` consumed the first `@` of `@@login`, so
the second never matched and `@@login` still notified. A lookbehind
breaks each one. Found by the repo's own AI review.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---------
Co-authored-by: Brent Vatne <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
* feat: replace `includeSuggestions` with a `minSeverity` floor
`policy.minSeverity` names the least severe level a review keeps, and defaults to
`design`. A config that still sets `includeSuggestions` loads as before (`true` keeps
everything, `false` keeps `critical`, `warning` and `design`) with a setup warning that
names the replacement, because zod would otherwise strip the key and silently change what
a repo's reviews show.
This repo's own reviewer setup moves to `minSeverity: "design"`, and its prompts ask for
`design` findings the same way the templates do.
* fix: approve when the severity floor drops every finding, and only warn on includeSuggestions in doctor
- applyReviewPolicy approved only an approve_with_comments left with no
findings. A raised floor (critical, say) can drop every finding a
request_changes rested on, which read "Request changes" over "No findings."
and exited 1 locally. Approve whenever nothing remains, the same rule as
decisionAfterVerification.
- The includeSuggestions deprecation rode in setupWarnings, which ecr doctor
fails on, so every repo still setting the key failed doctor although its
reviews work. It is now its own `deprecations` field: still a setup note on
every review, but a ⚠ in doctor.
- A test that a scope config's includeSuggestions sets its own floor.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---------
Co-authored-by: Brent Vatne <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
* feat: add `design` and `nit` severities
The scale is now `critical`, `warning`, `design`, `suggestion` and `nit`. `design` is a
should-fix level for design problems that does not block the PR, and `includeSuggestions`
gates `nit` together with `suggestion`. The comment headings use the deep-code-review
skill's labels. The templates define the new levels and ask for `critical`, `warning` and
`design`, where a `design` finding names a concrete cost in place of a failure path.
* fix: render a stored severity this engine doesn't know, and let design findings rate by cost
- Findings read back from a comment's embedded state aren't schema-validated,
so an engine that doesn't know a stored severity crashed in groupBySeverity
on /dismiss and on a partial --scopes re-render. That is what an engine
older than this PR does with `design` and `nit`. From this engine on,
knownSeverity renders an unknown severity as a warning (shown, not
blocking) and keeps the stored value, so the newer engine reads it back.
- The rubric defined confidence and impact by a failure or exploit path, so a
`design` finding rated low confidence and the coordinator dropped it. Rate
it by its cost instead, in shared.md, correctness.md and the verifier.
- Tests: an unknown stored severity re-renders, the terminal reporter's
DESIGN and NIT labels, and the verifier's cost wording.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
---------
Co-authored-by: Brent Vatne <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
gh colors `gh pr diff` even into a pipe when CLICOLOR_FORCE or GH_FORCE_TTY
is set. The escape codes before each `diff --git` made the whole PR parse as
no files, so a CI review reported "No reviewable changes". Pass
`--color never` in both places that read the PR diff.
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
templates/dismiss.yml defaulted ECR_VERSION to '^0.2.3'. On 0.x a caret
range only matches its own minor, so every scaffolded /dismiss ran 0.2.3.
That engine writes back only { review, dismissed }, silently dropping
feedback, pins, scopes and inline links from the comment state, and it
would throw on any severity it doesn't know. Default to 'latest' like the
other templates, and test that every template does.
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
* feat: post a run's new inline comments in one review
Each new inline comment went out as its own `POST /pulls/{n}/comments`, and
GitHub makes every such comment its own review. A run with three findings
put three "reviewed" entries and three notifications on the PR.
The inline sync now posts all of a run's new threads in one
`POST /pulls/{n}/reviews` with `event: "COMMENT"`, the reviewed head as
`commit_id`, and a short fixed body, which GitHub requires for a COMMENT
review. The response doesn't list the comments, so the sync reads the
review's comments and maps each one to its finding by the inline marker.
Patches, stubs and deletes stay per comment and create no review.
GitHub rejects the whole review when it refuses one comment, so any failure
other than a rate limit falls back to one comment per thread. A review that
was created but returned no usable id is not posted again; its findings
render in full in the main comment until the next run links them.
* docs: describe the single review for new inline comments in the README
* docs: describe the single review for new inline comments in the config template
* fix: retry inline comments one by one only after a 422, and read every page of the review
A review POST that failed with a 5xx or a dropped connection fell back to
posting each comment, but GitHub can have created the review anyway, so the
fallback could post every thread twice. Only a 422, which means GitHub
created nothing, now falls back. Any other failure counts the threads as
failed and does not retry; the main comment renders those findings in full,
and the next run creates the threads that are still missing.
`inline.maxComments` has no upper bound, so the read of the new review's
comments now goes through every page, like the other review-comment reads.
* fix: classify review and comment POST failures by GitHub's response body
- Both inline POSTs run with check: false and throw ghApiFailure, which keeps
the response body's distinct messages (each capped) next to gh's one-line
stderr, since gh prints errors[] only to stdout.
- The spam throttle ("submitted too quickly") counts as a rate limit, so it
aborts the sync for the review and inside the one-by-one fallback. A
pending review on the posting account fails the new threads without a
fallback; stubs and deletes still run.
- Reading back the review's comments goes page by page: a failed page keeps
the links before it, and a rate limit there stops the remaining mutations.
- Tests: payload bodies, a mixed create/delete run, read-back rate limit and
partial pages, pending-review and spam 422s, a rate limit while patching,
spam during the fallback, and ghApiFailure.
- README, template and LLP 0008: one review per scope in per-scope mode, the
review body stays after its threads are deleted, and GitHub and gh
behavior that hasn't been checked live is tagged [inferred].
* fix: post the inline review with an empty body, and record GitHub's observed behavior
A probe against a private scratch PR showed that GitHub accepts a COMMENT
review with comments and an empty or missing body, despite its docs. A
submitted review can't be deleted, so the fixed body stayed on the PR after
its threads were torn down; an empty one leaves nothing.
The same probe confirmed that one unanchorable line makes GitHub refuse the
whole review and create nothing, that gh prints errors[] only to stdout, and
what both POSTs return while the account has a pending review. LLP 0008 now
tags those [observed].
---------
Co-authored-by: Brent Vatne <[email protected]>
* fix: parse local diffs whatever the user's git diff config says
`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.
* fix: also disable textconv and copy detection in local diffs, and harden the tests
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.
* fix: show submodule pointer changes whatever the diff config, and isolate the local-git tests
- 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.
---------
Co-authored-by: Brent Vatne <[email protected]>
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.
Kudo Chien