diff --git a/changelog.d/gsta195-prbody-intro.md b/changelog.d/gsta195-prbody-intro.md index e7ba32451..2c22e458c 100644 --- a/changelog.d/gsta195-prbody-intro.md +++ b/changelog.d/gsta195-prbody-intro.md @@ -1,7 +1,7 @@ **A partial `gstack-post pr-body` post keeps the PR's intro.** Since GST-412, any body file with a `## ` section replaced the text above the PR's first heading, so a lane that posted only `## Screenshots` deleted a human intro or a `Review-loop exception:` line under the receipt (GStack Minder: "a partial pr-body post must keep the human intro"). ### Fixed -- **Only a full post owns the text above the first heading.** A file with a `## ` section and a receipt block or its own lead text, as /ship composes it, still replaces stale lead text and stale notes under the receipt. A file with `## ` sections only keeps them. +- **Only a full post owns the text above the first heading.** A file with a receipt block above its first `## ` section, as /ship composes it, still replaces stale lead text and stale notes under the receipt. Any other file keeps them; its own lead text still replaces the PR's lead text. ### Tests -- `test/gstack-post.test.ts`: a partial post keeps the intro and the note under the receipt; the full-post cases now carry a receipt, as /ship's body does. +- `test/gstack-post.test.ts`: a partial post keeps the intro and the note under the receipt, even with a caption of its own or a receipt further down; the full-post cases now carry a receipt, as /ship's body does. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1b940f5a6..28861c2a6 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2518,14 +2518,13 @@ 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, and new sections are appended after them. The text above the first -`## ` heading belongs to the file when the file is a full post: a `## ` -section plus a receipt block or text of its own above its first heading, as -/ship composes it. Then the current body's text there (the opening lines, and +`## ` heading belongs to the file when the file is a full post: a +receipt block above its first `## ` section, as /ship composes it. Then 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 partial post (`## ` sections -only, such as a lane posting `## Screenshots`) and a file with no `## ` -section (a receipt-only or text-only post) leave that text alone, so a human +with no receipt above them, such as a lane posting `## Screenshots`) and a +file with no `## ` section (a receipt-only or text-only post) leave that text alone, so a human intro or a `Review-loop exception:` line under the receipt survives. One stderr line names what was replaced, kept, added and dropped, a dropped piece with its first line, or with that line withheld when the redaction scan flags it (`gstack-post: pr-body merged diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index beae0132d..1860170ac 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -9,9 +9,9 @@ * `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, and a file with a `## ` section - * plus a receipt or its own lead text replaces the text above the first - * heading; `--replace` overwrites instead. + * rewrite are kept, new ones are appended, and a file with a receipt above + * its first `## ` 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; @@ -213,16 +213,16 @@ 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 full post (a file with at least one `## ` section and a receipt block or - * text of its own above its first heading) manages the preamble (GST-412): every + * A full post (a file with a receipt block above its first `## ` section, as + * /ship composes it) 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 (withheld when the redaction scan * flags it, since the merged bytes no longer hold it), and the receipt keeps the rules - * above. A partial post (`## ` sections only) and a file without a `## ` - * section merge as described above, so the current intro is kept. + * above. Any other file (`## ` sections with no receipt above them, or no + * `## ` section) merges as described above, so the current intro is kept. */ export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: string[] } { const now = splitSections(current); @@ -233,7 +233,8 @@ export function mergeBodySections(current: string, incoming: string): { body: st 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 managed = next.some(n => n.key.startsWith("h2:")) && (next.some(n => n.key === "receipt") || preamble(next).length > 0); + const firstHeading = next.findIndex(n => n.key.startsWith("h2:")); + const managed = firstHeading > 0 && next.slice(0, firstHeading).some(n => n.key === "receipt"); const stale = managed ? preamble(now) : []; const ours = managed ? preamble(next) : []; const role = (s: Section) => (s.key === "text:lead" ? "lead" : "trailing"); diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index 96e0acdc0..72e6ecbf7 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -446,12 +446,12 @@ 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 + // Value: protects=a full post (a receipt above its first heading, as /ship composes it) 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", () => { + test("a full post 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"); @@ -469,8 +469,8 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(own.stderr()).toBe("gstack-post: pr-body merged by section: replaced leading text, ship receipt, trailing text, Why; kept none; added none\n"); }); - // Value: protects=a partial post (`## ` sections only, no receipt and no text of its own above its first heading) keeps the human intro and - // the note under the receipt, the same as a full post that brings no replacement for them; fails_when=a lane posting only `## Screenshots` + // Value: protects=a partial post (no receipt above its first `## ` heading, even with a caption of its own or a receipt further down) keeps + // the human intro and the note under the receipt; fails_when=a lane posting only `## Screenshots` // deletes the intro or a Review-loop exception line; why_new=GST-412 made any file with a section own the preamble (GstackMinder, // 2026-10-31: "a partial pr-body post must keep the human intro"); seam=fake run; no_claim=real gh test("a partial post keeps the current intro and the note under the receipt", () => { @@ -478,6 +478,12 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(run(["pr-body", "5", "--body-file", "/b"], partial)).toBe(EXIT.posted); expect(sent(partial).input).toBe("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n\n## Screenshots\n\n![shot](u)\n"); expect(partial.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept leading text, ship receipt, trailing text, What changed; added Screenshots\n"); + + for (const file of ["Screenshots below.\n\n## Screenshots\n\n![shot](u)\n", "## Screenshots\n\n![shot](u)\n\n" + RECEIPT("bbbb")]) { + const merged = mergeBodySections("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n", file); + expect(merged.body).toContain("Review-loop exception: accepted by owner"); + expect(merged.dropped).toEqual([]); + } }); // Value: protects=the dropped-piece excerpt on stderr never repeats a secret the merge removed from the body (the scan only reads the merged bytes); @@ -515,11 +521,11 @@ describe("gstack-post pr-body merges the PR body by section", () => { }); // 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 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, + // section by a file without sections and dropped (and named) by a full post (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", () => { + test("--replace overwrites; a body without H2 headings is kept by a file without a receipt and dropped by a full post; 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");