diff --git a/apps/server/test/integration/phase-review-diff.test.ts b/apps/server/test/integration/phase-review-diff.test.ts new file mode 100644 index 0000000..036e12d --- /dev/null +++ b/apps/server/test/integration/phase-review-diff.test.ts @@ -0,0 +1,256 @@ +/** + * Integration tests for getPhaseReviewDiff and getFileDiff tRPC procedures. + * + * Uses real git repos on disk (no cassettes) + an in-memory SQLite database. + * No network calls — purely local git operations. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtemp, rm, writeFile, mkdir } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { simpleGit } from 'simple-git'; +import { router, publicProcedure, createCallerFactory } from '../../trpc/trpc.js'; +import { phaseProcedures } from '../../trpc/routers/phase.js'; +import { createTestDatabase } from '../../db/repositories/drizzle/test-helpers.js'; +import { + DrizzleInitiativeRepository, + DrizzlePhaseRepository, + DrizzleProjectRepository, +} from '../../db/repositories/drizzle/index.js'; +import { SimpleGitBranchManager } from '../../git/simple-git-branch-manager.js'; +import { getProjectCloneDir } from '../../git/project-clones.js'; +import type { TRPCContext } from '../../trpc/context.js'; + +// ============================================================================ +// Test router & caller factory +// ============================================================================ + +const testRouter = router({ + ...phaseProcedures(publicProcedure), +}); + +const createCaller = createCallerFactory(testRouter); + +// ============================================================================ +// Shared test state (set up once for the whole suite) +// ============================================================================ + +let workspaceRoot: string; +let cleanup: () => Promise; +let phaseId: string; +let pendingPhaseId: string; + +/** + * Build the test git repo with the required branches and files. + * + * Repo layout on the phase branch vs main: + * file1.txt through file5.txt — added (10–20 lines each) + * photo.bin — binary file (Buffer.alloc(32)) + * gone.txt — deleted (existed on main) + * has space.txt — added (contains text) + */ +async function setupGitRepo(clonePath: string): Promise { + await mkdir(clonePath, { recursive: true }); + + const git = simpleGit(clonePath); + await git.init(); + await git.addConfig('user.email', 'test@example.com'); + await git.addConfig('user.name', 'Test'); + + // Commit gone.txt on main so it can be deleted on the phase branch + await writeFile(path.join(clonePath, 'gone.txt'), Array.from({ length: 5 }, (_, i) => `line ${i + 1}`).join('\n') + '\n'); + await git.add('gone.txt'); + await git.commit('Initial commit on main'); + + // Create phase branch from main + // phaseBranchName('main', 'test-phase') => 'main-phase-test-phase' + await git.checkoutLocalBranch('main-phase-test-phase'); + + // Add 5 text files + for (let i = 1; i <= 5; i++) { + const lines = Array.from({ length: 15 }, (_, j) => `Line ${j + 1} in file${i}`).join('\n') + '\n'; + await writeFile(path.join(clonePath, `file${i}.txt`), lines); + } + await git.add(['file1.txt', 'file2.txt', 'file3.txt', 'file4.txt', 'file5.txt']); + + // Add binary file + await writeFile(path.join(clonePath, 'photo.bin'), Buffer.alloc(32)); + await git.add('photo.bin'); + + // Delete gone.txt + await git.rm(['gone.txt']); + + // Add file with space in name + await writeFile(path.join(clonePath, 'has space.txt'), 'content with spaces\n'); + await git.add('has space.txt'); + + await git.commit('Phase branch changes'); +} + +beforeAll(async () => { + // Create workspace root temp dir + workspaceRoot = await mkdtemp(path.join(tmpdir(), 'cw-phase-diff-test-')); + cleanup = async () => { + await rm(workspaceRoot, { recursive: true, force: true }); + }; + + // Set up in-memory database + const db = createTestDatabase(); + const initiativeRepo = new DrizzleInitiativeRepository(db); + const phaseRepo = new DrizzlePhaseRepository(db); + const projectRepo = new DrizzleProjectRepository(db); + + // Create initiative with branch='main' + const initiative = await initiativeRepo.create({ + name: 'Test Initiative', + branch: 'main', + }); + + // Create project — we'll set up the git repo at the expected clone path + const project = await projectRepo.create({ + name: 'test-repo', + url: 'file:///dev/null', // won't be cloned — we create the repo directly + defaultBranch: 'main', + }); + + // Link project to initiative + await projectRepo.addProjectToInitiative(initiative.id, project.id); + + // Set up git repo at the expected clone path + const relPath = getProjectCloneDir(project.name, project.id); + const clonePath = path.join(workspaceRoot, relPath); + await setupGitRepo(clonePath); + + // Create reviewable phase (pending_review) + const phase = await phaseRepo.create({ + initiativeId: initiative.id, + name: 'test-phase', + status: 'pending_review', + }); + phaseId = phase.id; + + // Create a non-reviewable phase (pending) for error test + const pendingPhase = await phaseRepo.create({ + initiativeId: initiative.id, + name: 'pending-phase', + status: 'pending', + }); + pendingPhaseId = pendingPhase.id; + + // Store db and repos so the caller can use them + // (stored in module-level vars to be accessed in test helper) + Object.assign(sharedCtx, { + initiativeRepository: initiativeRepo, + phaseRepository: phaseRepo, + projectRepository: projectRepo, + branchManager: new SimpleGitBranchManager(), + workspaceRoot, + }); +}); + +afterAll(async () => { + await cleanup?.(); +}); + +// ============================================================================ +// Shared context (filled in beforeAll) +// ============================================================================ + +const sharedCtx: Partial = { + eventBus: { emit: () => {}, on: () => {}, off: () => {}, once: () => {} } as any, + serverStartedAt: null, + processCount: 0, +}; + +function getCaller() { + return createCaller(sharedCtx as TRPCContext); +} + +// ============================================================================ +// Tests: getPhaseReviewDiff +// ============================================================================ + +describe('getPhaseReviewDiff', () => { + it('returns files array with correct metadata and no rawDiff field', async () => { + const start = Date.now(); + const result = await getCaller().getPhaseReviewDiff({ phaseId }); + const elapsed = Date.now() - start; + + expect(result).not.toHaveProperty('rawDiff'); + expect(result.files).toBeInstanceOf(Array); + // 5 text + 1 binary + 1 deleted + 1 spaced = 8 files + expect(result.files.length).toBeGreaterThanOrEqual(7); + expect(elapsed).toBeLessThan(3000); + }); + + it('includes binary file with status=binary, additions=0, deletions=0', async () => { + const result = await getCaller().getPhaseReviewDiff({ phaseId }); + const bin = result.files.find((f) => f.path === 'photo.bin'); + expect(bin?.status).toBe('binary'); + expect(bin?.additions).toBe(0); + expect(bin?.deletions).toBe(0); + }); + + it('includes deleted file with status=deleted and nonzero deletions', async () => { + const result = await getCaller().getPhaseReviewDiff({ phaseId }); + const del = result.files.find((f) => f.path === 'gone.txt'); + expect(del?.status).toBe('deleted'); + expect(del?.deletions).toBeGreaterThan(0); + }); + + it('computes totalAdditions and totalDeletions as sums over files', async () => { + const result = await getCaller().getPhaseReviewDiff({ phaseId }); + const sumAdd = result.files.reduce((s, f) => s + f.additions, 0); + const sumDel = result.files.reduce((s, f) => s + f.deletions, 0); + expect(result.totalAdditions).toBe(sumAdd); + expect(result.totalDeletions).toBe(sumDel); + }); + + it('throws NOT_FOUND for unknown phaseId', async () => { + const err = await getCaller().getPhaseReviewDiff({ phaseId: 'nonexistent' }).catch((e) => e); + expect(err.code).toBe('NOT_FOUND'); + }); + + it('throws BAD_REQUEST for phase not in reviewable status', async () => { + const err = await getCaller().getPhaseReviewDiff({ phaseId: pendingPhaseId }).catch((e) => e); + expect(err.code).toBe('BAD_REQUEST'); + }); +}); + +// ============================================================================ +// Tests: getFileDiff +// ============================================================================ + +describe('getFileDiff', () => { + it('returns rawDiff with unified diff for a normal file, under 1 second', async () => { + const start = Date.now(); + const result = await getCaller().getFileDiff({ phaseId, filePath: 'file1.txt' }); + const elapsed = Date.now() - start; + + expect(result.binary).toBe(false); + expect(result.rawDiff).toContain('+'); + expect(elapsed).toBeLessThan(1000); + }); + + it('returns binary=true and rawDiff="" for binary file', async () => { + const result = await getCaller().getFileDiff({ phaseId, filePath: 'photo.bin' }); + expect(result.binary).toBe(true); + expect(result.rawDiff).toBe(''); + }); + + it('returns removal hunks for a deleted file', async () => { + const result = await getCaller().getFileDiff({ phaseId, filePath: 'gone.txt' }); + expect(result.binary).toBe(false); + expect(result.rawDiff).toContain('-'); + }); + + it('handles URL-encoded file path with space', async () => { + const result = await getCaller().getFileDiff({ + phaseId, + filePath: encodeURIComponent('has space.txt'), + }); + expect(result.binary).toBe(false); + expect(result.rawDiff).toContain('has space.txt'); + }); +}); diff --git a/apps/server/trpc/routers/phase.ts b/apps/server/trpc/routers/phase.ts index be59ef2..0d9c999 100644 --- a/apps/server/trpc/routers/phase.ts +++ b/apps/server/trpc/routers/phase.ts @@ -4,11 +4,13 @@ import { TRPCError } from '@trpc/server'; import { z } from 'zod'; +import { simpleGit } from 'simple-git'; import type { Phase } from '../../db/schema.js'; import type { ProcedureBuilder } from '../trpc.js'; import { requirePhaseRepository, requireTaskRepository, requireBranchManager, requireInitiativeRepository, requireProjectRepository, requireExecutionOrchestrator, requireReviewCommentRepository, requireChangeSetRepository } from './_helpers.js'; import { phaseBranchName } from '../../git/branch-naming.js'; import { ensureProjectClone } from '../../git/project-clones.js'; +import type { FileStatEntry } from '../../git/types.js'; export function phaseProcedures(publicProcedure: ProcedureBuilder) { return { @@ -230,28 +232,93 @@ export function phaseProcedures(publicProcedure: ProcedureBuilder) { const initBranch = initiative.branch; const phBranch = phaseBranchName(initBranch, phase.name); - // For completed phases, use stored merge base; for pending_review, use initiative branch const diffBase = (phase.status === 'completed' && phase.mergeBase) ? phase.mergeBase : initBranch; const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId); - let rawDiff = ''; + const files: FileStatEntry[] = []; for (const project of projects) { const clonePath = await ensureProjectClone(project, ctx.workspaceRoot!); - const diff = await branchManager.diffBranches(clonePath, diffBase, phBranch); - if (diff) { - rawDiff += diff + '\n'; + const entries = await branchManager.diffBranchesStat(clonePath, diffBase, phBranch); + for (const entry of entries) { + const tagged: FileStatEntry = { ...entry, projectId: project.id }; + if (projects.length > 1) { + tagged.path = `${project.name}/${entry.path}`; + if (entry.oldPath) { + tagged.oldPath = `${project.name}/${entry.oldPath}`; + } + } + files.push(tagged); } } + const totalAdditions = files.reduce((sum, f) => sum + f.additions, 0); + const totalDeletions = files.reduce((sum, f) => sum + f.deletions, 0); + return { phaseName: phase.name, sourceBranch: phBranch, targetBranch: initBranch, - rawDiff, + files, + totalAdditions, + totalDeletions, }; }), + getFileDiff: publicProcedure + .input(z.object({ + phaseId: z.string().min(1), + filePath: z.string().min(1), + projectId: z.string().optional(), + })) + .query(async ({ ctx, input }) => { + const phaseRepo = requirePhaseRepository(ctx); + const initiativeRepo = requireInitiativeRepository(ctx); + const projectRepo = requireProjectRepository(ctx); + const branchManager = requireBranchManager(ctx); + + const phase = await phaseRepo.findById(input.phaseId); + if (!phase) { + throw new TRPCError({ code: 'NOT_FOUND', message: `Phase '${input.phaseId}' not found` }); + } + if (phase.status !== 'pending_review' && phase.status !== 'completed') { + throw new TRPCError({ code: 'BAD_REQUEST', message: `Phase is not reviewable (status: ${phase.status})` }); + } + + const initiative = await initiativeRepo.findById(phase.initiativeId); + if (!initiative?.branch) { + throw new TRPCError({ code: 'BAD_REQUEST', message: 'Initiative has no branch configured' }); + } + + const initBranch = initiative.branch; + const phBranch = phaseBranchName(initBranch, phase.name); + const diffBase = (phase.status === 'completed' && phase.mergeBase) ? phase.mergeBase : initBranch; + + const decodedPath = decodeURIComponent(input.filePath); + + const projects = await projectRepo.findProjectsByInitiativeId(phase.initiativeId); + let clonePath: string; + if (input.projectId) { + const project = projects.find((p) => p.id === input.projectId); + if (!project) { + throw new TRPCError({ code: 'NOT_FOUND', message: `Project '${input.projectId}' not found for this phase` }); + } + clonePath = await ensureProjectClone(project, ctx.workspaceRoot!); + } else { + clonePath = await ensureProjectClone(projects[0], ctx.workspaceRoot!); + } + + const git = simpleGit(clonePath); + // Binary files appear as "-\t-\t" in --numstat output + const numstatOut = await git.raw(['diff', '--numstat', `${diffBase}...${phBranch}`, '--', decodedPath]); + if (numstatOut.trim() && numstatOut.startsWith('-\t-\t')) { + return { binary: true, rawDiff: '' }; + } + + const rawDiff = await branchManager.diffFileSingle(clonePath, diffBase, phBranch, decodedPath); + return { binary: false, rawDiff }; + }), + approvePhaseReview: publicProcedure .input(z.object({ phaseId: z.string().min(1) })) .mutation(async ({ ctx, input }) => { diff --git a/docs/server-api.md b/docs/server-api.md index b064576..eb5e5ef 100644 --- a/docs/server-api.md +++ b/docs/server-api.md @@ -118,7 +118,8 @@ Each procedure uses `require*Repository(ctx)` helpers that throw `TRPCError(INTE | listInitiativePhaseDependencies | query | All dependency edges | | getPhaseDependencies | query | What this phase depends on | | getPhaseDependents | query | What depends on this phase | -| getPhaseReviewDiff | query | Full branch diff for pending_review phase | +| getPhaseReviewDiff | query | File-level metadata for pending_review phase: `{phaseName, sourceBranch, targetBranch, files: FileStatEntry[], totalAdditions, totalDeletions}` — no hunk content | +| getFileDiff | query | Per-file unified diff on demand: `{phaseId, filePath, projectId?}` → `{binary: boolean, rawDiff: string}`; `filePath` must be URL-encoded; binary files return `{binary: true, rawDiff: ''}` | | getPhaseReviewCommits | query | List commits between initiative and phase branch | | getCommitDiff | query | Diff for a single commit (by hash) in a phase | | approvePhaseReview | mutation | Approve and merge phase branch |