Skip to content

Commit 06c379a

Browse files
GitBalakeblakebrendan-kellam
authored
fix(worker): prefer newest refs when applying the 64 revision cap (#1122)
* fix(worker): prefer newest refs when applying the 64 revision cap * test(worker): address review feedback * changelog fix --------- Co-authored-by: blake <blake.knudson@tcnbroadcasting.com> Co-authored-by: Brendan Kellam <brendan@sourcebot.dev>
1 parent 44a5db8 commit 06c379a

File tree

4 files changed

+285
-9
lines changed

4 files changed

+285
-9
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
- Fixed revision selection so the 64-revision cap prefers the newest matching branches and tags instead of pruning by ref-name order. [#1122](https://github.com/sourcebot-dev/sourcebot/pull/1122)
12+
1013
## [4.16.11] - 2026-04-17
1114

1215
### Changed

packages/backend/src/git.test.ts

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { mkdtemp, rm, writeFile } from "node:fs/promises";
2+
import { join } from "node:path";
3+
import { tmpdir } from "node:os";
4+
import { execFileSync } from "node:child_process";
5+
import { afterEach, describe, expect, test } from "vitest";
6+
import { getBranches, getTags } from "./git.js";
7+
8+
const runGit = (
9+
repoPath: string,
10+
args: string[],
11+
env: Record<string, string> = {},
12+
) => {
13+
execFileSync("git", args, {
14+
cwd: repoPath,
15+
env: {
16+
...process.env,
17+
...env,
18+
},
19+
stdio: "pipe",
20+
});
21+
};
22+
23+
const createTempRepo = async () => {
24+
const repoPath = await mkdtemp(join(tmpdir(), "sourcebot-git-test-"));
25+
26+
runGit(repoPath, ["init", "--initial-branch=main"]);
27+
runGit(repoPath, ["config", "user.name", "Sourcebot Test"]);
28+
runGit(repoPath, ["config", "user.email", "sourcebot@example.com"]);
29+
runGit(repoPath, ["config", "tag.sort", "refname"]);
30+
runGit(repoPath, ["config", "branch.sort", "refname"]);
31+
32+
return repoPath;
33+
};
34+
35+
const commitFile = async ({
36+
repoPath,
37+
fileName,
38+
content,
39+
message,
40+
timestamp,
41+
}: {
42+
repoPath: string;
43+
fileName: string;
44+
content: string;
45+
message: string;
46+
timestamp: string;
47+
}) => {
48+
await writeFile(join(repoPath, fileName), content);
49+
runGit(repoPath, ["add", fileName]);
50+
runGit(repoPath, ["commit", "-m", message], {
51+
GIT_AUTHOR_DATE: timestamp,
52+
GIT_COMMITTER_DATE: timestamp,
53+
});
54+
};
55+
56+
describe("git ref ordering", () => {
57+
const repoPaths: string[] = [];
58+
59+
afterEach(async () => {
60+
await Promise.all(
61+
repoPaths
62+
.splice(0)
63+
.map((repoPath) =>
64+
rm(repoPath, { recursive: true, force: true }),
65+
),
66+
);
67+
});
68+
69+
test("getTags returns newest tags first by creator date", async () => {
70+
const repoPath = await createTempRepo();
71+
repoPaths.push(repoPath);
72+
73+
await commitFile({
74+
repoPath,
75+
fileName: "README.md",
76+
content: "base\n",
77+
message: "initial commit",
78+
timestamp: "2024-01-01T00:00:00Z",
79+
});
80+
81+
runGit(repoPath, ["tag", "-a", "a-oldest", "-m", "oldest tag"], {
82+
GIT_COMMITTER_DATE: "2024-01-02T00:00:00Z",
83+
});
84+
runGit(repoPath, ["tag", "-a", "z-newest", "-m", "newest tag"], {
85+
GIT_COMMITTER_DATE: "2024-01-03T00:00:00Z",
86+
});
87+
88+
const tags = await getTags(repoPath);
89+
90+
expect(tags).toContain("z-newest");
91+
expect(tags).toContain("a-oldest");
92+
expect(tags.indexOf("z-newest")).toBeLessThan(tags.indexOf("a-oldest"));
93+
});
94+
95+
test("getBranches returns newest branches first by last commit date", async () => {
96+
const repoPath = await createTempRepo();
97+
repoPaths.push(repoPath);
98+
99+
await commitFile({
100+
repoPath,
101+
fileName: "README.md",
102+
content: "base\n",
103+
message: "initial commit",
104+
timestamp: "2024-01-01T00:00:00Z",
105+
});
106+
107+
runGit(repoPath, ["checkout", "-b", "aaa-oldest"]);
108+
await commitFile({
109+
repoPath,
110+
fileName: "oldest.txt",
111+
content: "oldest\n",
112+
message: "oldest branch commit",
113+
timestamp: "2024-01-02T00:00:00Z",
114+
});
115+
116+
runGit(repoPath, ["checkout", "main"]);
117+
runGit(repoPath, ["checkout", "-b", "zzz-newest"]);
118+
await commitFile({
119+
repoPath,
120+
fileName: "newest.txt",
121+
content: "newest\n",
122+
message: "newest branch commit",
123+
timestamp: "2024-01-03T00:00:00Z",
124+
});
125+
126+
runGit(repoPath, ["checkout", "main"]);
127+
128+
const branches = await getBranches(repoPath);
129+
130+
expect(branches).toContain("zzz-newest");
131+
expect(branches).toContain("aaa-oldest");
132+
expect(branches.indexOf("zzz-newest")).toBeLessThan(
133+
branches.indexOf("aaa-oldest"),
134+
);
135+
});
136+
});

packages/backend/src/git.ts

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,17 +285,48 @@ export const getOriginUrl = async (path: string) => {
285285
}
286286
}
287287

288-
export const getBranches = async (path: string) => {
288+
const parseRefNames = (refs: string) =>
289+
refs
290+
.split('\n')
291+
.map((ref) => ref.trim())
292+
.filter(Boolean);
293+
294+
const getSortedRefs = async ({
295+
path,
296+
sort,
297+
refNamespace,
298+
}: {
299+
path: string,
300+
sort: string,
301+
refNamespace: 'refs/heads' | 'refs/tags',
302+
}) => {
289303
const git = createGitClientForPath(path);
290-
const branches = await git.branch();
291-
return branches.all;
292-
}
304+
305+
return parseRefNames(
306+
await git.raw([
307+
"for-each-ref",
308+
`--sort=${sort}`,
309+
"--format=%(refname:short)",
310+
refNamespace,
311+
]),
312+
);
313+
};
314+
315+
export const getBranches = async (path: string) => {
316+
return getSortedRefs({
317+
path,
318+
sort: "-committerdate",
319+
refNamespace: "refs/heads",
320+
});
321+
};
293322

294323
export const getTags = async (path: string) => {
295-
const git = createGitClientForPath(path);
296-
const tags = await git.tags();
297-
return tags.all;
298-
}
324+
return getSortedRefs({
325+
path,
326+
sort: "-creatordate",
327+
refNamespace: "refs/tags",
328+
});
329+
};
299330

