diff --git a/changelog.d/gst-1227-post-preamble.md b/changelog.d/gst-1227-post-preamble.md new file mode 100644 index 000000000..8cdb694d3 --- /dev/null +++ b/changelog.d/gst-1227-post-preamble.md @@ -0,0 +1,10 @@ +**`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 badge at the end of the body stays there.** A `` … `` block after the first receipt or heading is now a piece of its own, so rewriting the section above it no longer deletes it, and new sections go above it. +- **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 id of that list (`run=` too) and an id after a bare `actions/runs/` path with no leading `/`. Each id needs its `/`, so `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/redact-context-exemptions.test.ts`: run ids in a `runs=` list or an `actions/runs/` path stay clean (6 cases fail on main), and 7 controls, `call +1 415 555 0123` among them, still report. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f17f55959..8893ad8d1 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2720,6 +2720,14 @@ 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 or a + leading `/` (`see actions/runs/41629861820`), and every 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=+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 +2771,27 @@ 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 marker block (`` through +``) after the first receipt or heading is a piece of its +own, so a badge at the end of the body stays there when the section above it +is rewritten, and new sections are added above it. 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..bfc213ea2 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, a marker block, or text after a receipt or marker block. */ interface Section { key: string; label: string; text: string } /** @@ -174,25 +175,38 @@ 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. + * After the first receipt or heading, a `` line outside + * a fence opens a marker block (a bot's badge, such as Capy's + * `…`) that runs through its + * `` line, or up to the next heading when it never + * closes, so a badge at the end of the body never leaves with the section + * above it (GST-1227). A marker block in the leading text stays part of it. */ function splitSections(body: string): Section[] { const out: Section[] = []; let fence: string | null = null; + let block: string | null = null; let plain = 0; const open = (key: string, label: string) => 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 = ""; + const endsBlock = () => { if (block !== null && bare.includes(``)) { block = null; openText(); } }; for (const line of body.split(/(?<=\n)/)) { bare = line.replace(/\r?\n$/, ""); + if (block !== null && /^ {0,3}## /.test(bare)) block = null; + if (block !== null) { out.at(-1)!.text += line; endsBlock(); continue; } const fenceOpen: RegExpExecArray | null = fence === null ? /^ {0,3}(`{3,}|~{3,})/.exec(bare) : null; + const marker = fence === null ? /^ {0,3}/.exec(bare) : null; 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); } + else if (marker && out.some(s => s.key === "receipt" || s.key.startsWith("h2:"))) { open(`block:${marker[1]}`, `${marker[1]} block`); block = marker[1]!; } if (out.length === 0) openText(); out.at(-1)!.text += line; + endsBlock(); } if (fence === "receipt") Object.assign(out.at(-1)!, { key: "unclosed-receipt", label: "unclosed receipt fence" }); return out.filter(s => s.text !== ""); @@ -202,8 +216,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 marker block that ends the + * body), 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 +230,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 +249,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[] = []; @@ -262,8 +275,12 @@ export function mergeBodySections(current: string, incoming: string): { body: st 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, 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; + let tail = merged.length; + while (tail > lead && (merged[tail - 1]!.key.startsWith("block:") || !merged[tail - 1]!.text.trim())) tail--; + if (!merged.slice(tail).some(s => s.key.startsWith("block:"))) tail = merged.length; + const all = [...merged.slice(0, lead), ...first, ...merged.slice(lead, tail), ...fresh.filter(n => n.key.startsWith("h2:")), ...merged.slice(tail)]; + 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..c6f90557a 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -356,8 +356,12 @@ 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 or a leading `/`), a `run_id` / `job_id` key, or any 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 +373,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]+|\bactions\/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..ad7ec59a3 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,4162986182/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"]) { + 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,30 @@ 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"); + }); + // 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 +537,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 +569,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 +582,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 +720,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..8c635dcdd 100644 --- a/test/redact-context-exemptions.test.ts +++ b/test/redact-context-exemptions.test.ts @@ -296,3 +296,28 @@ 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,4162986182/1", + "runs=4162986182/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", + ], + ); +});