From bd6f5ca705296465daabb03b4a2d84908e537260 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:00:57 -0400 Subject: [PATCH 1/2] fix(pr-merge): wrap the refusal comment's reasons to 100 columns enable-auto-merge's refusal comment put each reason on one fenced line, which GitHub scrolls sideways (up to 347 columns measured). Reasons now go through address-feedback's quote(): wrapped at a space to 100 columns, continuations indented two spaces, more than 20 lines folded into
. The marker lines and the head line are unchanged. refusalReasons() joins wrapped lines back and also accepts a comment posted in the old one-line format; personReasons, holdPathOnly and clear-escalations' nativeRefusal read through it, and refusalWrites treats an old-format comment with the same reason as unchanged. Co-Authored-By: Claude Opus 5.5 --- .../fix-refusal-comment-wrap-3b9d51e7.md | 15 +++ .../scripts/pr-merge/clear-escalations.mjs | 8 +- .../pr-merge/clear-escalations.test.mjs | 33 ++++++ .../scripts/pr-merge/enable-auto-merge.mjs | 59 +++++++--- .../pr-merge/enable-auto-merge.test.mjs | 108 ++++++++++++++++-- .../scripts/pr-unblock/address-feedback.mjs | 40 +++++++ .github/scripts/pr-unblock/verify-notes.mjs | 41 +------ scripts/ci-hold-human-gated.test.sh | 11 ++ 8 files changed, 248 insertions(+), 67 deletions(-) create mode 100644 .changes/pr-pipeline/fix-refusal-comment-wrap-3b9d51e7.md diff --git a/.changes/pr-pipeline/fix-refusal-comment-wrap-3b9d51e7.md b/.changes/pr-pipeline/fix-refusal-comment-wrap-3b9d51e7.md new file mode 100644 index 0000000000..8d04f1cf8a --- /dev/null +++ b/.changes/pr-pipeline/fix-refusal-comment-wrap-3b9d51e7.md @@ -0,0 +1,15 @@ +--- +bump: patch +--- + +### Fixed + +- enable-auto-merge's refusal comment wraps each reason line to 100 columns at a space, its + continuations indented two spaces, and folds more than 20 lines into a `
` block, so + GitHub no longer scrolls a long reason sideways. The marker lines and the line naming the head + are unchanged. Every reader joins a wrapped reason back into one line and still reads a comment + posted before the change, and a refusal whose reason is unchanged writes nothing new, so a + person-removed label stays off. clear-escalations reads the whole reason, so a person's code + after a wrapped "; " still keeps the label. +- `quote` and `wrapLine` move from verify-notes.mjs into address-feedback.mjs; verify-notes' + comments are unchanged. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 03a718e28b..8794363d26 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -191,7 +191,7 @@ import { } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; import { LANES_APP_LOGIN as DEFAULT_BOT, laneChange, REVIEW_LANES } from "./merge-holds.mjs"; -import { pythonVerify, READER_HOLDS, REFUSAL_MARKER, refusalBody, rotated, verifyHold } from "./enable-auto-merge.mjs"; +import { pythonVerify, READER_HOLDS, REFUSAL_MARKER, refusalReasons, rotated, verifyHold } from "./enable-auto-merge.mjs"; const REASON = /^- Reason: `([a-z-]+)`$/m; const clearedMarker = (reason, head) => ``; @@ -673,9 +673,9 @@ function nativeRefusal(appComments, merged, label, head) { if (!(Date.parse(newest.updated_at) <= Date.parse(newest.created_at))) { return null; } - const reason = lines(newest.body).slice(5, -1).join("\n"); - const kind = Object.keys(READER_HOLDS).find((key) => reason.startsWith(READER_HOLDS[key])); - return kind !== undefined && refusalBody(true, reason, head, label) === newest.body ? { kind, reason } : null; + const reason = refusalReasons(newest.body, true, head, label)?.join("\n"); + const kind = reason === undefined ? undefined : Object.keys(READER_HOLDS).find((key) => reason.startsWith(READER_HOLDS[key])); + return kind !== undefined ? { kind, reason } : null; } // Whether the reader's `evidence` at `head` lifts the `kind` refusal, whose diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index afb7828305..4090b3c074 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -780,6 +780,39 @@ test("enable-auto-merge's label at the current head comes off once the gate's re assert.deepEqual(removal, DELETE); }); +// The person refusal written out by hand, its reason lines as given: one line +// per reason before reasons wrapped, or a reason wrapped at 100 columns with +// two-space continuations. LONG_UNBOUND's ";" is its 99th column (the risk +// prefix is 44, then 17 + 37 + 1), so the wrap cuts right after "; " and the +// lane code after it is on the next line. +const handRefusal = (reasonLines, head = HEAD) => + [ + "", + "", + `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${head}\`. A person must clear this reason, so the pr-merge lane added the \`needs-human\` label: fix the reason, then remove the label, and the next scheduled run judges it again.`, + "", + "```text", + ...reasonLines, + "```", + ].join("\n"); +const LONG_UNBOUND = `pr-merge's gate holds it on its risk score (evidence-unbound:${"a".repeat(37)}; files-incomplete)`; +const LONG_SECURITY = `pr-merge's gate holds it on its risk score (evidence-unbound:${"a".repeat(37)}; security-finding: 1 open CRITICAL finding)`; +const wrapAfterSemicolon = (reason) => [reason.slice(0, 99), ` ${reason.slice(100)}`]; + +test("enable-auto-merge's long refusal comes off in the old one-line format and the wrapped one", async () => { + for (const body of [handRefusal([LONG_UNBOUND]), handRefusal(wrapAfterSemicolon(LONG_UNBOUND))]) { + const w = world({ timeline: mergeEscalated(body), author: BOT, evidence: clean }); + const { logs } = await run(w, NATIVE); + assert.deepEqual(w.writes().at(-1), DELETE, `${body}\n${logs.join("\n")}`); + } + // A person's code on the wrapped line still keeps the label. + for (const body of [handRefusal([LONG_SECURITY]), handRefusal(wrapAfterSemicolon(LONG_SECURITY))]) { + const w = world({ timeline: mergeEscalated(body), author: BOT, evidence: clean }); + await run(w, NATIVE); + assert.deepEqual(w.writes(), [], body); + } +}); + test("enable-auto-merge's label comes off when the reader now reads a refusal the lanes clear for the same cause", async () => { for (const [reason, evidence] of [ // A lane-class verifier label: any verdict the lanes clear, read whole. diff --git a/.github/scripts/pr-merge/enable-auto-merge.mjs b/.github/scripts/pr-merge/enable-auto-merge.mjs index 8250023c90..9fdb734a20 100755 --- a/.github/scripts/pr-merge/enable-auto-merge.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.mjs @@ -206,7 +206,7 @@ import { pathToFileURL } from "node:url"; import { createGitHub, errorLabel, GitHubError, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { rateLimitLatch } from "../../actions/log-api-budget/rate-guard.mjs"; -import { humanGatedLabelFrom } from "../pr-unblock/address-feedback.mjs"; +import { humanGatedLabelFrom, quote } from "../pr-unblock/address-feedback.mjs"; import { lines as bodyLines, foreignMarker, @@ -670,16 +670,30 @@ export const personReason = (reasons) => [...new Set(reasons.map(String))].sort( const partialLine = (head) => ``; export const isPartial = (body, head) => String(body ?? "").split("\n").at(-1) === partialLine(head); -// The reason lines of a person's refusal comment `body` at `head` as -// refusalBody renders them, or null when it is not one. -function personReasons(body, head, label) { +// The reason lines of a refusal comment `body` at `head` (a person's when +// `person`), each whole and as fenced renders it, when refusalBody renders it +// from them with `since`, `partial` and `records`, or rendered it before +// reasons wrapped (one fence line per reason); else null. A wrapped line's +// continuations start with two spaces, which no fenced line does. +export function refusalReasons(body, person, head, label, { since = null, partial = false, records = [] } = {}) { const lines = String(body ?? "").split("\n"); - const partial = isPartial(body, head); const open = lines.indexOf("```text"); - const reasons = open < 0 ? [] : lines.slice(open + 1, lines.indexOf("```", open + 1)); - return open >= 0 && refusalBody(true, reasons.join("\n"), head, label, null, partial, recordLines(body)) === body ? reasons : null; + const close = lines.indexOf("```", open + 1); + if (open < 0 || close < 0) { + return null; + } + const raw = lines.slice(open + 1, close); + const reasons = raw.reduce((out, line) => (line.startsWith(" ") && out.length > 0 ? [...out.slice(0, -1), `${out.at(-1)} ${line.slice(2)}`] : [...out, line]), []); + if (refusalBody(person, reasons.join("\n"), head, label, since, partial, records) === body) { + return reasons; + } + return renderRefusal(unwrappedFence, person, raw.join("\n"), head, label, since, partial, records) === body ? fenced(raw.join("\n")).split("\n") : null; } +// The reason lines of a person's refusal comment `body` at `head`, or null +// when it is not one. +const personReasons = (body, head, label) => refusalReasons(body, true, head, label, { partial: isPartial(body, head), records: recordLines(body) }); + // A person's refusal at `head` whose every reason is a hold path // (merge-holds.pathHoldKind's "it changes , a hold path"): the one // escalation a trusted person's approval of the head lifts, so the lane may @@ -802,8 +816,19 @@ async function liftBlocked(github, repo, number, options) { return liftBlocker({ timeline, reviewComments, reviews, ...options }); } -// A reason as the comment's fenced block carries it. -const fenced = (reason) => String(reason).replaceAll(/[\u0000-\u0008\u000b-\u001f\u007f]/g, "?").replaceAll("```", "'''"); +// A reason as the comment carried it before reasons wrapped. +const unfolded = (reason) => String(reason).replaceAll(/[\u0000-\u0008\u000b-\u001f\u007f]/g, "?").replaceAll("```", "'''"); +// A reason as the comment's fenced block carries it: each line trimmed, its +// whitespace runs one space, so a wrap at a space joins back exactly. +const fenced = (reason) => + unfolded(reason) + .split("\n") + .map((line) => line.replaceAll(/\s+/g, " ").trim()) + .join("\n"); +// The fenced block: each reason line wrapped (address-feedback quote), or one +// fence line per reason as before reasons wrapped. +const wrappedFence = (reason) => quote(fenced(reason).split("\n"), (count) => `Its reasons (${count} lines)`); +const unwrappedFence = (reason) => ["```text", unfolded(reason), "```"].join("\n"); // A whole-review request's record line (babysit_whole_review.py RECORD_RE), // kept on every rewrite of the refusal comment, a person's included, since it @@ -826,7 +851,10 @@ export const recordLines = (body) => { }; // `records`: the comment's whole-review record lines, before its last. -export function refusalBody(person, reason, head, label, since = null, partial = false, records = []) { +export const refusalBody = (person, reason, head, label, since = null, partial = false, records = []) => + renderRefusal(wrappedFence, person, reason, head, label, since, partial, records); + +function renderRefusal(fence, person, reason, head, label, since, partial, records) { const who = person ? `A person must clear this reason, so the pr-merge lane added the \`${label}\` label: fix the reason, then remove the label, and the next scheduled run judges it again.` : "The lanes clear this reason on their own; the next scheduled run judges it again."; @@ -835,9 +863,7 @@ export function refusalBody(person, reason, head, label, since = null, partial = REFUSAL_MARKER, `Unattended merge: native auto-merge leaves this pull request unarmed at head \`${head}\`. ${who}`, "", - "```text", - fenced(reason), - "```", + fence(reason), ...(records.length === 0 ? [] : ["", ...records]), ...(person || since === null ? [] : [laneSince(head, since)]), ...(person && partial ? [partialLine(head)] : []), @@ -896,6 +922,11 @@ export function refusalWrites(pull, comments, { clears, reason, comment, partial const lane = refusalBody(false, reason, pull.headRefOid, label, since, false, records); const person = refusalBody(true, reason, pull.headRefOid, label, null, partial && !answered, records); const body = clears === "person" ? person : lane; + // The same comment as written before reasons wrapped, which says this too. + const unwrapped = + clears === "person" + ? renderRefusal(unwrappedFence, true, reason, pull.headRefOid, label, null, partial && !answered, records) + : renderRefusal(unwrappedFence, false, reason, pull.headRefOid, label, since, false, records); const carried = (pull.labels?.nodes ?? []).some((node) => String(node?.name).toLowerCase() === String(label).toLowerCase()); // The verifier's answer adding nothing to a partial refusal at the head: // its mark comes off in place, notifying no one of what it already said. @@ -907,7 +938,7 @@ export function refusalWrites(pull, comments, { clears, reason, comment, partial // goes back on unless a person removed it after the comment last changed, // which the caller reads from the timeline (`liftedSince`). const standing = partial && body === person ? personReasons(prior?.body, pull.headRefOid, label) : null; - if (prior?.body === body || (standing !== null && fenced(reason).split("\n").every((line) => standing.includes(line)))) { + if (prior?.body === body || (prior !== undefined && prior.body === unwrapped) || (standing !== null && fenced(reason).split("\n").every((line) => standing.includes(line)))) { return body === person && !carried ? { body: null, commentId: prior.databaseId ?? null, label, liftedSince: String(prior.updatedAt ?? prior.createdAt) } : null; } // An escalation to a person posts a new comment: an edit notifies no one diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 8582b280ab..0011f63af8 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -35,6 +35,7 @@ import { holdPathOnly, isPartial, refusalBody, + refusalReasons, refusalWrites, main, pythonRoom, @@ -460,7 +461,13 @@ test("an enforcement floor path the gate holds for its security review is a lane const verdict = { refusal: null, needsHuman: false, reviewsPerson: false, head: HEAD, ineligible: null, reviews }; const { writes, notes } = await run([pull(1, { files: [{ filename: ".github/scripts/work-resolve-issue/limits.mjs" }], verdict })]); assert.deepEqual(writes, []); - assert.deepEqual(notes, [{ method: "POST", path: `${ISSUES}/1/comments`, body: { body: laneNote(`pr-merge's gate holds it on its risk score (${reviews})`) } }]); + const wrapped = [ + "pr-merge's gate holds it on its risk score", + " (review-path:.github/scripts/work-resolve-issue/limits.mjs:", + " .github/scripts/work-resolve-issue/limits.mjs; a clean claude-security-review bound to this head", + " clears it)", + ]; + assert.deepEqual(notes, [{ method: "POST", path: `${ISSUES}/1/comments`, body: { body: laneNote(wrapped.join("\n")) } }]); }); // #10908: a `review-path:` hold whose security findings the lanes App @@ -471,6 +478,12 @@ test("an enforcement floor path the gate holds for its security review is a lane // comment first, then the draft toggle starts the whole review. const PATH_HOLD = "review-path:.github/scripts/pr-unblock/gate-source.mjs: .github/scripts/pr-unblock/gate-source.mjs; a clean claude-security-review bound to this head clears it"; const PATH_REASON = `pr-merge's gate holds it on its risk score (${PATH_HOLD})`; +// PATH_REASON as the comment wraps it at 100 columns. +const PATH_NOTE = [ + "pr-merge's gate holds it on its risk score (review-path:.github/scripts/pr-unblock/gate-source.mjs:", + " .github/scripts/pr-unblock/gate-source.mjs; a clean claude-security-review bound to this head", + " clears it)", +].join("\n"); const PATH_HELD = { refusal: null, needsHuman: false, head: HEAD, ineligible: null, reviews: PATH_HOLD, reviewsPerson: false, wholeReviewWanted: true, queue: UNEJECTED }; // babysit_whole_review.record_line's shape, checked against it below. const recordAt = (head, at) => `Whole review requested at head \`${head}\` at ${at}: an incremental review cannot clear the findings below.`; @@ -480,7 +493,7 @@ const withRecords = (body, records) => body.replace(/\n(`].join("\n"); + for (const prior of [personNote(LONG_RISK), wrappedPerson]) { + // A person took the label off: it goes back only if they removed it + // before the comment last changed, never with a second comment. + const writes = refusalWrites({ headRefOid: HEAD }, [lanesSaid(prior)], { clears: "person", reason: LONG_RISK }, { bot: LANES.login, label: "needs-human" }); + assert.deepEqual(writes, { body: null, commentId: 77, label: "needs-human", liftedSince: "2026-10-23T10:00:00Z" }); + } + for (const prior of [laneNote(LONG_RISK), wrappedLane]) { + assert.equal(refusalWrites({ headRefOid: HEAD }, [lanesSaid(prior)], { clears: "lane", reason: LONG_RISK }, { bot: LANES.login, label: "needs-human", now: Date.parse(NOW) }), null); + } + // A changed reason over an old comment is written in the wrapped format. + const moved = refusalWrites({ headRefOid: HEAD }, [lanesSaid(laneNote(LONG_RISK))], { clears: "lane", reason: MISSING_REASON + LONG_RISK }, { bot: LANES.login, label: "needs-human", now: Date.parse(NOW) }); + assert.ok(fits(moved.body)); + assert.equal(moved.commentId, 77); +}); + test("an armed pull request, an event run and report-only write no refusal note", async () => { assert.deepEqual((await run([pull(1)])).notes, []); const armedHeld = pull(2, { autoMergeRequest: {}, files: [{ filename: "docs/held.md" }] }); @@ -1974,6 +2062,8 @@ test("a human-merge-hold label holds a lanes App PR whose body lost its hold lin // label, as for a configured denied path; it puts the label back unless a // person removed it. A person's own hold text gets neither: they set it. const LANE_HOLD_REASON = "its body carries the lane's own 'Hold: a human merges this' line: a person merges it by hand, or removes that line"; +// LANE_HOLD_REASON as a new comment wraps it at 100 columns. +const LANE_HOLD_NOTE = "its body carries the lane's own 'Hold: a human merges this' line: a person merges it by hand, or\n removes that line"; const APP_ONLY = { totalCount: 0, nodes: [] }; const laneHeld = (number, overrides = {}) => pull(number, { body: `Closes #5\n\n${HOLD_LINE}`, userContentEdits: APP_ONLY, ...overrides }); test("the lane's own hold line is commented on once and labeled human-gated", async () => { @@ -1981,7 +2071,7 @@ test("the lane's own hold line is commented on once and labeled human-gated", as assert.equal(first.code, 0); assert.deepEqual(first.writes, []); assert.deepEqual(first.notes, [ - { method: "POST", path: `${ISSUES}/1/comments`, body: { body: personNote(LANE_HOLD_REASON) } }, + { method: "POST", path: `${ISSUES}/1/comments`, body: { body: personNote(LANE_HOLD_NOTE) } }, { method: "POST", path: `${ISSUES}/1/labels`, body: { labels: ["needs-human"] } }, ]); // The next run: labeled, it reads the discussion for a partial refusal to @@ -2003,10 +2093,10 @@ test("the lane's hold line over a lane's comment posts once, then reads the post const laneComment = lanesSaid(laneNote(MISSING_REASON)); const first = await run([laneHeld(1, { discussion: { comments: [laneComment] } })]); assert.deepEqual(first.notes, [ - { method: "POST", path: `${ISSUES}/1/comments`, body: { body: personNote(LANE_HOLD_REASON) } }, + { method: "POST", path: `${ISSUES}/1/comments`, body: { body: personNote(LANE_HOLD_NOTE) } }, { method: "POST", path: `${ISSUES}/1/labels`, body: { labels: ["needs-human"] } }, ]); - const posted = { ...lanesSaid(personNote(LANE_HOLD_REASON), "2026-10-23T10:05:00Z"), databaseId: 78, createdAt: "2026-10-23T10:05:00Z" }; + const posted = { ...lanesSaid(personNote(LANE_HOLD_NOTE), "2026-10-23T10:05:00Z"), databaseId: 78, createdAt: "2026-10-23T10:05:00Z" }; const both = [laneComment, posted]; const lifted = await run([laneHeld(1, { discussion: { comments: both }, unlabeled: [unlabeled("2026-10-23T10:30:00Z")] })]); assert.deepEqual(lifted.notes, []); diff --git a/.github/scripts/pr-unblock/address-feedback.mjs b/.github/scripts/pr-unblock/address-feedback.mjs index 1a5e8ee369..4fe1ca5476 100644 --- a/.github/scripts/pr-unblock/address-feedback.mjs +++ b/.github/scripts/pr-unblock/address-feedback.mjs @@ -82,6 +82,46 @@ const FILES_CAP = 3000; const firstLine = (body) => String(body ?? "").split("\n")[0].replace(/\r$/, ""); +// GitHub refuses a comment body over 65536 characters; leave room for the +// framing (judgment). +const BODY_MAX = 60000; + +// GitHub scrolls a long line in a code block instead of wrapping it, so each +// quoted line is broken at a space to fit WRAP columns, its continuations +// indented two spaces; a word longer than that (a URL, a path) stays whole. A +// quote over FOLD_LINES lines folds into a
block (judgment). +export const WRAP = 100; +const FOLD_LINES = 20; + +export function wrapLine(line, width = WRAP) { + const out = []; + let lead = ""; + let rest = line; + while (lead.length + rest.length > width) { + const room = width - lead.length; + let cut = rest.lastIndexOf(" ", room); + if (cut <= 0) cut = rest.indexOf(" ", room); + if (cut <= 0) break; + out.push(lead + rest.slice(0, cut).trimEnd()); + rest = rest.slice(cut + 1).trimStart(); + lead = " "; + } + out.push(lead + rest); + return out; +} + +// One fenced quote, with no fence inside it and no control character. The +// fence keeps the quoted text inert: no mention, reference, link or HTML +// renders, and no line of it reads as a lane footer, rating or fix request. +// `summary` labels the
block a long quote folds into. +export function quote(lines, summary = (count) => `What it wrote (${count} lines)`) { + // biome-ignore lint/suspicious/noControlCharactersInRegex: control characters are what it removes + const text = lines.join("\n").replace(/\r\n/g, "\n").replace(/[\u0000-\u0008\u000b-\u001f\u007f]/g, "?").replaceAll("```", "'''"); + const wrapped = (text.length > BODY_MAX ? `${text.slice(0, BODY_MAX)} [cut]` : text).split("\n").flatMap((line) => wrapLine(line)); + const fence = ["```text", ...wrapped, "```"].join("\n"); + return wrapped.length > FOLD_LINES ? [`
${summary(wrapped.length)}`, "", fence, "", "
"].join("\n") : fence; +} + // The ids of the unresolved threads (listThreads) a pr-verify FAIL's notes // opened on a head other than `head`: the first comment is // github-actions[bot]'s, opens with VERIFY_NOTE_MARKER, and was posted on diff --git a/.github/scripts/pr-unblock/verify-notes.mjs b/.github/scripts/pr-unblock/verify-notes.mjs index 9d1378d9ab..9a27c60f9f 100644 --- a/.github/scripts/pr-unblock/verify-notes.mjs +++ b/.github/scripts/pr-unblock/verify-notes.mjs @@ -50,11 +50,7 @@ import { pathToFileURL } from "node:url"; import { rateLimitLatch } from "../../actions/log-api-budget/rate-guard.mjs"; import { headRefDigest, isVerdictName, NAME } from "../pr-verify/write-verdict.mjs"; -import { createGitHubApi, MARKER_AUTHOR_ID, need, staleVerifyNotes, VERIFY_NOTE_MARKER } from "./address-feedback.mjs"; - -// GitHub refuses a comment body over 65536 characters; leave room for the -// framing (judgment). -const BODY_MAX = 60000; +import { createGitHubApi, MARKER_AUTHOR_ID, need, quote, staleVerifyNotes, VERIFY_NOTE_MARKER } from "./address-feedback.mjs"; // The label that holds a person's pull request after a FAIL (ci-status's // merge-queue label step reads the same name). @@ -79,41 +75,6 @@ export function notePath(note, files) { return files.includes(path) ? path : null; } -// GitHub scrolls a long line in a code block instead of wrapping it, so each -// quoted line is broken at a space to fit WRAP columns, its continuations -// indented two spaces; a word longer than that (a URL, a path) stays whole. A -// quote over FOLD_LINES lines folds into a
block (judgment). -const WRAP = 100; -const FOLD_LINES = 20; - -function wrapLine(line, width = WRAP) { - const out = []; - let lead = ""; - let rest = line; - while (lead.length + rest.length > width) { - const room = width - lead.length; - let cut = rest.lastIndexOf(" ", room); - if (cut <= 0) cut = rest.indexOf(" ", room); - if (cut <= 0) break; - out.push(lead + rest.slice(0, cut).trimEnd()); - rest = rest.slice(cut + 1).trimStart(); - lead = " "; - } - out.push(lead + rest); - return out; -} - -// One fenced quote, with no fence inside it and no control character. The -// fence keeps the model's text inert: no mention, reference, link or HTML -// renders, and no line of it reads as a lane footer, rating or fix request. -function quote(lines) { - // biome-ignore lint/suspicious/noControlCharactersInRegex: control characters are what it removes - const text = lines.join("\n").replace(/\r\n/g, "\n").replace(/[\u0000-\u0008\u000b-\u001f\u007f]/g, "?").replaceAll("```", "'''"); - const wrapped = (text.length > BODY_MAX ? `${text.slice(0, BODY_MAX)} [cut]` : text).split("\n").flatMap((line) => wrapLine(line)); - const fence = ["```text", ...wrapped, "```"].join("\n"); - return wrapped.length > FOLD_LINES ? [`
What it wrote (${wrapped.length} lines)`, "", fence, "", "
"].join("\n") : fence; -} - // The comments to post, each { path, body }, in the pull request's file order. export function noteComments({ head, summary, notes, files }) { const byPath = new Map(); diff --git a/scripts/ci-hold-human-gated.test.sh b/scripts/ci-hold-human-gated.test.sh index b80d2f0e90..b8c7e3936b 100755 --- a/scripts/ci-hold-human-gated.test.sh +++ b/scripts/ci-hold-human-gated.test.sh @@ -223,6 +223,17 @@ REFUSAL="$(hold "$APP" "$REFUSAL_BODY")" export STUB_COMMENTS="[$NEW,$REFUSAL]" STUB_PIPELINE=$'merge:\n agent-identity: melodic-automation-lanes[bot]' expect "a newer pr-merge refusal beats an older security hold, naming its fenced reasons" 1 1 "needs-human" 0 \ "(it changes docs/held.md, a hold path pr-merge's gate holds it on its risk score (cost-review-required: infra/main.tf)). $APPROVE" "Changes a trust gate" +# A reason line past 100 columns wraps at a space, its continuation indented +# two spaces, and more than 20 lines fold into
: the reason reads +# back as one line either way. +WRAPPED_BODY=$'\n\nUnattended merge: native auto-merge leaves this pull request unarmed at head `bbbb`. A person must clear this reason, so the pr-merge lane added the `needs-human` label: fix the reason, then remove the label, and the next scheduled run judges it again.\n\n```text\npr-merge\'s gate holds it on its risk score (manifest-review-required:\n plugins/a/.claude-plugin/plugin.json: hooks)\n```' +export STUB_COMMENTS="[$NEW,$(hold "$APP" "$WRAPPED_BODY")]" STUB_PIPELINE=$'merge:\n agent-identity: melodic-automation-lanes[bot]' +expect "a wrapped pr-merge refusal names its reason on one line" 1 1 "needs-human" 0 \ + "(pr-merge's gate holds it on its risk score (manifest-review-required: plugins/a/.claude-plugin/plugin.json: hooks)). $APPROVE" "Changes a trust gate" +FOLDED_BODY="${WRAPPED_BODY/$'```text'/$'
Its reasons (21 lines)\n\n```text'}"$'\n\n
' +export STUB_COMMENTS="[$NEW,$(hold "$APP" "$FOLDED_BODY")]" +expect "a folded pr-merge refusal names its reason too" 1 1 "needs-human" 0 \ + "(pr-merge's gate holds it on its risk score (manifest-review-required: plugins/a/.claude-plugin/plugin.json: hooks)). $APPROVE" "Its reasons" export STUB_COMMENTS="[$REFUSAL,$NEW]" expect "a newer security hold beats an older pr-merge refusal" 1 1 "needs-human" 0 "(Changes a trust gate:" "risk score" # A lane's refusal (no attend-queue marker) set no label, so a newer one From 4763aff42f51c8d932afeee8e3cf9862fb86ba92 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:03:43 -0400 Subject: [PATCH 2/2] test(pr-merge): declare the wrapped refusal fixtures before exporting them Co-Authored-By: Claude Opus 5.5 --- scripts/ci-hold-human-gated.test.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/ci-hold-human-gated.test.sh b/scripts/ci-hold-human-gated.test.sh index b8c7e3936b..a0a4d634ca 100755 --- a/scripts/ci-hold-human-gated.test.sh +++ b/scripts/ci-hold-human-gated.test.sh @@ -227,11 +227,13 @@ expect "a newer pr-merge refusal beats an older security hold, naming its fenced # two spaces, and more than 20 lines fold into
: the reason reads # back as one line either way. WRAPPED_BODY=$'\n\nUnattended merge: native auto-merge leaves this pull request unarmed at head `bbbb`. A person must clear this reason, so the pr-merge lane added the `needs-human` label: fix the reason, then remove the label, and the next scheduled run judges it again.\n\n```text\npr-merge\'s gate holds it on its risk score (manifest-review-required:\n plugins/a/.claude-plugin/plugin.json: hooks)\n```' -export STUB_COMMENTS="[$NEW,$(hold "$APP" "$WRAPPED_BODY")]" STUB_PIPELINE=$'merge:\n agent-identity: melodic-automation-lanes[bot]' +WRAPPED="$(hold "$APP" "$WRAPPED_BODY")" +export STUB_COMMENTS="[$NEW,$WRAPPED]" STUB_PIPELINE=$'merge:\n agent-identity: melodic-automation-lanes[bot]' expect "a wrapped pr-merge refusal names its reason on one line" 1 1 "needs-human" 0 \ "(pr-merge's gate holds it on its risk score (manifest-review-required: plugins/a/.claude-plugin/plugin.json: hooks)). $APPROVE" "Changes a trust gate" FOLDED_BODY="${WRAPPED_BODY/$'```text'/$'
Its reasons (21 lines)\n\n```text'}"$'\n\n
' -export STUB_COMMENTS="[$NEW,$(hold "$APP" "$FOLDED_BODY")]" +FOLDED="$(hold "$APP" "$FOLDED_BODY")" +export STUB_COMMENTS="[$NEW,$FOLDED]" expect "a folded pr-merge refusal names its reason too" 1 1 "needs-human" 0 \ "(pr-merge's gate holds it on its risk score (manifest-review-required: plugins/a/.claude-plugin/plugin.json: hooks)). $APPROVE" "Its reasons" export STUB_COMMENTS="[$REFUSAL,$NEW]"