300331
export const getCommitHashForRefName = async ({
301332
path,

packages/backend/src/repoIndexManager.test.ts

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,13 @@ vi.mock('redlock', () => ({
131131
import { existsSync } from 'fs';
132132
import { readdir, rm } from 'fs/promises';
133133
import { ExecutionError } from 'redlock';
134-
import { cloneRepository, fetchRepository, isPathAValidGitRepoRoot } from './git.js';
134+
import {
135+
cloneRepository,
136+
fetchRepository,
137+
getBranches,
138+
getTags,
139+
isPathAValidGitRepoRoot,
140+
} from './git.js';
135141
import { RepoIndexManager } from './repoIndexManager.js';
136142
import { indexGitRepository } from './zoekt.js';
137143

@@ -410,6 +416,106 @@ describe('RepoIndexManager', () => {
410416
);
411417
});
412418

419+
test('keeps default branch and truncates to the first 63 matching tags', async () => {
420+
const newestTagsFirst = Array.from(
421+
{ length: 70 },
422+
(_, index) => `v${70 - index}.0.0`,
423+
);
424+
const repo = createMockRepoWithConnections({
425+
metadata: {
426+
tags: ['**'],
427+
},
428+
});
429+
(existsSync as Mock).mockReturnValue(true);
430+
(getTags as Mock).mockResolvedValue(newestTagsFirst);
431+
432+
manager = new RepoIndexManager(mockPrisma, mockSettings, mockRedis, mockPromClient as any);
433+
434+
(mockPrisma.repoIndexingJob.findUniqueOrThrow as Mock).mockResolvedValue({
435+
status: RepoIndexingJobStatus.PENDING,
436+
});
437+
(mockPrisma.repoIndexingJob.update as Mock).mockResolvedValue({
438+
type: RepoIndexingJobType.INDEX,
439+
repo,
440+
});
441+
442+
const mockJob = {
443+
data: {
444+
jobId: 'job-1',
445+
type: 'INDEX',
446+
repoId: repo.id,
447+
repoName: repo.name,
448+
},
449+
moveToDelayed: vi.fn(),
450+
} as unknown as Job;
451+
452+
const { Worker } = await import('bullmq');
453+
const processor = (Worker as unknown as Mock).mock.calls[0][1];
454+
await processor(mockJob);
455+
456+
expect(indexGitRepository).toHaveBeenCalledWith(
457+
repo,
458+
mockSettings,
459+
[
460+
'refs/heads/main',
461+
...newestTagsFirst
462+
.slice(0, 63)
463+
.map((tag) => `refs/tags/${tag}`),
464+
],
465+
expect.any(Object)
466+
);
467+
});
468+
469+
test('de-duplicates the default branch before truncating matching branches', async () => {
470+
const newestBranchesFirst = [
471+
'feature/newest',
472+
'main',
473+
...Array.from(
474+
{ length: 68 },
475+
(_, index) => `feature/${68 - index}`,
476+
),
477+
];
478+
const repo = createMockRepoWithConnections({
479+
metadata: {
480+
branches: ['main', 'feature/**'],
481+
},
482+
});
483+
(existsSync as Mock).mockReturnValue(true);
484+
(getBranches as Mock).mockResolvedValue(newestBranchesFirst);
485+
486+
manager = new RepoIndexManager(mockPrisma, mockSettings, mockRedis, mockPromClient as any);
487+
488+
(mockPrisma.repoIndexingJob.findUniqueOrThrow as Mock).mockResolvedValue({
489+
status: RepoIndexingJobStatus.PENDING,
490+
});
491+
(mockPrisma.repoIndexingJob.update as Mock).mockResolvedValue({
492+
type: RepoIndexingJobType.INDEX,
493+
repo,
494+
});
495+
496+
const mockJob = {
497+
data: {
498+
jobId: 'job-1',
499+
type: 'INDEX',
500+
repoId: repo.id,
501+
repoName: repo.name,
502+
},
503+
moveToDelayed: vi.fn(),
504+
} as unknown as Job;
505+
506+
const { Worker } = await import('bullmq');
507+
const processor = (Worker as unknown as Mock).mock.calls[0][1];
508+
await processor(mockJob);
509+
510+
const revisions = (indexGitRepository as Mock).mock.calls.at(-1)?.[2] as string[];
511+
512+
expect(revisions).toHaveLength(64);
513+
expect(revisions.filter((revision) => revision === 'refs/heads/main')).toHaveLength(1);
514+
expect(revisions[0]).toBe('refs/heads/main');
515+
expect(revisions).toContain('refs/heads/feature/newest');
516+
expect(revisions).not.toContain('refs/heads/feature/6');
517+
});
518+
413519
test('updates repo.indexedAt and indexedCommitHash on completion', async () => {
414520
const repo = createMockRepoWithConnections();
415521
(existsSync as Mock).mockReturnValue(true);

0 commit comments

Comments
 (0)