diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e40735e..1eea7d93a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## [1.91.320.0] - 2026-10-29 + +**A branch that adds its own pregate lane now learns why it still fails, and `gstack-post pr-body` no longer keeps a stale copy of a section.** Pregate reads its policy from the base branch so a branch cannot loosen its own gate. A branch that had already declared a lane in its own `.gstack/pregate.json` was still told to declare it, and went round in a circle. Separately, a PR body that held `## What changed` twice (an old description pasted under the /ship body) kept the old copy every time /ship reposted. + +### Fixed +- **`PREGATE_NO_LANE`**: when the branch edits `.gstack/pregate.json` and the policy comes from the base copy or the user-owned file, the fix line names that policy, says a lane the branch adds counts only once it is on the base, and offers both routes: land the policy change in its own PR first, or rerun with `--policy-from head --allow-repo-commands`, which records the run as unreviewed. Pregate still never reads the branch's policy on its own. (#3574) +- **`gstack-post pr-body`**: when the PR's current body holds a title from the file more than once, it posts nothing, exits 64 and says to rerun with `--replace` and the whole body. /ship's PR-body step stops on that refusal and reports the duplicated titles instead of reposting with `--replace` itself, since a person chooses which copy stays. (#3575) + +### Docs +- `docs/troubleshooting.md#pregate-no-lane-own-policy` explains the own-policy case and its two routes; `docs/pregate.md`'s trust boundary links it. The `gstack-post-pr-body-merge` entry covers the duplicate-title refusal. + +### Tests +- `test/pregate-selection.test.ts`: the own-policy fix line under the base policy; a head-policy run and a branch that deletes `pregate.json` get the plain fix. +- `test/pregate-user-policy.test.ts`: the own-policy line names the user file as the policy source. +- `test/gstack-post.test.ts`: a doubled title refuses and posts nothing, `--replace` posts the file, and a body with two receipt blocks refuses too. + +### Gate integrity +- `test/gstack-post.test.ts`: the duplicate-titles assertion that expected the first copy replaced and the second kept now expects the refusal, plus a case where duplicates the file does not touch are still kept. + ## [1.91.319.0] - 2026-10-29 **`gstack-pregate sealed` no longer prints a line that reads like a push confirmation.** On gstack-evals#29 the lane read `pushed aeb2413` in the `SEALED:` line as the push landing, while the remote stayed at 9036ab1 (#3565). `sealed` never pushes: it checks the commits a push would publish. `pushed ` was its label for the commit whose `sealed` policy it read beside the base branch's. diff --git a/VERSION b/VERSION index c6d249a3f..5b93c3984 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.319.0 +1.91.320.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 3a2793d5c..00e18aa32 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.319.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.320.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/docs/pregate.md b/docs/pregate.md index 9e8aaefce..09a7ee76c 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -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. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b06d26507..ccb23ea08 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2516,11 +2516,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`. ### `MEDIUM pii.email :` from the pre-push hook @@ -3361,6 +3370,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. + +**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/` (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/@:.gstack/pregate.json so a branch cannot loosen +its own gate, and a lane it adds counts only once it is on ` 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. + ### `PREGATE_LANE_FAILED` diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index f633c10df..f7e216183 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -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
(); const replaced: string[] = [], kept: string[] = [], added: string[] = []; const merged: Section[] = now.map(s => { diff --git a/lib/pregate/checks/lanes.ts b/lib/pregate/checks/lanes.ts index 47df77d37..7d5feaf76 100644 --- a/lib/pregate/checks/lanes.ts +++ b/lib/pregate/checks/lanes.ts @@ -518,9 +518,19 @@ async function run(ctx: PregateContext): Promise { 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 }; diff --git a/package.json b/package.json index bb309fc9b..fa7f662ea 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.319", + "version": "1.91.320", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index a19f4aee7..c25c35999 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -294,7 +294,10 @@ Branch on the exit code of either block: output back into the body file and rerun the block, which scans again. Only when the user accepts the findings as they are, rerun the same block with `--confirm ` added to the command that printed it; any edit needs a new token. -- **3** `gh`/`glab` failed, or **64** no usable remote or CLI: print the error, the +- **64** with stderr saying the current body holds a title `more than once`: nothing was + posted. STOP and report the duplicated titles; never rerun with `--replace` yourself, + since a human picks which copy stays (docs/troubleshooting.md#gstack-post-pr-body-merge). +- **3** `gh`/`glab` failed, or any other **64** (no usable remote or CLI): print the error, the branch name, the remote URL and the block for posting by hand. Do not stop — the code is pushed — but never report a PR that was not created. With open gate findings, also say the gate edits were not disclosed and leave the Step 16 open-items report diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index c9c727af0..46b2a7d3c 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -277,7 +277,10 @@ Branch on the exit code of either block: output back into the body file and rerun the block, which scans again. Only when the user accepts the findings as they are, rerun the same block with `--confirm ` added to the command that printed it; any edit needs a new token. -- **3** `gh`/`glab` failed, or **64** no usable remote or CLI: print the error, the +- **64** with stderr saying the current body holds a title `more than once`: nothing was + posted. STOP and report the duplicated titles; never rerun with `--replace` yourself, + since a human picks which copy stays (docs/troubleshooting.md#gstack-post-pr-body-merge). +- **3** `gh`/`glab` failed, or any other **64** (no usable remote or CLI): print the error, the branch name, the remote URL and the block for posting by hand. Do not stop — the code is pushed — but never report a PR that was not created. With open gate findings, also say the gate edits were not disclosed and leave the Step 16 open-items report diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index 9ed9cc2b7..fc952a844 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -526,15 +526,42 @@ 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=a current body holding two ship-receipt blocks (an old receipt pasted below) refuses like a doubled title, so the stale + // receipt is never kept beside the new one; fails_when=receipt keys are exempted from the duplicate check or matched one to one again; + // why_new=only doubled ## titles were tested; seam=none; no_claim=how /ship reports the refusal + test("a current body with two receipt blocks refuses a file that carries a receipt", () => { + const current = RECEIPT("aaaa") + "\n## A\n\na\n\n" + RECEIPT("bbbb") + "\n## B\n\nb\n"; + expect(() => mergeBodySections(current, RECEIPT("cccc") + "\n## A\n\nnew\n")).toThrow('holds "ship receipt" more than once'); + }); + // 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 diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index d6eb6ef0c..489bb1982 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -495,6 +495,45 @@ 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'); + }); + + // Value: protects=the own-policy fix line appears only when the base or user copy decides and the branch still carries its own + // pregate.json: a --policy-from head run and a branch that deletes the file get the plain fix; fails_when=the configKind 'head' guard or + // the status 'D' filter in noLaneFix is dropped; why_new=the head run above passes and never reaches noLaneFix, and no test deletes the + // pin file on a NO_LANE run; seam=fixture repo; no_claim=the wording of the plain fix + test('lanes: a head-policy run and a branch that deletes pregate.json get the plain no-lane fix', () => { + const repo = fixture(); + commit(repo, { 'lib/orphan.js': "exports.lonely = () => 2;\n", '.gstack/pregate.json': JSON.stringify({ ...BASE_PREGATE, dependencies: { ...BASE_PREGATE.dependencies, 'lib/orphan.js': ['test/util.test.js'] } }) + '\n' }); + commit(repo, { 'lib/orphan2.js': "exports.alone = () => 3;\n" }); + const head = pregate(repo, ['--stage', 'tests', '--policy-from', 'head']); + expect(head.status).toBe(EXIT.fail); + expect(head.err).toContain('(PREGATE_NO_LANE)'); + expect(check(repo, 'lanes').fix).toMatch(/^add a test that names the file/); + commit(repo, { '.gstack/pregate.json': null }); + const deleted = pregate(repo, ['--stage', 'tests']); + expect(deleted.status).toBe(EXIT.fail); + expect(deleted.err).toContain('(PREGATE_NO_LANE)'); + expect(check(repo, 'lanes').fix).toMatch(/^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'); diff --git a/test/pregate-user-policy.test.ts b/test/pregate-user-policy.test.ts index b56a32032..1cd0646fa 100644 --- a/test/pregate-user-policy.test.ts +++ b/test/pregate-user-policy.test.ts @@ -131,6 +131,19 @@ describe('gstack-pregate with a user-owned policy', () => { expect(lanes.fix).toContain(`under dependencies — or empty_lanes for other documentation — in ${userPolicy}`); }, 60_000); + // Value: protects=under a user-owned policy, a branch that adds its own .gstack/pregate.json is told the user file is the policy, by path; + // fails_when=the own-policy line is limited to the base copy or hardcodes origin as the source; why_new=the own-policy line is new and its + // test covers the base copy only; seam=none; no_claim=which route suits a user-policy repository + test('the own-policy no-lane fix names the user file as the policy source', () => { + const { repo, userPolicy, run } = setup(); + writePolicy(userPolicy, { local: LOCAL_ALL }); + commit(repo, { 'lib/orphan.js': 'exports.lonely = () => 2;\n', '.gstack/pregate.json': JSON.stringify({ runner: 'node --test {files}', dependencies: { 'lib/orphan.js': ['test/util.test.js'] } }) }); + expect(run().status).not.toBe(EXIT.ok); + const lanes = JSON.parse(fs.readFileSync(path.join(repo.work, '.gstack/tmp/pregate.json'), 'utf8')).checks.find((c: { id: string }) => c.id === 'lanes'); + expect(lanes.code).toBe('PREGATE_NO_LANE'); + expect(lanes.fix).toStartWith(`this branch edits .gstack/pregate.json, but the policy comes from user ${userPolicy} so a branch cannot loosen its own gate`); + }, 60_000); + // Value: protects=an invalid, sealed-bearing or unreadable user file refuses with PREGATE_CONFIG_INVALID naming the file, with a fix // that points at the file rather than the base branch; fails_when=user-file errors run on defaults, or sealed/price_file are accepted // and look active while nothing reads them; why_new=the user source is new; seam=none; no_claim=the sealed guard itself diff --git a/test/size-ledger/ship/fix-pregate-own-policy-and-post-duplicate.json b/test/size-ledger/ship/fix-pregate-own-policy-and-post-duplicate.json new file mode 100644 index 000000000..9b2584d0f --- /dev/null +++ b/test/size-ledger/ship/fix-pregate-own-policy-and-post-duplicate.json @@ -0,0 +1,4 @@ +{ + "bytes": 281, + "why": "pr-body's exit-64 branch stops and reports a duplicate-title refusal instead of the no-remote by-hand path (#3575)" +}