fix(pr-pipeline): hold reuse PRs on real conditions and leak-scan the reused branch

.changes/pr-pipeline/fix-recovery-hold-paths-5b1e93c4.md

@@ -0,0 +1,9 @@
+---
+bump: patch
+---
+
+### Changed
+
+- A reuse PR from `work-resolve-issue` no longer carries `Hold: a human merges this` on every PR. It holds on the same conditions as a `done` run's PR, through one `mustHold` check in `report-outcome.mjs` that both paths call, and now reads `merge: human` on a reuse too. It also holds when the compare failed, when the runner's checks for the tip are missing or did not all pass, or when the new reuse leak scan is missing or not clean. This supersedes the fixed hold from [#7831](https://github.com/melodic-software/claude-code-plugins/pull/7831).
+- `run-checks` scans a reused branch before its checks run. It uses the read runner's own leak scan on the diff from the branch's merge base with the default branch tip and on every file that diff makes. The result goes into the checks record as `scan`.
+- The read runner's leak-scan passes now live in `lib/publish-gate.mjs` (`scanLeak`, `deeperLeak`, `changedBytes`): per-line classification, the UTF-16LE pass, the split-line pass and the scan of the files a patch makes. The scan job calls them from the gate it already loads at the default branch tip, so both scans run one copy of the code. Behavior is unchanged.

.changes/review/fix-recovery-hold-paths-5b1e93c4.md

@@ -0,0 +1,7 @@
+---
+bump: patch
+---
+
+### Changed
+
+- The shared `lib/publish-gate.mjs` copy gains the lanes' leak-scan functions (`scanLeak`, `deeperLeak`, `changedBytes`). Nothing in this plugin calls them, and its publish gate behaves as before.

.changes/visualization/fix-recovery-hold-paths-5b1e93c4.md

@@ -0,0 +1,7 @@
+---
+bump: patch
+---
+
+### Changed
+
+- The shared `lib/publish-gate.mjs` copy gains the lanes' leak-scan functions (`scanLeak`, `deeperLeak`, `changedBytes`). Nothing in this plugin calls them, and its publish gate behaves as before.

.github/actions/resolve-config/callers.test.mjs

@@ -3184,10 +3184,10 @@ test("leak scan: a credential-format file name blocks, and a PEM header alone bl
 });
 
 test("leak scan: a gate whose labels lack the warning label still blocks its credentials", async (t) => {
-  const gate = [
-    'export const SECRET_PATTERNS = Object.freeze([["Anthropic key", /\\bsk-ant-[A-Za-z0-9_-]{20,}/]]);',
-    "export const findSecret = (text) => (SECRET_PATTERNS[0][1].test(text) ? [SECRET_PATTERNS[0][0], 1] : null);",
-  ].join("\n");
+  // The main gate, its scan included, with a pattern list that has no warning label.
+  const main = read("../../../lib/publish-gate.mjs");
+  const gate = main.replace(/export const SECRET_PATTERNS = Object\.freeze\(\[[\s\S]*?\n\]\);/, 'export const SECRET_PATTERNS = Object.freeze([["Anthropic key", /\\bsk-ant-[A-Za-z0-9_-]{20,}/]]);');
+  assert.notEqual(gate, main, "the main gate's pattern list was replaced");
   assert.deepEqual((await scanWith(t, () => {}, gate)).clean, "true");
   const hit = await scanWith(t, ({ temp }) => writeFileSync(join(temp, "skill-reply", "reply.txt"), `${KEY}\n`), gate);
   assert.equal(hit.clean, "false");
@@ -4512,12 +4512,8 @@ test("leak scan: clean multi-line and clean UTF-16LE text stay clean in the repl
 });
 
 test("leak scan: the split-line pass joins patch hunk lines without their prefixes, and counts only a match across the join", async () => {
-  const script = scanSteps().find((s) => s.id === "leak-scan").with.script;
-  const body = script.slice(script.indexOf("const straddles"), script.indexOf("// Each source has a kind"));
-  const { findSecret, SECRET_PATTERNS } = await import(new URL("../../../lib/publish-gate.mjs", import.meta.url));
-  const WARN_ONLY = "password or secret assignment";
-  const blocking = SECRET_PATTERNS.filter(([label]) => label !== WARN_ONLY);
-  const deeper = new Function("blocking", "findSecret", "WARN_ONLY", `${body}; return deeper;`)(blocking, findSecret, WARN_ONLY);
+  // The leak scan's deeper passes, from the gate the scan job loads.
+  const { deeperLeak: deeper } = await import(new URL("../../../lib/publish-gate.mjs", import.meta.url));
   const hunk = `diff --git a/a b/a\n--- a/a\n+++ b/a\n@@ -1 +1,2 @@\n+the token is ${GHS.slice(0, 2)}\n+${GHS.slice(2)}\n`;
   assert.equal(deeper(hunk, true), "GitHub token", "an added-hunk split joins without the + prefixes");
   assert.equal(deeper(hunk, false), null, "kept, the + prefix breaks the token");

.github/scripts/work-resolve-issue/report-outcome.mjs

@@ -662,7 +662,9 @@ export function readSummary(dir, { issueNumber, branch }) {
 
 // The run-checks job's `checks` output: {tip, results: [{check, exit}]}, or
 // null unless it names this branch, a tip, and at most MAX_ITEMS whole-number
-// results, so no result is cut off. A check passes only on exit 0.
+// results, so no result is cut off. A check passes only on exit 0. On a reused
+// branch it adds `scan`, the leak scan's word for that tip (scan-reuse.mjs);
+// any word but clean holds the PR, and so does none.
 export function readChecks(text, { branch }) {
   let parsed;
   try {
@@ -680,8 +682,13 @@ export function readChecks(text, { branch }) {
     Array.isArray(results) &&
     results.length <= MAX_ITEMS &&
     results.every((result) => typeof result?.check === "string" && Number.isInteger(result.exit));
-  return valid ? { tip: parsed.tip, results: results.map(({ check, exit }) => ({ check: check.slice(0, MAX_ITEM), exit })) } : null;
+  if (!valid) {
+    return null;
+  }
+  const record = { tip: parsed.tip, results: results.map(({ check, exit }) => ({ check: check.slice(0, MAX_ITEM), exit })) };
+  return Object.hasOwn(parsed, "scan") ? { ...record, scan: SCAN_WORDS.has(parsed.scan) ? parsed.scan : "failed" } : record;
 }
+const SCAN_WORDS = new Set(["clean", "found", "failed"]);
 
 const checkFailures = (checks) =>
   checks === null
@@ -692,6 +699,35 @@ const checkFailures = (checks) =>
 // posted: at least one check ran and none failed.
 export const checksPassed = (checks) => Boolean(checks?.results.length) && !checkFailures(checks).length;
 
+// Whether the lane's PR holds for a person. decide() and reusePlan() both ask
+// here, so a reused branch holds on exactly the main path's conditions plus
+// its own leak scan: a trusted `merge: human`, an issue labeled
+// untrusted-provenance, an unreadable denied-path list or changed-file list,
+// a listing cut at the cap while some path is denied (it may hide one), a
+// changed path the merge lane refuses, runner checks missing or not all
+// passed, and on a reused branch a leak scan that is missing or not clean.
+export function mustHold({ issueLabels, humanMerge, holdPaths, changedFiles, checks, reused = false }) {
+  return (
+    humanMerge ||
+    issueLabels.includes(UNTRUSTED_PROVENANCE) ||
+    holdPaths === null ||
+    changedFiles === null ||
+    (holdPaths.length > 0 && changedFiles.length >= COMPARE_FILES_CAP) ||
+    changedFiles.some((path) => holdPaths.some((glob) => posix.matchesGlob(path, glob))) ||
+    !checks ||
+    checkFailures(checks).length > 0 ||
+    (reused && checks.scan !== "clean")
+  );
+}
+
+// The reused branch's leak scan as the PR body's Verification section shows it.
+const scanLine = (checks) =>
+  checks?.scan === "clean"
+    ? "The runner's leak scan read everything the branch changes since its merge base with the default branch and found nothing credential-shaped."
+    : checks?.scan === "found"
+      ? "The runner's leak scan found a credential shape or a password or secret assignment in what the branch changes, so a person merges this."
+      : "The runner's leak scan of the branch did not finish or could not be read, so a person merges this.";
+
 // The runner's check results as the PR body's Verification section shows them.
 const checkLines = (checks) =>
   checks === null
@@ -729,20 +765,23 @@ function fileList(files, holdPaths) {
 }
 
 // A lane branch an earlier run left with commits and no report, whose outcome
-// this run cannot read: a PR a human merges, listing the files it changes.
-// With the summary pass's summary it carries that text; without it, titled
-// from the issue, no model text.
+// this run cannot read: a PR listing the files it changes, held on mustHold's
+// conditions. With the summary pass's summary it carries that text; without
+// it, titled from the issue, no model text.
 export function reusePlan({
   issueNumber,
+  issueLabels = [],
   issueTitle = "",
   branch,
   runUrl = "",
   earlierRunUrl = "",
   summary = null,
   changedFiles = null,
   holdPaths = null,
-  checks,
+  humanMerge = false,
+  checks = null,
 }) {
+  const hold = mustHold({ issueLabels, humanMerge, holdPaths, changedFiles, checks, reused: true });
   const opened = `Opened by the work-resolve-issue lane from \`${branch}\`, which an earlier run left with commits and no report.`;
   const [about, fix, earlierChecks, leftOut] = summary
     ? [
@@ -760,8 +799,7 @@ export function reusePlan({
   const body = [
     `Closes #${issueNumber}`,
     "",
-    HOLD_LINE,
-    "",
+    ...(hold ? [HOLD_LINE, ""] : []),
     "## Summary",
     "",
     ...about,
@@ -774,12 +812,12 @@ export function reusePlan({
     "",
     "## Verification",
     "",
-    checks === undefined ? earlierChecks : `${earlierChecks}\n\n${checkLines(checks)}`,
+    `${earlierChecks}\n\n${checkLines(checks)}\n\n${scanLine(checks)}`,
     "",
     earlierRunUrl ? `- The earlier lane run: ${earlierRunUrl}` : "- The earlier lane run.",
     runUrl ? `- This lane run: ${runUrl}` : "- This lane run.",
     "",
-    `Left out: ${leftOut}; a person reviews the diff before merging.`,
+    hold ? `Left out: ${leftOut}; a person reviews the diff before merging.` : `Left out: ${leftOut}.`,
     "",
     "## Related",
     "",
@@ -974,14 +1012,7 @@ export function decide({
   }
   // A PR that names work it leaves for later links without closing.
   const refs = outcome.remaining.length > 0;
-  // A listing cut at the cap may hide a denied path, so it holds only while
-  // some path is denied.
-  const hold =
-    humanMerge ||
-    issueLabels.includes(UNTRUSTED_PROVENANCE) ||
-    holdPaths === null ||
-    (holdPaths.length > 0 && changedFiles.length >= COMPARE_FILES_CAP) ||
-    changedFiles.some((path) => holdPaths.some((glob) => posix.matchesGlob(path, glob)));
+  const hold = mustHold({ issueLabels, humanMerge, holdPaths, changedFiles, checks });
   // The sections of the PR body contract (.claude/rules/pr-body-contract.md),
   // in the org PR template's order, with the run's gaps above the work.
   const gaps = outcome.notDone.map(({ what }) => what);
@@ -1638,13 +1669,12 @@ export async function main({ env = process.env, fetch = globalThis.fetch, now =
     const resolvedPath = env.RESOLVED_CONFIG || join(env.RUNNER_TEMP ?? "", "resolved-config.json");
     const holdPaths = readHoldPaths(resolvedPath);
     // A reuse whose outcome cannot be read never takes the limit record: the
-    // skipped run hit no limit, and a limited marker would loop restarts. Its
-    // file list is only text, so a failed compare never blocks the PR.
+    // skipped run hit no limit, and a limited marker would loop restarts. A
+    // failed compare leaves its file list unread, which holds the PR.
     const issueTitle = typeof issue.title === "string" ? issue.title : "";
     const issueLabels = (issue.labels ?? []).map((label) => label.name);
     // A labeler that cannot be read holds the PR, like an unreadable denied-path list.
     const humanMerge =
-      !unreported &&
       issueLabels.includes(HUMAN_MERGE_LABEL) &&
       (await paginate(github, `${repo}/issues/${issueNumber}/events`)
         .then((events) => trustedLabeler(events, HUMAN_MERGE_LABEL, { bot, humans: readHumans(env.TRUSTED_ACTORS ?? "") }))
@@ -1674,13 +1704,15 @@ export async function main({ env = process.env, fetch = globalThis.fetch, now =
     let plan = unreported
       ? reusePlan({
           issueNumber,
+          issueLabels,
           issueTitle,
           branch,
           runUrl,
           earlierRunUrl: runUrlOf(reused[1]),
           summary,
           changedFiles: await compare().catch(() => null),
           holdPaths,
+          humanMerge,
           checks,
         })
       : decide({