diff --git a/.github/scripts/stack-breadcrumb.cjs b/.github/scripts/stack-breadcrumb.cjs index d7e7e67a..959454a9 100644 --- a/.github/scripts/stack-breadcrumb.cjs +++ b/.github/scripts/stack-breadcrumb.cjs @@ -298,6 +298,7 @@ async function reconcile(root, deps) { const updated = []; const skipped = []; + const frozen = []; const failed = []; for (const number_ of tree.members) { @@ -305,6 +306,14 @@ async function reconcile(root, deps) { if (!pullRequest) { continue; } + // A merged/closed member stays in the tree — so open PRs keep listing it and + // the last open PR captures the full construction history — but its own body + // is frozen (never rewritten). Members without a `state` are treated as open + // (open-PR list and unit fixtures both omit or set it to "open"). + if (pullRequest.state !== undefined && pullRequest.state !== "open") { + frozen.push(number_); + continue; + } const region = renderRegion(tree, number_); const plan = planUpdate(pullRequest, region); if (!plan.changed) { @@ -322,7 +331,7 @@ async function reconcile(root, deps) { } } - return { root, updated, skipped, failed }; + return { root, updated, skipped, frozen, failed }; } module.exports = { diff --git a/.github/scripts/stack-breadcrumb.test.cjs b/.github/scripts/stack-breadcrumb.test.cjs index 15c42292..7a971dd8 100644 --- a/.github/scripts/stack-breadcrumb.test.cjs +++ b/.github/scripts/stack-breadcrumb.test.cjs @@ -276,3 +276,40 @@ test("reconcile: records a failed write and continues", async () => { assert.deepEqual(result.failed, [10]); assert.deepEqual(result.updated, [11]); }); + +test("reconcile: freezes a merged member but keeps it in the open PR's breadcrumb", async () => { + // #10 open (root), #11 merged (tip). #11 stays in the tree so #10 keeps + // listing it, but its own body is never rewritten. + const prs = [ + { ...pr(10, "a", "main"), state: "open" }, + { ...pr(11, "b", "a"), state: "closed" }, + ]; + const calls = []; + const result = await reconcile(10, { + pullRequests: prs, + writeBody: (number_, body) => calls.push([number_, body]), + }); + assert.deepEqual(result.updated, [10]); + assert.deepEqual(result.frozen, [11]); + assert.equal(calls.length, 1); + // #10's breadcrumb still lists the merged #11 and the marker keeps full membership. + assert.match(calls[0][1], /#11/); + assert.match(calls[0][1], /pr=10,11/); +}); + +test("reconcile: a lone open root with only merged descendants still renders", async () => { + // The whole stack has merged except the root — the breadcrumb must NOT clear. + const prs = [ + { ...pr(10, "a", "main"), state: "open" }, + { ...pr(11, "b", "a"), state: "closed" }, + { ...pr(12, "c", "b"), state: "closed" }, + ]; + const calls = []; + const result = await reconcile(10, { + pullRequests: prs, + writeBody: (number_, body) => calls.push([number_, body]), + }); + assert.deepEqual(result.updated, [10]); + assert.deepEqual(result.frozen, [11, 12]); + assert.match(calls[0][1], /pr=10,11,12/); +}); diff --git a/.github/workflows/stack-breadcrumb.yml b/.github/workflows/stack-breadcrumb.yml index f5e02304..b0002670 100644 --- a/.github/workflows/stack-breadcrumb.yml +++ b/.github/workflows/stack-breadcrumb.yml @@ -145,8 +145,74 @@ jobs: headRefName: pr.head.ref, baseRefName: pr.base.ref, body: pr.body ?? "", + state: "open", })); + // Preserve construction history: pull in stack members recorded in + // existing breadcrumb markers that are no longer open (merged/closed), + // so the tree keeps listing them and the last open PR captures the + // whole stack. Their bodies are frozen (reconcile never rewrites a + // non-open member); they're fetched only for tree structure/rendering. + // Root resolution below still uses the open-PR list only. + const openNumbers = new Set(pullRequests.map((pr) => pr.number)); + const historicalNumbers = new Set(); + for (const pr of pullRequests) { + const marker = stack.parseStackComment(pr.body); + if (!marker) continue; + for (const member of marker.members) { + // Guard against a malformed marker (non-positive / non-integer) + // and skip members that are already open. + if (Number.isInteger(member) && member > 0 && !openNumbers.has(member)) { + historicalNumbers.add(member); + } + } + } + // PR bodies are user-editable, so a crafted/malformed marker could + // list many members; cap the historical fetch defensively (with a log). + const MAX_HISTORICAL_MEMBERS = 50; + let historicalList = [...historicalNumbers].toSorted((a, b) => a - b); + if (historicalList.length > MAX_HISTORICAL_MEMBERS) { + core.info(`stack-breadcrumb: marker lists ${historicalList.length} historical members; capping at ${MAX_HISTORICAL_MEMBERS}`); + historicalList = historicalList.slice(0, MAX_HISTORICAL_MEMBERS); + } + // Fetch in parallel (bounded by the cap above); keep same-repo PRs. + const fetched = ( + await Promise.all( + historicalList.map(async (number) => { + try { + const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: number }); + if (pr.head.repo?.full_name !== `${owner}/${repo}`) return undefined; + return { + number: pr.number, + title: pr.title, + headRefName: pr.head.ref, + baseRefName: pr.base.ref, + body: pr.body ?? "", + state: pr.state, // "closed" (merged or closed) + }; + } catch (error) { + core.info(`stack-breadcrumb: could not fetch historical PR #${number}: ${String(error)}`); + return undefined; + } + }) + ) + ).filter((pr) => pr !== undefined); + // The tree is keyed by headRefName, so admit a historical PR only if + // its head ref is not already claimed — by an open PR OR an earlier + // historical one. A reused branch name would otherwise corrupt tree + // derivation. + const claimedHeadRefs = new Set(pullRequests.map((pr) => pr.headRefName)); + const historical = []; + for (const pr of fetched) { + if (claimedHeadRefs.has(pr.headRefName)) { + core.info(`stack-breadcrumb: skipping historical PR #${pr.number}; head ref "${pr.headRefName}" already claimed by another PR`); + continue; + } + claimedHeadRefs.add(pr.headRefName); + historical.push(pr); + } + const allPullRequests = [...pullRequests, ...historical]; + const writeBody = async (number, body) => { await github.rest.pulls.update({ owner, repo, pull_number: number, body }); }; @@ -207,7 +273,7 @@ jobs: // failure so an inconsistent breadcrumb doesn't pass silently. const failures = []; for (const root of roots) { - const result = await stack.reconcile(root, { pullRequests, writeBody }); + const result = await stack.reconcile(root, { pullRequests: allPullRequests, writeBody }); core.info(`reconciled root #${root}: ${JSON.stringify(result)}`); failures.push(...result.failed); }