diff --git a/app/gateway-api/src/orchestrator/buildRunner.ts b/app/gateway-api/src/orchestrator/buildRunner.ts index be1fa97..a8e62ab 100644 --- a/app/gateway-api/src/orchestrator/buildRunner.ts +++ b/app/gateway-api/src/orchestrator/buildRunner.ts @@ -17,6 +17,11 @@ export interface BuildRunner { run(commands: BuildCommand[]): Promise; } +export const MAX_BUILD_OUTPUT_CHARS = 64 * 1024; + +const appendOutputTail = (current: string, chunk: unknown): string => + `${current}${String(chunk)}`.slice(-MAX_BUILD_OUTPUT_CHARS); + const runCommand = (command: BuildCommand): Promise => new Promise((resolve) => { const child = spawn(command.command, command.args, { @@ -26,10 +31,17 @@ const runCommand = (command: BuildCommand): Promise => }); let output = ''; child.stdout.on('data', (chunk) => { - output += chunk.toString(); + output = appendOutputTail(output, chunk); }); child.stderr.on('data', (chunk) => { - output += chunk.toString(); + output = appendOutputTail(output, chunk); + }); + child.on('error', (error) => { + resolve({ + ok: false, + exitCode: null, + output: appendOutputTail(output, error.message), + }); }); child.on('close', (code) => { resolve({ @@ -45,7 +57,7 @@ export class PnpmBuildRunner implements BuildRunner { let mergedOutput = ''; for (const command of commands) { const result = await runCommand(command); - mergedOutput += result.output; + mergedOutput = appendOutputTail(mergedOutput, result.output); if (!result.ok) { return { ok: false, diff --git a/app/gateway-api/src/orchestrator/workspaceManager.ts b/app/gateway-api/src/orchestrator/workspaceManager.ts index d9782d7..931bb1c 100644 --- a/app/gateway-api/src/orchestrator/workspaceManager.ts +++ b/app/gateway-api/src/orchestrator/workspaceManager.ts @@ -28,6 +28,9 @@ const runGit = (args: string[], cwd: string, env?: Record): Prom child.stderr.on('data', (chunk) => { output += chunk.toString(); }); + child.on('error', (error) => { + resolve({ ok: false, output: `${output}${error.message}` }); + }); child.on('close', (code) => { resolve({ ok: code === 0, output }); }); @@ -41,6 +44,7 @@ const ensureDir = (dir: string): void => { const hasInstallMarker = (dir: string): boolean => fs.existsSync(path.join(dir, 'node_modules', '.pnpm')); const GIT_REF_PATTERN = /^[0-9A-Za-z._/-]+$/; +const COMMIT_SHA_PATTERN = /^[0-9a-f]{40}$/i; const assertGitRef = (value: string): string => { const ref = value.trim(); @@ -84,6 +88,9 @@ export class GitWorkspaceManager { } async prepare(commitSha: string): Promise { + if (!COMMIT_SHA_PATTERN.test(commitSha)) { + throw new Error('Invalid commit SHA.'); + } const workspacePath = path.join(this.worktreeRoot, commitSha); ensureDir(this.worktreeRoot); @@ -101,6 +108,8 @@ export class GitWorkspaceManager { if (!result.ok) { throw new Error(result.output || 'Failed to create git worktree.'); } + } else { + await this.assertReusableWorkspace(workspacePath, commitSha); } return { @@ -111,18 +120,61 @@ export class GitWorkspaceManager { } async remove(workspacePath: string): Promise { - const resolved = path.resolve(workspacePath); - const root = path.resolve(this.worktreeRoot); - if (!resolved.startsWith(root)) { - throw new Error('Workspace path is outside the configured worktree root.'); - } + const resolved = this.assertManagedWorkspacePath(workspacePath); if (!fs.existsSync(resolved)) { return false; } + await this.assertRegisteredWorkspace(resolved); const result = await runGit(['worktree', 'remove', '--force', resolved], this.repoRoot, this.baseEnv); if (!result.ok) { fs.rmSync(resolved, { recursive: true, force: true }); } return true; } + + private assertManagedWorkspacePath(workspacePath: string): string { + const resolved = path.resolve(workspacePath); + const root = path.resolve(this.worktreeRoot); + const relative = path.relative(root, resolved); + if (!relative || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) { + throw new Error('Workspace path must be a child of the configured worktree root.'); + } + if (relative.includes(path.sep) || !COMMIT_SHA_PATTERN.test(relative)) { + throw new Error('Workspace path is not a managed commit workspace.'); + } + return resolved; + } + + private async assertRegisteredWorkspace(workspacePath: string, expectedCommitSha?: string): Promise { + const listed = await runGit(['worktree', 'list', '--porcelain'], this.repoRoot, this.baseEnv); + if (!listed.ok) { + throw new Error(listed.output || 'Failed to inspect git worktrees.'); + } + const blocks = listed.output.split(/\n\n+/); + const matchingBlock = blocks.find((block) => { + const worktreeLine = block.split('\n').find((line) => line.startsWith('worktree ')); + return worktreeLine && path.resolve(worktreeLine.slice('worktree '.length)) === workspacePath; + }); + if (!matchingBlock) { + throw new Error('Workspace is not registered as a git worktree.'); + } + if (expectedCommitSha) { + const headLine = matchingBlock.split('\n').find((line) => line.startsWith('HEAD ')); + if (headLine?.slice('HEAD '.length).toLowerCase() !== expectedCommitSha.toLowerCase()) { + throw new Error('Existing workspace HEAD does not match the requested commit.'); + } + } + } + + private async assertReusableWorkspace(workspacePath: string, expectedCommitSha: string): Promise { + const resolved = this.assertManagedWorkspacePath(workspacePath); + await this.assertRegisteredWorkspace(resolved, expectedCommitSha); + const status = await runGit(['status', '--porcelain'], resolved, this.baseEnv); + if (!status.ok) { + throw new Error(status.output || 'Failed to inspect existing workspace.'); + } + if (status.output.trim()) { + throw new Error('Existing workspace has uncommitted changes.'); + } + } } diff --git a/app/gateway-api/test/buildRunner.test.ts b/app/gateway-api/test/buildRunner.test.ts new file mode 100644 index 0000000..2cef6f4 --- /dev/null +++ b/app/gateway-api/test/buildRunner.test.ts @@ -0,0 +1,43 @@ +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { MAX_BUILD_OUTPUT_CHARS, PnpmBuildRunner } from '../src/orchestrator/buildRunner.js'; + +describe('PnpmBuildRunner', () => { + it('returns a failed result when a command cannot be spawned', async () => { + const runner = new PnpmBuildRunner(); + + const result = await runner.run([ + { + command: path.join(process.cwd(), 'missing-build-command'), + args: [], + cwd: process.cwd(), + }, + ]); + + expect(result.ok).toBe(false); + expect(result.exitCode).toBeNull(); + expect(result.output).toContain('ENOENT'); + }); + + it('retains only a bounded tail across command output', async () => { + const runner = new PnpmBuildRunner(); + const result = await runner.run([ + { + command: process.execPath, + args: ['-e', `process.stdout.write('a'.repeat(${MAX_BUILD_OUTPUT_CHARS}));`], + cwd: process.cwd(), + }, + { + command: process.execPath, + args: ['-e', "process.stdout.write('tail-marker');"], + cwd: process.cwd(), + }, + ]); + + expect(result.ok).toBe(true); + expect(result.output.length).toBe(MAX_BUILD_OUTPUT_CHARS); + expect(result.output.endsWith('tail-marker')).toBe(true); + }); +}); diff --git a/app/gateway-api/test/workspaceManager.test.ts b/app/gateway-api/test/workspaceManager.test.ts index 989ec01..e1a1685 100644 --- a/app/gateway-api/test/workspaceManager.test.ts +++ b/app/gateway-api/test/workspaceManager.test.ts @@ -85,4 +85,65 @@ describe('GitWorkspaceManager source resolution', () => { await expect(manager.resolveCommit('BRANCH', '--upload-pack=bad')).rejects.toThrow('Invalid git ref'); await expect(manager.resolveCommit('COMMIT', 'HEAD..main')).rejects.toThrow('Invalid git ref'); }); + + it('reuses only a clean registered worktree at the requested commit', async () => { + const fixture = createRepositoryFixture(); + const manager = new GitWorkspaceManager({ + repoRoot: fixture.checkout, + worktreeRoot: fixture.worktrees, + }); + + const created = await manager.prepare(fixture.firstCommit); + expect(created.created).toBe(true); + const reused = await manager.prepare(fixture.firstCommit); + expect(reused).toMatchObject({ root: created.root, created: false }); + + fs.writeFileSync(path.join(created.root, 'untracked.txt'), 'dirty\n'); + await expect(manager.prepare(fixture.firstCommit)).rejects.toThrow('uncommitted changes'); + }); + + it('rejects an unregistered directory that occupies a commit workspace path', async () => { + const fixture = createRepositoryFixture(); + const manager = new GitWorkspaceManager({ + repoRoot: fixture.checkout, + worktreeRoot: fixture.worktrees, + }); + const occupied = path.join(fixture.worktrees, fixture.firstCommit); + fs.mkdirSync(occupied, { recursive: true }); + + await expect(manager.prepare(fixture.firstCommit)).rejects.toThrow('not registered as a git worktree'); + }); + + it('removes only registered direct commit workspaces and never the root or sibling prefixes', async () => { + const fixture = createRepositoryFixture(); + const manager = new GitWorkspaceManager({ + repoRoot: fixture.checkout, + worktreeRoot: fixture.worktrees, + }); + const workspace = await manager.prepare(fixture.firstCommit); + const siblingPrefix = `${fixture.worktrees}-outside`; + fs.mkdirSync(siblingPrefix, { recursive: true }); + + await expect(manager.remove(fixture.worktrees)).rejects.toThrow('must be a child'); + await expect(manager.remove(path.join(siblingPrefix, fixture.firstCommit))).rejects.toThrow('must be a child'); + expect(fs.existsSync(siblingPrefix)).toBe(true); + await expect(manager.remove(workspace.root)).resolves.toBe(true); + expect(fs.existsSync(workspace.root)).toBe(false); + }); + + it('rejects deletion of unregistered and nested paths under the worktree root', async () => { + const fixture = createRepositoryFixture(); + const manager = new GitWorkspaceManager({ + repoRoot: fixture.checkout, + worktreeRoot: fixture.worktrees, + }); + const unregistered = path.join(fixture.worktrees, fixture.firstCommit); + fs.mkdirSync(unregistered, { recursive: true }); + + await expect(manager.remove(unregistered)).rejects.toThrow('not registered as a git worktree'); + await expect(manager.remove(path.join(unregistered, 'nested'))).rejects.toThrow( + 'not a managed commit workspace' + ); + expect(fs.existsSync(unregistered)).toBe(true); + }); });