Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 115 additions & 2 deletions src/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,15 @@ type TempRepoReleaseBranch = {
};
};

type TempRepoStaleMerge = {
cwd: string;
commits: {
base: string;
staleMerge: string; // Merge of feat/ABC-1-stale — edited app-a/ only, merged after app-b/ landed
subjectMerge: string; // Merge of feat/XYZ-2-impl — edited app-b/, key only on the merge subject (HEAD)
};
};

function runGit(command: string, cwd: string): string {
return execSync(`git ${command}`, {
cwd,
Expand Down Expand Up @@ -729,6 +738,52 @@ function createTempRepoReleaseBranch(): TempRepoReleaseBranch {
return { cwd, commits: { base, headMerge } };
}

/**
* Two PR merges into main, each carrying its issue key only in the branch name
* (no content commit carries a key):
* - feat/ABC-1-stale is rooted at `base`, edits app-a/ only, and is merged
* AFTER app-b/ appears on main — a stale branch never rebased. The merge
* differs from its first parent for app-b/ only because app-b/ advanced on
* main while the branch was open, so `--full-history` keeps it under an app-b
* pathspec even though the branch delivered nothing to app-b/.
* - feat/XYZ-2-impl is rooted at the stale merge and genuinely edits app-b/.
* Its key lives only on the merge subject, so dropping the merge would lose
* the key entirely even though the merge did deliver app-b/ changes.
*/
function createTempRepoStaleMerge(): TempRepoStaleMerge {
const { cwd, base } = initTempRepo({
prefix: "linear-release-stale-merge-",
dirs: ["app-a", "app-b"],
seedFile: { path: "app-a/file.txt", content: "a0" },
});

runGit(`checkout -b feat/ABC-1-stale ${base}`, cwd);
writeFileSync(join(cwd, "app-a", "file.txt"), "a1");
runGit("add .", cwd);
runGit('commit -m "rework app-a internals"', cwd);

runGit("checkout main", cwd);
writeFileSync(join(cwd, "app-b", "file.txt"), "b0");
runGit("add .", cwd);
runGit('commit -m "add app-b on main"', cwd);

runGit('merge --no-ff feat/ABC-1-stale -m "Merge pull request #1 from owner/feat/ABC-1-stale"', cwd);
const staleMerge = runGit("rev-parse HEAD", cwd);
runGit("branch -D feat/ABC-1-stale", cwd);

runGit(`checkout -b feat/XYZ-2-impl ${staleMerge}`, cwd);
writeFileSync(join(cwd, "app-b", "file.txt"), "b1");
runGit("add .", cwd);
runGit('commit -m "implement the thing"', cwd);

runGit("checkout main", cwd);
runGit('merge --no-ff feat/XYZ-2-impl -m "Merge pull request #2 from owner/feat/XYZ-2-impl"', cwd);
const subjectMerge = runGit("rev-parse HEAD", cwd);
runGit("branch -D feat/XYZ-2-impl", cwd);

return { cwd, commits: { base, staleMerge, subjectMerge } };
}

describe("getCommitContextsBetweenShas", () => {
let repo: TempRepo;

Expand Down Expand Up @@ -1044,8 +1099,9 @@ describe("merge commit handling", () => {
const shas = new Set(result.map((c) => c.sha));
expect(shas.has(multiRepo.commits.merge100)).toBe(true);
expect(shas.has(multiRepo.commits.merge200)).toBe(true);
// merge300 only touched infra/ — kept by the merges-only scan, then dropped
// by commitTouchesPaths so LIN-300 doesn't leak into a frontend release.
// merge300 only touched infra/, so under the frontend/backend pathspec it
// changed nothing relative to its parents and `--full-history` drops it
// natively — LIN-300 never reaches a frontend release.
expect(shas.has(multiRepo.commits.merge300)).toBe(false);
expect(shas.has(multiRepo.commits.headMerge)).toBe(true);

Expand Down Expand Up @@ -1115,6 +1171,63 @@ describe("merge commit handling", () => {
expect(branchNames).not.toContain("feature/LIN-300-mobile");
});
});

describe("getCommitContextsBetweenShas with stale-branch merges under a path filter", () => {
let repo: TempRepoStaleMerge;

beforeAll(() => {
repo = createTempRepoStaleMerge();
});

afterAll(() => {
rmSync(repo.cwd, { recursive: true, force: true });
});

it("drops a stale-branch merge that delivered no change to the filtered paths", () => {
// feat/ABC-1-stale edited app-a/ only but merged after app-b/ landed, so
// `--full-history` keeps its merge under the app-b pathspec. The merge
// delivered nothing to app-b/, so its subject key must not be attributed.
const result = getCommitContextsBetweenShas(repo.commits.base, repo.commits.subjectMerge, {
includePaths: ["app-b/**"],
cwd: repo.cwd,
});

const shas = new Set(result.map((c) => c.sha));
expect(shas.has(repo.commits.staleMerge)).toBe(false);

const branchNames = result.map((c) => c.branchName).filter((b): b is string => !!b);
expect(branchNames).not.toContain("feat/ABC-1-stale");
});

it("retains a merge whose key lives only on the subject when it delivered the filtered paths", () => {
// feat/XYZ-2-impl genuinely edited app-b/ and carries its key only on the
// merge subject, so dropping the merge would lose the key entirely.
const result = getCommitContextsBetweenShas(repo.commits.base, repo.commits.subjectMerge, {
includePaths: ["app-b/**"],
cwd: repo.cwd,
});

const shas = new Set(result.map((c) => c.sha));
expect(shas.has(repo.commits.subjectMerge)).toBe(true);

const branchNames = result.map((c) => c.branchName).filter((b): b is string => !!b);
expect(branchNames).toContain("feat/XYZ-2-impl");
});

it("still attributes a stale merge to the surface it actually touched", () => {
// The same stale merge DID deliver app-a/ changes, so under an app-a filter
// its subject key is correctly retained — the fix discards leaks, not work.
// And the app-b-only merge must not leak into the app-a surface.
const result = getCommitContextsBetweenShas(repo.commits.base, repo.commits.subjectMerge, {
includePaths: ["app-a/**"],
cwd: repo.cwd,
});

const branchNames = result.map((c) => c.branchName).filter((b): b is string => !!b);
expect(branchNames).toContain("feat/ABC-1-stale");
expect(branchNames).not.toContain("feat/XYZ-2-impl");
});
});
});

describe("assertGitAvailable", () => {
Expand Down
63 changes: 56 additions & 7 deletions src/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,20 @@ export function extractBranchNameFromMergeMessage(message: string | null | undef
* Prefers branch name from merge message over decorations for issue tracking.
*/
function parseCommitChunk(chunk: string): CommitContext {
const [sha, rawMessage, rawDecorations] = chunk.split("\x1f");
const [sha, rawMessage, rawDecorations, rawParents] = chunk.split("\x1f");
// 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 };
// %P is the parent SHAs, space-separated and empty for a root commit. Keep only
// full 40-char hashes so a root commit yields [] rather than [""], letting
// parents.length reliably tell a merge (2+) from a normal commit (1).
const parents = (rawParents ?? "")
.trim()
.split(/\s+/)
.filter((p) => /^[0-9a-f]{40}$/i.test(p));

return { sha: sha.trim(), branchName, message, parents };
}

/**
Expand Down Expand Up @@ -383,7 +390,7 @@ export function ensureCommitAvailable(sha: string, cwd: string = process.cwd()):
}

function runLog(rangeArgs: string, cwd: string): CommitContext[] {
const output = execSync(`git log --format=%H%x1f%B%x1f%D%x1e ${rangeArgs}`, {
const output = execSync(`git log --format=%H%x1f%B%x1f%D%x1f%P%x1e ${rangeArgs}`, {
cwd,
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf8",
Expand All @@ -394,14 +401,54 @@ function runLog(rangeArgs: string, cwd: string): CommitContext[] {
.map(parseCommitChunk);
}

/**
* Whether merge `commit` delivered net changes to the filtered paths, compared to
* its first parent (the branch it was merged into). Non-merges pass through.
*
* Without this, `--full-history` retains a stale branch's merge for paths it never
* touched — they differ across the merge only because the target branch advanced
* while the branch was open — leaking the issue key on the merge subject. The
* first-parent diff is empty for those and non-empty when the merge really
* delivered the paths, so a merge whose issue key lives only on its subject is
* still kept when it actually touched the paths. On an unexpected diff failure,
* keep the merge rather than drop real work.
*/
function mergeDeliversToPaths(commit: CommitContext, pathspec: string, cwd: string): boolean {
const parents = commit.parents ?? [];
if (parents.length <= 1) {
return true;
}
try {
execSync(`git diff --quiet ${parents[0]} ${commit.sha} ${pathspec}`, {
cwd,
stdio: ["ignore", "ignore", "pipe"],
});
return false;
} catch (e) {
const status = (e as { status?: number }).status;
if (status === 1) {
return true;
}
warn(
`Could not diff merge ${commit.sha.slice(0, 7)} against its first parent; keeping it under the path filter. ${
e instanceof Error ? e.message : String(e)
}`,
);
return true;
}
}

/**
* Returns commits between two SHAs, optionally filtered by file paths.
*
* `--full-history` (only when `includePaths` is set): a non-evil merge's
* tree equals one of its parents' trees, so under a pathspec it's TREESAME
* and git's default simplification drops it. That's true of every provider's
* merge commit (GitHub, GitLab MR, Bitbucket PR, plain `git merge --no-ff`)
* and would lose the issue keys encoded in their feature-branch names.
* and would lose the issue keys encoded in their feature-branch names. Merges
* it keeps are then passed through `mergeDeliversToPaths`, which drops a
* stale-branch merge that delivered no net change to the filtered paths so its
* subject key isn't attributed to a surface it never touched.
*
* `--no-walk` (only when `fromSha === toSha`): without it, `git log -1 <sha>
* -- <paths>` walks back from `<sha>` to the first ancestor matching the
Expand Down Expand Up @@ -432,14 +479,16 @@ export function getCommitContextsBetweenShas(
}

const inspectingSingleCommit = fromSha === toSha && inspectSingleCommit;
const pathspec = buildPathspecArgs(includePaths);
const args = [
includePaths?.length ? "--full-history" : "",
inspectingSingleCommit ? `--no-walk ${toSha}` : `${fromSha}..${toSha}`,
buildPathspecArgs(includePaths),
pathspec,
]
.filter(Boolean)
.join(" ");
const commits = runLog(args, cwd);
const logged = runLog(args, cwd);
const commits = pathspec ? logged.filter((commit) => mergeDeliversToPaths(commit, pathspec, cwd)) : logged;

if (commits.length === 0) {
if (inspectingSingleCommit) {
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export type CommitContext = {
sha: string;
branchName?: string | null;
message?: string | null;
parents?: string[];
};

export type GitInfo = {
Expand Down
Loading