From 771cd71c1e42a0841d5d24cbafc5b5aaa748b56f Mon Sep 17 00:00:00 2001 From: Lukas May Date: Tue, 10 Feb 2026 11:46:00 +0100 Subject: [PATCH] feat: Validate default branch exists in repo when setting project defaultBranch registerProject and updateProject now verify via remoteBranchExists that the specified branch actually exists in the cloned repository before saving. --- docs/git-process-logging.md | 15 +++ docs/server-api.md | 4 +- src/git/branch-manager.ts | 7 ++ src/git/simple-git-branch-manager.test.ts | 110 ++++++++++++++++++++++ src/git/simple-git-branch-manager.ts | 12 +++ src/trpc/routers/project.ts | 28 ++++++ 6 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 src/git/simple-git-branch-manager.test.ts diff --git a/docs/git-process-logging.md b/docs/git-process-logging.md index e9e23cd..3579a0a 100644 --- a/docs/git-process-logging.md +++ b/docs/git-process-logging.md @@ -31,6 +31,21 @@ Worktrees stored in `.cw-worktrees/` subdirectory of the repo. Each agent gets a 4. On conflict: `git merge --abort`, emit `worktree:conflict` with conflicting file list 5. Restore original branch +### BranchManager (`src/git/branch-manager.ts`) +- **Port**: `BranchManager` interface +- **Adapter**: `SimpleGitBranchManager` using simple-git + +| Method | Purpose | +|--------|---------| +| `ensureBranch(repoPath, branch, baseBranch)` | Create branch from base if it doesn't exist (idempotent) | +| `mergeBranch(repoPath, source, target)` | Merge via ephemeral worktree, returns conflict info | +| `diffBranches(repoPath, base, head)` | Three-dot diff between branches | +| `deleteBranch(repoPath, branch)` | Delete local branch (no-op if missing) | +| `branchExists(repoPath, branch)` | Check local branches | +| `remoteBranchExists(repoPath, branch)` | Check remote tracking branches (`origin/`) | + +`remoteBranchExists` is used by `registerProject` and `updateProject` to validate that a project's default branch actually exists in the cloned repository before saving. + ### Project Clones - `cloneProject(url, destPath)` — Simple git clone wrapper - `ensureProjectClone(project, workspaceRoot)` — Idempotent: checks if clone exists, clones if not diff --git a/docs/server-api.md b/docs/server-api.md index 7f5c889..d2c7e14 100644 --- a/docs/server-api.md +++ b/docs/server-api.md @@ -140,10 +140,10 @@ Each procedure uses `require*Repository(ctx)` helpers that throw `TRPCError(INTE ### Projects | Procedure | Type | Description | |-----------|------|-------------| -| registerProject | mutation | Clone git repo, create record | +| registerProject | mutation | Clone git repo, create record. Validates defaultBranch exists in repo | | listProjects | query | All projects | | getProject | query | Single project | -| updateProject | mutation | Update project settings (defaultBranch) | +| updateProject | mutation | Update project settings (defaultBranch). Validates branch exists in repo | | deleteProject | mutation | Delete clone and record | | getInitiativeProjects | query | Projects for initiative | | updateInitiativeProjects | mutation | Sync junction table | diff --git a/src/git/branch-manager.ts b/src/git/branch-manager.ts index ad28768..d61b4dd 100644 --- a/src/git/branch-manager.ts +++ b/src/git/branch-manager.ts @@ -38,4 +38,11 @@ export interface BranchManager { * Check if a branch exists in the repository. */ branchExists(repoPath: string, branch: string): Promise; + + /** + * Check if a branch exists as a remote tracking branch (origin/). + * Useful for validating branch names against what the remote has, + * since local branches may not include all remote branches. + */ + remoteBranchExists(repoPath: string, branch: string): Promise; } diff --git a/src/git/simple-git-branch-manager.test.ts b/src/git/simple-git-branch-manager.test.ts new file mode 100644 index 0000000..18cb50a --- /dev/null +++ b/src/git/simple-git-branch-manager.test.ts @@ -0,0 +1,110 @@ +/** + * SimpleGitBranchManager Tests + * + * Tests for remoteBranchExists validation used when setting + * a project's default branch. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { simpleGit } from 'simple-git'; +import { SimpleGitBranchManager } from './simple-git-branch-manager.js'; + +/** + * Create a "remote" bare repo and a clone of it for testing. + * The bare repo has branches that the clone can see as remote tracking branches. + */ +async function createTestRepoWithRemote(): Promise<{ + clonePath: string; + barePath: string; + cleanup: () => Promise; +}> { + const tmpBase = await mkdtemp(path.join(tmpdir(), 'cw-branch-test-')); + const barePath = path.join(tmpBase, 'bare.git'); + const workPath = path.join(tmpBase, 'work'); + const clonePath = path.join(tmpBase, 'clone'); + + // Create a bare repo + const bareGit = simpleGit(); + await bareGit.init([barePath, '--bare']); + + // Clone it to a working directory, add commits and branches, push + await simpleGit().clone(barePath, workPath); + const workGit = simpleGit(workPath); + await workGit.addConfig('user.email', 'test@example.com'); + await workGit.addConfig('user.name', 'Test User'); + await writeFile(path.join(workPath, 'README.md'), '# Test\n'); + await workGit.add('README.md'); + await workGit.commit('Initial commit'); + await workGit.push('origin', 'main'); + + // Create additional branches + await workGit.checkoutLocalBranch('develop'); + await writeFile(path.join(workPath, 'dev.txt'), 'dev\n'); + await workGit.add('dev.txt'); + await workGit.commit('Dev commit'); + await workGit.push('origin', 'develop'); + + await workGit.checkoutLocalBranch('feature/auth'); + await writeFile(path.join(workPath, 'auth.txt'), 'auth\n'); + await workGit.add('auth.txt'); + await workGit.commit('Auth commit'); + await workGit.push('origin', 'feature/auth'); + + // Clone from bare to simulate what project registration does + await simpleGit().clone(barePath, clonePath); + + return { + clonePath, + barePath, + cleanup: async () => { + await rm(tmpBase, { recursive: true, force: true }); + }, + }; +} + +describe('SimpleGitBranchManager', () => { + let clonePath: string; + let cleanup: () => Promise; + let branchManager: SimpleGitBranchManager; + + beforeEach(async () => { + const setup = await createTestRepoWithRemote(); + clonePath = setup.clonePath; + cleanup = setup.cleanup; + branchManager = new SimpleGitBranchManager(); + }); + + afterEach(async () => { + await cleanup(); + }); + + describe('remoteBranchExists', () => { + it('should return true for a branch that exists on the remote', async () => { + expect(await branchManager.remoteBranchExists(clonePath, 'main')).toBe(true); + expect(await branchManager.remoteBranchExists(clonePath, 'develop')).toBe(true); + expect(await branchManager.remoteBranchExists(clonePath, 'feature/auth')).toBe(true); + }); + + it('should return false for a branch that does not exist', async () => { + expect(await branchManager.remoteBranchExists(clonePath, 'nonexistent')).toBe(false); + expect(await branchManager.remoteBranchExists(clonePath, 'feature/nope')).toBe(false); + }); + + it('should return false for an invalid repo path', async () => { + expect(await branchManager.remoteBranchExists('/tmp/no-such-repo', 'main')).toBe(false); + }); + + it('should detect remote branches not checked out locally', async () => { + // After clone, only 'main' is checked out locally. + // 'develop' exists only as origin/develop. + const localExists = await branchManager.branchExists(clonePath, 'develop'); + const remoteExists = await branchManager.remoteBranchExists(clonePath, 'develop'); + + expect(localExists).toBe(false); + expect(remoteExists).toBe(true); + }); + }); +}); diff --git a/src/git/simple-git-branch-manager.ts b/src/git/simple-git-branch-manager.ts index 6fa0016..a6b2be8 100644 --- a/src/git/simple-git-branch-manager.ts +++ b/src/git/simple-git-branch-manager.ts @@ -104,4 +104,16 @@ export class SimpleGitBranchManager implements BranchManager { return false; } } + + async remoteBranchExists(repoPath: string, branch: string): Promise { + try { + const git = simpleGit(repoPath); + const result = await git.branch(['-r']); + return result.all.some( + (ref) => ref === `origin/${branch}` || ref === `remotes/origin/${branch}`, + ); + } catch { + return false; + } + } } diff --git a/src/trpc/routers/project.ts b/src/trpc/routers/project.ts index 0433751..b8d34c2 100644 --- a/src/trpc/routers/project.ts +++ b/src/trpc/routers/project.ts @@ -52,6 +52,21 @@ export function projectProcedures(publicProcedure: ProcedureBuilder) { message: `Failed to clone repository: ${(cloneError as Error).message}`, }); } + + // Validate that the specified default branch exists in the cloned repo + const branchToValidate = input.defaultBranch ?? 'main'; + if (ctx.branchManager) { + const exists = await ctx.branchManager.remoteBranchExists(clonePath, branchToValidate); + if (!exists) { + // Clean up: remove project and clone + await rm(clonePath, { recursive: true, force: true }).catch(() => {}); + await repo.delete(project.id); + throw new TRPCError({ + code: 'BAD_REQUEST', + message: `Branch '${branchToValidate}' does not exist in the repository`, + }); + } + } } return project; @@ -105,6 +120,19 @@ export function projectProcedures(publicProcedure: ProcedureBuilder) { message: `Project '${id}' not found`, }); } + + // Validate that the new default branch exists in the repo + if (data.defaultBranch && ctx.workspaceRoot && ctx.branchManager) { + const clonePath = join(ctx.workspaceRoot, getProjectCloneDir(existing.name, existing.id)); + const exists = await ctx.branchManager.remoteBranchExists(clonePath, data.defaultBranch); + if (!exists) { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: `Branch '${data.defaultBranch}' does not exist in the repository`, + }); + } + } + return repo.update(id, data); }),