diff --git a/changelog.d/gst-1227-post-preamble.md b/changelog.d/gst-1227-post-preamble.md new file mode 100644 index 000000000..4f50a51cf --- /dev/null +++ b/changelog.d/gst-1227-post-preamble.md @@ -0,0 +1,11 @@ +**`gstack-post pr-body` keeps a PR's opening paragraph and Capy badge, and a GitHub Actions run list no longer reads as a phone number (GST-1227).** Reported from truther#1174 on /ship 1.94.15. + +### Fixed +- **A /ship post keeps the text above the PR's first heading.** Since GST-412, a body file with a receipt above its first `## ` section dropped everything above the PR's first heading, so a /ship run on a Capy-opened PR deleted the opening paragraph and the `…` badge. That text is now kept byte for byte and the new receipt goes right under it, so the next /ship post doesn't read it as a stale note under the receipt. Opening lines the file brings of its own still replace it, and a stale note between the receipt and the first heading is still dropped and named on stderr. +- **A Capy badge at the end of the body stays there.** A `` … `` block that ends the body after its first receipt or heading is now a piece of its own, so rewriting the section above it no longer deletes it, new sections go above it, and a badge only the file brings goes last, once, so reposting the same file gives the same bytes. Every other marker block, a badge with text after it and an unclosed marker stay inside their section, as before. +- **Every run id in a `runs=/,/` list is exempt from `pii.phone.e164`.** Only the id right after `runs=` was exempt (and only at 11 digits), so the CI carry and run clear receipts `lib/run-evidence.ts` prints needed `--confirm` from their second run on. The scanner now exempts each 11- or 12-digit id of that list (`run=` too) and an id after an `actions/runs/` path with no leading `/` when it starts the line or follows whitespace, `(`, `[`, `<` or a quote. Each id needs its `/`, so `runs=4155550123`, `runs=4155550123/1`, `sales-actions/runs/4155550123` and `+1 415 555 0123` on the same line still report. See docs/troubleshooting.md#redact-run-id-as-phone. + +### Tests +- `test/gstack-post.test.ts`: "a full post keeps the opening text and a badge block above the first heading or at the end of the body byte for byte" and "run ids in a runs= list or an actions/runs/ path are not phone numbers; a phone number beside them still is" both fail on main. Four GST-412 tests that expected the opening text to be dropped now expect it kept. +- `test/gstack-post.test.ts`: "a marker block that is not a trailing Capy badge stays inside its section", "a trailing badge the file brings lands at the end once and reposts byte for byte", "a fenced heading inside a marker block and an unclosed badge marker change nothing about the sections" and "two marker blocks with the same name merge by their sections" pin the main-branch handling of every other marker block. +- `test/redact-context-exemptions.test.ts`: run ids in a `runs=` list or an `actions/runs/` path stay clean (5 cases, all failing on main), and 10 controls, `call +1 415 555 0123` and `sales-actions/runs/4155550123` among them, still report. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 382d7daf3..793bff5ff 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2720,6 +2720,16 @@ exact tokens it finds in these positions: - a 12-digit value right after `tree`, `gated_tree`, `head` or `predecessor` with `=` or a space (`tree=704992822996`, `tree 704992822996`); - an 11-digit id inside an `/actions/runs/` URL or as that link's text; +- any id inside an `actions/runs/` path, with or without a host, when + `actions/runs/` starts the line or follows whitespace, `/`, `(`, `[`, + `<` or a quote (`see actions/runs/41629861820`; `sales-actions/runs/…` + is not one), and every 11- or 12-digit id of a `runs=/` list + a CI carry or run clear prints (`runs=41629861819/1,41629861820/2`; + `run=` too). The scanner itself exempts both (`isCiRunIdentifier`), and + only that shape counts: each id needs its `/` of one to three + digits, so `runs=4155550123`, `runs=4155550123/1`, + `runs=+14155550123/1` and a phone number elsewhere on the line still + report (GST-1227); - an 11-digit id right after a run word on the same line, with an optional `:`, `#` or `=` between: `run`, `runs`, `screen`, `screens`, `gate`, `gates`, `backstop`, `bisect`, `rerun`, `dispatched`, `dispatch`, @@ -2763,20 +2773,30 @@ same-titled section (title compared case-insensitively), the 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 -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 +receipt block above its first `## ` section, as /ship composes it. Then any +lines between the current receipt and the first heading are replaced by the +file's own text under its receipt, or dropped when the file has none. The +opening text above the first receipt or heading (a paragraph, a bot's badge +such as `…`, or the whole +body when it has no `## ` heading) is kept byte for byte, and a new receipt +lands right under it; only opening lines the file brings of its own replace +it (GST-1227). A Capy badge that ends the body after its first receipt or +heading (`` through ``, outside +a code fence, with only blank lines after it) is a piece of its own: it stays +at the end when the section above it is rewritten, new sections go above it, +and a badge only the file brings goes last, once. Every other marker block, a +badge with text after it and an unclosed badge marker stay inside the section +they sit in. A partial post (`## ` sections 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; only -opening lines the file brings of its own replace the PR's opening lines. One stderr line names what was replaced, kept, added +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 -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. +by section: replaced ship receipt, What changed; kept leading text, Why; +added Tests; dropped trailing text ("Old note under the receipt")`). A body +file holding only its first section therefore no longer wipes the rest of the +PR, and a stale note no longer sits under a fresh receipt. To retire an old +intro, post the whole body with `--replace` or bring new opening lines. 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 diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index 1860170ac..25e0b9d2f 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -10,7 +10,8 @@ * 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 receipt above - * its first `## ` section replaces the text above the first heading; + * its first `## ` section replaces the text between the receipt and the + * first heading while the opening text above the receipt is kept; * `--replace` overwrites instead. * The scan runs on the merged bytes, which are the bytes sent. * @@ -159,7 +160,7 @@ function parseArgs(argv: string[]): Request { return req; } -/** One piece of a PR body: the leading text, a `## ` section, the ship-receipt block, or text after a receipt block. */ +/** One piece of a PR body: the leading text, a `## ` section, the ship-receipt block, text after a receipt block, or a Capy badge that ends the body. */ interface Section { key: string; label: string; text: string } /** @@ -174,26 +175,43 @@ interface Section { key: string; label: string; text: string } * nothing; a receipt fence that never closes makes the body from that line * on one section the merge never matches (`unclosed-receipt`), so it is kept * byte for byte instead of being read as a receipt or as replaceable text. + * A Capy badge (`` through ``, + * both outside a fence) that ends the body after its first receipt or + * heading, with only blank lines after it, is a piece of its own + * (`badge:capy-badge`), so it never leaves with the section above it + * (GST-1227). A badge anywhere else, an unclosed badge marker and every other + * marker block stay inside the section they sit in. */ function splitSections(body: string): Section[] { const out: Section[] = []; let fence: string | null = null; + let badgeAt = -1; + let inBadge = false; let plain = 0; - const open = (key: string, label: string) => out.push({ key, label, text: "" }); + const open = (key: string, label: string) => { badgeAt = -1; inBadge = false; out.push({ key, label, text: "" }); }; const openText = () => out.length === 0 ? open("text:lead", "leading text") : open(`text:${plain++}`, "trailing text"); const closes = (run: string) => new RegExp(`^ {0,3}${run[0] === "`" ? "`" : "~"}{${run.length},}\\s*$`).test(bare); let bare = ""; for (const line of body.split(/(?<=\n)/)) { bare = line.replace(/\r?\n$/, ""); const fenceOpen: RegExpExecArray | null = fence === null ? /^ {0,3}(`{3,}|~{3,})/.exec(bare) : null; + const outside = fence === null && !fenceOpen; if (fenceOpen && bare.trim() === "```" + RECEIPT_FENCE) { open("receipt", "ship receipt"); fence = "receipt"; } else if (fence === "receipt") { out.at(-1)!.text += line; if (closes("```")) { fence = null; openText(); } continue; } else if (fenceOpen) fence = fenceOpen[1]!; else if (fence && closes(fence)) fence = null; else if (fence === null && /^ {0,3}## /.test(bare)) { const title = bare.replace(/^ {0,3}## /, "").trim(); open(`h2:${title.toLowerCase()}`, title); } if (out.length === 0) openText(); + if (outside && /^ {0,3}/.test(bare)) { badgeAt = out.at(-1)!.text.length; inBadge = true; } + else if (inBadge && outside && //.test(bare)) inBadge = false; + else if (!inBadge && bare.trim() !== "") badgeAt = -1; out.at(-1)!.text += line; } + const last = out.at(-1); + if (last && badgeAt >= 0 && !inBadge && fence === null && last.key !== "text:lead") { + out.push({ key: "badge:capy-badge", label: "capy-badge block", text: last.text.slice(badgeAt) }); + last.text = last.text.slice(0, badgeAt); + } if (fence === "receipt") Object.assign(out.at(-1)!, { key: "unclosed-receipt", label: "unclosed receipt fence" }); return out.filter(s => s.text !== ""); } @@ -202,8 +220,10 @@ function splitSections(body: string): Section[] { * The body to send for `pr-body` without `--replace`: each section of the * file replaces the same-keyed section of the current body in place, current * sections the file does not rewrite are kept byte for byte, new `## ` - * sections are appended after them, and a new receipt or leading text goes - * first (where lib/ship-receipt.ts puts the receipt). Text outside sections + * sections are appended after them (above a Capy badge that ends the body; + * a badge only the file brings goes last), new leading text goes first, and a new receipt goes right after the + * current leading text, or first when there is none (where + * lib/ship-receipt.ts puts the receipt). Text outside sections * matches by role (the file's leading text replaces the leading text; text * after a receipt block never replaces an intro), and whitespace-only text in * the file is dropped, so reposting the body /ship composes (receipt, blank @@ -214,15 +234,17 @@ function splitSections(body: string): Section[] { * 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 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 + * /ship composes it) manages the text under the receipt (GST-412): every + * non-blank text section after a receipt block and before the current body's + * first heading is dropped, the file's own text after its receipt takes its + * place, right after the receipt, and 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. Any other file (`## ` sections with no receipt above them, or no - * `## ` section) merges as described above, so the current intro is kept. + * flags it, since the merged bytes no longer hold it). The current leading text + * (the opening paragraph and a bot's badge above the first receipt or heading) + * is kept byte for byte unless the file brings leading text of its own, which + * replaces it as in any merge (GST-1227). Any other file (`## ` sections with + * no receipt above them, or no `## ` section) merges as described above, so + * the text under the receipt is kept too. */ export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: string[] } { const now = splitSections(current); @@ -231,18 +253,13 @@ export function mergeBodySections(current: string, incoming: string): { body: st 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 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() !== ""); + return list.slice(0, end < 0 ? list.length : end).filter(s => s.key.startsWith("text:") && s.key !== "text:lead" && s.text.trim() !== ""); }; 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"); - 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 paired = new Set
([...stale.slice(0, ours.length), ...ours.slice(0, stale.length)]); const used = new Set
(ours); const replaced: string[] = [], kept: string[] = [], added: string[] = [], dropped: string[] = []; const merged: Section[] = []; @@ -260,10 +277,13 @@ export function mergeBodySections(current: string, incoming: string): { body: st } const fresh = next.filter(n => !used.has(n)); 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:")); + const badge = (s: Section) => s.key === "badge:capy-badge"; + const first = fresh.filter(n => !n.key.startsWith("h2:") && !badge(n)); 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 lead = merged[0]?.key === "text:lead" && merged[0].text.trim() ? 1 : 0; + const tail = merged.length && badge(merged.at(-1)!) ? merged.length - 1 : merged.length; + const all = [...merged.slice(0, lead), ...first, ...merged.slice(lead, tail), ...fresh.filter(n => n.key.startsWith("h2:")), ...merged.slice(tail), ...fresh.filter(badge)]; + for (const t of [...ours].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]; diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 5bac73526..f90de67dc 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -356,8 +356,14 @@ export function isNumericMetadataValue(span: string, match: RegExpExecArray): bo * (`bun run eval:pass-rates --run 37235771700`), and redacting it breaks that * command. Only the position decides: a run/job flag (`--run 37235771700`, * `--job=111534615007`), a `gh run view|watch|rerun|download|cancel` - * argument, an `actions/runs/` or `/job/` URL segment, or a - * `run_id` / `job_id` key. An 11-digit id (every GitHub Actions run id since + * argument, an `actions/runs/` or `/job/` URL segment (with or + * without a host: `actions/runs/` at the line start or after whitespace, + * `/`, `(`, `[`, `<` or a quote, so `sales-actions/runs/` is not one), + * a `run_id` / `job_id` key, or any 11- or 12-digit id of a + * `runs=/[,/…]` list (`run=` too), the receipt a + * CI carry or run clear prints (lib/run-evidence.ts runReceiptFields); the + * `/` of one to three digits after every id is what tells that list + * from a number (GST-1227). An 11-digit id (every GitHub Actions run id since * 2025) also counts right after a run word on the same line, the way a ship * receipt, a READY block or an approval record names one (`screen 40477638211 * GREEN`, `gate=40323989742`, `rerun passed 40287239206`, `run #19368712345`); @@ -369,7 +375,8 @@ export function isNumericMetadataValue(span: string, match: RegExpExecArray): bo * else still report. */ const CI_ID_POSITION_BEFORE = - /(?:--(?:run|job)(?:-id)?(?:[ \t]+|[ \t]*=[ \t]*)|\bgh[ \t]+run[ \t]+(?:view|watch|rerun|download|cancel)[ \t]+|\/actions\/runs\/|\/jobs?\/|(?:run|job)_id["']?[ \t]*[:=][ \t]*["']?)$/i; + /(?:--(?:run|job)(?:-id)?(?:[ \t]+|[ \t]*=[ \t]*)|\bgh[ \t]+run[ \t]+(?:view|watch|rerun|download|cancel)[ \t]+|(?:^|[\s\/(\[<"'])actions\/runs\/|\/jobs?\/|(?:run|job)_id["']?[ \t]*[:=][ \t]*["']?)$/i; +const CI_RUN_LIST = /(? m.index! < at && at + span.length <= m.index! + m[0].length); } /** diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index 72e6ecbf7..c9f6a5948 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -267,6 +267,28 @@ describe("gstack-post owns the redaction scan", () => { } }); + // Value: protects=the run list a CI carry or run clear prints (`runs=/,/`, lib/run-evidence.ts runReceiptFields) and a bare + // `actions/runs/` path post without a pii.phone.e164 confirmation, while a phone number in prose next to them still needs one (GST-1227); + // fails_when=every run id after the first in a `runs=` list, or an `actions/runs/` path with no leading slash, needs --confirm, or the exemption + // swallows +1 415 555 0123 on the same line; why_new=the run-word test covers only the id right after the word; seam=fake run; no_claim=real gh + test("run ids in a runs= list or an actions/runs/ path are not phone numbers; a phone number beside them still is", () => { + const clean = [ + "CI_CARRY: cleared pregate.json/remote_gate run tree=704992822996 tests=412 runs=41629861819/1,41629861820/2 profiles=free-tests,windows-smoke heads=2f9d11a0c3e4 verified=2026-11-03T18:00:00.000Z", + "carried run=41629861819/1,416298618201/3", + "logs at garrytan/gstack actions/runs/41629861819/attempts/2", + ]; + for (const body of clean) { + const f = fakeEnv({ files: { "/b": body + "\n" }, visibility: "public" }); + expect([body, run(["pr-comment", "12", "--body-file", "/b"], f)]).toEqual([body, EXIT.posted]); + } + for (const body of ["call +1 415 555 0123 about runs=41629861819/1,41629861820/2", "runs=+14155550123/1", "runs=1/4155550123", "runs=4155550123", "runs=4155550123/1", "sales-actions/runs/4155550123"]) { + const f = fakeEnv({ files: { "/b": body + "\n" }, visibility: "public" }); + expect([body, run(["pr-comment", "12", "--body-file", "/b"], f)]).toEqual([body, EXIT.confirm]); + expect(f.stdout()).toContain("RULE: pii.phone.e164 LINE: 1 PART: body"); + expect(f.posts()).toEqual([]); + } + }); + test("the title is scanned too, and the pusher's own email is not a finding", () => { const f = fakeEnv({ files: { "/t": "v1 fix for jane.roe@acme-corp.io", "/b": "body" } }); expect(run(["pr-create", "--base", "main", "--title-file", "/t", "--body-file", "/b"], f)).toBe(EXIT.confirm); @@ -446,16 +468,18 @@ 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 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; + // Value: protects=a full post (a receipt above its first heading, as /ship composes it) owns the text between the receipt and the first heading + // (GST-412): a stale note there is dropped and named on stderr with its first line, the file's own text under its receipt takes its place, the + // opening text above the receipt is kept unless the file brings its own (GST-1227), 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 note under the receipt after /ship reposts the + // sections, or drops the opening text the file does not rewrite; why_new=lead text was kept unless the file brought its own; seam=fake run; // no_claim=real gh - 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" }); + test("a full post replaces the text under the receipt, keeps the opening text it does not rewrite, and names each dropped piece", () => { + const intro = "Draft intro from gh pr create, long enough to need truncation in the summary line\nsecond line\n\n"; + const stale = withBody(intro + 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"); + expect(sent(stale).input).toBe(intro + 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 leading text, Why, Approvals; added none\n"); const between = withBody(RECEIPT("aaaa") + "Old note under the receipt\n\n## What changed\n\n- old\n", { "/b": RECEIPT("bbbb") + "## What changed\n\n- new\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], between)).toBe(EXIT.posted); @@ -469,6 +493,79 @@ 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 full post keeps the PR's opening text (a Capy badge block and a paragraph above the first heading) byte for byte, lands a new + // receipt right after it, and keeps a badge block that ends the body after the section above it is rewritten, on the first post and on a repost + // (GST-1227); fails_when=pr-body drops the opening paragraph and the badge (truther#1174), a new receipt lands above them so the next post drops them + // as a note under the receipt, or the trailing badge leaves with the section it follows; why_new=the GST-412 tests read every opening line as a + // stale intro; seam=fake run; no_claim=real gh + test("a full post keeps the opening text and a badge block above the first heading or at the end of the body byte for byte", () => { + const BADGE = "\n[![Capy](https://capy.ai/badge.svg)](https://capy.ai)\n\n"; + const PREAMBLE = BADGE + "\nThis PR stops the login retry from looping.\n\n"; + const first = withBody(PREAMBLE + "## What changed\n\n- the old summary\n\n## Why\n\nBecause.\n", { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], first)).toBe(EXIT.posted); + expect(sent(first).input).toBe(PREAMBLE + RECEIPT("bbbb") + "\n## What changed\n\n- new\n\n## Why\n\nBecause.\n"); + expect(first.stderr()).toBe("gstack-post: pr-body merged by section: replaced What changed; kept leading text, Why; added ship receipt\n"); + + const again = withBody(sent(first).input!, { "/b": RECEIPT("cccc") + "\n## What changed\n\n- newer\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], again)).toBe(EXIT.posted); + expect(sent(again).input).toBe(PREAMBLE + RECEIPT("cccc") + "\n## What changed\n\n- newer\n\n## Why\n\nBecause.\n"); + expect(again.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, What changed; kept leading text, Why; added none\n"); + + const trailing = withBody(CURRENT + "\n" + BADGE, { "/b": RECEIPT("bbbb") + "\n## Approvals\n\n- owner: ship it\n\n## Tests\n\n- one\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], trailing)).toBe(EXIT.posted); + expect(sent(trailing).input).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- the old summary\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: ship it\n\n## Tests\n\n- one\n\n" + BADGE); + expect(trailing.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, Approvals; kept What changed, Why, capy-badge block; added Tests\n"); + }); + + const BADGE = "\n[![Capy](https://capy.ai/badge.svg)](https://capy.ai)\n\n"; + const SHOTS = (alt: string) => "\n![" + alt + "](u)\n\n"; + + // Value: protects=a marker block other than a Capy badge that ends the body stays inside its section exactly as before GST-1227, so a rewrite + // replaces it with the section, a new section lands after it, text after a badge in mid-section never pairs with the note under a receipt, + // and the file's own block stays in its section; fails_when=the old block and the stale lines after it survive a rewrite, a shots block is + // split from its section, "Fixes #12" is dropped, or the file's block moves above the first heading; why_new=GST-1227 review P1; seam=none + // (mergeBodySections); no_claim=the gh round trip + test("a marker block that is not a trailing Capy badge stays inside its section", () => { + const mid = mergeBodySections(RECEIPT("aaaa") + "\n## What changed\n\n- old\n" + SHOTS("a") + "stale line\n\n## Why\n\nBecause.\n", RECEIPT("bbbb") + "\n## What changed\n\n- new\n"); + expect([mid.body, mid.kept]).toEqual([RECEIPT("bbbb") + "\n## What changed\n\n- new\n\n## Why\n\nBecause.\n", ["Why"]]); + const last = mergeBodySections(CURRENT + SHOTS("a"), RECEIPT("bbbb") + "\n## Tests\n\n- one\n"); + expect(last.body).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- the old summary\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: do it\n" + SHOTS("a") + "\n## Tests\n\n- one\n"); + const collide = mergeBodySections("Intro\n\n## What changed\n\n- x\n" + BADGE + "Fixes #12\n", RECEIPT("bbbb") + "Review-loop exception: foo\n"); + expect(collide.body).toBe("Intro\n\n" + RECEIPT("bbbb") + "\nReview-loop exception: foo\n\n## What changed\n\n- x\n" + BADGE + "Fixes #12\n"); + const own = mergeBodySections(CURRENT, RECEIPT("bbbb") + "\n## What changed\n\n" + SHOTS("n") + "after the shots\n"); + expect(own.body).toBe(RECEIPT("bbbb") + "\n## What changed\n\n" + SHOTS("n") + "after the shots\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: do it\n"); + }); + + // Value: protects=a trailing badge the file brings into a body without one lands at the end once and a repost is byte-identical; fails_when=each + // identical post adds another badge or moves it above the sections; why_new=GST-1227 review P2; seam=none (mergeBodySections); no_claim=the gh round trip + test("a trailing badge the file brings lands at the end once and reposts byte for byte", () => { + const file = RECEIPT("bbbb") + "\n## Tests\n\n- one\n\n" + BADGE; + const once = mergeBodySections(CURRENT, file).body; + expect(once).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- the old summary\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: do it\n\n## Tests\n\n- one\n\n" + BADGE); + const twice = mergeBodySections(once, file).body; + expect(twice).toBe(once); + expect(mergeBodySections(twice, file).body).toBe(once); + }); + + // Value: protects=fences inside a marker block keep their meaning (a fenced `## x` opens no section), a badge inside a fence is text, and an + // unclosed badge marker swallows nothing; fails_when=the fenced `## x` ends the block and opens a section, or the unclosed marker keeps the + // rest of the old section after a rewrite; why_new=GST-1227 review P2; seam=none (mergeBodySections); no_claim=the gh round trip + test("a fenced heading inside a marker block and an unclosed badge marker change nothing about the sections", () => { + const fenced = mergeBodySections("## What changed\n\n\n```md\n## x\n```\n\n\n## Why\n\nold\n", "## Why\n\nnew\n"); + expect([fenced.body, fenced.replaced, fenced.kept]).toEqual(["## What changed\n\n\n```md\n## x\n```\n\n\n## Why\n\nnew\n", ["Why"], ["What changed"]]); + const example = mergeBodySections("## Docs\n\n```md\n" + BADGE + "```\n", "## Docs\n\nnew\n"); + expect([example.body, example.kept]).toEqual(["## Docs\n\nnew\n", []]); + const unclosed = mergeBodySections(RECEIPT("aaaa") + "\n## Why\n\nBecause.\n\n## Approvals\n\n- x\n\nmore approvals\n", RECEIPT("bbbb") + "\n## Approvals\n\n- y\n"); + expect([unclosed.body, unclosed.kept]).toEqual([RECEIPT("bbbb") + "\n## Why\n\nBecause.\n\n## Approvals\n\n- y\n", ["Why"]]); + }); + + // Value: protects=two marker blocks with one name merge by their sections; fails_when=the merge refuses them as a duplicated section; + // why_new=GST-1227 review; seam=none (mergeBodySections); no_claim=the gh round trip + test("two marker blocks with the same name merge by their sections", () => { + const two = mergeBodySections("## A\n\n" + SHOTS("a") + "\n## B\n\n" + SHOTS("b"), "## A\n\n" + SHOTS("a2")); + expect([two.body, two.replaced, two.kept]).toEqual(["## A\n\n" + SHOTS("a2") + "\n## B\n\n" + SHOTS("b"), ["A"], ["B"]]); + }); + // 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, @@ -489,10 +586,10 @@ describe("gstack-post pr-body merges the PR body by section", () => { // 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); // fails_when=a stale intro holding a credential is echoed to stderr while it leaves the PR; why_new=the excerpt is new in GST-412; seam=fake run; no_claim=real gh test("a dropped piece whose first line the redaction scan flags is named without its text", () => { - const leak = withBody(`Deploy key ${AWS} for the demo\n\n` + CURRENT, { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" }); + const leak = withBody(RECEIPT("aaaa") + `Deploy key ${AWS} for the demo\n\n## What changed\n\n- old\n`, { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], leak)).toBe(EXIT.posted); expect(leak.stderr()).not.toContain(AWS); - expect(leak.stderr()).toContain("dropped leading text (first line withheld: the redaction scan flagged it)"); + expect(leak.stderr()).toContain("dropped trailing text (first line withheld: the redaction scan flagged it)"); }); test("receipt-only and text-only files keep the older merge; a repost is byte-identical; a callout in the preamble is never doubled", () => { @@ -521,11 +618,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 full post (GST-412), an empty current body posts the new body byte for byte, + // section, above a new receipt, by a file without sections and by a full post (GST-1227), 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 a receipt and dropped by a full post; the merged bytes are what is scanned; a failed read refuses", () => { + test("--replace overwrites; a body without H2 headings is kept above a new receipt by any file; 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"); @@ -534,11 +631,11 @@ describe("gstack-post pr-body merges the PR body by section", () => { const plain = withBody("Just a paragraph.\nAnother line.", { "/b": RECEIPT("bbbb") + "\n## Tests\n\n- one\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], plain)).toBe(EXIT.posted); - expect(sent(plain).input).toBe(RECEIPT("bbbb") + "\n## Tests\n\n- one\n"); - expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept none; added ship receipt, Tests; dropped leading text (\"Just a paragraph.\")\n"); + expect(sent(plain).input).toBe("Just a paragraph.\nAnother line.\n\n" + RECEIPT("bbbb") + "\n## Tests\n\n- one\n"); + expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept leading text; added ship receipt, Tests\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(sent(plainText).input).toBe("Just a paragraph.\nAnother line.\n\n" + RECEIPT("bbbb")); 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" }); @@ -672,8 +769,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).toBe(receipt + "\nnote\n\n## Summary\n\nnew\n"); + expect(afterReceipt.body).toBe("Intro line\n\n" + receipt + "\nnote\n\n## Summary\n\nnew\n"); expect(afterReceipt.replaced).toEqual(["Summary"]); - expect(afterReceipt.dropped).toEqual(['leading text ("Intro line")']); + expect(afterReceipt.dropped).toEqual([]); }); }); diff --git a/test/redact-context-exemptions.test.ts b/test/redact-context-exemptions.test.ts index a5134b7f3..8c8383a1b 100644 --- a/test/redact-context-exemptions.test.ts +++ b/test/redact-context-exemptions.test.ts @@ -296,3 +296,30 @@ describe("pii.phone.e164: GitHub Actions run and job ids in their id positions", ], ); }); + +describe("pii.phone.e164: run ids in a runs= list or an actions/runs/ path (GST-1227)", () => { + table( + "pii.phone.e164", + "MEDIUM", + [ + // The run list a CI carry or run clear prints (lib/run-evidence.ts runReceiptFields): every id, not only the first. + "CI_CARRY: cleared pregate.json/remote_gate run tests=412 runs=41629861819/1,41629861820/2 profiles=free-tests heads=2f9d11a0c3e4", + '{"remote_gate": "carried runs=41629861819/1,41629861820/1,41629861821/3 profiles=free-tests,windows-smoke"}', + "run=41629861819/1,416298618201/1", + "see actions/runs/41629861820 for the log", + "garrytan/gstack actions/runs/4162986182/attempts/2", + ], + [ + "call +1 415 555 0123 tomorrow", + "call +1 415 555 0123 about runs=41629861819/1,41629861820/2", + "runs=+14155550123/1", + "runs=1/4155550123", + "runs=4155550123", + "reruns=4155550123/1", + "factions/runs/4155550123", + "sales-actions/runs/4155550123", + "runs=4162986182/1", + "runs=1234567890123/1", + ], + ); +});