Skip to content

Commit 994e413

Browse files
CodeCasterXclaudecodex
committed
chore(meta): make post-review gate coverage fail-closed
- Replace the post-review allowlist with a fail-closed denylist: default pathspec ":/" covers all tracked changes, default excludes are empty, and projects opt in via review.post_review_exclude_globs - Drop the deprecated review.post_review_globs block from .agents/.airc.json and mirror the shared lib to templates/ - Add e2e tests for coverage expansion, package-lock.json default coverage, and project opt-in exclusion, plus two template parity guards Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
1 parent 4360354 commit 994e413

6 files changed

Lines changed: 102 additions & 51 deletions

File tree

.agents/.airc.json

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -103,20 +103,5 @@
103103
},
104104
"task": {
105105
"shortIdLength": 2
106-
},
107-
"review": {
108-
"post_review_globs": [
109-
".agents/.airc.json",
110-
".agents/rules",
111-
".agents/scripts",
112-
".agents/skills",
113-
".agents/workflows",
114-
".git-hooks/pre-commit",
115-
".github/workflows",
116-
"bin",
117-
"lib",
118-
"src",
119-
"templates"
120-
]
121106
}
122107
}

.agents/scripts/lib/post-review-commit.js

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,22 @@ import path from "node:path";
33

44
import { artifactName, maxRound } from "./review-artifacts.js";
55

6-
export const DEFAULT_POST_REVIEW_GLOBS = [
7-
".agents/skills",
8-
".agents/scripts",
9-
".agents/rules",
10-
".agents/workflows",
11-
"bin",
12-
"lib",
13-
"src",
14-
"templates"
15-
];
16-
6+
// Paths excluded from the post-review coverage gate. Fail-closed: the gate
7+
// covers ALL tracked changes by default. This default denylist is EMPTY —
8+
// projects opt into exclusions explicitly via review.post_review_exclude_globs.
9+
export const DEFAULT_POST_REVIEW_EXCLUDES = [];
10+
11+
// Returns git pathspecs selecting "all tracked changes except project excludes".
12+
// The leading ":/" makes coverage fail-closed (everything from the repo root);
13+
// each exclude becomes a top-level negative pathspec. Projects extend the
14+
// denylist via review.post_review_exclude_globs (union, deduped).
1715
export function resolvePostReviewGlobs(config = {}, reviewConfig = {}) {
18-
if (Array.isArray(config.post_review_globs)) {
19-
return config.post_review_globs;
20-
}
21-
if (Array.isArray(reviewConfig.post_review_globs)) {
22-
return reviewConfig.post_review_globs;
23-
}
24-
return DEFAULT_POST_REVIEW_GLOBS;
16+
const projectExcludes = [
17+
...(Array.isArray(reviewConfig.post_review_exclude_globs) ? reviewConfig.post_review_exclude_globs : []),
18+
...(Array.isArray(config.post_review_exclude_globs) ? config.post_review_exclude_globs : [])
19+
];
20+
const excludes = [...new Set([...DEFAULT_POST_REVIEW_EXCLUDES, ...projectExcludes])];
21+
return [":/", ...excludes.map((p) => `:(top,exclude)${p}`)];
2522
}
2623

