diff --git a/src/extractors.test.ts b/src/extractors.test.ts index 01ffb54..994ad7f 100644 --- a/src/extractors.test.ts +++ b/src/extractors.test.ts @@ -1,12 +1,18 @@ import { describe, expect, it } from "vitest"; import { + ExtractedIdentifier, extractLinearIssueIdentifiersForCommit, extractPullRequestNumbersForCommit, + extractRevertedIssueIdentifiersForCommit, getRevertBranchDepth, getRevertMessageDepth, } from "./extractors"; import { CommitContext } from "./types"; +function ids(result: ExtractedIdentifier[]): string[] { + return result.map((r) => r.identifier); +} + describe("extractLinearIssueIdentifiersForCommit", () => { it("extracts identifiers from branch name", () => { const commit: CommitContext = { @@ -17,7 +23,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result).toEqual(["ENG-123"]); + expect(ids(result)).toEqual(["ENG-123"]); }); it("extracts identifiers from commit message with magic words", () => { @@ -29,7 +35,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result.sort()).toEqual(["ENG-7", "PLAT-42"].sort()); + expect(ids(result).sort()).toEqual(["ENG-7", "PLAT-42"].sort()); }); it("deduplicates identifiers across branch and message (case-insensitive)", () => { @@ -41,7 +47,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result).toEqual(["ENG-123"]); + expect(ids(result)).toEqual(["ENG-123"]); }); it("returns empty array when no identifiers are present", () => { @@ -53,7 +59,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("matches team keys with 1-7 alphanumeric characters", () => { @@ -65,7 +71,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result.sort()).toEqual(["A-1", "ABCDEFG-999", "X1Y2Z3A-100"].sort()); + expect(ids(result).sort()).toEqual(["A-1", "ABCDEFG-999", "X1Y2Z3A-100"].sort()); }); it("does not extract identifiers from commit message without magic words", () => { @@ -77,7 +83,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("does not match team keys longer than 7 characters", () => { @@ -89,7 +95,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { const result = extractLinearIssueIdentifiersForCommit(commit); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); }); @@ -108,7 +114,7 @@ describe("version suffix handling", () => { branchName: branch, message: null, }); - expect(result).toEqual(expected); + expect(ids(result)).toEqual(expected); }); // Legitimate identifiers should match @@ -125,7 +131,7 @@ describe("version suffix handling", () => { branchName: branch, message: null, }); - expect(result).toEqual(expected); + expect(ids(result)).toEqual(expected); }); }); @@ -136,7 +142,7 @@ describe("leading zero rejection", () => { branchName: "feature/LIN-0004-test", message: null, }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); }); @@ -150,7 +156,7 @@ describe("underscore handling", () => { branchName: branch, message: null, }); - expect(result).toEqual(expected); + expect(ids(result)).toEqual(expected); }); }); @@ -164,7 +170,7 @@ describe("multiple identifiers", () => { branchName: null, message, }); - expect(result.sort()).toEqual(expected.sort()); + expect(ids(result).sort()).toEqual(expected.sort()); }); }); @@ -175,7 +181,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("extracts with contributing phrase", () => { @@ -184,7 +190,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Related to LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("does not extract without magic words", () => { @@ -193,7 +199,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "See LIN-123 for details", }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("extracts multiple keys after keyword", () => { @@ -202,7 +208,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes LIN-123, LIN-456 and ENG-789", }); - expect(result.sort()).toEqual(["ENG-789", "LIN-123", "LIN-456"]); + expect(ids(result).sort()).toEqual(["ENG-789", "LIN-123", "LIN-456"]); }); it("extracts magic word in title line", () => { @@ -211,7 +217,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fix LIN-123: something", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("does not extract key in title without keyword", () => { @@ -220,7 +226,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "LIN-123: Fix something", }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it.each([ @@ -246,7 +252,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: `${keyword} LIN-100`, }); - expect(result).toEqual(["LIN-100"]); + expect(ids(result)).toEqual(["LIN-100"]); }); it.each(["ref", "refs", "references", "part of", "related to", "relates to", "contributes to", "towards", "toward"])( @@ -257,7 +263,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: `${phrase} LIN-200`, }); - expect(result).toEqual(["LIN-200"]); + expect(ids(result)).toEqual(["LIN-200"]); }, ); @@ -267,7 +273,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Closes: LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("only extracts keys preceded by magic word on same line", () => { @@ -276,7 +282,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "See LIN-111, fixes LIN-222", }); - expect(result).toEqual(["LIN-222"]); + expect(ids(result)).toEqual(["LIN-222"]); }); it("does not extract from the original bug scenario", () => { @@ -285,7 +291,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Title\nSeparate issue to follow up on that here LIN-60064", }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("branch provides keys independently of message magic words", () => { @@ -294,7 +300,7 @@ describe("commit message magic word behavior", () => { branchName: "feature/LIN-100-something", message: "See LIN-200 for details", }); - expect(result).toEqual(["LIN-100"]); + expect(ids(result)).toEqual(["LIN-100"]); }); it("is case insensitive for magic words", () => { @@ -303,7 +309,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "fIXES LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("extracts with multi-word phrase 'Part of'", () => { @@ -312,7 +318,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Part of LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("extracts with multi-word phrase 'Related to'", () => { @@ -321,7 +327,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Related to LIN-456", }); - expect(result).toEqual(["LIN-456"]); + expect(ids(result)).toEqual(["LIN-456"]); }); it("extracts issue from Linear URL with slug after magic word", () => { @@ -330,7 +336,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes https://linear.app/myorg/issue/LIN-123/fix-auth", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("extracts issue from Linear URL without slug after magic word", () => { @@ -339,7 +345,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes https://linear.app/myorg/issue/LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("does not extract Linear URL without magic word", () => { @@ -348,7 +354,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "See https://linear.app/myorg/issue/LIN-123/fix", }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("extracts mixed Linear URLs and raw IDs after magic word", () => { @@ -357,7 +363,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes https://linear.app/myorg/issue/LIN-123/slug, ENG-456 and LIN-789", }); - expect(result.sort()).toEqual(["ENG-456", "LIN-123", "LIN-789"]); + expect(ids(result).sort()).toEqual(["ENG-456", "LIN-123", "LIN-789"]); }); it("extracts issue from http Linear URL after magic word", () => { @@ -366,7 +372,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes http://linear.app/myorg/issue/LIN-123", }); - expect(result).toEqual(["LIN-123"]); + expect(ids(result)).toEqual(["LIN-123"]); }); it("extracts issue from Linear URL with trailing slash", () => { @@ -375,7 +381,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Fixes https://linear.app/my-org/issue/LIN-213/", }); - expect(result).toEqual(["LIN-213"]); + expect(ids(result)).toEqual(["LIN-213"]); }); it("extracts issue from Linear URL with contributing phrase", () => { @@ -384,7 +390,7 @@ describe("commit message magic word behavior", () => { branchName: null, message: "Part of https://linear.app/myorg/issue/LIN-213/some-slug", }); - expect(result).toEqual(["LIN-213"]); + expect(ids(result)).toEqual(["LIN-213"]); }); }); @@ -395,7 +401,7 @@ describe("revert branch handling", () => { branchName: "revert-571-romain/bac-39", message: "Merge pull request #572 from org/revert-571-romain/bac-39", }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("blocks extraction when revert branch has multiple identifiers", () => { @@ -404,7 +410,7 @@ describe("revert branch handling", () => { branchName: "revert-571-romain/drive-320-and-drive-321", message: null, }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); }); it("blocks PR number extraction from revert branch", () => { @@ -422,7 +428,16 @@ describe("revert branch handling", () => { branchName: null, message: 'Revert "Fixes ENG-100"', }); - expect(result).toEqual([]); + expect(ids(result)).toEqual([]); + }); + + it("reverted extractor picks up message-only revert with magic word", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + branchName: null, + message: 'Revert "Fixes ENG-100"', + }); + expect(ids(result)).toEqual(["ENG-100"]); }); it("allows extraction from revert-of-revert branch (even depth)", () => { @@ -431,7 +446,73 @@ describe("revert branch handling", () => { branchName: "revert-572-revert-571-romain/bac-39", message: null, }); - expect(result).toEqual(["BAC-39"]); + expect(ids(result)).toEqual(["BAC-39"]); + }); +}); + +describe("extractRevertedIssueIdentifiersForCommit", () => { + it("extracts identifier from unwrapped revert message with magic word", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + message: 'Revert "Fixes DRIVE-320: memory leak in background location service"', + }); + expect(ids(result)).toEqual(["DRIVE-320"]); + }); + + it("ignores identifier in unwrapped message without magic word", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + message: 'Revert "DRIVE-320: Fix memory leak in background location service"', + }); + expect(ids(result)).toEqual([]); + }); + + it("extracts identifier from revert branch name", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + branchName: "revert-571-romain/bac-39", + message: 'Revert "Add TEST variable to .env.example"', + }); + expect(ids(result)).toEqual(["BAC-39"]); + }); + + it("extracts from both message and branch, deduplicates", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + branchName: "revert-566-romain/drive-320", + message: 'Revert "Fixes DRIVE-320: memory leak"', + }); + expect(ids(result)).toEqual(["DRIVE-320"]); + }); + + it("extracts multiple identifiers from unwrapped message", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + message: 'Revert "Fixes DRIVE-320 and DRIVE-321"', + }); + expect(ids(result).sort()).toEqual(["DRIVE-320", "DRIVE-321"]); + }); + + it("returns empty for non-revert commit", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + branchName: "romain/drive-320", + message: "DRIVE-320: Fix memory leak", + }); + expect(ids(result)).toEqual([]); + }); + + it("returns empty when revert message has no identifiers", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + message: 'Revert "Fix bug"', + }); + expect(ids(result)).toEqual([]); + }); + + it("returns empty for null/undefined inputs", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit({ sha: "abc", message: null }))).toEqual([]); + expect(ids(extractRevertedIssueIdentifiersForCommit({ sha: "abc" }))).toEqual([]); }); }); diff --git a/src/extractors.ts b/src/extractors.ts index 1fb3390..44f552a 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -129,7 +129,12 @@ function matchMagicWordIdentifiers(text: string): IdentifierMatch[] { return results; } -export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): string[] { +export type ExtractedIdentifier = { + identifier: string; + source: "branch_name" | "commit_message"; +}; + +export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): ExtractedIdentifier[] { if (!commit) { return []; } @@ -147,12 +152,12 @@ export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): s return []; } - const found = new Map(); + const found = new Map(); if (strippedBranch.length > 0) { for (const match of matchAllIdentifiers(strippedBranch)) { if (!found.has(match.identifier)) { - found.set(match.identifier, match.rawIdentifier); + found.set(match.identifier, { identifier: match.identifier, source: "branch_name" }); } } } @@ -162,12 +167,12 @@ export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): s if (message.length > 0) { for (const match of matchMagicWordIdentifiers(message)) { if (!found.has(match.identifier)) { - found.set(match.identifier, match.rawIdentifier); + found.set(match.identifier, { identifier: match.identifier, source: "commit_message" }); } } } - return Array.from(found.keys()); + return Array.from(found.values()); } export function extractPullRequestNumbersForCommit(commit: CommitContext): number[] { @@ -219,7 +224,10 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe return [...new Set(prNumbers)]; } -function parseRevertBranch(branchName: string): { depth: number; inner: string } { +function parseRevertBranch(branchName: string): { + depth: number; + inner: string; +} { // Full refs can have org/ prefixes (e.g. "org/revert-571-..."), strip to the revert pattern. // Non-greedy so we stop at the first revert-N- match, not the last (preserves nested depth). let name = branchName.replace(/^.*?\/(?=revert-\d+-)/i, ""); @@ -260,3 +268,36 @@ export function getRevertMessageDepth(message: string | null | undefined): numbe if (!message) return 0; return parseRevertMessage(message).depth; } + +/** Extract identifiers being reverted. Returns [] if not an odd-depth revert. */ +export function extractRevertedIssueIdentifiersForCommit(commit: CommitContext): ExtractedIdentifier[] { + if (!commit) return []; + + const { depth: branchDepth, inner: originalBranch } = parseRevertBranch(commit.branchName ?? ""); + const { depth: messageDepth, inner: innerMessage } = parseRevertMessage(commit.message ?? ""); + + // At least one of branch/message must have odd depth (i.e., be a revert) to extract + if (branchDepth % 2 === 0 && messageDepth % 2 === 0) return []; + + const found = new Map(); + + if (branchDepth % 2 === 1) { + for (const match of matchAllIdentifiers(originalBranch)) { + if (!found.has(match.identifier)) { + found.set(match.identifier, { identifier: match.identifier, source: "branch_name" }); + } + } + } + + // 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)) { + if (!found.has(match.identifier)) { + found.set(match.identifier, { identifier: match.identifier, source: "commit_message" }); + } + } + } + + return Array.from(found.values()); +} diff --git a/src/index.ts b/src/index.ts index 89f8e2f..b6f8feb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -191,7 +191,13 @@ async function syncCommand(): Promise<{ return null; } - const { issueReferences, prNumbers, debugSink } = scanCommits(commits, effectiveIncludePaths); + // git log returns newest-first; scanCommits needs chronological (oldest-first) for last-write-wins + commits.reverse(); + + const { issueReferences, revertedIssueReferences, prNumbers, debugSink } = scanCommits( + commits, + effectiveIncludePaths, + ); log(`Debug sink: ${JSON.stringify(debugSink, null, 2)}`); @@ -201,9 +207,13 @@ async function syncCommand(): Promise<{ log(`Retrieved issue keys: ${issueReferences.map((f) => f.identifier).join(", ")}`); } + if (revertedIssueReferences.length > 0) { + log(`Reverted issue keys: ${revertedIssueReferences.map((f) => f.identifier).join(", ")}`); + } + const repoInfo = getRepoInfo(); - const release = await syncRelease(issueReferences, prNumbers, repoInfo, debugSink); + const release = await syncRelease(issueReferences, revertedIssueReferences, prNumbers, repoInfo, debugSink); log( `Issues [${issueReferences.map((f) => f.identifier).join(", ")}] and pull requests [${prNumbers.join( ", ", @@ -342,6 +352,7 @@ async function getPipelineSettings(): Promise<{ includePathPatterns: string[] }> async function syncRelease( issueReferences: IssueReference[], + revertedIssueReferences: IssueReference[], prNumbers: number[], repoInfo: RepoInfo | null, debugSink: DebugSink, @@ -379,6 +390,7 @@ async function syncRelease( version: releaseVersion, commitSha: currentSha, issueReferences, + revertedIssueReferences: revertedIssueReferences.length > 0 ? revertedIssueReferences : undefined, pullRequestReferences: prNumbers.map((number) => ({ repositoryOwner: owner, repositoryName: name, diff --git a/src/scan.test.ts b/src/scan.test.ts new file mode 100644 index 0000000..d4536a2 --- /dev/null +++ b/src/scan.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from "vitest"; +import { scanCommits } from "./scan"; +import { CommitContext } from "./types"; + +function ids(refs: { identifier: string }[]): string[] { + return refs.map((r) => r.identifier).sort(); +} + +describe("scanCommits", () => { + describe("add → revert → re-add chain", () => { + // Oldest first, as scanCommits expects + const commits: CommitContext[] = [ + { sha: "a1", message: "Add TEST variable to .env.example" }, + { + sha: "a2", + branchName: "romain/bac-39", + message: "Merge pull request #571 from org/romain/bac-39 Add TEST variable", + }, + { sha: "r1", message: 'Revert "Add TEST variable to .env.example"' }, + { + sha: "r2", + branchName: "revert-571-romain/bac-39", + message: 'Merge pull request #572 from org/revert-571-romain/bac-39 Revert "Add TEST variable"', + }, + { sha: "ra1", message: 'Revert "Revert "Add TEST variable to .env.example""' }, + { + sha: "ra2", + branchName: "revert-572-revert-571-romain/bac-39", + message: "Merge pull request #573 from org/revert-572-revert-571-romain/bac-39 Custom name", + }, + ]; + + it("full chain: last action is re-add, so identifier is in added list", () => { + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual(["BAC-39"]); + expect(ids(result.revertedIssueReferences)).toEqual([]); + }); + + it("add only: identifier is in added list", () => { + const result = scanCommits(commits.slice(0, 2), null); + expect(ids(result.issueReferences)).toEqual(["BAC-39"]); + expect(ids(result.revertedIssueReferences)).toEqual([]); + }); + + it("add + revert: identifier is in reverted list", () => { + const result = scanCommits(commits.slice(0, 4), null); + expect(ids(result.issueReferences)).toEqual([]); + expect(ids(result.revertedIssueReferences)).toEqual(["BAC-39"]); + }); + }); + + describe("squash revert with magic word in message", () => { + it("identifier extracted from unwrapped message via magic word, ends up reverted", () => { + const commits: CommitContext[] = [ + { sha: "a1", message: "Fixes DRIVE-320: memory leak in background location service" }, + { sha: "r1", message: 'Revert "Fixes DRIVE-320: memory leak in background location service"' }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual([]); + expect(ids(result.revertedIssueReferences)).toEqual(["DRIVE-320"]); + }); + + it("identifier without magic word is ignored (no false positives)", () => { + const commits: CommitContext[] = [ + { sha: "a1", message: "Bump v1-2 to v1-3" }, + { sha: "r1", message: 'Revert "Bump v1-2 to v1-3"' }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual([]); + expect(ids(result.revertedIssueReferences)).toEqual([]); + }); + }); + + describe("last-write-wins edge cases", () => { + it("different issues go to their respective lists", () => { + const commits: CommitContext[] = [ + { sha: "a1", branchName: "user/eng-100", message: "Fixes ENG-100" }, + { sha: "r1", branchName: "revert-1-user/eng-200", message: 'Revert "ENG-200: something"' }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual(["ENG-100"]); + expect(ids(result.revertedIssueReferences)).toEqual(["ENG-200"]); + }); + + it("add then revert same issue: only in reverted", () => { + const commits: CommitContext[] = [ + { sha: "a1", branchName: "user/eng-100" }, + { sha: "r1", branchName: "revert-1-user/eng-100", message: 'Revert "ENG-100: fix"' }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual([]); + expect(ids(result.revertedIssueReferences)).toEqual(["ENG-100"]); + }); + + it("revert then add same issue: only in added", () => { + const commits: CommitContext[] = [ + { sha: "r1", branchName: "revert-1-user/eng-100", message: 'Revert "ENG-100: fix"' }, + { sha: "a1", branchName: "user/eng-100" }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual(["ENG-100"]); + expect(ids(result.revertedIssueReferences)).toEqual([]); + }); + + it("revert merge commit with magic word in message does not add identifier", () => { + const commits: CommitContext[] = [ + { sha: "a1", branchName: "user/eng-100" }, + { + sha: "r1", + branchName: "revert-1-user/eng-100", + message: "Merge pull request #2\n\nFixes ENG-100", + }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual([]); + expect(ids(result.revertedIssueReferences)).toEqual(["ENG-100"]); + }); + + it("message-only revert with magic word is reverted, not added", () => { + const commits: CommitContext[] = [ + { sha: "a1", branchName: "user/eng-100", message: "Fixes ENG-100" }, + { sha: "r1", message: 'Revert "Fixes ENG-100"' }, + ]; + const result = scanCommits(commits, null); + expect(ids(result.issueReferences)).toEqual([]); + expect(ids(result.revertedIssueReferences)).toEqual(["ENG-100"]); + }); + }); +}); diff --git a/src/scan.ts b/src/scan.ts index 91f7307..99a01bb 100644 --- a/src/scan.ts +++ b/src/scan.ts @@ -1,20 +1,34 @@ -import { extractLinearIssueIdentifiersForCommit, extractPullRequestNumbersForCommit } from "./extractors"; +import { + extractLinearIssueIdentifiersForCommit, + extractPullRequestNumbersForCommit, + extractRevertedIssueIdentifiersForCommit, +} from "./extractors"; import { log } from "./log"; -import { CommitContext, DebugSink, IssueReference, IssueSource, PullRequestSource } from "./types"; +import { CommitContext, DebugSink, IssueReference, PullRequestSource } from "./types"; +/** + * Scan commits and produce added/reverted issue references using last-write-wins. + * Expects commits in chronological order (oldest first). The caller must reverse + * git log output before passing it here. + */ export function scanCommits( commits: CommitContext[], includePaths: string[] | null, ): { issueReferences: IssueReference[]; + revertedIssueReferences: IssueReference[]; prNumbers: number[]; debugSink: DebugSink; } { - const seen = new Map(); + const lastAction = new Map(); + const addedRefs = new Map(); + const revertedRefs = new Map(); + const prNumbersSet = new Set(); const debugSink: DebugSink = { inspectedShas: [], issues: {}, + revertedIssues: {}, pullRequests: [], includePaths, }; @@ -22,56 +36,36 @@ export function scanCommits( for (const commit of commits) { debugSink.inspectedShas.push(commit.sha); - const fromBranch = extractLinearIssueIdentifiersForCommit({ - sha: commit.sha, - branchName: commit.branchName, - message: null, - }); - - for (const key of fromBranch) { - if (!debugSink.issues[key]) { - debugSink.issues[key] = []; + for (const { identifier, source } of extractRevertedIssueIdentifiersForCommit(commit)) { + if (!debugSink.revertedIssues[identifier]) { + debugSink.revertedIssues[identifier] = []; } - - const source: IssueSource = { + debugSink.revertedIssues[identifier].push({ sha: commit.sha, - source: "branch_name", - value: commit.branchName ?? "", - }; - debugSink.issues[key].push(source); + source, + value: source === "branch_name" ? (commit.branchName ?? "") : (commit.message ?? ""), + }); - if (seen.has(key)) { - continue; - } - - seen.set(key, { identifier: key, commitSha: commit.sha }); - log(`Detected issue key ${key} from branch name "${commit.branchName ?? ""}"`); + lastAction.set(identifier, "reverted"); + revertedRefs.set(identifier, { identifier, commitSha: commit.sha }); + log(`Detected reverted issue key ${identifier} from commit ${commit.sha}`); } - const fromMessage = extractLinearIssueIdentifiersForCommit({ - sha: commit.sha, - branchName: null, - message: commit.message, - }); - - for (const key of fromMessage) { - if (!debugSink.issues[key]) { - debugSink.issues[key] = []; + for (const { identifier, source } of extractLinearIssueIdentifiersForCommit(commit)) { + if (!debugSink.issues[identifier]) { + debugSink.issues[identifier] = []; } - - const source: IssueSource = { + debugSink.issues[identifier].push({ sha: commit.sha, - source: "commit_message", - value: commit.message ?? "", - }; - debugSink.issues[key].push(source); - - if (seen.has(key)) { - continue; - } + source, + value: source === "branch_name" ? (commit.branchName ?? "") : (commit.message ?? ""), + }); - seen.set(key, { identifier: key, commitSha: commit.sha }); - log(`Detected issue key ${key} from commit message "${commit.message ?? ""}"`); + lastAction.set(identifier, "added"); + addedRefs.set(identifier, { identifier, commitSha: commit.sha }); + log( + `Detected issue key ${identifier} from ${source === "branch_name" ? `branch "${commit.branchName}"` : `message "${commit.message}"`}`, + ); } for (const prNumber of extractPullRequestNumbersForCommit(commit)) { @@ -88,8 +82,19 @@ export function scanCommits( } } + const issueReferences: IssueReference[] = []; + const revertedIssueReferences: IssueReference[] = []; + for (const [identifier, action] of lastAction) { + if (action === "added") { + issueReferences.push(addedRefs.get(identifier)!); + } else { + revertedIssueReferences.push(revertedRefs.get(identifier)!); + } + } + return { - issueReferences: Array.from(seen.values()), + issueReferences, + revertedIssueReferences, prNumbers: Array.from(prNumbersSet), debugSink, }; diff --git a/src/types.ts b/src/types.ts index 478cae5..a807549 100644 --- a/src/types.ts +++ b/src/types.ts @@ -104,6 +104,7 @@ export type PullRequestSource = { export type DebugSink = { inspectedShas: string[]; // From oldest to newest issues: Record; // Issue identifier -> array of sources + revertedIssues: Record; // Issue identifier -> array of sources (reverted) pullRequests: PullRequestSource[]; // PR numbers found in commits includePaths: string[] | null; // Path filters applied during commit scanning };