From 819fc25ad4dbc89c81482872b6a72c8b8ef3a666 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 06:12:49 +0000 Subject: [PATCH] fix(ship): goldens, head= in the base-red line test, Severity field after a closing quote (ggstack#71); repin ship/sections/review-army.md under GST-508+GST-464+GST-967+GSTA-258-L4 (brief-identity no-loss check: every cut brief unchanged) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta258-l4-ready-receipts.md | 4 ++-- lib/review-ledger.ts | 2 +- test/base-red.test.ts | 2 +- test/fixtures/golden/agy-ship-SKILL.md | 2 +- test/fixtures/golden/claude-ship-SKILL.md | 2 +- test/fixtures/golden/codex-ship-SKILL.md | 2 +- test/fixtures/golden/factory-ship-SKILL.md | 2 +- test/review-ledger-verdict-line.test.ts | 10 ++++++++++ test/review-packets-removed.test.ts | 3 ++- 9 files changed, 20 insertions(+), 9 deletions(-) diff --git a/changelog.d/gsta258-l4-ready-receipts.md b/changelog.d/gsta258-l4-ready-receipts.md index 0790d9a30..2b8b4845d 100644 --- a/changelog.d/gsta258-l4-ready-receipts.md +++ b/changelog.d/gsta258-l4-ready-receipts.md @@ -3,7 +3,7 @@ ### Changed - **`gstack-review-scope summary` binds by finding id.** A row finding carries `ledger_id` (`/`), each fix commit names what it fixed in a `Review-Finding: reader=` trailer, rows that record their head dedupe by kind, pass, head and finding ids, and a late defer counts once. Rows and findings without an id keep the path:line pairing. - **What can't bind is named, never a silent zero.** A row with no start token or readable time, a `pN` ledger with no review row, a `ledger_id` no ledger holds and a trailer naming nothing each print `REVIEW_SCOPE_UNBOUND: : `, and the line and receipt carry `unbound=N`. -- **The native reader's own verdict line no longer collects as a blocker.** "There are three low-severity findings, all FIXABLE, and none blocks." (#3901) collected as CRITICAL beside the findings it summarized; it now collects at FIXABLE. A line that says something blocks, names a file:line or leads with FIXABLE still collects as CRITICAL. +- **The native reader's own verdict line no longer collects as a blocker.** "There are three low-severity findings, all FIXABLE, and none blocks." (#3901) collected as CRITICAL beside the findings it summarized; it now collects at FIXABLE. A line that says something blocks, names a file:line or leads with FIXABLE still collects as CRITICAL. A finding's `Severity: Low. Blocks: no` after a closing quote (`… its base." Severity: Low`) now counts as its own field: on ggstack#71 that one quote made one of four identical findings CRITICAL. - **`gstack-ship-receipt carry` offers a scoped re-check (J1).** On `RECEIPT_CARRY_PRODUCT_CHANGED` with linear history against the reviewed base and a range-diff (plus the diff of the changed product paths) of at most 400 lines, carry prints `RECHECK: eligible … packet=`. One reader on the packet and `carry --recheck ` rebind the receipt, unless HEAD needs a deeper review depth or another risk class. Rebased branches, retargeted stacks and anything over the cap still need /ship. - **Every /ship run's receipt is kept.** When `write` or `carry` replaces a different receipt, the old one's identifiers and counters move to an append-only `gstack-receipt-runs` block in the PR body (newest 30). - **`gstack-ship-queue ready` prints the whole READY.** It adds the receipt's `REVIEW_LOOP:` line, a `TRIAL_MERGES:` line for the lane to fill and an `Attention:` stub that opens with `reviewed by ` on risk-class PRs and `self-audited` on every other PR. docs/unattended.md now asks for the cross-family trail read only on risk-class PRs. @@ -12,6 +12,6 @@ ### Tests - `test/review-loop-binding.test.ts`: the GBEV-3, GEVA-223, #3906, #3892/#3903/#3911 and #3911 duplicate-row cases. On main the GBEV-3 case reproduces its receipt exactly (3 found, 2 fixed) and GEVA-223's pattern (18 found, 9 unrecorded). -- `test/review-ledger-verdict-line.test.ts`: the captured #3901 output, with negative controls. +- `test/review-ledger-verdict-line.test.ts`: the captured #3901 output and ggstack#71's findings, with negative controls. - `test/ship-recheck.test.ts`: every refusal that does and doesn't qualify, the record binding, and the carry end to end. - `test/ship-receipt-runs.test.ts`, `test/ship-receipt-result-head.test.ts`: the runs block, the READY tail, `no_stop` validation and the `head=` token. diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index e1fa339be..67c4d7838 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -73,7 +73,7 @@ const STATED_SEVERITY = new RegExp(`(?]\\s+|\\d+[.)]\\s+)?\\**\\s*Severity\\**\\s*:?\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])|(?:[.;,|()]\\s*|\\s[—–-]\\s*)\\**\\s*Severity\\**\\s*:\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])(?=\\**\\s*(?:$|[.;,|()*—–]|-\\s|Blocks)))`, 'gi'); +const SEVERITY_FIELD = new RegExp(`(?:^(?:[-*+>]\\s+|\\d+[.)]\\s+)?\\**\\s*Severity\\**\\s*:?\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])|(?:[.;,|()]["'”’\`]*\\s*|\\s[—–-]\\s*)\\**\\s*Severity\\**\\s*:\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])(?=\\**\\s*(?:$|[.;,|()*—–]|-\\s|Blocks)))`, 'gi'); /** * A label leading its line past list, quote, heading, emphasis and emoji marks (one mark per step, so a long divider stays linear) * and an optional file:line, closed by a mark or a separator, or by a space, comma or parenthesis after an all-caps word ("[P0] …", diff --git a/test/base-red.test.ts b/test/base-red.test.ts index c2f868b8d..221957d7c 100644 --- a/test/base-red.test.ts +++ b/test/base-red.test.ts @@ -144,7 +144,7 @@ describe('pause, receipt and wait', () => { fs.writeFileSync(bodyFile, placeReceiptFirst('## Summary\n', { schema_version: 1, pr: 7, head, base: 'main', base_red: receiptBaseRed(pause) } as any)); const res = bin('gstack-ship-receipt', ['result', '--body', bodyFile, '--head', head, '--out', path.join(root, 'run')]); expect(res.status).toBe(EXIT.fail); - expect(res.stdout.trim()).toBe(`GSTACK_RESULT: skill=ship status=incomplete run=${path.join(root, 'run')}`); + expect(res.stdout.trim()).toBe(`GSTACK_RESULT: skill=ship status=incomplete head=${head} run=${path.join(root, 'run')}`); expect(res.stderr).toContain(`SHIP_PAUSED: base-red cases=1 base=${pause.base_sha.slice(0, 12)} lane=tests`); expect(JSON.parse(fs.readFileSync(path.join(root, 'run', 'ship-run.json'), 'utf8')).status).toBe('incomplete'); const carry = bin('gstack-ship-receipt', ['carry', '--body', bodyFile]); diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 45130e661..e7f441d1e 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -957,7 +957,7 @@ The last line printed is the result line a parent greps (docs/unattended.md): ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 125947cba..47496e238 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -956,7 +956,7 @@ The last line printed is the result line a parent greps (docs/unattended.md): ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index bf8e4ddcc..6cac22e78 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -966,7 +966,7 @@ $GSTACK_ROOT/bin/gstack-ship-receipt result --pr # handoff: --body ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index a34726a90..fdf11bc92 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -968,7 +968,7 @@ $GSTACK_ROOT/bin/gstack-ship-receipt result --pr # handoff: --body ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/review-ledger-verdict-line.test.ts b/test/review-ledger-verdict-line.test.ts index 14582acd9..4f42cc3eb 100644 --- a/test/review-ledger-verdict-line.test.ts +++ b/test/review-ledger-verdict-line.test.ts @@ -32,6 +32,16 @@ describe('native verdict line (collector severity inflation)', () => { expect(labels('- FIXABLE, and nothing blocks')).toEqual(['CRITICAL']); }); + // ggstack#71 (GGST-15, gstack 1.94.4.0, gate-class plan): four identically shaped native findings, the first labeled CRITICAL + // because its Severity field followed a closing quote ('… its base." Severity: Low'). Excerpt of findings 1 and 2, as returned. + test('ggstack#71: a Severity field after a closing quote is still the finding\'s own field', () => { + const out = ['## Findings', '', "**1. The CHANGELOG describes the \"behind main\" rule wrongly (CHANGELOG.md:10).** It says \"a pull request that doesn't contain main's tip runs the suite here as before.\" gstack's actual rule is different: the run's merge-commit tree must equal the head's tree. That means the head had to contain its base as it was when CI ran, and that base isn't always main (stacked PRs). Nothing in `findCarry` or `verifyRun` compares against main's current tip, so a run from before main moved still carries. /ship's own merge of the base usually changes the tree before this matters, but the text states a guarantee the code doesn't make. FIXABLE: reword it to something like \"a pull_request run made while the branch was behind its base.\" Severity: Low. Blocks: no", '', "**2. `files` is a static file listing, and run.sh skips Godot silently (test.yml:46, pregate.json:7).** tree.json's `files` comes from the same `ls -1 test/*.sh test/*.py test/*.gd` as the inventory, so the coverage check passes by construction. The receipt's `tests=9` counts files, not tests. run.sh prints SKIP and exits 0 when `$GODOT` isn't executable or neither `xvfb-run` nor `DISPLAY` is available. Web has `REQUIRE_WEB`, but nothing equivalent makes a missing Godot fail. Today the Godot and xvfb install steps fail hard, so nothing falsely carries. If a later workflow edit makes either step soft, CI would still list the `.gd` tests as covered. A local run without Godot skips them the same way, so the carry is never weaker than the lane it replaces. FIXABLE: add `REQUIRE_GODOT=1`, or fail the record step when the run.sh output contains `test: SKIP`. Severity: Low. Blocks: no", '', 'Recommendation: Ship as-is.'].join('\n'); + expect(labels(out)).toEqual(['Low', 'Low']); + expect(labels('1. **Wording (a.md:3).** FIXABLE: say \'the base\'. Severity: Low. Blocks: no')).toEqual(['Low']); + expect(labels('1. **Wording (a.md:3).** FIXABLE: say \u201cthe base.\u201d Severity: Low. Blocks: no')).toEqual(['Low']); + expect(labels('1. **Data loss (a.ts:3).** FIXABLE: guard it." Severity: Low. Blocks: yes')).toEqual(['CRITICAL']); + }); + test('a stated Blocks field on the line wins: Blocks: yes stays CRITICAL', () => { expect(labels('All FIXABLE, none blocks. Blocks: yes')).toEqual(['CRITICAL']); }); diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 89eaf5aa5..9b54cc7fb 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -73,10 +73,11 @@ const NO_LOSS_REPLAYS = { 'GST-967': "GST-967 dispatch-superset no-loss check (test/review-depth.test.ts, 'GST-967 dispatch-superset no-loss check'): over 1,440 planReview cases the branch plans exactly what main planned at 1022f4f except the 71 gate-class stops before Step 11, which add only Step 11's adversarial, outside and targeted voices, so no reviewer main dispatches is dropped or re-efforted; the brief bodies lib/review-brief.ts cuts from the ship sections hash to main's; only the lead's repeat-decision and Step 11 instructions changed; reviewer text is unchanged", 'GST-508+GST-464+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: ship/sections/review-army.md and adversarial.md are main's GST-967 bytes plus exactly the GST-508+GST-464 lines (the merged file minus main equals the wave minus fc292e22f, line for line), so GST-967's dispatch-superset no-loss check and GST-508's no-loss replay both hold; merged-head checks in GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md", 'GST-508+R51+GST-911+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: lib/review-depth.ts is main's GST-967 bytes plus exactly the GST-508+R51+GST-911 lines (the merged file minus main equals the wave minus fc292e22f, line for line); GST-967's dispatch-superset no-loss check (test/review-depth.test.ts) passes on the merged file, and the GST-508+R51+GST-911 checks are rerun at the merged head (GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md)", + 'GST-508+GST-464+GST-967+GSTA-258-L4': "GSTA-258 L4 (GST-1168) brief-identity no-loss check (the brief-body test below): ship/sections/review-army.md is the GST-508+GST-464+GST-967 bytes plus Step 9.4 lead instructions outside the cut spans (a finding's ledger_id, the Review-Finding fix-commit trailer, logging a late defer when it is decided); every reviewer brief lib/review-brief.ts cuts from it still hashes to BRIEF_BODIES, so no reviewer sees a changed byte", } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { - 'ship/sections/review-army.md': { sha: 'de3e0d35f2af56492c42fd287b6868fbe19bce96b86321e74a4947ba909b74eb', replay: 'GST-508+GST-464+GST-967' }, + 'ship/sections/review-army.md': { sha: '22c37a64fd937f94ce231f8d815c401f232e0b06da8a1e57d3d15a18d3c9f2c0', replay: 'GST-508+GST-464+GST-967+GSTA-258-L4' }, 'ship/sections/adversarial.md': { sha: '0142d6bc3f2a5bceb9f8b2adef1d3a206e5836c99a0667fe995568da2e80a5a6', replay: 'GST-508+GST-464+GST-967' }, 'review/sections/review-army.md': { sha: '78329b64640260cd8831ca81a9189f3f8dcade9c406431631d3f2cd798cd3967', replay: '#3638' }, 'review/sections/adversarial.md': { sha: 'e6962fa25b78f362654f2954f79683e524d2ebd452d1494f8a2b099888e8e255', replay: 'G1' },