Repository navigation
feat: add a per-agent paths filter to agent frontmatter - #81
Merged
Merged
Conversation
An agent can now declare `paths`, a list of repo-relative globs in the routing dialect, as a flow list (`paths: ["packages/ios/**"]`) or a block list of `- "glob"` lines. The agent then reviews only changes that touch a matching file. runReview applies the filter to the reviewable files (after scope isolation and noise filtering) before any engine starts, so a skipped agent costs no model call. When no agent is left, the run returns like a noise-only diff. On a chunked diff the agent also skips each chunk with no matching file; a chunk it reviews keeps its full diff. The router and `alwaysRun` choose only among the agents the filter kept. An explicit `--agents` list bypasses the filter, because the caller asked for those agents. Skips go to the job log, the Actions step summary, and the run log (`pathsSkipped`). They are not coverage notes: a non-empty `incomplete` makes a result uncacheable, so a skip there would force a cache miss on every unchanged re-run. For the same reason the cross-file pass's configured model is now the first roster agent's, which is the model it runs on, so dropping the first agent no longer reports a false model substitution. An invalid `paths` (not a list, empty, or a glob that can never match a repo-relative path) is a load-time setup error that names the agent file and the fix. Agents without `paths` load with no `paths` key, so their prompts and review-cache keys are unchanged.
🤖 AI code reviewDecision: Ready for human review Overall PR risk: Medium. The change modifies existing agent selection and frontmatter parsing logic rather than being purely additive. It affects multiple surfaces: agent roster computation,
No findings. This review is advisory — it never blocks a merge and never auto-approves. |
…` entries The prompt templates opened with an `@ref` comment above `---`, and parseFrontmatter reads frontmatter only at the first byte. Scaffolded agents therefore lost their description, `alwaysRun`, and model pins, and an uncommented `paths` example would have done nothing. The templates now open with `---` and carry the `@ref` comment as the first body line. The reader also split lines on LF only, so with CRLF it read just the last key. It now splits on LF or CRLF. A trailing `# comment` stays in a frontmatter value. A `paths` entry with one, or with text after its closing quote, never matched a file and silently skipped the agent. It is now a load-time setup error that names the fix. The README example puts its comments on their own lines.
tsapeta
marked this pull request as ready for review
October 19, 2026 22:06
…tch renames on both paths
An invalid `paths` was a load error. In CI, `ecr ci` reports a load error to
stderr and posts nothing, and nothing checks agent files before they merge (CI
reviews a PR under its base config; `ecr verify-config` doesn't read agents), so
one typo would silently stop every review in the repo. It is now a setup
warning: the agent ignores `paths` and reviews every change, and the warning
names the file and the fix.
Warnings go in `LoadedConfig.setupWarnings`. Every review result, early returns
included, carries them as setup notes, so the PR comment's "Review setup" note
shows them. `ecr doctor` fails on them. The same warnings now also report:
- frontmatter below an HTML comment, a byte-order mark or blank lines, which
the reader ignores (repos scaffolded before the templates moved their `@ref`
comment have this). The file still loads as before.
- a trailing `# comment` that breaks `alwaysRun`, `model` or `temperature`.
The parser no longer turns these typos into a glob that silently matches
nothing:
- a missing comma, or text after a closing quote
- a block item written `-x`
- a `{a,b}` brace set, or an unquoted one split at its comma
A rename now records its old path (`rename from`), and both `paths` filters
match it, so moving a file out of an agent's globs still reaches that agent.
Member
|
This is Claude, commenting on @brentvatne's behalf. I pushed edc4694 on top of this PR:
The README and LLPs 0002, 0004 and 0006 are updated. The full test suite passes, and so do typecheck, lint, fmt and ref-check. A separate PR will fix two older bugs: |
This was referenced Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Every agent runs on every review, even when it can't apply to the change. In expo/expo we're adding a
native-abiagent (expo/expo#51862) that only matters whenexpo-modules-jsiorexpo-modules-corechanges, and on a PR without Swift changes it still costs a model round trip per chunk just to say there's nothing to do (about 4 s and two turns each). The same need comes up for docs-only PRs, where only thedocsagent should run.routing.jsoncdoesn't fit either case, since it splits a PR into separate reviews with separate config folders instead of switching one agent on or off.How
An agent can declare
paths, a list of repo-relative globs in the routing dialect, as a flow list (paths: ["packages/ios/**"]) or a block list.runReviewapplies it to the reviewable files before any engine starts, so a skipped agent costs no model call, and on a chunked diff the agent also skips chunks with no matching file (a chunk it does review keeps its full diff, so files like.changeset/*.mdstill reach it). The router andalwaysRunonly see the agents the filter kept, and an explicit--agentslist bypasses the filter since the caller asked for those agents.Skips are reported in the job log, the Actions step summary and the run log, not as coverage notes, because a non-empty
incompletemakes a result uncacheable. For the same reason the cross-file pass's configured model is now the first roster agent's, which is what both engines actually run it on; otherwise dropping the first agent reported a false model substitution. An invalidpathsis a load-time setup error that names the agent file and the fix. Agents withoutpathsload exactly as before, so prompts and cache keys don't change.I also fixed three frontmatter bugs that
pathswould have inherited. The prompt templates opened with an@refcomment above---, andparseFrontmatteronly reads frontmatter at the first byte, so every scaffolded agent silently lost its description,alwaysRunand model pin. The templates now open with---, but repos scaffolded earlier keep the old layout until they move the comment. The reader also split lines on LF only, so a CRLF file kept just its last key. And since a trailing# commentstays in the value, a commentedpathsentry is now a setup error, and the README example no longer puts comments after values.Test Plan
bun test(775 pass, up from 749, with new tests for parsing, run-level and chunk-level skips, the--agentsbypass,alwaysRun, routing and the cache key),bun run typecheck,bun run lint,bun run fmt:checkandbun run llp:checkall pass. The tests for the scaffolded templates, CRLF and commented entries fail on the code without those fixes.