diff --git a/src/extractors.test.ts b/src/extractors.test.ts index d33e0e2..1ef02b8 100644 --- a/src/extractors.test.ts +++ b/src/extractors.test.ts @@ -546,6 +546,125 @@ describe("getRevertMessageDepth", () => { }); }); +describe("squash sub-commit blocks", () => { + // `git merge --squash` followed by `git commit` writes a body that begins + // with a "Squashed commit of the following:" header and dumps every commit + // pulled in via the squash, including upstream commits merged into the + // feature branch. Those references describe branch history, not the change + // landing here, so they must not be re-attributed to this release. + + it("ignores PR refs inside a squash sub-commit dump (no real title)", () => { + const message = `Squashed commit of the following: + +commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + Older shipped PR title (#85) + + Fixes LIN-50`; + expect(extractPullRequestNumbersForCommit({ sha: "abc", message })).toEqual([]); + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual([]); + }); + + it("ignores PR refs inside a squash dump but keeps a real title prepended", () => { + const message = `New dashboard widget (#100) + +Squashed commit of the following: + +commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + Older shipped PR title (#85) + + Fixes LIN-50 + +commit bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + + Bug fix for graph render + + Fixes LIN-100`; + // Title's PR # is the legitimate one; body's #85 must not leak. + expect(extractPullRequestNumbersForCommit({ sha: "abc", message })).toEqual([100]); + // LIN-100 is in a sub-commit body — also nested history. With the squash + // dump stripped, neither LIN-50 (already shipped) nor LIN-100 (whose + // attribution belongs to a different commit) are re-extracted from here. + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual([]); + }); + + it("keeps magic-word refs from the PR description body above the squash dump", () => { + const message = `New dashboard widget (#100) + +Closes LIN-100 + +Squashed commit of the following: + +commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + Fixes LIN-50`; + expect(extractPullRequestNumbersForCommit({ sha: "abc", message })).toEqual([100]); + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual(["LIN-100"]); + }); + + it("does not extract PR # from body cross-reference like 'builds on #85'", () => { + const message = `Add settings page (#100) + +This builds on #85 and #87. Closes LIN-200.`; + expect(extractPullRequestNumbersForCommit({ sha: "abc", message })).toEqual([100]); + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual(["LIN-200"]); + }); + + it("preserves a user-authored footer appended after the squash dump", () => { + // Default `git merge --squash` puts the dump at the top, so any footer + // (e.g., trailers like `Closes LIN-X`, `Co-authored-by: ...`) typically + // ends up below the dump after the developer edits the message. + const message = `Squashed commit of the following: + +commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +Author: Dev +Date: Wed May 6 16:45:34 2026 +0000 + + Edge cases + +commit bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +Author: Dev +Date: Wed May 6 16:45:34 2026 +0000 + + Implement search filter + +Closes LIN-200`; + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual(["LIN-200"]); + }); + + it("preserves both a prepended title and an appended footer around the dump", () => { + const message = `Search filter (#100) + +Squashed commit of the following: + +commit aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + Older shipped PR title (#85) + + Fixes LIN-50 + +Closes LIN-200`; + expect(extractPullRequestNumbersForCommit({ sha: "abc", message })).toEqual([100]); + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message })).sort()).toEqual(["LIN-200"]); + }); + + it("handles the marker without recognizable commit blocks below", () => { + const message = `Squashed commit of the following: + +Closes LIN-200`; + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual(["LIN-200"]); + }); + + it("does not strip squash blocks from revert add-extraction (revert path is already blocked)", () => { + // A revert message that wraps a squash dump shouldn't add anything. + const message = `Revert "Squashed commit of the following: + + Fixes LIN-50"`; + expect(ids(extractLinearIssueIdentifiersForCommit({ sha: "abc", message }))).toEqual([]); + }); +}); + describe("extractPullRequestNumbersForCommit", () => { // Messages that should extract PR numbers it.each([ @@ -553,7 +672,6 @@ describe("extractPullRequestNumbersForCommit", () => { ["Fix #124 with better handling", [124], "hash in middle of title"], ["Merge pull request #431 from org/branch", [431], "GitHub merge format"], ["Merge pull request #42 from owner/branch\n\nDescription", [42], "merge with multiline description"], - ["Fix bug\n\nRelated to (#999)", [999], "PR in commit body"], ])("message %j should yield %j (%s)", (message, expected) => { const result = extractPullRequestNumbersForCommit({ sha: "abc", message }); expect(result).toEqual(expected); @@ -577,15 +695,15 @@ describe("extractPullRequestNumbersForCommit", () => { [ "FLEX-2816: fix something\n\nTwo issues from cursor[bot] review #4211934690.", [], - "fallback drops oversized #NNN (cursor review id)", + "title has no PR number and body scan is no longer attempted", ], [ "Fix something (#51876)\n\nTwo issues from cursor[bot] review #4211934690.", [51876], - "squash match keeps valid PR; fallback would have grabbed the oversized id but is skipped", + "squash match keeps valid PR; body cross-references are ignored", ], - ["Fix bug\n\nRelated to (#4211934690)", [], "fallback drops oversized parens form"], - ["Fix bug\n\nSee #123 and sentry #9999999999", [123], "fallback keeps small numbers and drops oversized"], + ["Fix bug\n\nRelated to (#4211934690)", [], "body fallback removed: cross-reference in body is ignored"], + ["Fix bug\n\nSee #123 and sentry #9999999999", [], "body fallback removed: numbers in body don't leak"], ["Title (#4211934690)", [], "squash format with oversized number is dropped"], ["Merge pull request #4211934690 from x/y", [], "merge format with oversized number is dropped"], [`Title (#${2_147_483_647})`, [2_147_483_647], "Int32 max is allowed"], diff --git a/src/extractors.ts b/src/extractors.ts index a5e5d49..5e535e8 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -29,6 +29,45 @@ const ISSUE_IDENTIFIER_REGEX = new RegExp( const LINEAR_ISSUE_URL_REGEX = /https?:\/\/linear\.app\/[\w-]+\/issue\/(\w{1,7}-[0-9]{1,9})(?:\/[\w-]*)*/gi; +/** + * `git merge --squash` followed by `git commit` writes a body containing this + * header and then dumps the full message of every commit pulled in via the + * squash — including upstream history merged into the feature branch via + * `git merge`. Issue / PR references inside that dump describe branch history, + * not the change being squashed, so they must not feed release association. + * + * We excise *only* the dump itself: any real subject the developer prepended + * and any footer they appended (e.g. `Closes LIN-X`, `Co-authored-by: …`) are + * preserved. The dump is bounded by recognizable structural lines (`commit + * `, `Author:`, `Date:`, `Merge:`, blank lines, and indented body + * content); the first non-indented, non-empty line that doesn't match those + * patterns marks the start of user-authored footer content. + */ +const SQUASH_BLOCK_MARKER = /^Squashed commit of the following:/i; +const SQUASH_COMMIT_HEADER = /^commit [0-9a-f]{7,40}\b/i; +const SQUASH_METADATA_HEADER = /^(?:Author|AuthorDate|Commit|CommitDate|Date|Merge):\s/i; + +function stripSquashBlock(message: string): string { + const lines = message.split(/\r?\n/); + const markerIdx = lines.findIndex((l) => SQUASH_BLOCK_MARKER.test(l)); + if (markerIdx === -1) return message; + + let i = markerIdx + 1; + for (; i < lines.length; i++) { + const line = lines[i]!; + if (line === "") continue; + if (/^[ \t]/.test(line)) continue; + if (SQUASH_COMMIT_HEADER.test(line)) continue; + if (SQUASH_METADATA_HEADER.test(line)) continue; + break; + } + + const before = lines.slice(0, markerIdx).join("\n").trimEnd(); + const after = lines.slice(i).join("\n").trim(); + if (before && after) return `${before}\n\n${after}`; + return before || after; +} + function normalizeLinearUrls(text: string): string { return text.replace(LINEAR_ISSUE_URL_REGEX, "$1"); } @@ -174,8 +213,10 @@ export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): E } } - // Commit message: only extract when preceded by a magic word - const message = commit.message ?? ""; + // Commit message: only extract when preceded by a magic word. + // Strip any squashed sub-commit dump first so references that came from + // already-merged branch history don't get re-attributed to this commit. + const message = stripSquashBlock(commit.message ?? ""); if (message.length > 0) { for (const match of matchMagicWordIdentifiers(message)) { if (!found.has(match.identifier)) { @@ -192,11 +233,11 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe return []; } - const message = commit.message ?? ""; + const rawMessage = commit.message ?? ""; // Skip reverts - they reference the original PR, not a new one - if (/^Revert "/i.test(message)) { - verbose(`Skipping revert commit ${commit.sha} with message: "${message}"`); + if (/^Revert "/i.test(rawMessage)) { + verbose(`Skipping revert commit ${commit.sha} with message: "${rawMessage}"`); return []; } @@ -207,6 +248,11 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe return []; } + // Drop nested squash sub-commit dumps before scanning so `(#NNN)` references + // from already-shipped commits pulled in via `git merge` don't get attributed + // to this commit's release. + const message = stripSquashBlock(rawMessage); + const prNumbers: number[] = []; const pushIfValid = (raw: string, source: string): void => { const number = Number.parseInt(raw, 10); @@ -233,10 +279,13 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe pushIfValid(mergeMatch[1]!, "merge format"); } - // Only use fallback if no matches from squash/merge formats + // Fallback for non-canonical merge titles (e.g. a direct push that put the PR + // number somewhere other than the trailing parens). Restrict to the title line + // — scanning the body would re-pick up cross-references like "builds on #85" + // and stale references inside squashed-in sub-commit history. if (prNumbers.length === 0) { - for (const match of message.matchAll(/#(\d+)/g)) { - pushIfValid(match[1]!, "message scan"); + for (const match of title.matchAll(/#(\d+)/g)) { + pushIfValid(match[1]!, "title scan"); } } @@ -311,7 +360,8 @@ export function extractRevertedIssueIdentifiersForCommit(commit: CommitContext): // Use magic-word gating on the inner message, same as the add path, to avoid // false positives from generic word-number tokens (e.g. "Bump v1-2 to v1-3"). if (messageDepth % 2 === 1) { - for (const match of matchMagicWordIdentifiers(innerMessage)) { + const innerStripped = stripSquashBlock(innerMessage); + for (const match of matchMagicWordIdentifiers(innerStripped)) { if (!found.has(match.identifier)) { found.set(match.identifier, { identifier: match.identifier, source: "commit_message" }); } diff --git a/src/git.test.ts b/src/git.test.ts index b9e8083..389bfb3 100644 --- a/src/git.test.ts +++ b/src/git.test.ts @@ -588,16 +588,39 @@ describe("getCommitContextsBetweenShas", () => { expect(result[0]?.sha).toBe(repo.commits.first); }); - it("should normalize commit message whitespace", () => { + it("should collapse horizontal whitespace but preserve newlines", () => { const result = getCommitContextsBetweenShas(repo.commits.first, repo.commits.first, { cwd: repo.cwd, }); expect(result).toHaveLength(1); - // The first commit has "feat: add src file with extra spaces" - multiple spaces should be normalized - expect(result[0]?.message).not.toMatch(/\s{2,}/); + // Multiple spaces in the subject should be collapsed expect(result[0]?.message).toBe("feat: add src file with extra spaces"); }); + it("should preserve newlines so extractors can distinguish title from body", () => { + // Standalone tempdir so the multiline body is independent of the shared fixture. + const cwd = mkdtempSync(join(tmpdir(), "linear-release-multiline-")); + try { + runGit("init", cwd); + runGit('config user.email "test@example.com"', cwd); + runGit('config user.name "Test User"', cwd); + writeFileSync(join(cwd, "file.txt"), "x"); + runGit("add .", cwd); + runGit('commit -m "Add feature (#100)" -m "Closes LIN-200" -m "Co-authored-by: Other "', cwd); + const sha = runGit("rev-parse HEAD", cwd); + + const result = getCommitContextsBetweenShas(sha, sha, { cwd }); + expect(result).toHaveLength(1); + expect(result[0]?.message).toBe( + "Add feature (#100)\n\nCloses LIN-200\n\nCo-authored-by: Other ", + ); + // First line is the actual title (not the entire flattened body) + expect(result[0]!.message!.split("\n")[0]).toBe("Add feature (#100)"); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }); + it("should return empty array when no commits in range", () => { // third..first is empty because first is an ancestor of third const result = getCommitContextsBetweenShas(repo.commits.third, repo.commits.first, { diff --git a/src/git.ts b/src/git.ts index aab13ec..154ea46 100644 --- a/src/git.ts +++ b/src/git.ts @@ -195,7 +195,9 @@ export function extractBranchNameFromMergeMessage(message: string | null | undef */ function parseCommitChunk(chunk: string): CommitContext { const [sha, rawMessage, rawDecorations] = chunk.split("\x1f"); - const message = (rawMessage ?? "").trim().replace(/\s+/g, " "); + // Collapse runs of horizontal whitespace, but keep newlines so downstream + // extractors can tell the title from the body and skip nested commit blocks. + const message = (rawMessage ?? "").trim().replace(/[ \t]+/g, " "); const branchName = extractBranchNameFromMergeMessage(message) ?? extractBranchName(rawDecorations); return { sha: sha.trim(), branchName, message };