From 22d0e4de146b4c8b54c62e01e886a39ee15757ff Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Fri, 27 Feb 2026 10:56:38 +0000 Subject: [PATCH 1/4] Better revert detection --- src/extractors.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/extractors.ts b/src/extractors.ts index 1fb3390..ad574d0 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -219,7 +219,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, ""); From 37b0984a5ef3fb4e3597e18dac357ee45591a98c Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Fri, 27 Feb 2026 11:00:52 +0000 Subject: [PATCH 2/4] Support prefix before revert-xxx --- src/extractors.ts | 50 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/src/extractors.ts b/src/extractors.ts index ad574d0..c2c7332 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -19,7 +19,8 @@ const ISSUE_IDENTIFIER_REGEX = new RegExp( "gi", ); -const LINEAR_ISSUE_URL_REGEX = /https?:\/\/linear\.app\/[\w-]+\/issue\/(\w{1,7}-[0-9]{1,9})(?:\/[\w-]*)*/gi; +const LINEAR_ISSUE_URL_REGEX = + /https?:\/\/linear\.app\/[\w-]+\/issue\/(\w{1,7}-[0-9]{1,9})(?:\/[\w-]*)*/gi; function normalizeLinearUrls(text: string): string { return text.replace(LINEAR_ISSUE_URL_REGEX, "$1"); @@ -82,7 +83,12 @@ type IdentifierMatch = { function parseMatch(match: RegExpExecArray): IdentifierMatch | undefined { const [, rawIdentifier, teamKey, numberString] = match; // Reject leading zeros (e.g., LIN-0004) - if (!rawIdentifier || !teamKey || !numberString || Number(numberString).toString().length !== numberString.length) { + if ( + !rawIdentifier || + !teamKey || + !numberString || + Number(numberString).toString().length !== numberString.length + ) { return; } return { @@ -129,21 +135,29 @@ function matchMagicWordIdentifiers(text: string): IdentifierMatch[] { return results; } -export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): string[] { +export function extractLinearIssueIdentifiersForCommit( + commit: CommitContext, +): string[] { if (!commit) { return []; } // Odd depth = the commit is undoing previous work (a revert), so we must not // count its identifiers as "added". Even depth = revert-of-revert (re-add). - const { depth: branchDepth, inner: strippedBranch } = parseRevertBranch(commit.branchName ?? ""); + const { depth: branchDepth, inner: strippedBranch } = parseRevertBranch( + commit.branchName ?? "", + ); if (branchDepth % 2 === 1) { - log(`Skipping revert branch "${commit.branchName}" (depth ${branchDepth}) for commit ${commit.sha}`); + log( + `Skipping revert branch "${commit.branchName}" (depth ${branchDepth}) for commit ${commit.sha}`, + ); return []; } const { depth: messageDepth } = parseRevertMessage(commit.message ?? ""); if (messageDepth % 2 === 1) { - log(`Skipping revert message (depth ${messageDepth}) for commit ${commit.sha}`); + log( + `Skipping revert message (depth ${messageDepth}) for commit ${commit.sha}`, + ); return []; } @@ -170,7 +184,9 @@ export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): s return Array.from(found.keys()); } -export function extractPullRequestNumbersForCommit(commit: CommitContext): number[] { +export function extractPullRequestNumbersForCommit( + commit: CommitContext, +): number[] { if (!commit) { return []; } @@ -196,14 +212,18 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe const title = message.split(/\r?\n/)[0] ?? ""; const squashMatch = title.match(/\(#(\d+)\)$/); if (squashMatch) { - log(`Found PR number ${squashMatch[1]} in commit ${commit.sha} using squash format: "${message}"`); + log( + `Found PR number ${squashMatch[1]} in commit ${commit.sha} using squash format: "${message}"`, + ); prNumbers.push(Number.parseInt(squashMatch[1]!, 10)); } // GitHub merge: "Merge pull request #123 from ..." - must be at start const mergeMatch = message.match(/^Merge pull request #(\d+)/i); if (mergeMatch) { - log(`Found PR number ${mergeMatch[1]} in commit ${commit.sha} using merge format: "${message}"`); + log( + `Found PR number ${mergeMatch[1]} in commit ${commit.sha} using merge format: "${message}"`, + ); prNumbers.push(Number.parseInt(mergeMatch[1]!, 10)); } @@ -211,7 +231,9 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe if (prNumbers.length === 0) { const messageMatches = message.matchAll(/#(\d+)/g); for (const match of messageMatches) { - log(`Found PR number ${match[1]} in commit ${commit.sha} by extracting from message: "${message}"`); + log( + `Found PR number ${match[1]} in commit ${commit.sha} by extracting from message: "${message}"`, + ); prNumbers.push(Number.parseInt(match[1]!, 10)); } } @@ -238,7 +260,9 @@ function parseRevertBranch(branchName: string): { * Strip revert-N- prefixes from a branch name and count nesting depth. * e.g. "revert-572-revert-571-romain/bac-39" → { depth: 2, inner: "romain/bac-39" } */ -export function getRevertBranchDepth(branchName: string | null | undefined): number { +export function getRevertBranchDepth( + branchName: string | null | undefined, +): number { if (!branchName) return 0; return parseRevertBranch(branchName).depth; } @@ -259,7 +283,9 @@ function parseRevertMessage(message: string): { depth: number; inner: string } { * Unwrap Revert "..." layers from a commit message and count nesting depth. * e.g. 'Revert "Revert "DRIVE-320: Fix""' → { depth: 2, inner: "DRIVE-320: Fix" } */ -export function getRevertMessageDepth(message: string | null | undefined): number { +export function getRevertMessageDepth( + message: string | null | undefined, +): number { if (!message) return 0; return parseRevertMessage(message).depth; } From be8161f9bc67ec41146b8d01f506a0e699234236 Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Fri, 27 Feb 2026 13:05:33 +0000 Subject: [PATCH 3/4] Proper revert extraction --- src/extractors.test.ts | 263 +++++++++++++++++++++++++++++++++++------ src/extractors.ts | 96 ++++++++------- src/index.ts | 16 ++- src/scan.test.ts | 148 +++++++++++++++++++++++ src/scan.ts | 97 ++++++++------- src/types.ts | 1 + 6 files changed, 493 insertions(+), 128 deletions(-) create mode 100644 src/scan.test.ts diff --git a/src/extractors.test.ts b/src/extractors.test.ts index 01ffb54..0de1e6f 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,179 @@ 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("ignores non-issue tokens in unwrapped message", () => { + const result = extractRevertedIssueIdentifiersForCommit({ + sha: "abc", + message: 'Revert "Bump v1-2 to v1-3"', + }); + 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([]); + }); +}); + +describe("revert chain: add → revert → re-add", () => { + const mergeAdd: CommitContext = { + sha: "c7f3c4b1", + branchName: "romain/bac-39", + message: "Merge pull request #571 from org/romain/bac-39 Add TEST variable", + }; + const innerAdd: CommitContext = { + sha: "439fe0e5", + message: "Add TEST variable to .env.example", + }; + const mergeRevert: CommitContext = { + sha: "69c6d923", + branchName: "revert-571-romain/bac-39", + message: 'Merge pull request #572 from org/revert-571-romain/bac-39 Revert "Add TEST variable"', + }; + const innerRevert: CommitContext = { + sha: "986e4383", + message: 'Revert "Add TEST variable to .env.example"', + }; + const mergeReAdd: CommitContext = { + sha: "cc13b9c5", + branchName: "revert-572-revert-571-romain/bac-39", + message: "Merge pull request #573 from org/revert-572-revert-571-romain/bac-39 Custom name for the revert revert", + }; + const innerReAdd: CommitContext = { + sha: "9c83cecb", + message: 'Revert "Revert "Add TEST variable to .env.example""', + }; + const inlineAdd: CommitContext = { sha: "c041d48b", message: "More revert test" }; + const inlineRevert: CommitContext = { sha: "fa20f72f", message: 'Revert "More revert test"' }; + const inlineReapply: CommitContext = { sha: "1086658b", message: 'Reapply "More revert test"' }; + const inlineMerge: CommitContext = { + sha: "f685bbbc", + branchName: "romain/test-revert", + message: 'Merge pull request #575 from org/romain/test-revert Reapply "More revert test"', + }; + + describe("issue extraction (add path)", () => { + it("merge add → extracts identifier from branch", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(mergeAdd))).toEqual(["BAC-39"]); + }); + + it("inner add → nothing (no magic word, no branch)", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(innerAdd))).toEqual([]); + }); + + it("merge revert → blocked (odd-depth revert branch)", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(mergeRevert))).toEqual([]); + }); + + it("inner revert → nothing (Revert message has no magic word)", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(innerRevert))).toEqual([]); + }); + + it("merge re-add → identifier re-added (even depth = revert-of-revert)", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(mergeReAdd))).toEqual(["BAC-39"]); + }); + + it("inner re-add → nothing (no magic word, no branch)", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(innerReAdd))).toEqual([]); + }); + + it("inline revert/reapply → nothing (no identifiers)", () => { + expect(ids(extractLinearIssueIdentifiersForCommit(inlineAdd))).toEqual([]); + expect(ids(extractLinearIssueIdentifiersForCommit(inlineRevert))).toEqual([]); + expect(ids(extractLinearIssueIdentifiersForCommit(inlineReapply))).toEqual([]); + expect(ids(extractLinearIssueIdentifiersForCommit(inlineMerge))).toEqual([]); + }); + }); + + describe("issue extraction (revert path)", () => { + it("merge add → not a revert", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit(mergeAdd))).toEqual([]); + }); + + it("merge revert → extracts identifier from stripped branch", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit(mergeRevert))).toEqual(["BAC-39"]); + }); + + it("inner revert → nothing (no identifier in unwrapped message)", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit(innerRevert))).toEqual([]); + }); + + it("merge re-add → not a revert (even depth)", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit(mergeReAdd))).toEqual([]); + }); + + it("inner re-add → not a revert (even depth)", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit(innerReAdd))).toEqual([]); + }); + + it("inline revert/reapply → nothing (no identifiers)", () => { + expect(ids(extractRevertedIssueIdentifiersForCommit(inlineRevert))).toEqual([]); + expect(ids(extractRevertedIssueIdentifiersForCommit(inlineReapply))).toEqual([]); + }); }); }); diff --git a/src/extractors.ts b/src/extractors.ts index c2c7332..f905728 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -19,8 +19,7 @@ const ISSUE_IDENTIFIER_REGEX = new RegExp( "gi", ); -const LINEAR_ISSUE_URL_REGEX = - /https?:\/\/linear\.app\/[\w-]+\/issue\/(\w{1,7}-[0-9]{1,9})(?:\/[\w-]*)*/gi; +const LINEAR_ISSUE_URL_REGEX = /https?:\/\/linear\.app\/[\w-]+\/issue\/(\w{1,7}-[0-9]{1,9})(?:\/[\w-]*)*/gi; function normalizeLinearUrls(text: string): string { return text.replace(LINEAR_ISSUE_URL_REGEX, "$1"); @@ -83,12 +82,7 @@ type IdentifierMatch = { function parseMatch(match: RegExpExecArray): IdentifierMatch | undefined { const [, rawIdentifier, teamKey, numberString] = match; // Reject leading zeros (e.g., LIN-0004) - if ( - !rawIdentifier || - !teamKey || - !numberString || - Number(numberString).toString().length !== numberString.length - ) { + if (!rawIdentifier || !teamKey || !numberString || Number(numberString).toString().length !== numberString.length) { return; } return { @@ -135,38 +129,35 @@ 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 []; } // Odd depth = the commit is undoing previous work (a revert), so we must not // count its identifiers as "added". Even depth = revert-of-revert (re-add). - const { depth: branchDepth, inner: strippedBranch } = parseRevertBranch( - commit.branchName ?? "", - ); + const { depth: branchDepth, inner: strippedBranch } = parseRevertBranch(commit.branchName ?? ""); if (branchDepth % 2 === 1) { - log( - `Skipping revert branch "${commit.branchName}" (depth ${branchDepth}) for commit ${commit.sha}`, - ); + log(`Skipping revert branch "${commit.branchName}" (depth ${branchDepth}) for commit ${commit.sha}`); return []; } const { depth: messageDepth } = parseRevertMessage(commit.message ?? ""); if (messageDepth % 2 === 1) { - log( - `Skipping revert message (depth ${messageDepth}) for commit ${commit.sha}`, - ); + log(`Skipping revert message (depth ${messageDepth}) for commit ${commit.sha}`); 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" }); } } } @@ -176,17 +167,15 @@ export function extractLinearIssueIdentifiersForCommit( 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[] { +export function extractPullRequestNumbersForCommit(commit: CommitContext): number[] { if (!commit) { return []; } @@ -212,18 +201,14 @@ export function extractPullRequestNumbersForCommit( const title = message.split(/\r?\n/)[0] ?? ""; const squashMatch = title.match(/\(#(\d+)\)$/); if (squashMatch) { - log( - `Found PR number ${squashMatch[1]} in commit ${commit.sha} using squash format: "${message}"`, - ); + log(`Found PR number ${squashMatch[1]} in commit ${commit.sha} using squash format: "${message}"`); prNumbers.push(Number.parseInt(squashMatch[1]!, 10)); } // GitHub merge: "Merge pull request #123 from ..." - must be at start const mergeMatch = message.match(/^Merge pull request #(\d+)/i); if (mergeMatch) { - log( - `Found PR number ${mergeMatch[1]} in commit ${commit.sha} using merge format: "${message}"`, - ); + log(`Found PR number ${mergeMatch[1]} in commit ${commit.sha} using merge format: "${message}"`); prNumbers.push(Number.parseInt(mergeMatch[1]!, 10)); } @@ -231,9 +216,7 @@ export function extractPullRequestNumbersForCommit( if (prNumbers.length === 0) { const messageMatches = message.matchAll(/#(\d+)/g); for (const match of messageMatches) { - log( - `Found PR number ${match[1]} in commit ${commit.sha} by extracting from message: "${message}"`, - ); + log(`Found PR number ${match[1]} in commit ${commit.sha} by extracting from message: "${message}"`); prNumbers.push(Number.parseInt(match[1]!, 10)); } } @@ -260,9 +243,7 @@ function parseRevertBranch(branchName: string): { * Strip revert-N- prefixes from a branch name and count nesting depth. * e.g. "revert-572-revert-571-romain/bac-39" → { depth: 2, inner: "romain/bac-39" } */ -export function getRevertBranchDepth( - branchName: string | null | undefined, -): number { +export function getRevertBranchDepth(branchName: string | null | undefined): number { if (!branchName) return 0; return parseRevertBranch(branchName).depth; } @@ -283,9 +264,40 @@ function parseRevertMessage(message: string): { depth: number; inner: string } { * Unwrap Revert "..." layers from a commit message and count nesting depth. * e.g. 'Revert "Revert "DRIVE-320: Fix""' → { depth: 2, inner: "DRIVE-320: Fix" } */ -export function getRevertMessageDepth( - message: string | null | undefined, -): number { +export function getRevertMessageDepth(message: string | null | undefined): number { 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(); + + // 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" }); + } + } + } + + 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" }); + } + } + } + + 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..f73b9c9 --- /dev/null +++ b/src/scan.test.ts @@ -0,0 +1,148 @@ +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("inline revert/reapply cycle without identifiers", () => { + const commits: CommitContext[] = [ + { sha: "a1", message: "More revert test" }, + { sha: "r1", message: 'Revert "More revert test"' }, + { sha: "ra1", message: 'Reapply "More revert test"' }, + { + sha: "m1", + branchName: "romain/test-revert", + message: 'Merge pull request #575 from org/romain/test-revert Reapply "More revert test"', + }, + ]; + + it("no identifiers found in either list", () => { + 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 }; From a9b073d836471fae05e79f4b31042e55e9294159 Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Mon, 2 Mar 2026 08:59:11 +0000 Subject: [PATCH 4/4] Tests cleanup --- src/extractors.test.ts | 106 ----------------------------------------- src/extractors.ts | 16 +++---- src/scan.test.ts | 19 -------- 3 files changed, 8 insertions(+), 133 deletions(-) diff --git a/src/extractors.test.ts b/src/extractors.test.ts index 0de1e6f..994ad7f 100644 --- a/src/extractors.test.ts +++ b/src/extractors.test.ts @@ -467,14 +467,6 @@ describe("extractRevertedIssueIdentifiersForCommit", () => { expect(ids(result)).toEqual([]); }); - it("ignores non-issue tokens in unwrapped message", () => { - const result = extractRevertedIssueIdentifiersForCommit({ - sha: "abc", - message: 'Revert "Bump v1-2 to v1-3"', - }); - expect(ids(result)).toEqual([]); - }); - it("extracts identifier from revert branch name", () => { const result = extractRevertedIssueIdentifiersForCommit({ sha: "abc", @@ -524,104 +516,6 @@ describe("extractRevertedIssueIdentifiersForCommit", () => { }); }); -describe("revert chain: add → revert → re-add", () => { - const mergeAdd: CommitContext = { - sha: "c7f3c4b1", - branchName: "romain/bac-39", - message: "Merge pull request #571 from org/romain/bac-39 Add TEST variable", - }; - const innerAdd: CommitContext = { - sha: "439fe0e5", - message: "Add TEST variable to .env.example", - }; - const mergeRevert: CommitContext = { - sha: "69c6d923", - branchName: "revert-571-romain/bac-39", - message: 'Merge pull request #572 from org/revert-571-romain/bac-39 Revert "Add TEST variable"', - }; - const innerRevert: CommitContext = { - sha: "986e4383", - message: 'Revert "Add TEST variable to .env.example"', - }; - const mergeReAdd: CommitContext = { - sha: "cc13b9c5", - branchName: "revert-572-revert-571-romain/bac-39", - message: "Merge pull request #573 from org/revert-572-revert-571-romain/bac-39 Custom name for the revert revert", - }; - const innerReAdd: CommitContext = { - sha: "9c83cecb", - message: 'Revert "Revert "Add TEST variable to .env.example""', - }; - const inlineAdd: CommitContext = { sha: "c041d48b", message: "More revert test" }; - const inlineRevert: CommitContext = { sha: "fa20f72f", message: 'Revert "More revert test"' }; - const inlineReapply: CommitContext = { sha: "1086658b", message: 'Reapply "More revert test"' }; - const inlineMerge: CommitContext = { - sha: "f685bbbc", - branchName: "romain/test-revert", - message: 'Merge pull request #575 from org/romain/test-revert Reapply "More revert test"', - }; - - describe("issue extraction (add path)", () => { - it("merge add → extracts identifier from branch", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(mergeAdd))).toEqual(["BAC-39"]); - }); - - it("inner add → nothing (no magic word, no branch)", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(innerAdd))).toEqual([]); - }); - - it("merge revert → blocked (odd-depth revert branch)", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(mergeRevert))).toEqual([]); - }); - - it("inner revert → nothing (Revert message has no magic word)", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(innerRevert))).toEqual([]); - }); - - it("merge re-add → identifier re-added (even depth = revert-of-revert)", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(mergeReAdd))).toEqual(["BAC-39"]); - }); - - it("inner re-add → nothing (no magic word, no branch)", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(innerReAdd))).toEqual([]); - }); - - it("inline revert/reapply → nothing (no identifiers)", () => { - expect(ids(extractLinearIssueIdentifiersForCommit(inlineAdd))).toEqual([]); - expect(ids(extractLinearIssueIdentifiersForCommit(inlineRevert))).toEqual([]); - expect(ids(extractLinearIssueIdentifiersForCommit(inlineReapply))).toEqual([]); - expect(ids(extractLinearIssueIdentifiersForCommit(inlineMerge))).toEqual([]); - }); - }); - - describe("issue extraction (revert path)", () => { - it("merge add → not a revert", () => { - expect(ids(extractRevertedIssueIdentifiersForCommit(mergeAdd))).toEqual([]); - }); - - it("merge revert → extracts identifier from stripped branch", () => { - expect(ids(extractRevertedIssueIdentifiersForCommit(mergeRevert))).toEqual(["BAC-39"]); - }); - - it("inner revert → nothing (no identifier in unwrapped message)", () => { - expect(ids(extractRevertedIssueIdentifiersForCommit(innerRevert))).toEqual([]); - }); - - it("merge re-add → not a revert (even depth)", () => { - expect(ids(extractRevertedIssueIdentifiersForCommit(mergeReAdd))).toEqual([]); - }); - - it("inner re-add → not a revert (even depth)", () => { - expect(ids(extractRevertedIssueIdentifiersForCommit(innerReAdd))).toEqual([]); - }); - - it("inline revert/reapply → nothing (no identifiers)", () => { - expect(ids(extractRevertedIssueIdentifiersForCommit(inlineRevert))).toEqual([]); - expect(ids(extractRevertedIssueIdentifiersForCommit(inlineReapply))).toEqual([]); - }); - }); -}); - describe("getRevertBranchDepth", () => { it.each([ [null, 0], diff --git a/src/extractors.ts b/src/extractors.ts index f905728..44f552a 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -281,20 +281,20 @@ export function extractRevertedIssueIdentifiersForCommit(commit: CommitContext): const found = new Map(); - // 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 (branchDepth % 2 === 1) { + for (const match of matchAllIdentifiers(originalBranch)) { if (!found.has(match.identifier)) { - found.set(match.identifier, { identifier: match.identifier, source: "commit_message" }); + found.set(match.identifier, { identifier: match.identifier, source: "branch_name" }); } } } - if (branchDepth % 2 === 1) { - for (const match of matchAllIdentifiers(originalBranch)) { + // 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: "branch_name" }); + found.set(match.identifier, { identifier: match.identifier, source: "commit_message" }); } } } diff --git a/src/scan.test.ts b/src/scan.test.ts index f73b9c9..d4536a2 100644 --- a/src/scan.test.ts +++ b/src/scan.test.ts @@ -71,25 +71,6 @@ describe("scanCommits", () => { }); }); - describe("inline revert/reapply cycle without identifiers", () => { - const commits: CommitContext[] = [ - { sha: "a1", message: "More revert test" }, - { sha: "r1", message: 'Revert "More revert test"' }, - { sha: "ra1", message: 'Reapply "More revert test"' }, - { - sha: "m1", - branchName: "romain/test-revert", - message: 'Merge pull request #575 from org/romain/test-revert Reapply "More revert test"', - }, - ]; - - it("no identifiers found in either list", () => { - 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[] = [