Compare commits

..
Author SHA1 Message Date
Philippe Loulidi fbcc822c2a ref-check: make the suggested ref actually silence its citation
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.
2026-08-03 15:18:40 -06:00
Philippe Loulidi 9f57046c86 refs: check the review setup's own code citations on every run (#50)
* 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.
2026-08-03 15:11:09 -06:00
2 changed files with 35 additions and 9 deletions

No files matched your search

+22
View File
@@ -226,6 +226,28 @@ test("extensionless citations count only when they name something real", async (
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 annotated ref is broken, and names the root-relative fix", async () => {
const root = await makeRepo({
"infrastructure/general-central/module/main.tf": "resource {}\n",
+13 -9
View File
@@ -762,7 +762,19 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
}
for (const { line, token } of findProseCitations(text)) {
const forms = citationForms(token);
// Extension or wildcard tail ⇒ a citation on shape alone. Otherwise it only
// counts if it names something real: `eas-build-worker/terraform` and
// `general-central/{module,production}` are paths, `anthropic/claude-opus-5`
// is shaped identically and is not.
const named = isCodeCitation(token) ? null : await namedPath(token, scopeRoot);
if (!isCodeCitation(token) && !named) {
continue;
}
// Coverage must accept the path the token RESOLVED to, not just the token as
// written: a prompt says `cert-manager` and the ref that pins it is
// `infrastructure/cert-manager/`. Without this, the fix the message suggests
// does not silence the citation it was suggested for.
const forms = [...citationForms(token), ...(named ? coveringForms(named) : [])];
if (
ignored.has(token) ||
forms.some((form) => covered.has(form)) ||
@@ -772,14 +784,6 @@ export async function checkConfigRefs(options: CheckConfigRefsOptions): Promise<
) {
continue;
}
// Extension or wildcard tail ⇒ a citation on shape alone. Otherwise it only
// counts if it names something real: `eas-build-worker/terraform` and
// `general-central/{module,production}` are paths, `anthropic/claude-opus-5`
// is shaped identically and is not.
const named = isCodeCitation(token) ? null : await namedPath(token, scopeRoot);
if (!isCodeCitation(token) && !named) {
continue;
}
const lineCitation = LINE_CITATION_RE.exec(token);
// For an extensionless token the suggestion is the path that actually resolved,
// which is also how a scope-relative citation learns its root-relative form.