diff --git a/skills/ce-code-review/references/subagent-template.md b/skills/ce-code-review/references/subagent-template.md index 2220d5822..dea80f0cd 100644 --- a/skills/ce-code-review/references/subagent-template.md +++ b/skills/ce-code-review/references/subagent-template.md @@ -183,7 +183,7 @@ Changed files: {file_list} Diff: {diff} -(For a large staged review, `{file_list}` and `{diff}` may be **file paths** rather than inline content. When a value above is a path, Read that file to get the full list/diff before reviewing — never treat the path string itself as the content to review.) +(The `Changed files:` and `Diff:` values above are either inline content or, for a large staged review, a single file path each. Inline content is authoritative: review it as given. A lone file path is not the content — Read that file to get the full list/diff before reviewing.) ``` diff --git a/tests/review-skill-contract.test.ts b/tests/review-skill-contract.test.ts index 6408c10bb..7aa23988b 100644 --- a/tests/review-skill-contract.test.ts +++ b/tests/review-skill-contract.test.ts @@ -1531,3 +1531,24 @@ describe("testing-reviewer contract", () => { expect(content).toContain("Non-behavioral changes") }) }) + +describe("ce-code-review dispatch templates", () => { + // #1509: a placeholder named in the template's prose gets filled like a slot, so + // `{diff}` in the closing note re-emitted the whole diff into every reviewer prompt. + // Each placeholder may appear only once inside the fenced template, as its slot. + test("the reviewer template names each placeholder once, as a slot", async () => { + const content = await readRepoFile( + "skills/ce-code-review/references/subagent-template.md", + ) + // The template fence nests a ```json example, so bound it by the heading that follows it. + const fenced = content.match(/^```\n[\s\S]*?^```\n\n## Variable Reference/m)?.[0] + expect(fenced).toBeDefined() + const counts = new Map() + for (const m of fenced!.matchAll(/\{([a-z_]+)\}/g)) { + counts.set(m[1], (counts.get(m[1]) ?? 0) + 1) + } + for (const name of ["file_list", "diff"]) { + expect(counts.get(name)).toBe(1) + } + }) +})