Skip to content

review: enable scoped re-reviews (follow-up to the v1.7.0 bump) #277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jwbron merged 4 commits into from
Jul 21, 2026
Merged

review: enable scoped re-reviews (follow-up to the v1.7.0 bump) #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d54d921
[jwies/review-bump-1.6.1] review: bump installed reviewer to review-v…
jwbron Jul 20, 2026
b97b384
[jwies/review-bump-1.6.1] review: guard the hand-merged install; spli…
jwbron Jul 21, 2026
e2857d8
[jwies/review-bump-1.6.1] review: bump the installed reviewer to revi…
jwbron Jul 21, 2026
f19c2be
[jwies/review-rereview-scoped] review: enable scoped re-reviews on th…
jwbron Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/aw/review/ROUTING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@

enable holistic,completeness,test-adequacy,first-principles,conventions

# Repeat reviews of the same PR run thread reconciliation plus the full
# enabled roster over the new hunks (scoped staging); the first full review
# of a ready PR runs everything, and the divergence tripwire re-arms full
# review when a push rewrites enough of the PR. Scoped rather than
# flip-gated: composite actions here run in consuming repos' CI with their
# credentials, so a small post-review push should still be seen by the
# whole-change reviewers, not only the correctness pass. Graduate to

@github-actions github-actions Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought (non-blocking): This rationale also holds under flip-gated: both modes stage the same new-hunks diff, and flip-gated's always-on correctness pass still sees every post-review push. The real delta is whether the whole-change reviewers also see the new hunks — and their distinctive value is whole-change context that new-hunks staging strips. So scoped-vs-flip-gated is a risk-posture call, not a "new hunks must be seen" one (the README's measured evidence backs scoped over reconcile-only, not over flip-gated). The planned graduation step reasonably hedges this.

# flip-gated once re-reviews here show scoped is overpaying (see

@github-actions github-actions Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (non-blocking): "Graduate to flip-gated once re-reviews here show scoped is overpaying" names no measurable signal. Consider naming a falsifiable exit condition (e.g. N consecutive scoped re-reviews where no whole-change reviewer produced a validated finding the correctness pass missed) so the dial has a defined trigger rather than an open-ended judgment call.

# workflows/review/README.md for the dial).
re-review scoped

# Shipped composite actions run inside consuming repos' CI with their credentials.
actions/** tier=high
# This repo's own CI, the publish pipeline, and the compiled agentic workflows.
Expand Down
87 changes: 87 additions & 0 deletions .github/workflows/review-pins.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
* utils/sync-workflow-versions.ts and its backstop in
* workflows/review/version-sync.test.ts; neither covers these files.)
*/
import {spawnSync} from "node:child_process";
import * as fs from "fs";
import * as os from "os";
import * as path from "path";
import {describe, expect, it} from "vitest";

const reviewMd = fs.readFileSync(
Expand Down Expand Up @@ -56,3 +59,87 @@ describe("compiled review.lock.yml pins", () => {
expect(new Set(literals)).toEqual(new Set([sourceRef]));
});
});

/**
* Content guard for the hand-merged install. `gh aw update` cannot resolve
* changesets-style tags (review-v*), so bumps of the installed copy are
* manual 3-way merges; the pins above check version consistency but nothing
* verified the merged CONTENT. This diffs the installed copy against the
* shared source at the pinned release (this repo hosts both) and requires
* every hunk to carry a `KHAN/ACTIONS LOCAL OVERRIDE` marker, so a manual
* bump that silently drops an override or an upstream hunk fails CI instead
* of surfacing in a live run. Convention enforced as a side effect: each
* override edit inserts its marker comment adjacent to the edited lines
* (within the diff hunk's context window).
*/
describe("installed review.md content vs the pinned source", () => {
const repoRoot = path.resolve(
new URL(".", import.meta.url).pathname,
"../..",
);
const sourcePath = "workflows/review/review.md";

const gitShow = (ref: string): string | null => {
const show = () =>
spawnSync("git", ["show", `${ref}:${sourcePath}`], {
cwd: repoRoot,
encoding: "utf-8",
maxBuffer: 32 * 1024 * 1024,
});
let result = show();
if (result.status !== 0) {
// A shallow or tag-less clone (CI checks out at depth 1): fetch
// just the pinned tag, then retry.
spawnSync(
"git",
["fetch", "--quiet", "--depth=1", "origin", "tag", ref],
{cwd: repoRoot, encoding: "utf-8"},
);
result = show();
}
return result.status === 0 ? result.stdout : null;
};

it("differs from the pinned release only inside LOCAL OVERRIDE hunks", () => {
expect(sourceRef).toBeDefined();
const source = gitShow(sourceRef as string);
if (source === null) {
throw new Error(
`cannot read ${sourcePath} at tag ${sourceRef}: fetch the ` +
`tag (git fetch origin tag ${sourceRef}) and re-run`,
);
}
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "review-pins-"));
try {
const sourceFile = path.join(dir, "source.md");
fs.writeFileSync(sourceFile, source);
const installedFile = path.join(dir, "installed.md");
fs.writeFileSync(installedFile, reviewMd);
const diff = spawnSync("diff", ["-u", sourceFile, installedFile], {
encoding: "utf-8",
maxBuffer: 32 * 1024 * 1024,
});
// 0: identical, 1: differences found, 2: trouble.
expect([0, 1]).toContain(diff.status);
const hunks: string[][] = [];
for (const line of diff.stdout.split("\n")) {
if (line.startsWith("@@")) {
hunks.push([line]);
} else {
hunks.at(-1)?.push(line);
}
}
const unmarked = hunks.filter(
(hunk) =>
!hunk.some((line) =>
line.includes("KHAN/ACTIONS LOCAL OVERRIDE"),
),
);
expect(
unmarked.map((hunk) => hunk.slice(0, 8).join("\n")),
).toEqual([]);
} finally {
fs.rmSync(dir, {recursive: true, force: true});
}
});
});
Loading
Loading