diff --git a/CHANGELOG.md b/CHANGELOG.md index 963f56fd7..1a37c5e36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- 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) + ## [4.16.11] - 2026-04-17 ### Changed diff --git a/packages/backend/src/git.test.ts b/packages/backend/src/git.test.ts new file mode 100644 index 000000000..c9c32cf57 --- /dev/null +++ b/packages/backend/src/git.test.ts @@ -0,0 +1,136 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { execFileSync } from "node:child_process"; +import { afterEach, describe, expect, test } from "vitest"; +import { getBranches, getTags } from "./git.js"; + +const runGit = ( + repoPath: string, + args: string[], + env: Record = {}, +) => { + execFileSync("git", args, { + cwd: repoPath, + env: { + ...process.env, + ...env, + }, + stdio: "pipe", + }); +}; + +const createTempRepo = async () => { + const repoPath = await mkdtemp(join(tmpdir(), "sourcebot-git-test-")); + + runGit(repoPath, ["init", "--initial-branch=main"]); + runGit(repoPath, ["config", "user.name", "Sourcebot Test"]); + runGit(repoPath, ["config", "user.email", "sourcebot@example.com"]); + runGit(repoPath, ["config", "tag.sort", "refname"]); + runGit(repoPath, ["config", "branch.sort", "refname"]); + + return repoPath; +}; + +const commitFile = async ({ + repoPath, + fileName, + content, + message, + timestamp, +}: { + repoPath: string; + fileName: string; + content: string; + message: string; + timestamp: string; +}) => { + await writeFile(join(repoPath, fileName), content); + runGit(repoPath, ["add", fileName]); + runGit(repoPath, ["commit", "-m", message], { + GIT_AUTHOR_DATE: timestamp, + GIT_COMMITTER_DATE: timestamp, + }); +}; + +describe("git ref ordering", () => { + const repoPaths: string[] = []; + + afterEach(async () => { + await Promise.all( + repoPaths + .splice(0) + .map((repoPath) => + rm(repoPath, { recursive: true, force: true }), + ), + ); + }); + + test("getTags returns newest tags first by creator date", async () => { + const repoPath = await createTempRepo(); + repoPaths.push(repoPath); + + await commitFile({ + repoPath, + fileName: "README.md", + content: "base\n", + message: "initial commit", + timestamp: "2024-01-01T00:00:00Z", + }); + + runGit(repoPath, ["tag", "-a", "a-oldest", "-m", "oldest tag"], { + GIT_COMMITTER_DATE: "2024-01-02T00:00:00Z", + }); + runGit(repoPath, ["tag", "-a", "z-newest", "-m", "newest tag"], { + GIT_COMMITTER_DATE: "2024-01-03T00:00:00Z", + }); + + const tags = await getTags(repoPath); + + expect(tags).toContain("z-newest"); + expect(tags).toContain("a-oldest"); + expect(tags.indexOf("z-newest")).toBeLessThan(tags.indexOf("a-oldest")); + }); + + test("getBranches returns newest branches first by last commit date", async () => { + const repoPath = await createTempRepo(); + repoPaths.push(repoPath); + + await commitFile({ + repoPath, + fileName: "README.md", + content: "base\n", + message: "initial commit", + timestamp: "2024-01-01T00:00:00Z", + }); + + runGit(repoPath, ["checkout", "-b", "aaa-oldest"]); + await commitFile({ + repoPath, + fileName: "oldest.txt", + content: "oldest\n", + message: "oldest branch commit", + timestamp: "2024-01-02T00:00:00Z", + }); + + runGit(repoPath, ["checkout", "main"]); + runGit(repoPath, ["checkout", "-b", "zzz-newest"]); + await commitFile({ + repoPath, + fileName: "newest.txt", + content: "newest\n", + message: "newest branch commit", + timestamp: "2024-01-03T00:00:00Z", + }); + + runGit(repoPath, ["checkout", "main"]); + + const branches = await getBranches(repoPath); + + expect(branches).toContain("zzz-newest"); + expect(branches).toContain("aaa-oldest"); + expect(branches.indexOf("zzz-newest")).toBeLessThan( + branches.indexOf("aaa-oldest"), + ); + }); +}); diff --git a/packages/backend/src/git.ts b/packages/backend/src/git.ts index 9dc838fa0..523143538 100644 --- a/packages/backend/src/git.ts +++ b/packages/backend/src/git.ts @@ -285,17 +285,48 @@ export const getOriginUrl = async (path: string) => { } } -export const getBranches = async (path: string) => { +const parseRefNames = (refs: string) => + refs + .split('\n') + .map((ref) => ref.trim()) + .filter(Boolean); + +const getSortedRefs = async ({ + path, + sort, + refNamespace, +}: { + path: string, + sort: string, + refNamespace: 'refs/heads' | 'refs/tags', +}) => { const git = createGitClientForPath(path); - const branches = await git.branch(); - return branches.all; -} + + return parseRefNames( + await git.raw([ + "for-each-ref", + `--sort=${sort}`, + "--format=%(refname:short)", + refNamespace, + ]), + ); +}; + +export const getBranches = async (path: string) => { + return getSortedRefs({ + path, + sort: "-committerdate", + refNamespace: "refs/heads", + }); +}; export const getTags = async (path: string) => { - const git = createGitClientForPath(path); - const tags = await git.tags(); - return tags.all; -} + return getSortedRefs({ + path, + sort: "-creatordate", + refNamespace: "refs/tags", + }); +}; export const getCommitHashForRefName = async ({ path, diff --git a/packages/backend/src/repoIndexManager.test.ts b/packages/backend/src/repoIndexManager.test.ts index a3abe158d..3a65d092f 100644 --- a/packages/backend/src/repoIndexManager.test.ts +++ b/packages/backend/src/repoIndexManager.test.ts @@ -131,7 +131,13 @@ vi.mock('redlock', () => ({ import { existsSync } from 'fs'; import { readdir, rm } from 'fs/promises'; import { ExecutionError } from 'redlock'; -import { cloneRepository, fetchRepository, isPathAValidGitRepoRoot } from './git.js'; +import { + cloneRepository, + fetchRepository, + getBranches, + getTags, + isPathAValidGitRepoRoot, +} from './git.js'; import { RepoIndexManager } from './repoIndexManager.js'; import { indexGitRepository } from './zoekt.js'; @@ -410,6 +416,106 @@ describe('RepoIndexManager', () => { ); }); + test('keeps default branch and truncates to the first 63 matching tags', async () => { + const newestTagsFirst = Array.from( + { length: 70 }, + (_, index) => `v${70 - index}.0.0`, + ); + const repo = createMockRepoWithConnections({ + metadata: { + tags: ['**'], + }, + }); + (existsSync as Mock).mockReturnValue(true); + (getTags as Mock).mockResolvedValue(newestTagsFirst); + + manager = new RepoIndexManager(mockPrisma, mockSettings, mockRedis, mockPromClient as any); + + (mockPrisma.repoIndexingJob.findUniqueOrThrow as Mock).mockResolvedValue({ + status: RepoIndexingJobStatus.PENDING, + }); + (mockPrisma.repoIndexingJob.update as Mock).mockResolvedValue({ + type: RepoIndexingJobType.INDEX, + repo, + }); + + const mockJob = { + data: { + jobId: 'job-1', + type: 'INDEX', + repoId: repo.id, + repoName: repo.name, + }, + moveToDelayed: vi.fn(), + } as unknown as Job; + + const { Worker } = await import('bullmq'); + const processor = (Worker as unknown as Mock).mock.calls[0][1]; + await processor(mockJob); + + expect(indexGitRepository).toHaveBeenCalledWith( + repo, + mockSettings, + [ + 'refs/heads/main', + ...newestTagsFirst + .slice(0, 63) + .map((tag) => `refs/tags/${tag}`), + ], + expect.any(Object) + ); + }); + + test('de-duplicates the default branch before truncating matching branches', async () => { + const newestBranchesFirst = [ + 'feature/newest', + 'main', + ...Array.from( + { length: 68 }, + (_, index) => `feature/${68 - index}`, + ), + ]; + const repo = createMockRepoWithConnections({ + metadata: { + branches: ['main', 'feature/**'], + }, + }); + (existsSync as Mock).mockReturnValue(true); + (getBranches as Mock).mockResolvedValue(newestBranchesFirst); + + manager = new RepoIndexManager(mockPrisma, mockSettings, mockRedis, mockPromClient as any); + + (mockPrisma.repoIndexingJob.findUniqueOrThrow as Mock).mockResolvedValue({ + status: RepoIndexingJobStatus.PENDING, + }); + (mockPrisma.repoIndexingJob.update as Mock).mockResolvedValue({ + type: RepoIndexingJobType.INDEX, + repo, + }); + + const mockJob = { + data: { + jobId: 'job-1', + type: 'INDEX', + repoId: repo.id, + repoName: repo.name, + }, + moveToDelayed: vi.fn(), + } as unknown as Job; + + const { Worker } = await import('bullmq'); + const processor = (Worker as unknown as Mock).mock.calls[0][1]; + await processor(mockJob); + + const revisions = (indexGitRepository as Mock).mock.calls.at(-1)?.[2] as string[]; + + expect(revisions).toHaveLength(64); + expect(revisions.filter((revision) => revision === 'refs/heads/main')).toHaveLength(1); + expect(revisions[0]).toBe('refs/heads/main'); + expect(revisions).toContain('refs/heads/feature/newest'); + expect(revisions).not.toContain('refs/heads/feature/6'); + }); + test('updates repo.indexedAt and indexedCommitHash on completion', async () => { const repo = createMockRepoWithConnections(); (existsSync as Mock).mockReturnValue(true);