Skip to content

Commit 96ab707

Browse files
authored
Ensure merges are properly handled for detection of issues & PRs (#62)
* Ensure merges are properly handled for detection of issues & PRs * Better comments * More targeted test
1 parent 17b7854 commit 96ab707

3 files changed

Lines changed: 370 additions & 96 deletions

File tree

src/git.test.ts

Lines changed: 295 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import {
1111
extractBranchNameFromMergeMessage,
1212
getCommitContext,
1313
getCommitContextsBetweenShas,
14+
getCommitParents,
1415
getRepoInfo,
15-
isMergeCommit,
1616
normalizePathspec,
1717
parseRepoUrl,
18+
resolveFirstSyncBoundary,
1819
} from "./git";
1920

2021
describe("normalizePathspec", () => {
@@ -422,6 +423,25 @@ type TempRepoWithMerge = {
422423
};
423424
};
424425

426+
type TempRepoWithMultipleMerges = {
427+
cwd: string;
428+
commits: {
429+
base: string;
430+
merge100: string; // Merge of feature/LIN-100 (touches frontend/)
431+
merge200: string; // Merge of feature/LIN-200 (touches backend/)
432+
merge300: string; // Merge of feature/LIN-300 (touches infra/ — outside includePaths)
433+
headMerge: string; // Merge of release branch into main
434+
};
435+
};
436+
437+
type TempRepoReleaseBranch = {
438+
cwd: string;
439+
commits: {
440+
base: string;
441+
headMerge: string; // The rel-branch → main merge (HEAD)
442+
};
443+
};
444+
425445
function runGit(command: string, cwd: string): string {
426446
return execSync(`git ${command}`, {
427447
cwd,
@@ -430,6 +450,53 @@ function runGit(command: string, cwd: string): string {
430450
}).trim();
431451
}
432452

453+
/**
454+
* Initializes a tmpdir repo, configures user, creates the listed directories,
455+
* lands a seed commit, and renames the branch to `main`. Returns the cwd and
456+
* base SHA.
457+
*/
458+
function initTempRepo(opts: { prefix: string; dirs: string[]; seedFile: { path: string; content: string } }): {
459+
cwd: string;
460+
base: string;
461+
} {
462+
const cwd = mkdtempSync(join(tmpdir(), opts.prefix));
463+
runGit("init", cwd);
464+
runGit('config user.email "test@example.com"', cwd);
465+
runGit('config user.name "Test User"', cwd);
466+
for (const dir of opts.dirs) {
467+
mkdirSync(join(cwd, dir), { recursive: true });
468+
}
469+
writeFileSync(join(cwd, opts.seedFile.path), opts.seedFile.content);
470+
runGit("add .", cwd);
471+
runGit('commit -m "Initial"', cwd);
472+
runGit("branch -M main", cwd);
473+
return { cwd, base: runGit("rev-parse HEAD", cwd) };
474+
}
475+
476+
/**
477+
* Cuts `branch` off `baseBranch`, lands one file change, merges back via
478+
* `--no-ff` with a GitHub-style PR-merge message, then deletes `branch` to
479+
* mirror a CI checkout (merged feature branches gone). Returns the merge SHA.
480+
*/
481+
function mergeFeatureBranch(opts: {
482+
cwd: string;
483+
baseBranch: string;
484+
branch: string;
485+
file: string;
486+
prNumber: number;
487+
}): string {
488+
const { cwd, baseBranch, branch, file, prNumber } = opts;
489+
runGit(`checkout -b ${branch} ${baseBranch}`, cwd);
490+
writeFileSync(join(cwd, file), "x");
491+
runGit("add .", cwd);
492+
runGit(`commit -m "feature work on ${branch}"`, cwd);
493+
runGit(`checkout ${baseBranch}`, cwd);
494+
runGit(`merge --no-ff ${branch} -m "Merge pull request #${prNumber} from owner/${branch}"`, cwd);
495+
const sha = runGit("rev-parse HEAD", cwd);
496+
runGit(`branch -D ${branch}`, cwd);
497+
return sha;
498+
}
499+
433500
/**
434501
* Build a deterministic git repo for integration tests.
435502
*
@@ -521,6 +588,96 @@ function createTempRepoWithMerge(): TempRepoWithMerge {
521588
return { cwd, commits: { base, featureBranch, mergeCommit } };
522589
}
523590

591+
/**
592+
* Three feature branches merged into main, then a release branch with one
593+
* commit merged back as HEAD. `merge300` touches `infra/` only.
594+
*/
595+
function createTempRepoWithMultipleMerges(): TempRepoWithMultipleMerges {
596+
const { cwd, base } = initTempRepo({
597+
prefix: "linear-release-multi-merge-",
598+
dirs: ["frontend", "backend", "infra"],
599+
seedFile: { path: "frontend/seed.txt", content: "seed" },
600+
});
601+
602+
const merge100 = mergeFeatureBranch({
603+
cwd,
604+
baseBranch: "main",
605+
branch: "feature/LIN-100-add-foo",
606+
file: "frontend/foo.txt",
607+
prNumber: 100,
608+
});
609+
const merge200 = mergeFeatureBranch({
610+
cwd,
611+
baseBranch: "main",
612+
branch: "feature/LIN-200-fix-bar",
613+
file: "backend/bar.txt",
614+
prNumber: 200,
615+
});
616+
const merge300 = mergeFeatureBranch({
617+
cwd,
618+
baseBranch: "main",
619+
branch: "feature/LIN-300-infra",
620+
file: "infra/three.txt",
621+
prNumber: 300,
622+
});
623+
624+
// rel branch needs at least one of its own commits, otherwise --no-ff is a
625+
// no-op when the branches are identical.
626+
runGit("checkout -b rel/2026-05-06 main", cwd);
627+
writeFileSync(join(cwd, "frontend", "release-notes.txt"), "notes");
628+
runGit("add .", cwd);
629+
runGit('commit -m "release notes"', cwd);
630+
runGit("checkout main", cwd);
631+
runGit('merge --no-ff rel/2026-05-06 -m "Merge pull request #324 from owner/rel/2026-05-06"', cwd);
632+
const headMerge = runGit("rev-parse HEAD", cwd);
633+
runGit("branch -D rel/2026-05-06", cwd);
634+
635+
return { cwd, commits: { base, merge100, merge200, merge300, headMerge } };
636+
}
637+
638+
/**
639+
* Release-branch workflow: features merged INTO `rel/2026-05-06`, then rel
640+
* merged into main as HEAD. `feature/LIN-300-mobile` touches `mobile-android/`
641+
* only.
642+
*/
643+
function createTempRepoReleaseBranch(): TempRepoReleaseBranch {
644+
const { cwd, base } = initTempRepo({
645+
prefix: "linear-release-rel-branch-",
646+
dirs: ["frontend-nuxt3", "backend", "mobile-android"],
647+
seedFile: { path: "frontend-nuxt3/seed.ts", content: "seed" },
648+
});
649+
650+
runGit("checkout -b rel/2026-05-06 main", cwd);
651+
mergeFeatureBranch({
652+
cwd,
653+
baseBranch: "rel/2026-05-06",
654+
branch: "feature/LIN-100-foo",
655+
file: "frontend-nuxt3/foo.ts",
656+
prNumber: 100,
657+
});
658+
mergeFeatureBranch({
659+
cwd,
660+
baseBranch: "rel/2026-05-06",
661+
branch: "feature/LIN-200-bar",
662+
file: "backend/bar.ts",
663+
prNumber: 200,
664+
});
665+
mergeFeatureBranch({
666+
cwd,
667+
baseBranch: "rel/2026-05-06",
668+
branch: "feature/LIN-300-mobile",
669+
file: "mobile-android/m.kt",
670+
prNumber: 300,
671+
});
672+
673+
runGit("checkout main", cwd);
674+
runGit('merge --no-ff rel/2026-05-06 -m "Merge pull request #324 from owner/rel/2026-05-06"', cwd);
675+
const headMerge = runGit("rev-parse HEAD", cwd);
676+
runGit("branch -D rel/2026-05-06", cwd);
677+
678+
return { cwd, commits: { base, headMerge } };
679+
}
680+
524681
describe("getCommitContextsBetweenShas", () => {
525682
let repo: TempRepo;
526683

@@ -652,8 +809,9 @@ describe("getCommitContextsBetweenShas", () => {
652809
try {
653810
process.chdir(join(repo.cwd, "src"));
654811

655-
// Without the fix, this would fail because git would look for "src/**" relative to
656-
// the subdirectory (i.e., src/src/**) which doesn't exist
812+
// The `:(top,...)` magic prefix in buildPathspecArgs anchors the glob
813+
// at the repo root regardless of cwd; without it git would resolve
814+
// "src/**" against the subdirectory (i.e., src/src/**).
657815
const result = getCommitContextsBetweenShas(
658816
repo.commits.first,
659817
repo.commits.third,
@@ -697,21 +855,6 @@ describe("merge commit handling", () => {
697855
rmSync(mergeRepo.cwd, { recursive: true, force: true });
698856
});
699857

700-
describe("isMergeCommit", () => {
701-
it("should return true for a merge commit", () => {
702-
expect(isMergeCommit(mergeRepo.commits.mergeCommit, mergeRepo.cwd)).toBe(true);
703-
});
704-
705-
it("should return false for a regular commit", () => {
706-
expect(isMergeCommit(mergeRepo.commits.featureBranch, mergeRepo.cwd)).toBe(false);
707-
expect(isMergeCommit(mergeRepo.commits.base, mergeRepo.cwd)).toBe(false);
708-
});
709-
710-
it("should return false for invalid SHA", () => {
711-
expect(isMergeCommit("invalid-sha", mergeRepo.cwd)).toBe(false);
712-
});
713-
});
714-
715858
describe("getCommitContext", () => {
716859
it("should return commit context for a valid SHA", () => {
717860
const context = getCommitContext(mergeRepo.commits.mergeCommit, mergeRepo.cwd);
@@ -733,17 +876,52 @@ describe("merge commit handling", () => {
733876
});
734877
});
735878

879+
describe("getCommitParents", () => {
880+
it("returns 2 parents for a merge commit", () => {
881+
const parents = getCommitParents(mergeRepo.commits.mergeCommit, mergeRepo.cwd);
882+
expect(parents).toEqual([mergeRepo.commits.base, mergeRepo.commits.featureBranch]);
883+
});
884+
885+
it("returns 1 parent for a regular commit", () => {
886+
expect(getCommitParents(mergeRepo.commits.featureBranch, mergeRepo.cwd)).toEqual([mergeRepo.commits.base]);
887+
});
888+
889+
it("returns [] for the root commit", () => {
890+
expect(getCommitParents(mergeRepo.commits.base, mergeRepo.cwd)).toEqual([]);
891+
});
892+
893+
it("returns [] for an unknown SHA", () => {
894+
expect(getCommitParents("0000000000000000000000000000000000000000", mergeRepo.cwd)).toEqual([]);
895+
});
896+
});
897+
898+
describe("resolveFirstSyncBoundary", () => {
899+
it("expands to HEAD^1 when HEAD is a merge commit", () => {
900+
expect(resolveFirstSyncBoundary(mergeRepo.commits.mergeCommit, mergeRepo.cwd)).toBe(mergeRepo.commits.base);
901+
});
902+
903+
it("returns the commit itself when HEAD is a regular commit", () => {
904+
expect(resolveFirstSyncBoundary(mergeRepo.commits.featureBranch, mergeRepo.cwd)).toBe(
905+
mergeRepo.commits.featureBranch,
906+
);
907+
});
908+
909+
it("returns the commit itself when HEAD is the root commit", () => {
910+
expect(resolveFirstSyncBoundary(mergeRepo.commits.base, mergeRepo.cwd)).toBe(mergeRepo.commits.base);
911+
});
912+
});
913+
736914
describe("getCommitContextsBetweenShas with merge commits", () => {
737915
it("should include merge commit when path filtering would exclude it", () => {
738-
// Without the fix, path filtering for "src/**" would only return the feature branch commit
739-
// because merge commits don't have direct file changes.
740-
// With the fix, the merge commit should be included for metadata extraction.
916+
// The merge node itself adds no file changes, so default simplification
917+
// would drop it; `--full-history` keeps it for metadata (PR number,
918+
// branch name) extraction.
741919
const result = getCommitContextsBetweenShas(mergeRepo.commits.base, mergeRepo.commits.mergeCommit, {
742920
includePaths: ["src/**"],
743921
cwd: mergeRepo.cwd,
744922
});
745923

746-
// Should include both: the merge commit (for metadata) and the feature commit (for file changes)
924+
// Both the merge (for metadata) and the feature commit (for file changes).
747925
expect(result.length).toBeGreaterThanOrEqual(2);
748926

749927
// The merge commit should be first (unshifted)
@@ -768,6 +946,101 @@ describe("merge commit handling", () => {
768946
expect(mergeCommitCount).toBe(1);
769947
});
770948
});
949+
950+
describe("getCommitContextsBetweenShas with multiple merges in range", () => {
951+
let multiRepo: TempRepoWithMultipleMerges;
952+
953+
beforeAll(() => {
954+
multiRepo = createTempRepoWithMultipleMerges();
955+
});
956+
957+
afterAll(() => {
958+
rmSync(multiRepo.cwd, { recursive: true, force: true });
959+
});
960+
961+
it("should return in-path merges and drop out-of-path merges across a multi-merge range", () => {
962+
// `--full-history` keeps merges whose contribution arrived via a non-
963+
// first parent. Their tree equals one parent's, so default simplification
964+
// would drop them — and with them the issue keys in their branch names.
965+
const result = getCommitContextsBetweenShas(multiRepo.commits.base, multiRepo.commits.headMerge, {
966+
includePaths: ["frontend/**", "backend/**"],
967+
cwd: multiRepo.cwd,
968+
});
969+
970+
const shas = new Set(result.map((c) => c.sha));
971+
expect(shas.has(multiRepo.commits.merge100)).toBe(true);
972+
expect(shas.has(multiRepo.commits.merge200)).toBe(true);
973+
// merge300 only touched infra/ — kept by the merges-only scan, then dropped
974+
// by commitTouchesPaths so LIN-300 doesn't leak into a frontend release.
975+
expect(shas.has(multiRepo.commits.merge300)).toBe(false);
976+
expect(shas.has(multiRepo.commits.headMerge)).toBe(true);
977+
978+
const branchNames = result.map((c) => c.branchName).filter((b): b is string => !!b);
979+
expect(branchNames).toEqual(
980+
expect.arrayContaining(["feature/LIN-100-add-foo", "feature/LIN-200-fix-bar", "rel/2026-05-06"]),
981+
);
982+
expect(branchNames).not.toContain("feature/LIN-300-infra");
983+
});
984+
985+
it("should return HEAD merge commit when fromSha === toSha and HEAD is a merge", () => {
986+
const result = getCommitContextsBetweenShas(multiRepo.commits.headMerge, multiRepo.commits.headMerge, {
987+
includePaths: ["frontend/**", "backend/**"],
988+
cwd: multiRepo.cwd,
989+
});
990+
991+
const headResult = result.find((c) => c.sha === multiRepo.commits.headMerge);
992+
expect(headResult).toBeDefined();
993+
expect(headResult?.branchName).toBe("rel/2026-05-06");
994+
});
995+
996+
it("should not drift to an unrelated ancestor when fromSha === toSha and HEAD is outside includePaths", () => {
997+
// `git log -1 <sha> -- <paths>` walks back from <sha> until something
998+
// matches the pathspec — `--no-walk` makes it return only <sha>, or
999+
// nothing if <sha> doesn't match.
1000+
const result = getCommitContextsBetweenShas(multiRepo.commits.merge300, multiRepo.commits.merge300, {
1001+
includePaths: ["frontend/**"],
1002+
cwd: multiRepo.cwd,
1003+
});
1004+
1005+
expect(result).toEqual([]);
1006+
});
1007+
});
1008+
1009+
describe("getCommitContextsBetweenShas with release-branch workflow", () => {
1010+
// First sync (no prior release SHA) on a merge HEAD: scanning HEAD alone
1011+
// finds no keys because HEAD's branch is the rel branch, not any feature.
1012+
// Caller passes HEAD^1 as the boundary so the rel branch's contents are in
1013+
// range.
1014+
let relRepo: TempRepoReleaseBranch;
1015+
1016+
beforeAll(() => {
1017+
relRepo = createTempRepoReleaseBranch();
1018+
});
1019+
1020+
afterAll(() => {
1021+
rmSync(relRepo.cwd, { recursive: true, force: true });
1022+
});
1023+
1024+
it("should surface feature merges from inside the rel branch when scanning the resolved first-sync boundary", () => {
1025+
// Mirrors the customer's first-sync flow: resolveFirstSyncBoundary picks
1026+
// HEAD^1 because HEAD is a merge, then getCommitContextsBetweenShas runs
1027+
// over that range.
1028+
const boundary = resolveFirstSyncBoundary(relRepo.commits.headMerge, relRepo.cwd);
1029+
expect(boundary).not.toBe(relRepo.commits.headMerge);
1030+
1031+
const result = getCommitContextsBetweenShas(boundary, relRepo.commits.headMerge, {
1032+
includePaths: ["frontend-nuxt3/**", "backend/**"],
1033+
cwd: relRepo.cwd,
1034+
});
1035+
1036+
const branchNames = result.map((c) => c.branchName).filter((b): b is string => !!b);
1037+
expect(branchNames).toEqual(
1038+
expect.arrayContaining(["feature/LIN-100-foo", "feature/LIN-200-bar", "rel/2026-05-06"]),
1039+
);
1040+
// LIN-300 is mobile-only — outside the path filter — must not leak.
1041+
expect(branchNames).not.toContain("feature/LIN-300-mobile");
1042+
});
1043+
});
7711044
});
7721045

7731046
describe("assertGitAvailable", () => {

0 commit comments

Comments
 (0)