-
Notifications
You must be signed in to change notification settings - Fork 261
fix(web): restore ServiceError boundary in getFileSourceForRepo
#1145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
brendan-kellam
merged 4 commits into
sourcebot-dev:main
from
fatmcgav:fix-ai-agent-permissions
Apr 23, 2026
+382
−5
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,329 @@ | ||
| import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; | ||
|
|
||
| // Mocks must be declared before imports | ||
| vi.mock('simple-git'); | ||
| vi.mock('@sourcebot/shared', () => ({ | ||
| getRepoPath: (repo: { id: number }) => ({ | ||
| path: `/mock/repos/${repo.id}`, | ||
| }), | ||
| env: { | ||
| AUTH_URL: 'https://sourcebot.example.com', | ||
| }, | ||
| createLogger: () => ({ | ||
| debug: vi.fn(), | ||
| info: vi.fn(), | ||
| warn: vi.fn(), | ||
| error: vi.fn(), | ||
| }), | ||
| })); | ||
| vi.mock('@/lib/utils', () => ({ | ||
| getCodeHostBrowseFileAtBranchUrl: vi.fn().mockReturnValue('https://github.com/owner/repo/blob/main/src/index.ts'), | ||
| isServiceError: (obj: unknown): boolean => { | ||
| return obj !== null && typeof obj === 'object' && 'errorCode' in (obj as object); | ||
| }, | ||
| })); | ||
| vi.mock('@/app/(app)/browse/hooks/utils', () => ({ | ||
| getBrowsePath: vi.fn().mockReturnValue('/browse/github.com/owner/repo/blob/main/src/index.ts'), | ||
| })); | ||
| vi.mock('@/lib/gitattributes', () => ({ | ||
| parseGitAttributes: vi.fn().mockReturnValue({}), | ||
| resolveLanguageFromGitAttributes: vi.fn().mockReturnValue(undefined), | ||
| })); | ||
| vi.mock('@/lib/languageDetection', () => ({ | ||
| detectLanguageFromFilename: vi.fn().mockReturnValue('TypeScript'), | ||
| })); | ||
| // Required for module load; not exercised by getFileSourceForRepo directly | ||
| vi.mock('next/headers', () => ({ | ||
| headers: vi.fn().mockResolvedValue(new Headers()), | ||
| })); | ||
| vi.mock('@/middleware/sew', () => ({ | ||
| sew: async <T>(fn: () => Promise<T> | T): Promise<T> => fn(), | ||
| })); | ||
| vi.mock('@/middleware/withAuth', () => ({ | ||
| withOptionalAuth: vi.fn(), | ||
| })); | ||
| vi.mock('@/ee/features/audit/factory', () => ({ | ||
| getAuditService: () => ({ | ||
| createAudit: vi.fn(), | ||
| }), | ||
| })); | ||
|
|
||
| import { simpleGit } from 'simple-git'; | ||
| import { getFileSourceForRepo } from './getFileSourceApi'; | ||
|
|
||
| const MOCK_ORG = { id: 1, name: 'test-org' } as Parameters<typeof getFileSourceForRepo>[1]['org']; | ||
|
|
||
| const MOCK_REPO = { | ||
| id: 123, | ||
| name: 'github.com/owner/repo', | ||
| orgId: 1, | ||
| defaultBranch: 'main', | ||
| webUrl: 'https://github.com/owner/repo', | ||
| external_codeHostType: 'GITHUB', | ||
| displayName: null, | ||
| }; | ||
|
|
||
| describe('getFileSourceForRepo', () => { | ||
| const mockGitRaw = vi.fn(); | ||
| const mockCwd = vi.fn(); | ||
| const mockSimpleGit = simpleGit as unknown as Mock; | ||
| const mockFindFirst = vi.fn(); | ||
|
|
||
| const mockPrisma = { | ||
| repo: { findFirst: mockFindFirst }, | ||
| } as Parameters<typeof getFileSourceForRepo>[1]['prisma']; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
|
|
||
| mockCwd.mockReturnValue({ raw: mockGitRaw }); | ||
| mockSimpleGit.mockReturnValue({ cwd: mockCwd }); | ||
| mockFindFirst.mockResolvedValue(MOCK_REPO); | ||
|
|
||
| // Default: file show succeeds; .gitattributes not present | ||
| mockGitRaw.mockImplementation(async (args: string[]) => { | ||
| if (args[1]?.endsWith('.gitattributes')) { | ||
| throw new Error('does not exist in HEAD'); | ||
| } | ||
| return 'console.log("hello");'; | ||
| }); | ||
| }); | ||
|
|
||
| describe('repository validation', () => { | ||
| it('returns NOT_FOUND when repo is absent from the database', async () => { | ||
| mockFindFirst.mockResolvedValue(null); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'NOT_FOUND' }); | ||
| }); | ||
|
|
||
| it('queries the database by repo name and orgId', async () => { | ||
| await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(mockFindFirst).toHaveBeenCalledWith({ | ||
| where: { name: 'github.com/owner/repo', orgId: 1 }, | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('input validation', () => { | ||
| it('returns FILE_NOT_FOUND for path traversal attempts', async () => { | ||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/../../etc/passwd', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); | ||
| }); | ||
|
|
||
| it('returns FILE_NOT_FOUND for null-byte paths', async () => { | ||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/\0evil', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); | ||
| }); | ||
|
|
||
| it('returns INVALID_GIT_REF for refs starting with "-" (flag injection guard)', async () => { | ||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: '--upload-pack=evil' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('git error handling', () => { | ||
| it('returns FILE_NOT_FOUND when git reports the file does not exist', async () => { | ||
| mockGitRaw.mockRejectedValueOnce( | ||
| new Error("fatal: path 'src/missing.ts' does not exist in 'main'"), | ||
| ); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/missing.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); | ||
| }); | ||
|
|
||
| it('returns FILE_NOT_FOUND for "fatal: path" errors', async () => { | ||
| mockGitRaw.mockRejectedValueOnce(new Error('fatal: path not found')); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'FILE_NOT_FOUND' }); | ||
| }); | ||
|
|
||
| it('returns INVALID_GIT_REF when head_sha has not been fetched on the local clone ("unknown revision")', async () => { | ||
| // This is the scenario from the v4.16.14 regression: the review agent passes | ||
| // pr_payload.head_sha as ref, but the bare clone hasn't fetched it yet. | ||
| mockGitRaw.mockRejectedValueOnce( | ||
| new Error("fatal: ambiguous argument 'deadbeef': unknown revision or path not in the working tree"), | ||
| ); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'deadbeef' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' }); | ||
| }); | ||
|
|
||
| it('returns INVALID_GIT_REF for "bad revision" errors', async () => { | ||
| mockGitRaw.mockRejectedValueOnce(new Error('fatal: bad revision')); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'nonexistent' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' }); | ||
| }); | ||
|
|
||
| it('returns INVALID_GIT_REF for "invalid object name" errors', async () => { | ||
| mockGitRaw.mockRejectedValueOnce(new Error('fatal: invalid object name HEAD')); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'INVALID_GIT_REF' }); | ||
| }); | ||
|
|
||
| it('returns UNEXPECTED_ERROR — not throw — for unrecognised git errors (regression: v4.16.14 fatal exception)', async () => { | ||
| // Before the fix, getFileSourceForRepo re-threw unknown errors. | ||
| // Outside sew(), this caused a fatal Next.js task-runner exception. | ||
| // After the fix, all errors are returned as ServiceError. | ||
| mockGitRaw.mockRejectedValueOnce(new Error('I/O error: device busy')); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ errorCode: 'UNEXPECTED_ERROR' }); | ||
| }); | ||
|
|
||
| it('never rejects its returned promise for unrecognised git errors', async () => { | ||
| mockGitRaw.mockRejectedValueOnce(new Error('transient I/O error')); | ||
|
|
||
| await expect( | ||
| getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ), | ||
| ).resolves.toMatchObject({ errorCode: 'UNEXPECTED_ERROR' }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('successful response', () => { | ||
| it('returns the file source with language detected from filename', async () => { | ||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ | ||
| source: 'console.log("hello");', | ||
| language: 'TypeScript', | ||
| path: 'src/index.ts', | ||
| repo: 'github.com/owner/repo', | ||
| }); | ||
| }); | ||
|
|
||
| it('uses the provided ref for the git show command', async () => { | ||
| await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo', ref: 'abc123sha' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(mockGitRaw).toHaveBeenCalledWith(['show', 'abc123sha:src/index.ts']); | ||
| }); | ||
|
|
||
| it('falls back to defaultBranch when ref is omitted', async () => { | ||
| await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(mockGitRaw).toHaveBeenCalledWith(['show', 'main:src/index.ts']); | ||
| }); | ||
|
|
||
| it('falls back to HEAD when both ref and defaultBranch are absent', async () => { | ||
| mockFindFirst.mockResolvedValue({ ...MOCK_REPO, defaultBranch: null }); | ||
|
|
||
| await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(mockGitRaw).toHaveBeenCalledWith(['show', 'HEAD:src/index.ts']); | ||
| }); | ||
|
|
||
| it('uses the repo path from getRepoPath for the git working directory', async () => { | ||
| await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| // getRepoPath mock returns `/mock/repos/${repo.id}` | ||
| expect(mockCwd).toHaveBeenCalledWith('/mock/repos/123'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('language detection', () => { | ||
| it('uses language from .gitattributes when present', async () => { | ||
| const { resolveLanguageFromGitAttributes, parseGitAttributes } = await import('@/lib/gitattributes'); | ||
| const mockResolve = resolveLanguageFromGitAttributes as unknown as Mock; | ||
| const mockParse = parseGitAttributes as unknown as Mock; | ||
|
|
||
| mockParse.mockReturnValue({ '*.ts': { linguist_language: 'TypeScript' } }); | ||
| mockResolve.mockReturnValue('TypeScript'); | ||
|
|
||
| // Override default: .gitattributes call succeeds | ||
| mockGitRaw.mockImplementation(async (args: string[]) => { | ||
| if (args[1]?.endsWith('.gitattributes')) { | ||
| return 'linguist-language=TypeScript'; | ||
| } | ||
| return 'file content'; | ||
| }); | ||
|
|
||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ language: 'TypeScript' }); | ||
| expect(mockResolve).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('falls back to filename-based detection when .gitattributes is absent', async () => { | ||
| const { detectLanguageFromFilename } = await import('@/lib/languageDetection'); | ||
| const mockDetect = detectLanguageFromFilename as unknown as Mock; | ||
| mockDetect.mockReturnValue('TypeScript'); | ||
|
|
||
| // Default beforeEach setup: .gitattributes throws → falls back to filename detection | ||
| const result = await getFileSourceForRepo( | ||
| { path: 'src/index.ts', repo: 'github.com/owner/repo' }, | ||
| { org: MOCK_ORG, prisma: mockPrisma }, | ||
| ); | ||
|
|
||
| expect(result).toMatchObject({ language: 'TypeScript' }); | ||
| expect(mockDetect).toHaveBeenCalledWith('src/index.ts'); | ||
| }); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.