fix(pregate,gstack-post): PREGATE_NO_LANE names the base policy when the branch adds its own lane; pr-body refuses a body holding a title twice

Fixes #3574
Fixes #3575

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-29 03:11:34 +00:00
1 parent 4729f4752b
commit 184eb1bb1d
6 files changed
+94 -7

No files matched your search

+4 -1
View File
@@ -25,7 +25,10 @@ passes. Standalone, `gstack-regen check` lists the commands and exits 3
(`REPO_COMMANDS_NOT_ALLOWED`); `gstack-pregate` marks `regen` and `lanes`
`incomplete`, never `pass`. `--policy-from head` (a PR that introduces or
changes a pin file) is accepted only with `--allow-repo-commands` and the record
says `policy head (unreviewed)`. An unresolvable base exits 3 with
says `policy head (unreviewed)`. So a lane or allowlist entry a branch adds to
its own `pregate.json` counts only once it is on the base; `PREGATE_NO_LANE`'s
fix line says so when the branch edits the file
([troubleshooting](troubleshooting.md#pregate-no-lane-own-policy)). An unresolvable base exits 3 with
`POLICY_SOURCE_UNAVAILABLE`. Globs are repo-relative (no `..`, no absolute
paths, at most 64 per list); commands are one line under 512 bytes.
+32 -2
View File
@@ -2409,11 +2409,20 @@ holding only its first section therefore no longer wipes the rest of the PR.
The redaction scan runs on the merged bytes, so a MEDIUM or HIGH finding can
come from a kept section; the `RULE:` line's `LINE:` is in the merged text.
When the current body cannot be read (`gh pr view --json body` fails) nothing
is posted, exit 3.
is posted, exit 3. When the current body holds a title the file rewrites more
than once (an earlier description pasted below a `---` under the new one, as
on gstack-evals#19), the file's section matches no single copy, so nothing is
posted, exit 64: `gstack-post: the PR's current body holds "What changed" more
than once, so the file's copy matches no single section; nothing was posted.
Rerun with --replace and the whole body`. Merging would replace one copy and
keep the stale other beside it.
**Fix.** Nothing, when the kept sections are wanted. To overwrite the whole
body with the file, rerun with `--replace`. To drop a section, post the body
with `--replace` or edit it on the host.
with `--replace` or edit it on the host. For the duplicate-title refusal, write
the whole body you want (keep the earlier description under a title of its own,
such as `## Original description`, if it should stay) and post it with
`--replace`.
<a id="redact-prepush-email"></a>
### `MEDIUM pii.email <file>:<line>` from the pre-push hook
@@ -3240,6 +3249,27 @@ specifiers in test files that resolved to no repo file (for example a
a test does cover the file, declare it under `dependencies`; if the resolver
should have found it, report the specifier as a resolver gap.
<a id="pregate-no-lane-own-policy"></a>
**The branch adds the lane itself.** A branch that edits `.gstack/pregate.json`
to give its own new files a lane still gets this error: pregate reads the policy
from `origin/<base>` (or the user-owned file), never from the branch, so a
branch cannot loosen its own gate ([trust boundary](pregate.md#trust-boundary)).
That is intended, and pregate never switches to the branch's copy on its own.
The fix line then opens `this branch edits .gstack/pregate.json, but the policy
comes from origin/<base>@<sha>:.gstack/pregate.json so a branch cannot loosen
its own gate, and a lane it adds counts only once it is on <base>` and names the
two routes:
1. Land the policy change in its own PR first (the reviewed route), then merge
the base into this branch and rerun.
2. Rerun with `--policy-from head --allow-repo-commands`. The run reads the
branch's copy and its record says `policy head (unreviewed)`, so a
reviewer sees that the gate ran under a policy nobody has reviewed yet;
`gstack-ship-queue` never accepts a head-policy run as a remote mirror.
An allowlist entry the branch adds to its own `pregate.json` is read the same
way: it clears a finding only once it is on the base.
<a id="pregate-lane-failed"></a>
### `PREGATE_LANE_FAILED`
+5
View File
@@ -208,10 +208,15 @@ function splitSections(body: string): Section[] {
* line, sections) yields the same bytes. A blank
* line separates the two sides of every seam the merge creates; two kept
* neighbours keep their bytes. An empty current body yields the file unchanged.
* A file section whose title the current body holds more than once (an old
* description pasted under the new one) matches no single section, so the
* merge refuses instead of replacing one copy and keeping the stale other.
*/
export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[] } {
const now = splitSections(current);
const next = splitSections(incoming).filter(n => !(n.key.startsWith("text:") && n.text.trim() === ""));
const ambiguous = next.filter(n => now.filter(s => s.key === n.key).length > 1).map(n => `"${n.label}"`);
if (ambiguous.length) throw new UsageError(`the PR's current body holds ${[...new Set(ambiguous)].join(", ")} more than once, so the file's copy matches no single section; nothing was posted. Rerun with --replace and the whole body`);
const used = new Set<Section>();
const replaced: string[] = [], kept: string[] = [], added: string[] = [];
const merged: Section[] = now.map(s => {
+11 -1
View File
@@ -518,9 +518,19 @@ async function run(ctx: PregateContext): Promise<CheckResult> {
return noPolicy ? withStarter(ctx, sel, result) : result;
}
/**
* The fix line for PREGATE_NO_LANE. When the branch edits `.gstack/pregate.json`
* while the policy comes from elsewhere (the base copy, or the user-owned file),
* a lane the branch adds does not count yet, and saying "declare its lane" again
* sends the author round in a circle. That is the trust boundary working (a
* branch cannot loosen its own gate), so the line says so and names the two
* routes; it never switches to the head policy on its own.
*/
function noLaneFix(ctx: PregateContext, noLane: readonly string[]): string {
const device = noLane.some(isDeviceDart) ? `; pregate does not run device tests, so declare "**/integration_test/**": [] (and "**/test_driver/**": []) and let CI run them` : '';
return `add a test that names the file, or declare its lane (or []) under dependencies — or empty_lanes for other documentation — in ${ctx.configFile}${device}`;
const fix = `add a test that names the file, or declare its lane (or []) under dependencies — or empty_lanes for other documentation — in ${ctx.configFile}${device}`;
if (ctx.configKind === 'head' || !ctx.touched.some(t => t.path === PREGATE_CONFIG_PATH && t.status !== 'D')) return fix;
return `this branch edits ${PREGATE_CONFIG_PATH}, but the policy comes from ${ctx.configLabel} so a branch cannot loosen its own gate, and a lane it adds counts only once it is on ${ctx.base}: land the policy change in its own PR first, or rerun with --policy-from head --allow-repo-commands, which records the run as policy head (unreviewed); otherwise ${fix}`;
}
export const lanesCheck: CheckDef = { id: 'lanes', stage: 'tests', tier: 1, downgradable: true, needsRepoCommands: true, run };
+22 -3
View File
@@ -526,15 +526,34 @@ describe("gstack-post pr-body merges the PR body by section", () => {
expect(fresh.body).toBe(receipt + "\n## A\n\nb\n");
expect([fresh.replaced, fresh.kept, fresh.added]).toEqual([["A"], [], ["ship receipt"]]);
const twice = mergeBodySections("## A\na\n## A\na2\n", "## A\nnew\n");
expect(twice.body).toBe("## A\nnew\n\n## A\na2\n");
expect([twice.replaced, twice.kept]).toEqual([["A"], ["A"]]);
expect(() => mergeBodySections("## A\na\n## A\na2\n", "## A\nnew\n")).toThrow('holds "A" more than once');
const untouched = mergeBodySections("## A\na\n## A\na2\n", "## B\nb\n");
expect(untouched.body).toBe("## A\na\n## A\na2\n\n## B\nb\n");
expect([untouched.kept, untouched.added]).toEqual([["A", "A"], ["B"]]);
const glued = mergeBodySections("## A\n\na", "## B\n\nb");
expect(glued.body).toBe("## A\n\na\n\n## B\n\nb");
expect(mergeBodySections("## A\n\na\n\n", "## B\n\nb\n").body).toBe("## A\n\na\n\n## B\n\nb\n");
});
// Value: protects=a PR body holding a section title twice (an old description pasted under the /ship body, gstack-evals#19) refuses to merge,
// posts nothing and names the fix, so the stale copy is never kept beside the new one; --replace posts the file as written;
// fails_when=the file's section replaces the first copy and the old one is kept (the #19 duplicate); why_new=the merge matched duplicate titles
// one to one and reported the leftover as kept; seam=the fake run answers gh pr view --json body; no_claim=real gh
test("a current body with a title twice refuses the merge and posts nothing; --replace posts the file", () => {
const fresh = RECEIPT("bbbb") + "\n## What changed\n\n- the /ship summary\n\n## Test plan\n\n- [x] bun test\n";
const current = RECEIPT("aaaa") + "\n## What changed\n\n- the /ship summary\n\n## Test plan\n\n- [x] bun test\n\n---\n\nOriginal intro.\n\n## What changed\n\n- the old summary\n\n## Checks\n\n- old checks\n";
const f = withBody(current, { "/b": fresh + "\n## Original description\n\nOriginal intro.\n" });
expect(run(["pr-body", "5", "--body-file", "/b"], f)).toBe(EXIT.usage);
expect(f.posts().some(c => c.args[1] === "edit")).toBe(false);
expect(f.stderr()).toContain(`the PR's current body holds "What changed" more than once`);
expect(f.stderr()).toContain("Rerun with --replace and the whole body");
const replaced = withBody(current, { "/b": fresh });
expect(run(["pr-body", "5", "--body-file", "/b", "--replace"], replaced)).toBe(EXIT.posted);
expect(sent(replaced).input).toBe(fresh);
});
// Value: protects=the body /ship composes (lib/ship-receipt.ts placeReceiptFirst: the receipt block, one blank line, then the sections) reposts
// idempotently: merged into a PR body of the same shape it yields the file's bytes, the receipt stays byte-first, and the stderr line reports no
// addition; fails_when=the blank line after the receipt is read as new leading text and prepended before the receipt (one more leading newline per
+20
View File
@@ -495,6 +495,26 @@ describe('gstack-pregate (bin)', () => {
expect(head.out).toMatch(/^tests {6}lanes {5}pass /m);
});
// Value: protects=a branch that adds its own lane in .gstack/pregate.json and fails PREGATE_NO_LANE under the base policy is told why (the base
// copy is the policy) and given both routes, while the base policy still decides; fails_when=the fix line repeats "declare its lane" with no
// mention of the base policy, or the run silently reads the head policy and passes; why_new=the circular fix line from gstack-evals #6/#15;
// seam=fixture repo; no_claim=ship-queue's handling of a head-policy record
test('lanes: a branch whose own pregate.json adds the lane still fails under the base policy, and the fix names the base copy and --policy-from head', () => {
const repo = fixture();
commit(repo, { 'lib/orphan.js': "exports.lonely = () => 2;\n" });
const plain = pregate(repo, ['--stage', 'tests']);
expect(plain.status).toBe(EXIT.fail);
expect(check(repo, 'lanes').fix).not.toContain('--policy-from head');
commit(repo, { '.gstack/pregate.json': JSON.stringify({ ...BASE_PREGATE, dependencies: { ...BASE_PREGATE.dependencies, 'lib/orphan.js': ['test/util.test.js'] } }) + '\n' });
const r = pregate(repo, ['--stage', 'tests']);
expect(r.status).toBe(EXIT.fail);
expect(r.err).toContain('(PREGATE_NO_LANE)');
const fix: string = check(repo, 'lanes').fix;
expect(fix).toMatch(/^this branch edits \.gstack\/pregate\.json, but the policy comes from origin\/main@[0-9a-f]{12}:\.gstack\/pregate\.json so a branch cannot loosen its own gate/);
expect(fix).toContain('land the policy change in its own PR first, or rerun with --policy-from head --allow-repo-commands');
expect(fix).toContain('otherwise add a test that names the file');
});
test('lanes: deleted and renamed files select the tests that named them; subprocess targets and computed reads map through declarations and joined literals', () => {
const repo = fixture();
repo.git('mv', 'lib/util.js', 'lib/maths.js');