From 6535cc6daba6df51e405a30d03e280941a7ff26b Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Fri, 27 Feb 2026 10:56:38 +0000 Subject: [PATCH 1/3] Better revert detection --- src/extractors.test.ts | 59 +++++++++++++++++++++++++++++++++---- src/extractors.ts | 66 +++++++++++++++++++++++++++++++++++------- 2 files changed, 109 insertions(+), 16 deletions(-) diff --git a/src/extractors.test.ts b/src/extractors.test.ts index 101b2b1..38850a3 100644 --- a/src/extractors.test.ts +++ b/src/extractors.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { extractLinearIssueIdentifiersForCommit, extractPullRequestNumbersForCommit } from "./extractors"; +import { + extractLinearIssueIdentifiersForCommit, + extractPullRequestNumbersForCommit, + getRevertBranchDepth, + getRevertMessageDepth, +} from "./extractors"; import { CommitContext } from "./types"; describe("extractLinearIssueIdentifiersForCommit", () => { @@ -88,7 +93,7 @@ describe("extractLinearIssueIdentifiersForCommit", () => { }); }); -describe("version suffix handling ", () => { +describe("version suffix handling", () => { // Version strings should NOT match it.each([ ["release/ios-1.57.1", []], @@ -124,7 +129,7 @@ describe("version suffix handling ", () => { }); }); -describe("leading zero rejection ", () => { +describe("leading zero rejection", () => { it("rejects LIN-0004 style identifiers", () => { const result = extractLinearIssueIdentifiersForCommit({ sha: "abc", @@ -135,8 +140,7 @@ describe("leading zero rejection ", () => { }); }); -describe("underscore handling ", () => { - // Underscores act as word boundaries +describe("underscore handling", () => { it.each([ ["story/LIN-123_LIN-321_hello_world", ["LIN-123", "LIN-321"]], ["username/lin-123_branch_name", ["LIN-123"]], @@ -150,7 +154,7 @@ describe("underscore handling ", () => { }); }); -describe("multiple identifiers ", () => { +describe("multiple identifiers", () => { it.each([ ["Fixes LIN-123 and LIN-321", ["LIN-123", "LIN-321"]], ["Closes LIN-123, LIN-321", ["LIN-123", "LIN-321"]], @@ -411,6 +415,49 @@ describe("revert branch handling", () => { }); expect(result).toEqual([]); }); + + it("blocks add-extraction from message-only revert with magic word", () => { + const result = extractLinearIssueIdentifiersForCommit({ + sha: "abc", + branchName: null, + message: 'Revert "Fixes ENG-100"', + }); + expect(result).toEqual([]); + }); + + it("allows extraction from revert-of-revert branch (even depth)", () => { + const result = extractLinearIssueIdentifiersForCommit({ + sha: "abc", + branchName: "revert-572-revert-571-romain/bac-39", + message: null, + }); + expect(result).toEqual(["BAC-39"]); + }); +}); + +describe("getRevertBranchDepth", () => { + it.each([ + [null, 0], + ["romain/bac-39", 0], + ["revert-571-romain/bac-39", 1], + ["revert-572-revert-571-romain/bac-39", 2], + ["revert-574-revert-573-revert-572-romain/bac-39", 3], + ])("branch %j → depth %d", (branch, expected) => { + expect(getRevertBranchDepth(branch)).toBe(expected); + }); +}); + +describe("getRevertMessageDepth", () => { + it.each([ + [null, 0], + ["Fix memory leak", 0], + ['Revert "DRIVE-320: Fix"', 1], + ['Revert "Revert "DRIVE-320: Fix""', 2], + ['Revert "Revert "Revert "DRIVE-320: Fix"""', 3], + ['Reapply "DRIVE-320: Fix"', 0], + ])("message %j → depth %d", (message, expected) => { + expect(getRevertMessageDepth(message)).toBe(expected); + }); }); describe("extractPullRequestNumbersForCommit", () => { diff --git a/src/extractors.ts b/src/extractors.ts index f030f37..8ed5bb3 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -134,19 +134,23 @@ export function extractLinearIssueIdentifiersForCommit(commit: CommitContext): s return []; } - // GitHub auto-generates branch names like "revert-571-romain/bac-39" for revert PRs. - // Block extraction to prevent the original issue identifier from being added to the release. - if (/(^|\/)revert-\d+-/i.test(commit.branchName ?? "")) { - log(`Skipping revert branch ${commit.branchName} for commit ${commit.sha}`); + // 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 ?? ""); + if (branchDepth % 2 === 1) { + 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}`); return []; } const found = new Map(); - // Branch name: extract all matches (branch names are always intentional) - const branchName = commit.branchName ?? ""; - if (branchName.length > 0) { - for (const match of matchAllIdentifiers(branchName)) { + if (strippedBranch.length > 0) { + for (const match of matchAllIdentifiers(strippedBranch)) { if (!found.has(match.identifier)) { found.set(match.identifier, match.rawIdentifier); } @@ -179,8 +183,9 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe return []; } - // Skip merge commits of revert PRs — branch name like "revert-571-romain/bac-39" - if (/(^|\/)revert-\d+-/i.test(commit.branchName ?? "")) { + // Revert merge commits reference the original PR number, not a new one. + // Even depth (revert-of-revert) falls through to normal extraction. + if (getRevertBranchDepth(commit.branchName) % 2 === 1) { log(`Skipping revert merge commit ${commit.sha}`); return []; } @@ -213,3 +218,44 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe return [...new Set(prNumbers)]; } + +/** + * 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 parseRevertBranch(branchName: string): { depth: number; inner: string } { + // Full refs can have org/ prefixes (e.g. "org/revert-571-..."), strip to the revert pattern + let name = branchName.replace(/^.*\/(?=revert-\d+-)/i, ""); + let depth = 0; + while (/^revert-\d+-/i.test(name)) { + name = name.replace(/^revert-\d+-/i, ""); + depth++; + } + return { depth, inner: name }; +} + +export function getRevertBranchDepth(branchName: string | null | undefined): number { + if (!branchName) return 0; + return parseRevertBranch(branchName).depth; +} + +/** + * 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 parseRevertMessage(message: string): { depth: number; inner: string } { + let text = message; + let depth = 0; + while (/^Revert "/i.test(text)) { + const match = text.match(/^Revert "(.+)"(.*)$/s); + if (!match) break; + text = match[1]!; + depth++; + } + return { depth, inner: text }; +} + +export function getRevertMessageDepth(message: string | null | undefined): number { + if (!message) return 0; + return parseRevertMessage(message).depth; +} From 8976268be4b8ea67cff2e6e6783d7a53ded2a66f Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Fri, 27 Feb 2026 11:00:52 +0000 Subject: [PATCH 2/3] Support prefix before revert-xxx --- src/extractors.test.ts | 1 + src/extractors.ts | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/extractors.test.ts b/src/extractors.test.ts index 38850a3..01ffb54 100644 --- a/src/extractors.test.ts +++ b/src/extractors.test.ts @@ -442,6 +442,7 @@ describe("getRevertBranchDepth", () => { ["revert-571-romain/bac-39", 1], ["revert-572-revert-571-romain/bac-39", 2], ["revert-574-revert-573-revert-572-romain/bac-39", 3], + ["org/revert-572-revert-571-romain/bac-39", 2], ])("branch %j → depth %d", (branch, expected) => { expect(getRevertBranchDepth(branch)).toBe(expected); }); diff --git a/src/extractors.ts b/src/extractors.ts index 8ed5bb3..e229c0a 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -224,8 +224,9 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe * e.g. "revert-572-revert-571-romain/bac-39" → { depth: 2, inner: "romain/bac-39" } */ export function parseRevertBranch(branchName: string): { depth: number; inner: string } { - // Full refs can have org/ prefixes (e.g. "org/revert-571-..."), strip to the revert pattern - let name = branchName.replace(/^.*\/(?=revert-\d+-)/i, ""); + // 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, ""); let depth = 0; while (/^revert-\d+-/i.test(name)) { name = name.replace(/^revert-\d+-/i, ""); From ef591348db514585d36e86fa2671ef038b77468b Mon Sep 17 00:00:00 2001 From: Romain Cascino Date: Fri, 27 Feb 2026 13:08:56 +0000 Subject: [PATCH 3/3] Fix exports --- src/extractors.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/extractors.ts b/src/extractors.ts index e229c0a..1fb3390 100644 --- a/src/extractors.ts +++ b/src/extractors.ts @@ -219,11 +219,7 @@ export function extractPullRequestNumbersForCommit(commit: CommitContext): numbe return [...new Set(prNumbers)]; } -/** - * 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 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, ""); @@ -235,16 +231,16 @@ export function parseRevertBranch(branchName: string): { depth: number; inner: s return { depth, inner: name }; } +/** + * 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 { if (!branchName) return 0; return parseRevertBranch(branchName).depth; } -/** - * 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 parseRevertMessage(message: string): { depth: number; inner: string } { +function parseRevertMessage(message: string): { depth: number; inner: string } { let text = message; let depth = 0; while (/^Revert "/i.test(text)) { @@ -256,6 +252,10 @@ export function parseRevertMessage(message: string): { depth: number; inner: str return { depth, inner: text }; } +/** + * 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 { if (!message) return 0; return parseRevertMessage(message).depth;