2724
export function findAuthoritativeReviewCodeArtifact(taskDir) {

templates/.agents/scripts/lib/post-review-commit.js

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,22 @@ import path from "node:path";
33

44
import { artifactName, maxRound } from "./review-artifacts.js";
55

6-
export const DEFAULT_POST_REVIEW_GLOBS = [
7-
".agents/skills",
8-
".agents/scripts",
9-
".agents/rules",
10-
".agents/workflows",
11-
"bin",
12-
"lib",
13-
"src",
14-
"templates"
15-
];
16-
6+
// Paths excluded from the post-review coverage gate. Fail-closed: the gate
7+
// covers ALL tracked changes by default. This default denylist is EMPTY —
8+
// projects opt into exclusions explicitly via review.post_review_exclude_globs.
9+
export const DEFAULT_POST_REVIEW_EXCLUDES = [];
10+
11+
// Returns git pathspecs selecting "all tracked changes except project excludes".
12+
// The leading ":/" makes coverage fail-closed (everything from the repo root);
13+
// each exclude becomes a top-level negative pathspec. Projects extend the
14+
// denylist via review.post_review_exclude_globs (union, deduped).
1715
export function resolvePostReviewGlobs(config = {}, reviewConfig = {}) {
18-
if (Array.isArray(config.post_review_globs)) {
19-
return config.post_review_globs;
20-
}
21-
if (Array.isArray(reviewConfig.post_review_globs)) {
22-
return reviewConfig.post_review_globs;
23-
}
24-
return DEFAULT_POST_REVIEW_GLOBS;
16+
const projectExcludes = [
17+
...(Array.isArray(reviewConfig.post_review_exclude_globs) ? reviewConfig.post_review_exclude_globs : []),
18+
...(Array.isArray(config.post_review_exclude_globs) ? config.post_review_exclude_globs : [])
19+
];
20+
const excludes = [...new Set([...DEFAULT_POST_REVIEW_EXCLUDES, ...projectExcludes])];
21+
return [":/", ...excludes.map((p) => `:(top,exclude)${p}`)];
2522
}
2623

2724
export function findAuthoritativeReviewCodeArtifact(taskDir) {

tests/e2e/core/review-diff-fingerprint.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,47 @@ test("post-review globs are shared by the validator and fingerprint helper", ()
8686
assert.match(validator, /from "\.\/lib\/post-review-commit\.js"/);
8787
assert.match(validator, /resolvePostReviewGlobs\(config, loadReviewConfig\(\)\)/);
8888
});
89+
90+
test("review diff fingerprint covers paths outside the legacy allowlist (fail-closed)", onPlatforms("linux", "darwin", "win32"), async () => {
91+
await withTempRoot("agent-infra-fingerprint-failclosed-", (tempRoot) => {
92+
const baseline = setupRepo(tempRoot);
93+
const clean = fingerprint(tempRoot, "worktree", baseline);
94+
95+
write(path.join(tempRoot, "scripts/x.js"), "// new generator change\n");
96+
const changed = fingerprint(tempRoot, "worktree", baseline);
97+
assert.notEqual(changed, clean);
98+
});
99+
});
100+
101+
test("review diff fingerprint covers package-lock.json by default", onPlatforms("linux", "darwin", "win32"), async () => {
102+
await withTempRoot("agent-infra-fingerprint-lockfile-", (tempRoot) => {
103+
const baseline = setupRepo(tempRoot);
104+
const clean = fingerprint(tempRoot, "worktree", baseline);
105+
106+
write(path.join(tempRoot, "package-lock.json"), "{}\n");
107+
const changed = fingerprint(tempRoot, "worktree", baseline);
108+
assert.notEqual(changed, clean);
109+
});
110+
});
111+
112+
test("review diff fingerprint honors project post_review_exclude_globs", onPlatforms("linux", "darwin", "win32"), async () => {
113+
await withTempRoot("agent-infra-fingerprint-exclude-", (tempRoot) => {
114+
setupRepo(tempRoot);
115+
write(
116+
path.join(tempRoot, ".agents/.airc.json"),
117+
JSON.stringify({ review: { post_review_exclude_globs: ["package-lock.json"] } }, null, 2) + "\n"
118+
);
119+
git(tempRoot, ["add", ".agents/.airc.json"]);
120+
git(tempRoot, ["commit", "-qm", "add exclude config"]);
121+
const baseline = git(tempRoot, ["rev-parse", "HEAD"]);
122+
const clean = fingerprint(tempRoot, "worktree", baseline);
123+
124+
write(path.join(tempRoot, "package-lock.json"), "{}\n");
125+
const excluded = fingerprint(tempRoot, "worktree", baseline);
126+
assert.equal(excluded, clean);
127+
128+
write(path.join(tempRoot, "scripts/included.js"), "// not excluded\n");
129+
const included = fingerprint(tempRoot, "worktree", baseline);
130+
assert.notEqual(included, clean);
131+
});
132+
});

tests/e2e/core/validate-artifact-post-review-commit.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,29 @@ test("post-review-commit blocks when the task is not inside a git repository", o
217217
assert.equal(payload.status, "blocked");
218218
});
219219
});
220+
221+
test("post-review-commit fails for a commit outside the legacy allowlist (fail-closed coverage)", onPlatforms("linux", "darwin", "win32"), async () => {
222+
await withTempRoot("agent-infra-prc-failclosed-", (tempRoot) => {
223+
const { taskDir } = setupRepo(tempRoot);
224+
const baseline = commitCodePath(tempRoot, ".agents/skills/x.md", "base\n", "base");
225+
write(path.join(taskDir, "task.md"), buildTask());
226+
write(path.join(taskDir, "review-code.md"), buildReviewCode(baseline));
227+
commitCodePath(tempRoot, "scripts/build-inline.js", "// generated\n", "post-review change to a previously-uncovered path");
228+
229+
const { payload } = runCheck(taskDir);
230+
assert.equal(payload.status, "fail");
231+
});
232+
});
233+
234+
test("post-review-commit covers package-lock.json by default (no hardcoded exclusion)", onPlatforms("linux", "darwin", "win32"), async () => {
235+
await withTempRoot("agent-infra-prc-lockfile-", (tempRoot) => {
236+
const { taskDir } = setupRepo(tempRoot);
237+
const baseline = commitCodePath(tempRoot, ".agents/skills/x.md", "base\n", "base");
238+
write(path.join(taskDir, "task.md"), buildTask());
239+
write(path.join(taskDir, "review-code.md"), buildReviewCode(baseline));
240+
commitCodePath(tempRoot, "package-lock.json", "{}\n", "post-review lockfile bump");
241+
242+
const { payload } = runCheck(taskDir);
243+
assert.equal(payload.status, "fail");
244+
});
245+
});

tests/unit/templates/templates.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ test("update-agent-infra template copies stay in sync with working files", () =>
209209
[".agents/skills/update-agent-infra/scripts/sync-templates.js", "templates/.agents/skills/update-agent-infra/scripts/sync-templates.js"],
210210
[".agents/scripts/validate-artifact.js", "templates/.agents/scripts/validate-artifact.js"],
211211
[".agents/scripts/lib/review-artifacts.js", "templates/.agents/scripts/lib/review-artifacts.js"],
212+
[".agents/scripts/lib/post-review-commit.js", "templates/.agents/scripts/lib/post-review-commit.js"],
213+
[".agents/scripts/review-diff-fingerprint.js", "templates/.agents/scripts/review-diff-fingerprint.js"],
212214
[".agents/workflows/feature-development.yaml", "templates/.agents/workflows/feature-development.en.yaml"],
213215
[".agents/workflows/bug-fix.yaml", "templates/.agents/workflows/bug-fix.en.yaml"],
214216
[".agents/workflows/refactoring.yaml", "templates/.agents/workflows/refactoring.en.yaml"],

0 commit comments

Comments
 (0)