diff --git a/changelog.d/capy-gsta156-sealed-skip.md b/changelog.d/capy-gsta156-sealed-skip.md index 108f9c3c9..6126e96fe 100644 --- a/changelog.d/capy-gsta156-sealed-skip.md +++ b/changelog.d/capy-gsta156-sealed-skip.md @@ -1,7 +1,7 @@ **The sealed guard no longer rescans commits the destination already holds, so a stacked branch retargeted after its parent squash-landed stops being refused for the parent's history.** gstack-evals#22 listed its parent #6's seven commits after #6 landed as one squash. One of them had added and later deleted a held-out file, so the guard refused a push whose every byte GitHub already had as `refs/pull/6/head`, and the queue sat blocked for about 50 minutes until a waiver landed. ### Changed -- **`lib/sealed-guard.ts`** (`evaluateSealed`, shared by the pre-push hook and `gstack-pregate sealed`): a listed commit is skipped when it is an ancestor of the destination's own `refs/pull//head` for a squash `(#n)` on the base since the merge-base (at most 50 refs, a 20-second budget, an egress receipt per remote call, fetched refs deleted afterwards), or when its own `git patch-id --verbatim` and diff equal a base commit's (off above 2,000 base or 400 listed commits). Nothing is matched against a combined diff, and any failed lookup scans everything with one line saying why. docs/pregate.md#sealed-skip. +- **`lib/sealed-guard.ts`** (`evaluateSealed`, shared by the pre-push hook and `gstack-pregate sealed`): a listed commit is skipped when it is an ancestor of the destination's own `refs/pull//head` for a squash `(#n)` on the base since the merge-base (at most 50 refs, a 20-second budget, an egress receipt per remote call, fetched refs deleted afterwards), or when its own `git patch-id --verbatim` and diff equal a base commit's (off above 2,000 base or 400 listed commits; a landed commit still gets the `sealed.label_join` check, which reads the whole file). Nothing is matched against a combined diff, and any failed lookup scans everything with one line saying why. docs/pregate.md#sealed-skip. - **`gstack-pregate sealed`**: the `SEALED:` line gains `skipped=` (and `commits=` counts the commits scanned), stderr names each skipped group, and `--no-skip` scans every listed commit. ### Tests diff --git a/docs/pregate.md b/docs/pregate.md index c5c687390..4d16f28f9 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -1130,7 +1130,9 @@ gstack-pregate sealed [--range ] [--remote origin] [--base ] [--no `sealed-guard`) and deletes the fetched refs afterwards. Landed: the commit's own `git patch-id --verbatim` equals a base commit's since the merge-base and so does its diff, apart from blob ids and line numbers; a merge never matches, - and above 2,000 base or 400 listed commits this check is off. Nothing + and above 2,000 base or 400 listed commits this check is off. A landed commit + skips only the path and text rules: `sealed.label_join` judges the whole file + at that commit, which the base may never have held, so it still runs. Nothing matches a combined diff, so a parent whose commits were never published is scanned whole even when its squash landed. A lookup that fails, runs out of time or exceeds a limit prints one line and scans everything. Each PR group diff --git a/lib/sealed-guard.ts b/lib/sealed-guard.ts index 3ca74826d..773671b6a 100644 --- a/lib/sealed-guard.ts +++ b/lib/sealed-guard.ts @@ -35,7 +35,9 @@ * for a squash `(#n)` on the base since the merge-base (at most 50 refs, a * 20 s budget, fetched from the destination remote), is not rescanned, nor is * one whose own `git patch-id --verbatim` and diff equal a base commit's - * since the merge-base (off above 2,000 base or 400 listed commits). No + * since the merge-base (off above 2,000 base or 400 listed commits). A + * landed patch skips only the path and text rules: `sealed.label_join` judges + * the whole file at the commit, which the base may never have held. No * combined-diff matching; any lookup failure scans everything. */ import { spawnSync } from 'node:child_process'; @@ -200,10 +202,11 @@ function textFindings(repoRoot: string, commits: string[], p: SealedPolicy): Sea return out; } -export function scanCommits(repoRoot: string, commits: string[], policy: SealedPolicy): SealedFinding[] { - if (!commits.length) return []; +export function scanCommits(repoRoot: string, commits: string[], policy: SealedPolicy, joinCommits: string[] = commits): SealedFinding[] { + if (!commits.length && !joinCommits.length) return []; const seen = new Set(); - return [...pathFindings(repoRoot, commits, policy), ...textFindings(repoRoot, commits, policy), ...joinFindings(repoRoot, commits, policy)].filter(f => { + const own = commits.length ? [...pathFindings(repoRoot, commits, policy), ...textFindings(repoRoot, commits, policy)] : []; + return [...own, ...joinFindings(repoRoot, joinCommits, policy)].filter(f => { const key = `${f.rule}\0${f.file}\0${f.line ?? ''}\0${f.commit}`; if (seen.has(key)) return false; seen.add(key); @@ -378,8 +381,9 @@ export function evaluateSealed(o: EvaluateOptions): SealedReport { const { skipped, notes } = alreadyPublished(o, listed); const skip = new Set(skipped.map(k => k.commit)); const commits = listed.filter(c => !skip.has(c)); + const held = new Set(skipped.filter(k => k.pr !== undefined).map(k => k.commit)); let found: SealedFinding[]; - try { found = scanCommits(o.repoRoot, commits, policy); } + try { found = scanCommits(o.repoRoot, commits, policy, listed.filter(c => !held.has(c))); } catch (e: any) { return { ...blank, policyLabels: labels, commits: commits.length, skipped, notes, status: 'error', error: `could not read the published commits: ${e.message}` }; } const exception = globMatcher(policy.allow_paths, false); const exempt: ExemptFinding[] = []; diff --git a/test/sealed-skip.test.ts b/test/sealed-skip.test.ts index 58e91240a..40a5c1b74 100644 --- a/test/sealed-skip.test.ts +++ b/test/sealed-skip.test.ts @@ -156,6 +156,28 @@ describe('sealed skips what the destination already holds', () => { expect(preflight(s).out).toMatch(/^SEALED: refused findings=1 commits=1 skipped=0 /m); }); + test('a landed commit still gets the label_join check, because the join can form only on the branch', () => { + const s = scratch(); + const rec = (extra: Record) => ['{', ' "id": "r1",', ' "k1": 1,', ' "k2": 2,', ' "k3": 3,', ' "k4": 4,', ' "k5": 5,', ' "k6": 6,', ' "k7": 7', '}'].flatMap((l, i) => extra[i] ? [l, extra[i]!] : [l]).join('\n') + '\n'; + const labeled = JSON.stringify({ sealed: { paths: ['heldout/**'], label_keys: ['stratum'] } }, null, 2) + '\n'; + commit(s, { '.gstack/pregate.json': labeled, 'rec.json': rec({}) }, 'labeled policy'); + sh(s, ['push', '-q', 'origin', 'main']); + sh(s, ['checkout', '-q', '-b', 'results']); + commit(s, { 'rec.json': rec({ 1: ' "stratum": "decoy",' }) }, 'stratum'); + const head = commit(s, { 'rec.json': rec({ 1: ' "stratum": "decoy",', 7: ' "head": "abc1234def5678",' }) }, 'head'); + sh(s, ['checkout', '-q', 'main']); + sh(s, ['cherry-pick', head]); + sh(s, ['push', '-q', 'origin', 'main']); + expect(sh(s, ['show', 'HEAD:rec.json'])).not.toContain('stratum'); + sh(s, ['checkout', '-q', 'results']); + const h = hook(s); + expect(h.code).toBe(1); + expect(h.err).toMatch(/sealed\.label_join\s+rec\.json/); + const p = preflight(s); + expect(p.code).toBe(1); + expect(p.out).toMatch(/^SEALED: refused findings=1 commits=1 skipped=1 /m); + }); + test('a landed commit that differs only in bytes that are not UTF-8 is scanned and refuses', () => { const s = scratch(); sh(s, ['checkout', '-q', '-b', 'results']);