diff --git a/.gitguardian.yaml b/.gitguardian.yaml index 62f06156..83b6d7a6 100644 --- a/.gitguardian.yaml +++ b/.gitguardian.yaml @@ -17,6 +17,8 @@ paths-ignore: - '**/*.spec.tsx' - 'temp/**' - '**/*.md' + - '**/test/**' + - '**/__tests__/**' # Specific files to ignore files-ignore: diff --git a/backend/src/routes/repo-git.ts b/backend/src/routes/repo-git.ts index 490f8ed2..070f0094 100644 --- a/backend/src/routes/repo-git.ts +++ b/backend/src/routes/repo-git.ts @@ -303,6 +303,98 @@ export function createRepoGitRoutes(database: Database, gitAuthService: GitAuthS } }) + app.post('/:id/git/discard', async (c) => { + try { + const id = parseInt(c.req.param('id')) + const repo = db.getRepoById(database, id) + + if (!repo) { + return c.json({ error: 'Repo not found' }, 404) + } + + const body = await c.req.json() + const { paths, staged } = body + + if (!paths || !Array.isArray(paths)) { + return c.json({ error: 'paths is required and must be an array' }, 400) + } + + await git.discardChanges(id, paths, staged ?? false, database) + + const status = await git.getStatus(id, database) + return c.json(status) + } catch (error: unknown) { + logger.error('Failed to discard changes:', error) + const gitError = parseGitError(error) + return c.json( + { error: gitError.summary, detail: gitError.detail, code: gitError.code }, + gitError.statusCode as ContentfulStatusCode + ) + } + }) + + app.get('/:id/git/commit/:hash', async (c) => { + try { + const id = parseInt(c.req.param('id')) + const hash = c.req.param('hash') + const repo = db.getRepoById(database, id) + + if (!repo) { + return c.json({ error: 'Repo not found' }, 404) + } + + if (!hash) { + return c.json({ error: 'hash is required' }, 400) + } + + const commitDetails = await git.getCommitDetails(id, hash, database) + + if (!commitDetails) { + return c.json({ error: 'Commit not found' }, 404) + } + + return c.json(commitDetails) + } catch (error: unknown) { + logger.error('Failed to get commit details:', error) + const gitError = parseGitError(error) + return c.json( + { error: gitError.summary, detail: gitError.detail, code: gitError.code }, + gitError.statusCode as ContentfulStatusCode + ) + } + }) + + app.get('/:id/git/commit/:hash/diff', async (c) => { + try { + const id = parseInt(c.req.param('id')) + const hash = c.req.param('hash') + const filePath = c.req.query('path') + const repo = db.getRepoById(database, id) + + if (!repo) { + return c.json({ error: 'Repo not found' }, 404) + } + + if (!hash) { + return c.json({ error: 'hash is required' }, 400) + } + + if (!filePath) { + return c.json({ error: 'path query parameter is required' }, 400) + } + + const diff = await git.getCommitDiff(id, hash, filePath, database) + return c.json(diff) + } catch (error: unknown) { + logger.error('Failed to get commit diff:', error) + const gitError = parseGitError(error) + return c.json( + { error: gitError.summary, detail: gitError.detail, code: gitError.code }, + gitError.statusCode as ContentfulStatusCode + ) + } + }) + app.get('/:id/git/log', async (c) => { try { const id = parseInt(c.req.param('id')) diff --git a/backend/src/services/git/GitService.ts b/backend/src/services/git/GitService.ts index 32c4b259..8c85d7e4 100644 --- a/backend/src/services/git/GitService.ts +++ b/backend/src/services/git/GitService.ts @@ -7,7 +7,7 @@ import { resolveGitIdentity, createGitIdentityEnv, isSSHUrl } from '../../utils/ import { isNoUpstreamError, parseBranchNameFromError } from '../../utils/git-errors' import { SettingsService } from '../settings' import type { Database } from 'bun:sqlite' -import type { GitBranch, GitCommit, FileDiffResponse, GitDiffOptions, GitStatusResponse, GitFileStatus, GitFileStatusType } from '../../types/git' +import type { GitBranch, GitCommit, FileDiffResponse, GitDiffOptions, GitStatusResponse, GitFileStatus, GitFileStatusType, CommitDetails, CommitFile } from '../../types/git' import type { GitCredential } from '@opencode-manager/shared' import path from 'path' @@ -269,6 +269,252 @@ export class GitService { } } + async discardChanges(repoId: number, paths: string[], staged: boolean, database: Database): Promise { + try { + const repo = getRepoById(database, repoId) + if (!repo) { + throw new Error(`Repository not found`) + } + + const repoPath = repo.fullPath + const env = this.gitAuthService.getGitEnvironment() + + if (paths.length === 0) { + return '' + } + + if (staged) { + const args = ['git', '-C', repoPath, 'restore', '--staged', '--worktree', '--source', 'HEAD', '--', ...paths] + return await executeCommand(args, { env }) + } + + const statusOutput = await executeCommand( + ['git', '-C', repoPath, 'status', '--porcelain', '-u', '--', ...paths], + { env } + ) + + const untrackedPaths: string[] = [] + const trackedPaths: string[] = [] + + for (const line of statusOutput.split('\n')) { + if (!line.trim()) continue + const statusCode = line.substring(0, 2) + const filePath = line.substring(3).trim() + + if (statusCode === '??') { + untrackedPaths.push(filePath) + } else { + trackedPaths.push(filePath) + } + } + + const results: string[] = [] + + if (trackedPaths.length > 0) { + const args = ['git', '-C', repoPath, 'checkout', '--', ...trackedPaths] + results.push(await executeCommand(args, { env })) + } + + if (untrackedPaths.length > 0) { + try { + const args = ['git', '-C', repoPath, 'clean', '-fd', '--', ...untrackedPaths] + results.push(await executeCommand(args, { env })) + } catch (error: unknown) { + logger.error(`Failed to clean untracked files for repo ${repoId}:`, error) + throw error + } + } + + return results.join('\n') + } catch (error: unknown) { + logger.error(`Failed to discard changes for repo ${repoId}:`, error) + throw error + } + } + + private normalizeRenamePath(path: string): string { + const renamePattern = /\{[^=]+=>\s*([^}]+)\}/ + let normalized = path + while (renamePattern.test(normalized)) { + normalized = normalized.replace(renamePattern, '$1') + } + return normalized.trim() + } + + private parseNumstatOutput(output: string): Map { + const map = new Map() + const lines = output.trim().split('\n') + + for (const line of lines) { + if (!line.trim()) continue + + const parts = line.split('\t') + if (parts.length >= 3) { + const additions = parts[0] + const deletions = parts[1] + const filePath = parts.slice(2).join('\t') + const normalizedPath = this.normalizeRenamePath(filePath) + + if ( + additions?.match(/^\d+$/) && + deletions?.match(/^\d+$/) && + normalizedPath + ) { + map.set(normalizedPath, { + additions: parseInt(additions, 10), + deletions: parseInt(deletions, 10) + }) + } + } + } + + return map + } + + private parseCommitFiles( + output: string, + numstatMap: Map + ): CommitFile[] { + const files: CommitFile[] = [] + const lines = output.trim().split('\n') + + for (const line of lines) { + if (!line.trim()) continue + + const parts = line.split('\t') + if (parts.length >= 2 && parts[0] && parts[0].match(/^[AMDRC]/)) { + const statusCode = parts[0] + const fromPath = parts[1] || '' + const toPath = parts[2] || parts[1] || '' + const isRename = statusCode.startsWith('R') + const isCopy = statusCode.startsWith('C') + + let status: GitFileStatusType = 'modified' + switch (statusCode.charAt(0)) { + case 'A': + status = 'added' + break + case 'D': + status = 'deleted' + break + case 'R': + status = 'renamed' + break + case 'C': + status = 'copied' + break + case 'M': + status = 'modified' + break + } + + const numstatData = numstatMap.get(toPath) + const additions = numstatData?.additions ?? 0 + const deletions = numstatData?.deletions ?? 0 + + files.push({ + path: toPath, + status, + oldPath: isRename || isCopy ? fromPath : undefined, + additions, + deletions + }) + } + } + + return files + } + + async getCommitDetails(repoId: number, hash: string, database: Database): Promise { + try { + const repo = getRepoById(database, repoId) + if (!repo) { + throw new Error(`Repository not found: ${repoId}`) + } + + const repoPath = path.resolve(repo.fullPath) + const env = this.gitAuthService.getGitEnvironment(true) + + const commitOutput = await executeCommand( + ['git', '-C', repoPath, 'log', '-1', '--format=%H%x00%an%x00%ae%x00%at%x00%B', hash], + { env } + ) + + if (!commitOutput.trim()) { + return null + } + + const parts = commitOutput.trim().split('\0') + const [commitHash, authorName, authorEmail, timestamp, message] = parts + + if (!commitHash) { + return null + } + + const filesOutput = await executeCommand( + ['git', '-C', repoPath, 'show', '-M', '--name-status', '--format=', hash], + { env } + ) + + const numstatOutput = await executeCommand( + ['git', '-C', repoPath, 'show', '-M', '--numstat', '--format=', hash], + { env } + ) + + const numstatMap = this.parseNumstatOutput(numstatOutput) + const files = this.parseCommitFiles(filesOutput, numstatMap) + + return { + hash: commitHash, + authorName: authorName || '', + authorEmail: authorEmail || '', + date: timestamp || '', + message: message || '', + unpushed: await this.isCommitUnpushed(repoPath, commitHash, env), + files + } + } catch (error: unknown) { + logger.error(`Failed to get commit details for repo ${repoId}:`, error) + throw new Error(`Failed to get commit details: ${getErrorMessage(error)}`) + } + } + + async getCommitDiff(repoId: number, hash: string, filePath: string, database: Database): Promise { + try { + const repo = getRepoById(database, repoId) + if (!repo) { + throw new Error(`Repository not found: ${repoId}`) + } + + const repoPath = path.resolve(repo.fullPath) + const env = this.gitAuthService.getGitEnvironment(true) + + const diff = await executeCommand( + ['git', '-C', repoPath, 'show', '--format=', hash, '--', filePath], + { env } + ) + + const status = this.detectDiffStatus(diff) + return this.parseDiffOutput(diff, status, filePath) + } catch (error: unknown) { + logger.error(`Failed to get commit diff for repo ${repoId}:`, error) + throw new Error(`Failed to get commit diff: ${getErrorMessage(error)}`) + } + } + + private detectDiffStatus(diff: string): GitFileStatusType { + if (diff.includes('new file mode')) { + return 'added' + } + if (diff.includes('deleted file mode')) { + return 'deleted' + } + if (diff.includes('rename from') || diff.includes('rename to')) { + return 'renamed' + } + return 'modified' + } + private async setupSSHIfNeeded(repoUrl: string | undefined, database: Database): Promise { await this.gitAuthService.setupSSHForRepoUrl(repoUrl, database) } @@ -669,6 +915,7 @@ export class GitService { let additions = 0 let deletions = 0 let isBinary = false + const MAX_DIFF_SIZE = 500 * 1024 if (typeof diff === 'string') { if (diff.includes('Binary files') || diff.includes('GIT binary patch')) { @@ -682,13 +929,21 @@ export class GitService { } } + let diffOutput = typeof diff === 'string' ? diff : '' + let truncated = false + if (diffOutput.length > MAX_DIFF_SIZE) { + diffOutput = diffOutput.substring(0, MAX_DIFF_SIZE) + '\n\n... (diff truncated due to size)' + truncated = true + } + return { path: filePath || '', status: status as GitFileStatusType, - diff: typeof diff === 'string' ? diff : '', + diff: diffOutput, additions, deletions, - isBinary + isBinary, + truncated } } @@ -701,6 +956,11 @@ export class GitService { } } + private async isCommitUnpushed(repoPath: string, commitHash: string, env: Record): Promise { + const unpushedHashes = await this.getUnpushedCommitHashes(repoPath, env) + return unpushedHashes.has(commitHash) + } + private async getUnpushedCommitHashes(repoPath: string, env: Record): Promise> { try { const output = await executeCommand( diff --git a/backend/src/types/git.ts b/backend/src/types/git.ts index 324cf53d..effe8fd1 100644 --- a/backend/src/types/git.ts +++ b/backend/src/types/git.ts @@ -31,6 +31,7 @@ export interface FileDiffResponse { additions: number deletions: number isBinary: boolean + truncated?: boolean } export interface GitDiffOptions { @@ -48,3 +49,15 @@ export interface GitBranch { behind?: number isWorktree?: boolean } + +export interface CommitFile { + path: string + status: GitFileStatusType + oldPath?: string + additions: number + deletions: number +} + +export interface CommitDetails extends GitCommit { + files: CommitFile[] +} diff --git a/backend/test/routes/repo-git.test.ts b/backend/test/routes/repo-git.test.ts index 103a6fce..2ec70ee1 100644 --- a/backend/test/routes/repo-git.test.ts +++ b/backend/test/routes/repo-git.test.ts @@ -299,4 +299,121 @@ describe('Repo Git Routes', () => { expect(body).toHaveProperty('error') }) }) + + describe('POST /:id/git/discard', () => { + it('should return 404 when repo does not exist', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue(null) + const response = await app.request('/999/git/discard', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ paths: ['file1.ts'] }), + }) + const body = await response.json() + + expect(response.status).toBe(404) + expect(body).toHaveProperty('error', 'Repo not found') + }) + + it('should return 400 when paths is not an array', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1 } as any) + const response = await app.request('/1/git/discard', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ paths: 'not-an-array' }), + }) + const body = await response.json() + + expect(response.status).toBe(400) + expect(body).toHaveProperty('error', 'paths is required and must be an array') + }) + + it('should return 400 when paths is missing', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1 } as any) + const response = await app.request('/1/git/discard', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({}), + }) + const body = await response.json() + + expect(response.status).toBe(400) + expect(body).toHaveProperty('error', 'paths is required and must be an array') + }) + + it('should return 500 when git operation fails', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1, fullPath: '/path/to/repo' } as any) + const response = await app.request('/1/git/discard', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ paths: ['file1.ts'], staged: false }), + }) + const body = await response.json() + + expect(response.status).toBe(500) + expect(body).toHaveProperty('error') + }) + }) + + describe('GET /:id/git/commit/:hash', () => { + it('should return 404 when repo does not exist', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue(null) + const response = await app.request('/999/git/commit/abc123') + + expect(response.status).toBe(404) + const body = await response.json() + expect(body).toHaveProperty('error', 'Repo not found') + }) + + it('should return 400 when hash is missing', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1 } as any) + const response = await app.request('/1/git/commit/') + + expect(response.status).toBeGreaterThanOrEqual(400) + }) + + it('should return 500 when git operation fails', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1, fullPath: '/path/to/repo' } as any) + const response = await app.request('/1/git/commit/abc123') + + expect(response.status).toBe(500) + const body = await response.json() + expect(body).toHaveProperty('error') + }) + }) + + describe('GET /:id/git/commit/:hash/diff', () => { + it('should return 404 when repo does not exist', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue(null) + const response = await app.request('/999/git/commit/abc123/diff?path=file.ts') + + expect(response.status).toBe(404) + const body = await response.json() + expect(body).toHaveProperty('error', 'Repo not found') + }) + + it('should return 400 when hash is missing', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1 } as any) + const response = await app.request('/1/git/commit//diff?path=file.ts') + + expect(response.status).toBeGreaterThanOrEqual(400) + }) + + it('should return 400 when path query parameter is missing', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1 } as any) + const response = await app.request('/1/git/commit/abc123/diff') + const body = await response.json() + + expect(response.status).toBe(400) + expect(body).toHaveProperty('error', 'path query parameter is required') + }) + + it('should return 500 when git operation fails', async () => { + ;(db.getRepoById as MockedFunction).mockReturnValue({ id: 1, fullPath: '/path/to/repo' } as any) + const response = await app.request('/1/git/commit/abc123/diff?path=file.ts') + + expect(response.status).toBe(500) + const body = await response.json() + expect(body).toHaveProperty('error') + }) + }) }) diff --git a/backend/test/services/git/GitService.test.ts b/backend/test/services/git/GitService.test.ts index 783853f1..d096a605 100644 --- a/backend/test/services/git/GitService.test.ts +++ b/backend/test/services/git/GitService.test.ts @@ -29,6 +29,7 @@ vi.mock('../../../src/utils/git-auth', () => ({ resolveGitIdentity: vi.fn().mockResolvedValue(null), createGitIdentityEnv: vi.fn().mockReturnValue({}), createSilentGitEnv: vi.fn(), + filterGitCredentials: vi.fn().mockReturnValue([]), })) vi.mock('../../../src/utils/git-errors', () => ({ @@ -775,4 +776,624 @@ describe('GitService', () => { expect(result).toBe("Switched to branch 'main'") }) }) + + describe('discardChanges', () => { + it('discards staged changes using restore --staged --worktree', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockResolvedValue('Changes discarded') + + const result = await service.discardChanges(1, ['file.ts', 'dir/file2.ts'], true, database) + + expect(getRepoByIdMock).toHaveBeenCalledWith(database, 1) + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'restore', '--staged', '--worktree', '--source', 'HEAD', '--', 'file.ts', 'dir/file2.ts'], + { env: expect.any(Object) } + ) + expect(result).toBe('Changes discarded') + }) + + it('discards unstaged tracked changes using checkout', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('status')) { + return Promise.resolve('M file.ts\n') + } + if (args.includes('checkout')) { + return Promise.resolve('Updated 1 path') + } + return Promise.resolve('') + }) + + const result = await service.discardChanges(1, ['file.ts'], false, database) + + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'status', '--porcelain', '-u', '--', 'file.ts'], + { env: expect.any(Object) } + ) + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'checkout', '--', 'file.ts'], + { env: expect.any(Object) } + ) + expect(result).toBe('Updated 1 path') + }) + + it('removes unstaged untracked files using git clean', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('status')) { + return Promise.resolve('?? untracked.ts\n') + } + if (args.includes('clean')) { + return Promise.resolve('Removed untracked.ts') + } + return Promise.resolve('') + }) + + const result = await service.discardChanges(1, ['untracked.ts'], false, database) + + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'clean', '-fd', '--', 'untracked.ts'], + { env: expect.any(Object) } + ) + expect(result).toContain('Removed untracked.ts') + }) + + it('handles mixed tracked and untracked files', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('status')) { + return Promise.resolve('M modified.ts\n?? untracked.ts\n') + } + if (args.includes('checkout')) { + return Promise.resolve('Updated 1 path') + } + if (args.includes('clean')) { + return Promise.resolve('Removed untracked.ts') + } + return Promise.resolve('') + }) + + const result = await service.discardChanges(1, ['modified.ts', 'untracked.ts'], false, database) + + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'checkout', '--', 'modified.ts'], + { env: expect.any(Object) } + ) + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'clean', '-fd', '--', 'untracked.ts'], + { env: expect.any(Object) } + ) + expect(result).toContain('Updated 1 path') + expect(result).toContain('Removed untracked.ts') + }) + + it('returns early when no paths provided', async () => { + const result = await service.discardChanges(1, [], false, database) + + expect(executeCommandMock).not.toHaveBeenCalled() + expect(result).toBe('') + }) + + it('throws error when repository not found', async () => { + getRepoByIdMock.mockReturnValue(null) + + await expect(service.discardChanges(1, ['file.ts'], false, database)).rejects.toThrow('Repository not found') + }) + + it('logs and throws error on git command failure', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + const error = new Error('Permission denied') + executeCommandMock.mockImplementation((args) => { + if (args.includes('status')) { + return Promise.resolve('M file.ts\n') + } + if (args.includes('checkout')) { + return Promise.reject(error) + } + return Promise.resolve('') + }) + + await expect(service.discardChanges(1, ['file.ts'], false, database)).rejects.toThrow() + }) + + it('handles directories in untracked clean operation', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('status')) { + return Promise.resolve('?? dir/\n') + } + if (args.includes('clean')) { + return Promise.resolve('Removing dir/') + } + return Promise.resolve('') + }) + + const result = await service.discardChanges(1, ['dir/'], false, database) + + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', mockRepo.fullPath, 'clean', '-fd', '--', 'dir/'], + { env: expect.any(Object) } + ) + expect(result).toContain('Removing dir/') + }) + }) + + describe('getCommitDetails', () => { + it('returns full commit details with files', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('log') && !args.includes('--not')) { + return Promise.resolve('abc123\x00John Doe\x00john@example.com\x001609459200\x00Initial commit') + } + if (args.includes('show') && args.includes('--name-status')) { + return Promise.resolve('A\tfile1.ts\nM\tfile2.ts\nD\tfile3.ts\n') + } + if (args.includes('show') && args.includes('--numstat')) { + return Promise.resolve('10\t5\tfile1.ts\n20\t15\tfile2.ts\n0\t30\tfile3.ts\n') + } + if (args.includes('--not') && args.includes('--remotes')) { + return Promise.resolve('') + } + return Promise.resolve('') + }) + + const result = await service.getCommitDetails(1, 'abc123', database) + + expect(result).not.toBeNull() + expect(result?.hash).toBe('abc123') + expect(result?.authorName).toBe('John Doe') + expect(result?.authorEmail).toBe('john@example.com') + expect(result?.date).toBe('1609459200') + expect(result?.message).toBe('Initial commit') + expect(result?.files).toHaveLength(3) + expect(result?.files[0]).toEqual({ + path: 'file1.ts', + status: 'added', + additions: 10, + deletions: 5 + }) + expect(result?.files[1]).toEqual({ + path: 'file2.ts', + status: 'modified', + additions: 20, + deletions: 15 + }) + expect(result?.files[2]).toEqual({ + path: 'file3.ts', + status: 'deleted', + oldPath: undefined, + additions: 0, + deletions: 30 + }) + }) + + it('handles renamed files in commit details', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('log') && !args.includes('--not')) { + return Promise.resolve('abc123\x00John Doe\x00john@example.com\x001609459200\x00Rename file') + } + if (args.includes('show') && args.includes('--name-status')) { + return Promise.resolve('R\told.ts\tnew.ts\n') + } + if (args.includes('show') && args.includes('--numstat')) { + return Promise.resolve('0\t0\tnew.ts\n') + } + if (args.includes('--not') && args.includes('--remotes')) { + return Promise.resolve('') + } + return Promise.resolve('') + }) + + const result = await service.getCommitDetails(1, 'abc123', database) + + expect(result?.files).toHaveLength(1) + expect(result?.files[0]).toEqual({ + path: 'new.ts', + status: 'renamed', + oldPath: 'old.ts', + additions: 0, + deletions: 0 + }) + }) + + it('handles copied files in commit details', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('log') && !args.includes('--not')) { + return Promise.resolve('abc123\x00John Doe\x00john@example.com\x001609459200\x00Copy file') + } + if (args.includes('show') && args.includes('--name-status')) { + return Promise.resolve('C\toriginal.ts\tcopy.ts\n') + } + if (args.includes('show') && args.includes('--numstat')) { + return Promise.resolve('0\t0\tcopy.ts\n') + } + if (args.includes('--not') && args.includes('--remotes')) { + return Promise.resolve('') + } + return Promise.resolve('') + }) + + const result = await service.getCommitDetails(1, 'abc123', database) + + expect(result?.files).toHaveLength(1) + expect(result?.files[0]).toEqual({ + path: 'copy.ts', + status: 'copied', + oldPath: 'original.ts', + additions: 0, + deletions: 0 + }) + }) + + it('returns empty files array for empty commit', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('log') && !args.includes('--not')) { + return Promise.resolve('abc123\x00John Doe\x00john@example.com\x001609459200\x00Empty commit') + } + if (args.includes('show') && args.includes('--name-status')) { + return Promise.resolve('') + } + if (args.includes('show') && args.includes('--numstat')) { + return Promise.resolve('') + } + if (args.includes('--not') && args.includes('--remotes')) { + return Promise.resolve('') + } + return Promise.resolve('') + }) + + const result = await service.getCommitDetails(1, 'abc123', database) + + expect(result?.files).toHaveLength(0) + }) + + it('returns null when commit hash not found', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockResolvedValue('') + + const result = await service.getCommitDetails(1, 'nonexistent', database) + + expect(result).toBeNull() + }) + + it('throws error when repository not found', async () => { + getRepoByIdMock.mockReturnValue(null) + + await expect(service.getCommitDetails(1, 'abc123', database)).rejects.toThrow('Repository not found') + }) + + it('marks unpushed commits correctly', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('log') && !args.includes('--not')) { + return Promise.resolve('abc123\x00John Doe\x00john@example.com\x001609459200\x00Local commit') + } + if (args.includes('show') && args.includes('--name-status')) { + return Promise.resolve('M\tfile.ts\n') + } + if (args.includes('show') && args.includes('--numstat')) { + return Promise.resolve('1\t1\tfile.ts\n') + } + if (args.includes('--not') && args.includes('--remotes')) { + return Promise.resolve('abc123\n') + } + return Promise.resolve('') + }) + + const result = await service.getCommitDetails(1, 'abc123', database) + + expect(result?.unpushed).toBe(true) + }) + + it('handles commit message with pipes correctly', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockImplementation((args) => { + if (args.includes('log') && !args.includes('--not')) { + return Promise.resolve('abc123\x00John Doe\x00john@example.com\x001609459200\x00Fix: merge|conflict|handling') + } + if (args.includes('show') && args.includes('--name-status')) { + return Promise.resolve('') + } + if (args.includes('show') && args.includes('--numstat')) { + return Promise.resolve('') + } + if (args.includes('--not') && args.includes('--remotes')) { + return Promise.resolve('') + } + return Promise.resolve('') + }) + + const result = await service.getCommitDetails(1, 'abc123', database) + + expect(result?.message).toBe('Fix: merge|conflict|handling') + }) + + it('throws error on git command failure', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + const error = new Error('Git error') + executeCommandMock.mockRejectedValue(error) + + await expect(service.getCommitDetails(1, 'abc123', database)).rejects.toThrow('Failed to get commit details') + }) + }) + + describe('getCommitDiff', () => { + it('returns diff for specific file in commit', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + const diffOutput = `diff --git a/file.ts b/file.ts +index abc123..def456 100644 +--- a/file.ts ++++ b/file.ts +@@ -1,3 +1,4 @@ ++new line + existing line 1 + existing line 2 + existing line 3` + executeCommandMock.mockResolvedValue(diffOutput) + + const result = await service.getCommitDiff(1, 'abc123', 'file.ts', database) + + expect(getRepoByIdMock).toHaveBeenCalledWith(database, 1) + expect(executeCommandMock).toHaveBeenCalledWith( + ['git', '-C', expect.stringContaining('/path/to/repo'), 'show', '--format=', 'abc123', '--', 'file.ts'], + { env: expect.any(Object) } + ) + expect(result.path).toBe('file.ts') + expect(result.status).toBe('modified') + expect(result.diff).toContain('new line') + expect(result.additions).toBe(1) + expect(result.deletions).toBe(0) + expect(result.isBinary).toBe(false) + }) + + it('detects binary files', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + executeCommandMock.mockResolvedValue('Binary files a/image.png and b/image.png differ') + + const result = await service.getCommitDiff(1, 'abc123', 'image.png', database) + + expect(result.isBinary).toBe(true) + expect(result.diff).toContain('Binary files') + }) + + it('throws error when repository not found', async () => { + getRepoByIdMock.mockReturnValue(null) + + await expect(service.getCommitDiff(1, 'abc123', 'file.ts', database)).rejects.toThrow('Repository not found') + }) + + it('handles deleted files in commit', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + const diffOutput = `diff --git a/deleted.ts b/deleted.ts +deleted file mode 100644 +index abc123..0000000 +--- a/deleted.ts ++++ /dev/null +@@ -1,3 +0,0 @@ +-line 1 +-line 2 +-line 3` + executeCommandMock.mockResolvedValue(diffOutput) + + const result = await service.getCommitDiff(1, 'abc123', 'deleted.ts', database) + + expect(result.deletions).toBe(3) + expect(result.additions).toBe(0) + }) + + it('throws error on git command failure', async () => { + const mockRepo = { + id: 1, + fullPath: '/path/to/repo', + } + getRepoByIdMock.mockReturnValue(mockRepo as any) + const error = new Error('Fatal error') + executeCommandMock.mockRejectedValue(error) + + await expect(service.getCommitDiff(1, 'abc123', 'file.ts', database)).rejects.toThrow('Failed to get commit diff') + }) + }) + + describe('parseDiffOutput', () => { + it('parses small diff and counts additions/deletions', () => { + const diffOutput = `diff --git a/file.ts b/file.ts +index abc123..def456 100644 +--- a/file.ts ++++ b/file.ts +@@ -1,3 +1,4 @@ ++added line + existing line 1 + existing line 2 +-removed line + existing line 3` + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'file.ts') + + expect(result).toEqual({ + path: 'file.ts', + status: 'modified', + diff: diffOutput, + additions: 1, + deletions: 1, + isBinary: false, + truncated: false + }) + }) + + it('detects binary files correctly', () => { + const diffOutput = 'GIT binary patch\nliteral 1234\nzcmeAS@N?(olHy`uVBq!ia0vp0{ow;2j9U=!1jd#uLplI' + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'binary.bin') + + expect(result.isBinary).toBe(true) + }) + + it('detects "Binary files" indicator', () => { + const diffOutput = 'Binary files a/image.png and b/image.png differ' + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'image.png') + + expect(result.isBinary).toBe(true) + }) + + it('truncates diff when exceeding MAX_DIFF_SIZE (500KB)', () => { + const largeContent = '+' + 'x'.repeat(500 * 1024 + 1000) + const diffOutput = `diff --git a/large.ts b/large.ts\n${largeContent}` + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'large.ts') + + expect(result.truncated).toBe(true) + expect(result.diff.length).toBe(500 * 1024 + '\n\n... (diff truncated due to size)'.length) + expect(result.diff).toContain('... (diff truncated due to size)') + }) + + it('does not truncate diff under MAX_DIFF_SIZE', () => { + const diffOutput = '+' + 'x'.repeat(100 * 1024) + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'file.ts') + + expect(result.truncated).toBe(false) + expect(result.diff.length).toBeLessThan(500 * 1024) + }) + + it('handles empty diff', () => { + const diffOutput = '' + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'file.ts') + + expect(result).toEqual({ + path: 'file.ts', + status: 'modified', + diff: '', + additions: 0, + deletions: 0, + isBinary: false, + truncated: false + }) + }) + + it('correctly counts multiple additions and deletions', () => { + const diffOutput = `diff --git a/file.ts b/file.ts +@@ -1,5 +1,7 @@ ++added 1 + context ++added 2 +-removed 1 + context ++added 3 +-removed 2 +-removed 3` + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'file.ts') + + expect(result.additions).toBe(3) + expect(result.deletions).toBe(3) + }) + + it('ignores diff headers (+++/---) when counting', () => { + const diffOutput = `diff --git a/file.ts b/file.ts +--- a/file.ts ++++ b/file.ts +@@ -1,1 +1,2 @@ ++added` + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'file.ts') + + expect(result.additions).toBe(1) + expect(result.deletions).toBe(0) + }) + + it('handles different status types in parseDiffOutput', () => { + const diffOutput = '+new line' + + const resultAdded = (service as any).parseDiffOutput(diffOutput, 'added', 'new.ts') + const resultDeleted = (service as any).parseDiffOutput(diffOutput, 'deleted', 'old.ts') + const resultRenamed = (service as any).parseDiffOutput(diffOutput, 'renamed', 'moved.ts') + + expect(resultAdded.status).toBe('added') + expect(resultDeleted.status).toBe('deleted') + expect(resultRenamed.status).toBe('renamed') + }) + + it('handles diff with large change counts at truncation', () => { + const addedLines = '+' + 'x'.repeat(501 * 1024) + const diffOutput = `diff --git a/file.ts b/file.ts\n${addedLines}` + + const result = (service as any).parseDiffOutput(diffOutput, 'modified', 'file.ts') + + expect(result.truncated).toBe(true) + expect(result.diff).toContain('... (diff truncated due to size)') + }) + }) }) diff --git a/backend/test/utils/ssh-key-manager.test.ts b/backend/test/utils/ssh-key-manager.test.ts index 6b4218c6..803bb356 100644 --- a/backend/test/utils/ssh-key-manager.test.ts +++ b/backend/test/utils/ssh-key-manager.test.ts @@ -1,8 +1,6 @@ import { describe, it, expect } from 'vitest' import * as fs from 'fs/promises' -import { join } from 'path' import { parseSSHHost, normalizeHostPort, parseHostPort, writeTemporarySSHKey, cleanupSSHKey, cleanupAllSSHKeys } from '../../src/utils/ssh-key-manager' -import { getWorkspacePath } from '@opencode-manager/shared/config/env' describe('SSH Host Parsing', () => { it('should parse git@host:path format', () => { diff --git a/backend/test/utils/ssh-validation.test.ts b/backend/test/utils/ssh-validation.test.ts index 8b41b48c..1a685441 100644 --- a/backend/test/utils/ssh-validation.test.ts +++ b/backend/test/utils/ssh-validation.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll } from 'vitest' vi.mock('@opencode-manager/shared/config/env', () => ({ ENV: { diff --git a/frontend/src/api/git.ts b/frontend/src/api/git.ts index 40aa0bed..66ec61d3 100644 --- a/frontend/src/api/git.ts +++ b/frontend/src/api/git.ts @@ -1,7 +1,7 @@ import { useQuery } from '@tanstack/react-query' import { fetchWrapper, FetchError } from './fetchWrapper' import { API_BASE_URL } from '@/config' -import type { GitStatusResponse, FileDiffResponse, GitCommit } from '@/types/git' +import type { GitStatusResponse, FileDiffResponse, GitCommit, CommitDetails } from '@/types/git' export async function fetchGitStatus(repoId: number): Promise { return fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/status`) @@ -22,6 +22,12 @@ export async function fetchFileDiff(repoId: number, path: string, includeStaged? }) } +export async function fetchCommitFileDiff(repoId: number, commitHash: string, path: string): Promise { + return fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/commit/${commitHash}/diff`, { + params: { path }, + }) +} + export async function fetchGitDiff(repoId: number, path: string): Promise<{ diff: string }> { const data = await fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/diff`, { params: { path }, @@ -35,6 +41,10 @@ export async function fetchGitLog(repoId: number, limit?: number): Promise<{ com }) } +export async function fetchCommitDetails(repoId: number, hash: string): Promise { + return fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/commit/${hash}`) +} + export async function gitFetch(repoId: number): Promise { return fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/fetch`, { method: 'POST', @@ -79,6 +89,14 @@ export async function gitUnstageFiles(repoId: number, paths: string[]): Promise< }) } +export async function gitDiscardFiles(repoId: number, paths: string[], staged: boolean): Promise { + return fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/discard`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ paths, staged }), + }) +} + export async function gitReset(repoId: number, commitHash: string): Promise { return fetchWrapper(`${API_BASE_URL}/api/repos/${repoId}/git/reset`, { method: 'POST', @@ -104,6 +122,14 @@ export function useFileDiff(repoId: number | undefined, path: string | undefined }) } +export function useCommitFileDiff(repoId: number | undefined, commitHash: string | undefined, path: string | undefined) { + return useQuery({ + queryKey: ['commitFileDiff', repoId, commitHash, path], + queryFn: () => (repoId && commitHash && path) ? fetchCommitFileDiff(repoId, commitHash, path) : Promise.reject(new Error('Missing params')), + enabled: !!repoId && !!commitHash && !!path, + }) +} + export function useGitLog(repoId: number | undefined, limit?: number) { return useQuery({ queryKey: ['gitLog', repoId, limit], @@ -112,6 +138,14 @@ export function useGitLog(repoId: number | undefined, limit?: number) { }) } +export function useCommitDetails(repoId: number | undefined, hash: string | undefined) { + return useQuery({ + queryKey: ['commitDetails', repoId, hash], + queryFn: () => (repoId && hash) ? fetchCommitDetails(repoId, hash) : Promise.reject(new Error('Missing params')), + enabled: !!repoId && !!hash, + }) +} + function parseGitErrorMessage(message: string): string { if (message.includes('no upstream') || message.includes('does not have any commits yet')) { return 'No upstream branch configured. Push with --set-upstream or create commits first.' diff --git a/frontend/src/components/file-browser/FileDiffView.tsx b/frontend/src/components/file-browser/FileDiffView.tsx index 7440497a..eaca2544 100644 --- a/frontend/src/components/file-browser/FileDiffView.tsx +++ b/frontend/src/components/file-browser/FileDiffView.tsx @@ -1,4 +1,4 @@ -import { useFileDiff } from "@/api/git"; +import { useFileDiff, useCommitFileDiff } from "@/api/git"; import { Loader2, FileText, @@ -10,6 +10,7 @@ import { Minus, ArrowLeft, ExternalLink, + X, } from "lucide-react"; import { Button } from "@/components/ui/button"; import { CopyButton } from "@/components/ui/copy-button"; @@ -21,7 +22,9 @@ interface FileDiffViewProps { repoId: number; filePath: string; includeStaged?: boolean; + commitHash?: string; onBack?: () => void; + onClose?: () => void; onOpenFile?: (path: string, lineNumber?: number) => void; isMobile?: boolean; } @@ -136,7 +139,7 @@ function DiffLineComponent({ }) { if (line.type === "header") { return ( -
+
{line.content}
); @@ -144,7 +147,7 @@ function DiffLineComponent({ if (line.type === "hunk") { return ( -
+
{line.content}
); @@ -170,7 +173,7 @@ function DiffLineComponent({ return ( 
@@ -215,11 +218,15 @@ export function FileDiffView({
   repoId,
   filePath,
   includeStaged,
+  commitHash,
   onBack,
+  onClose,
   onOpenFile,
   isMobile = false,
 }: FileDiffViewProps) {
-  const { data: diffData, isLoading, error } = useFileDiff(repoId, filePath, includeStaged);
+  const workingDiff = useFileDiff(repoId, filePath, includeStaged);
+  const commitDiff = useCommitFileDiff(repoId, commitHash, filePath);
+  const { data: diffData, isLoading, error } = commitHash ? commitDiff : workingDiff;
 
   const fileName = filePath.split("/").pop() || filePath;
   const dirPath = filePath.includes("/")
@@ -259,7 +266,7 @@ export function FileDiffView({
   const diffLines = diffData.diff ? parseDiff(diffData.diff) : [];
 
   return (
-    

+    

       

           )}
+          {onClose && (
+            
+          )}
         

       

 
-      

+      

         {diffData.isBinary ? (
           

             
Binary file - cannot display diff

diff --git a/frontend/src/components/source-control/ChangesTab.tsx b/frontend/src/components/source-control/ChangesTab.tsx
index 706f6dc4..5d658450 100644
--- a/frontend/src/components/source-control/ChangesTab.tsx
+++ b/frontend/src/components/source-control/ChangesTab.tsx
@@ -4,23 +4,28 @@ import { useGit } from '@/hooks/useGit'
 import { GitFlatFileList } from './GitFlatFileList'
 import { Button } from '@/components/ui/button'
 import { Textarea } from '@/components/ui/textarea'
-import { Tabs, TabsList, TabsTrigger, TabsContent } from '@/components/ui/tabs'
 import { FileDiffView } from '@/components/file-browser/FileDiffView'
+import { DiscardDialog } from '@/components/ui/discard-dialog'
 import { Loader2, GitCommit, FileText, AlertCircle } from 'lucide-react'
 
 interface ChangesTabProps {
   repoId: number
   onFileSelect: (path: string, staged: boolean) => void
+  onClearFileSelection?: () => void
   selectedFile?: {path: string, staged: boolean}
   isMobile: boolean
   onError?: (error: unknown) => void
 }
 
-export function ChangesTab({ repoId, onFileSelect, selectedFile, isMobile, onError }: ChangesTabProps) {
+export function ChangesTab({ repoId, onFileSelect, onClearFileSelection, selectedFile, isMobile, onError }: ChangesTabProps) {
   const { data: status, isLoading, error } = useGitStatus(repoId)
   const git = useGit(repoId, onError)
   const [commitMessage, setCommitMessage] = useState('')
 
+  const [discardDialogOpen, setDiscardDialogOpen] = useState(false)
+  const [discardPaths, setDiscardPaths] = useState([])
+  const [discardStaged, setDiscardStaged] = useState(false)
+
   const stagedFiles = status?.files.filter(f => f.staged) || []
   const unstagedFiles = status?.files.filter(f => !f.staged) || []
   const canCommit = commitMessage.trim() && stagedFiles.length > 0 && !git.commit.isPending
@@ -33,6 +38,23 @@ export function ChangesTab({ repoId, onFileSelect, selectedFile, isMobile, onErr
     git.unstageFiles.mutate(paths)
   }
 
+  const handleDiscard = (paths: string[], staged: boolean) => {
+    setDiscardPaths(paths)
+    setDiscardStaged(staged)
+    setDiscardDialogOpen(true)
+  }
+
+  const confirmDiscard = () => {
+    git.discardFiles.mutate({ paths: discardPaths, staged: discardStaged })
+    setDiscardDialogOpen(false)
+    setDiscardPaths([])
+  }
+
+  const cancelDiscard = () => {
+    setDiscardDialogOpen(false)
+    setDiscardPaths([])
+  }
+
   const handleCommit = () => {
     git.commit.mutate({ message: commitMessage.trim() })
     setCommitMessage('')
@@ -60,83 +82,13 @@ export function ChangesTab({ repoId, onFileSelect, selectedFile, isMobile, onErr
 
   if (isMobile && selectedFile) {
     return (
-      

-        
-          
-            Files ({stagedFiles.length + unstagedFiles.length})
-            {selectedFile.path.split('/').pop() || selectedFile.path}
-          
-
-          
-            

-              

-                {status.hasChanges ? (
-                  <>
-                    {stagedFiles.length > 0 && (
-                      
-                    )}
-
-                    {unstagedFiles.length > 0 && (
-                      
-                    )}
-                  
-                ) : (
-                  

-                    
-                    
No uncommitted changes

-                  

-                )}
-              

-
-              {status.hasChanges && (
-                

-