diff --git a/changelog.d/gsta195-evidence-prose.md b/changelog.d/gsta195-evidence-prose.md new file mode 100644 index 000000000..05aaa2f33 --- /dev/null +++ b/changelog.d/gsta195-evidence-prose.md @@ -0,0 +1,14 @@ +**A passing test lane that leaves Bun's `.bun/` folder behind now grades FRESH, `--check-prose` no longer reads the Approvals table as a claim, and `gstack-post pr-body` drops a stale intro.** Three /ship false alarms from GST-421 and GST-412 (HARN-1, HARN-12, GSTA-2, GEVA-69, gstack-evals#90). + +### Fixed +- **GST-421: `gstack-wtree` leaves an untracked `.bun/` folder at the repo root out of the fingerprint.** A test that created it made `gstack-evidence` treat the run as a mid-run edit, so `check` graded the lane STALE with "record has no content fingerprint" and named no file. Tracked files under `.bun/` still count, and a tree without that folder hashes exactly as before. +- **GST-421: `gstack-evidence` names what changed during a run.** When the fingerprint before and after a run differ, the record stores up to five changed paths as `wtree_changed`, the stderr warning lists them, and `check` grades STALE with `content changed during the run: `. A record without them still says "record has no content fingerprint". +- **GST-412: `gstack-ship-receipt read --check-prose` skips the `## Approvals` block `gstack-approvals` renders** (the heading, the table and its `Source: gstack-approvals record` line), so "one Step 11 pass" in the owner's quoted words is no longer read as `gate..pass body=11`. Text after that block, a hand-written Approvals list and an example inside a code fence are still checked, and CRLF bodies work. + +### Changed +- **GST-412: `gstack-post pr-body` manages the text above the first heading when the file has a `## ` section.** The current body's opening text, any text between the receipt and the first heading, or a whole body with no `## ` heading, is replaced by the file's own text above its first heading, or dropped when the file has none. The stderr line names each dropped piece with its first line (`; dropped leading text ("…")`). A receipt-only or text-only file merges as before, and reposting the same body gives the same bytes. + +### Tests +- `test/evidence.test.ts`: a lane that writes `.bun/x` grades FRESH; one that writes `src/new.ts` grades STALE naming it; a tracked file under `.bun/` still moves the fingerprint; a tree without `.bun/` hashes like a plain `git add -A`. +- `test/ship-receipt-prose.test.ts`: the rendered Approvals table, text after its footer, a fenced example, a hand-written list and a CRLF body. +- `test/gstack-post.test.ts`: stale lead text and stale text under the receipt are dropped and named; receipt-only and text-only files keep the old merge; reposts are byte-identical; an attribution callout is never doubled. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 0cf827ca5..8bd658a6b 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -970,7 +970,12 @@ not a claim; a number that is the value of a `key=` or `key:` pair is that key's claim alone, so `passes=2 blocking_found=1` is two claims, never a `2 blocking_found` one, #3513). A body that quotes a different number is `RECEIPT_PROSE_MISMATCH` (exit 1) and the `PROSE:` line names each field with -both values; prose that quotes no counter is not a claim. Data PRs (evals, +both values; prose that quotes no counter is not a claim. The `## Approvals` +block `gstack-approvals` renders (its heading, the Date | Asked | Exact words +table and the `Source: gstack-approvals record` line) quotes the owner's +words, not this PR's numbers, so it is skipped: "one Step 11 pass" in that table +is not a `pass` claim. Text you write under the heading after that block, and +a `## Approvals` example inside a code fence, are still checked. Data PRs (evals, measurements, sweeps) run it before READY so the body's numbers are the run's. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 471282f4e..98fb793b3 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2510,11 +2510,18 @@ section instead of overwriting it: a section in the file replaces the same-titled section (title compared case-insensitively), the ````gstack-ship-receipt```` block is a section of its own so a receipt rewrite replaces only the receipt, sections the file does not rewrite stay in -place, new sections are appended after them, and a current body with no -`## ` headings is kept as the leading text. One stderr line names what was -replaced, kept and added (`gstack-post: pr-body merged by section: replaced -What changed; kept ship receipt, Why, Approvals; added Tests`). A body file -holding only its first section therefore no longer wipes the rest of the PR. +place, and new sections are appended after them. The text above the first +`## ` heading belongs to the file whenever the file has a `## ` section: the +current body's text there (the opening lines, and any lines between the receipt +and the first heading, or the whole body when it has no `## ` heading) is +replaced by the file's own text above its first heading, or dropped when the +file has none. A file with no `## ` section (a receipt-only or text-only post) +leaves that text alone. One stderr line names what was replaced, kept, added +and dropped, a dropped piece with its first line (`gstack-post: pr-body merged +by section: replaced What changed; kept ship receipt, Why, Approvals; added +Tests; dropped leading text ("Draft from gh pr create")`). A body file holding +only its first section therefore no longer wipes the rest of the PR, and an +old intro no longer sits above a fresh /ship body. 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 @@ -5059,7 +5066,8 @@ body that names a counter the receipt records (`passes=3` against `review_loop.passes: 2`, `1 deferred` against `deferred: 2`, `passed: 40` against `gate.passed: 41`) with a different value. The receipt is written from the run; the prose was typed, so a reader of the body would learn a number the -run did not produce. The `PROSE:` line names each field with both values. +run did not produce. The `PROSE:` line names each field with both values. The +`## Approvals` table `gstack-approvals` renders is never read as a claim. **Fix.** Correct the body to the receipt's number. When the receipt is the stale one (the run was repeated), rerun `/ship` so the receipt is rewritten diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index f7e216183..038894a14 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -9,7 +9,8 @@ * `pr-body` merges by `## ` section into the PR's current body (read through * the same runner): a section in the file replaces the same-titled one, the * ship-receipt block is a section of its own, sections the file does not - * rewrite are kept, new ones are appended; `--replace` overwrites instead. + * rewrite are kept, new ones are appended, and a file with a `## ` section + * replaces the text above the first heading; `--replace` overwrites instead. * The scan runs on the merged bytes, which are the bytes sent. * * Exit codes: 0 posted; 1 HIGH finding (or oversize input) refused; @@ -211,24 +212,52 @@ function splitSections(body: string): Section[] { * 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. + * A file with at least one `## ` section manages the preamble (GST-412): every + * non-blank text section before the current body's first heading (the leading + * text, text after a receipt block) is dropped, the file's own text before its + * first heading takes its place (leading text first, text after the file's + * receipt right after the receipt), a dropped piece the file brings no text for + * is listed in `dropped` with its first line, and the receipt keeps the rules + * above. A file without a `## ` section merges as described above. */ -export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[] } { +export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: 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 => { + const managed = next.some(n => n.key.startsWith("h2:")); + const preamble = (list: Section[]) => { + const end = list.findIndex(s => s.key.startsWith("h2:")); + return list.slice(0, end < 0 ? list.length : end).filter(s => s.key.startsWith("text:") && s.text.trim() !== ""); + }; + const stale = managed ? preamble(now) : []; + const ours = managed ? preamble(next) : []; + const role = (s: Section) => (s.key === "text:lead" ? "lead" : "trailing"); + const paired = new Set
(); + for (const r of ["lead", "trailing"]) { + const theirs = ours.filter(n => role(n) === r); + stale.filter(s => role(s) === r).slice(0, theirs.length).forEach((s, i) => { paired.add(s); paired.add(theirs[i]!); }); + } + const used = new Set
(ours); + const replaced: string[] = [], kept: string[] = [], added: string[] = [], dropped: string[] = []; + const merged: Section[] = []; + for (const s of now) { + if (stale.includes(s)) { + const line = s.text.trim().split(/\r?\n/)[0]!; + if (paired.has(s)) replaced.push(s.label); + else dropped.push(`${s.label} ("${line.length > 60 ? line.slice(0, 59).trimEnd() + "…" : line}")`); + continue; + } const match = s.key === "unclosed-receipt" ? undefined : next.find(n => !used.has(n) && n.key === s.key); - if (!match) { if (s.text.trim()) kept.push(s.label); return s; } - used.add(match); replaced.push(s.label); return match; - }); + if (!match) { if (s.text.trim()) kept.push(s.label); merged.push(s); continue; } + used.add(match); replaced.push(s.label); merged.push(match); + } const fresh = next.filter(n => !used.has(n)); - for (const n of fresh) added.push(n.label); + for (const n of next) if (fresh.includes(n) || (ours.includes(n) && !paired.has(n))) added.push(n.label); const first = fresh.filter(n => !n.key.startsWith("h2:")); - if (now.length === 0) return { body: incoming, replaced, kept, added }; - const all = [...first, ...merged, ...fresh.filter(n => n.key.startsWith("h2:"))]; + if (now.length === 0) return { body: incoming, replaced, kept, added, dropped }; + const all = [...ours.filter(n => role(n) === "lead"), ...first, ...merged, ...fresh.filter(n => n.key.startsWith("h2:"))]; + for (const t of ours.filter(n => role(n) === "trailing").reverse()) all.splice(all.indexOf(next[next.indexOf(t) - 1]!) + 1, 0, t); const fromFile = (s: Section) => next.includes(s); const body = all.map((s, i) => { const after = all[i + 1]; @@ -236,7 +265,7 @@ export function mergeBodySections(current: string, incoming: string): { body: st const seam = (fromFile(s) || fromFile(after)) && s.text.trim() !== "" && !/^\r?\n/.test(after.text); return seam ? s.text.replace(/\n*$/, "\n\n") : s.text.endsWith("\n") ? s.text : s.text + "\n"; }).join(""); - return { body, replaced, kept, added }; + return { body, replaced, kept, added, dropped }; } /** The PR's current body for a merge; GitHub's `--jq .body` prints it raw, GitLab's JSON carries `description`. */ @@ -508,7 +537,7 @@ export function runPost(argv: string[], env: PostEnv, configBin = "gstack-config const m = mergeBodySections(current.body, body!); body = m.body; const list = (names: string[]) => (names.length ? names.join(", ") : "none"); - env.err(`gstack-post: pr-body merged by section: replaced ${list(m.replaced)}; kept ${list(m.kept)}; added ${list(m.added)}\n`); + env.err(`gstack-post: pr-body merged by section: replaced ${list(m.replaced)}; kept ${list(m.kept)}; added ${list(m.added)}${m.dropped.length ? `; dropped ${m.dropped.join(", ")}` : ""}\n`); } if (body !== undefined && req.attribute) body = attributionCallout(req.attribute) + body; const parts: Array<[Part, string]> = []; diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index fc952a844..cc292c903 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -446,11 +446,60 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(after.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, Why; kept unclosed receipt fence; added none\n"); }); + // Value: protects=a file with sections owns the text above the first heading (GST-412): stale lead text and stale text between the receipt and the + // first heading are dropped and named on stderr with their first line, the file's own preamble takes their place, a receipt-only or text-only file + // keeps the older merge, a repost is byte-identical and an attribution callout is never doubled; fails_when=pr-body keeps a stale intro above the + // receipt or a stale note under it after /ship reposts the sections; why_new=lead text was kept unless the file brought its own; seam=fake run; + // no_claim=real gh + test("a file with sections replaces every text section above the current first heading and names each dropped piece", () => { + const stale = withBody("Draft intro from gh pr create, long enough to need truncation in the summary line\nsecond line\n\n" + CURRENT, { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], stale)).toBe(EXIT.posted); + expect(sent(stale).input).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- new\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: do it\n"); + expect(stale.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, What changed; kept Why, Approvals; added none; dropped leading text (\"Draft intro from gh pr create, long enough to need truncati…\")\n"); + + const between = withBody(RECEIPT("aaaa") + "Old note under the receipt\n\n## What changed\n\n- old\n", { "/b": "## What changed\n\n- new\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], between)).toBe(EXIT.posted); + expect(sent(between).input).toBe(RECEIPT("aaaa") + "\n## What changed\n\n- new\n"); + expect(between.stderr()).toBe("gstack-post: pr-body merged by section: replaced What changed; kept ship receipt; added none; dropped trailing text (\"Old note under the receipt\")\n"); + + const own = withBody("Stale intro\n\n" + RECEIPT("aaaa") + "Stale note\n\n## Why\n\nold\n", { "/b": "Fresh intro\n\n" + RECEIPT("bbbb") + "Fresh note\n\n## Why\n\nnew\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], own)).toBe(EXIT.posted); + expect(sent(own).input).toBe("Fresh intro\n\n" + RECEIPT("bbbb") + "\nFresh note\n\n## Why\n\nnew\n"); + expect(mergeBodySections(sent(own).input!, "Fresh intro\n\n" + RECEIPT("bbbb") + "Fresh note\n\n## Why\n\nnew\n").body).toBe(sent(own).input!); + expect(own.stderr()).toBe("gstack-post: pr-body merged by section: replaced leading text, ship receipt, trailing text, Why; kept none; added none\n"); + }); + + test("receipt-only and text-only files keep the older merge; a repost is byte-identical; a callout in the preamble is never doubled", () => { + const lead = "Human intro\n\n" + CURRENT; + const receiptOnly = withBody(lead, { "/b": RECEIPT("bbbb") }); + expect(run(["pr-body", "5", "--body-file", "/b"], receiptOnly)).toBe(EXIT.posted); + expect(sent(receiptOnly).input).toBe(lead.replace(RECEIPT("aaaa"), RECEIPT("bbbb"))); + expect(receiptOnly.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt; kept leading text, What changed, Why, Approvals; added none\n"); + + const textOnly = withBody(RECEIPT("aaaa") + "Old note\n\n## Why\n\nx\n", { "/b": "New intro\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], textOnly)).toBe(EXIT.posted); + expect(sent(textOnly).input).toBe("New intro\n\n" + RECEIPT("aaaa") + "Old note\n\n## Why\n\nx\n"); + expect(textOnly.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept ship receipt, trailing text, Why; added leading text\n"); + + const composed = "Intro\n\n" + RECEIPT("bbbb") + "\n## Summary\n\nnew\n\n## Test plan\n\n- [x] bun test\n"; + const again = mergeBodySections(composed, composed); + expect(again.body).toBe(composed); + expect([again.added, again.dropped]).toEqual([[], []]); + + const callout = attributionCallout("Claude Opus on behalf of octocat"); + const withCallout = callout + RECEIPT("bbbb") + "\n## Summary\n\nnew\n"; + const reposted = mergeBodySections(callout + RECEIPT("aaaa") + "\n## Summary\n\nold\n", withCallout); + expect(reposted.body).toBe(withCallout); + expect(reposted.body.split("[!NOTE]")).toHaveLength(2); + expect(mergeBodySections(reposted.body, withCallout).body).toBe(withCallout); + }); + // Value: protects=--replace keeps the plain overwrite for a caller that composed the whole body, a current body with no H2 headings is kept as the leading - // section, an empty current body posts the new body byte for byte, the scan runs on the merged bytes, and an unreadable current body is exit 3 with - // the --replace hint rather than an overwrite; fails_when=--replace merges, headingless text is dropped, the merge changes a lone new body, a secret in a - // kept section passes the scan, or a failed read overwrites; why_new=first pr-body merge tests; seam=fake run; no_claim=real gh - test("--replace overwrites; a body without H2 headings is kept as a leading section; the merged bytes are what is scanned; a failed read refuses", () => { + // section by a file without sections and dropped (and named) by a file with sections (GST-412), an empty current body posts the new body byte for byte, + // the scan runs on the merged bytes, and an unreadable current body is exit 3 with the --replace hint rather than an overwrite; fails_when=--replace + // merges, headingless text is dropped by a receipt-only post or kept under new sections, the merge changes a lone new body, a secret in a kept section + // passes the scan, or a failed read overwrites; why_new=first pr-body merge tests; seam=fake run; no_claim=real gh + test("--replace overwrites; a body without H2 headings is kept by a file without sections and dropped by one with sections; the merged bytes are what is scanned; a failed read refuses", () => { const replaced = withBody(CURRENT, { "/b": "## What changed\n\nonly this\n" }); expect(run(["pr-body", "5", "--replace", "--body-file", "/b"], replaced)).toBe(EXIT.posted); expect(sent(replaced).input).toBe("## What changed\n\nonly this\n"); @@ -459,8 +508,12 @@ describe("gstack-post pr-body merges the PR body by section", () => { const plain = withBody("Just a paragraph.\nAnother line.", { "/b": "## Tests\n\n- one\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], plain)).toBe(EXIT.posted); - expect(sent(plain).input).toBe("Just a paragraph.\nAnother line.\n\n## Tests\n\n- one\n"); - expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept leading text; added Tests\n"); + expect(sent(plain).input).toBe("## Tests\n\n- one\n"); + expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept none; added Tests; dropped leading text (\"Just a paragraph.\")\n"); + const plainText = withBody("Just a paragraph.\nAnother line.", { "/b": RECEIPT("bbbb") }); + expect(run(["pr-body", "5", "--body-file", "/b"], plainText)).toBe(EXIT.posted); + expect(sent(plainText).input).toBe(RECEIPT("bbbb") + "\nJust a paragraph.\nAnother line."); + expect(plainText.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept leading text; added ship receipt\n"); const empty = withBody("", { "/b": "new body" }); expect(run(["pr-body", "6", "--body-file", "/b"], empty)).toBe(EXIT.posted); @@ -593,8 +646,8 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(fresh.added).toEqual(["leading text"]); const receipt = "```gstack-ship-receipt\n{\n \"head\": \"a\"\n}\n```\n"; const afterReceipt = mergeBodySections("Intro line\n\n## Summary\n\nold\n", receipt + "note\n\n## Summary\n\nnew\n"); - expect(afterReceipt.body).toContain("Intro line\n"); - expect(afterReceipt.body).toContain("note\n"); + expect(afterReceipt.body).toBe(receipt + "\nnote\n\n## Summary\n\nnew\n"); expect(afterReceipt.replaced).toEqual(["Summary"]); + expect(afterReceipt.dropped).toEqual(['leading text ("Intro line")']); }); });