diff --git a/cli/src/execution/sandbox-git.test.ts b/cli/src/execution/sandbox-git.test.ts new file mode 100644 index 00000000..a3ac2fc8 --- /dev/null +++ b/cli/src/execution/sandbox-git.test.ts @@ -0,0 +1,217 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import simpleGit from "simple-git"; +import { commitSandboxChanges } from "./sandbox-git.ts"; + +const TEST_BASE = join(tmpdir(), "ralphy-sandbox-git-test"); + +async function makeGitRepo(): Promise { + const id = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + const dir = join(TEST_BASE, id); + mkdirSync(dir, { recursive: true }); + + const git = simpleGit(dir); + await git.init(); + await git.addConfig("user.name", "Test"); + await git.addConfig("user.email", "test@test.com"); + + // Create initial commit so we have a branch + writeFileSync(join(dir, "README.md"), "# Test"); + await git.add("README.md"); + await git.commit("initial commit"); + + return dir; +} + +describe("commitSandboxChanges", () => { + let originalDir: string; + let sandboxDir: string; + + beforeEach(async () => { + rmSync(TEST_BASE, { recursive: true, force: true }); + originalDir = await makeGitRepo(); + sandboxDir = join(TEST_BASE, "sandbox"); + mkdirSync(sandboxDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(TEST_BASE, { recursive: true, force: true }); + }); + + it("should return success with empty branch when no files provided", async () => { + const result = await commitSandboxChanges(originalDir, [], sandboxDir, "empty task", 1, "main"); + + expect(result.success).toBe(true); + expect(result.branchName).toBe(""); + expect(result.filesCommitted).toBe(0); + }); + + it("should commit valid source files to a new branch", async () => { + // Create a file in sandbox that will be copied to original + writeFileSync(join(sandboxDir, "feature.ts"), "export const feature = true;"); + + const git = simpleGit(originalDir); + const baseBranch = (await git.branch()).current; + + const result = await commitSandboxChanges( + originalDir, + ["feature.ts"], + sandboxDir, + "add feature", + 1, + baseBranch, + ); + + expect(result.success).toBe(true); + expect(result.branchName).toContain("ralphy/agent-1-"); + expect(result.branchName).toContain("add-feature"); + expect(result.filesCommitted).toBe(1); + + // Should return to the original branch + const currentBranch = (await git.branch()).current; + expect(currentBranch).toBe(baseBranch); + }); + + it("should filter out gitignored files and still commit valid ones", async () => { + // Add a .gitignore to the repo + writeFileSync(join(originalDir, ".gitignore"), ".DS_Store\n*.log\n"); + const git = simpleGit(originalDir); + await git.add(".gitignore"); + await git.commit("add gitignore"); + + const baseBranch = (await git.branch()).current; + + // Create files in sandbox -- one valid, two gitignored + writeFileSync(join(sandboxDir, "feature.ts"), "export const f = 1;"); + writeFileSync(join(sandboxDir, ".DS_Store"), "binary junk"); + writeFileSync(join(sandboxDir, "debug.log"), "log output"); + + const result = await commitSandboxChanges( + originalDir, + ["feature.ts", ".DS_Store", "debug.log"], + sandboxDir, + "feature with ignored files", + 2, + baseBranch, + ); + + expect(result.success).toBe(true); + + // Verify the commit only has the valid file + await git.checkout(result.branchName); + const diff = await git.diff(["--name-only", "HEAD~1", "HEAD"]); + const committedFiles = diff.trim().split("\n").filter(Boolean); + + expect(committedFiles).toContain("feature.ts"); + expect(committedFiles).not.toContain(".DS_Store"); + expect(committedFiles).not.toContain("debug.log"); + + await git.checkout(baseBranch); + }); + + it("should succeed with no commit when ALL files are gitignored", async () => { + // Add a .gitignore to the repo + writeFileSync(join(originalDir, ".gitignore"), ".DS_Store\nnode_modules/\n"); + const git = simpleGit(originalDir); + await git.add(".gitignore"); + await git.commit("add gitignore"); + + const baseBranch = (await git.branch()).current; + + // Only gitignored files + writeFileSync(join(sandboxDir, ".DS_Store"), "binary junk"); + + const result = await commitSandboxChanges( + originalDir, + [".DS_Store"], + sandboxDir, + "only ignored files", + 3, + baseBranch, + ); + + expect(result.success).toBe(true); + expect(result.branchName).toBe(""); + expect(result.filesCommitted).toBe(0); + + // Should still be on the original branch + const currentBranch = (await git.branch()).current; + expect(currentBranch).toBe(baseBranch); + }); + + it("should commit files in nested directories", async () => { + const nestedDir = join(sandboxDir, "src", "utils"); + mkdirSync(nestedDir, { recursive: true }); + writeFileSync(join(nestedDir, "helper.ts"), "export const h = 1;"); + + const git = simpleGit(originalDir); + const baseBranch = (await git.branch()).current; + + const result = await commitSandboxChanges( + originalDir, + [join("src", "utils", "helper.ts")], + sandboxDir, + "add helper", + 4, + baseBranch, + ); + + expect(result.success).toBe(true); + expect(result.filesCommitted).toBe(1); + }); + + it("should create branch name with agent number and slugified task name", async () => { + writeFileSync(join(sandboxDir, "file.ts"), "export {};"); + + const git = simpleGit(originalDir); + const baseBranch = (await git.branch()).current; + + const result = await commitSandboxChanges( + originalDir, + ["file.ts"], + sandboxDir, + "Implement POST /workloads/start with idempotent deployment", + 7, + baseBranch, + ); + + expect(result.success).toBe(true); + expect(result.branchName).toMatch(/^ralphy\/agent-7-\d+-[a-z0-9]+-implement-post-/); + }); + + it("should return to original branch after committing", async () => { + writeFileSync(join(sandboxDir, "file.ts"), "export {};"); + + const git = simpleGit(originalDir); + const baseBranch = (await git.branch()).current; + + await commitSandboxChanges(originalDir, ["file.ts"], sandboxDir, "task", 1, baseBranch); + + const currentBranch = (await git.branch()).current; + expect(currentBranch).toBe(baseBranch); + }); + + it("should return to base branch on error", async () => { + // Provide a file that does not exist in sandbox -- writeFileSync skipped + // so copying will silently skip, but staging a non-existent file may fail + const git = simpleGit(originalDir); + const baseBranch = (await git.branch()).current; + + await commitSandboxChanges( + originalDir, + ["nonexistent-file.ts"], + sandboxDir, + "broken task", + 99, + baseBranch, + ); + + // The file copy is skipped (existsSync check), then git.add on a file that + // doesn't exist in the work tree may either error or succeed with nothing. + // Either way, we should end up on the base branch. + const currentBranch = (await git.branch()).current; + expect(currentBranch).toBe(baseBranch); + }); +}); diff --git a/cli/src/execution/sandbox-git.ts b/cli/src/execution/sandbox-git.ts index 45ceb1e9..6cbdac5d 100644 --- a/cli/src/execution/sandbox-git.ts +++ b/cli/src/execution/sandbox-git.ts @@ -104,8 +104,33 @@ export async function commitSandboxChanges( } } - // Stage all modified files - await git.add(modifiedFiles); + // Filter out gitignored files before staging + let filesToStage = modifiedFiles; + try { + const ignored = await git.raw(["check-ignore", ...modifiedFiles]); + const ignoredSet = new Set( + ignored + .split("\n") + .map((l) => l.trim()) + .filter(Boolean), + ); + filesToStage = modifiedFiles.filter((f) => !ignoredSet.has(f)); + } catch { + // check-ignore exits non-zero when NO files are ignored -- that means all files are safe + } + + if (filesToStage.length === 0) { + logDebug(`Agent ${agentNum}: All modified files are gitignored, nothing to commit`); + await git.checkout(currentBranch); + return { + success: true, + branchName: "", + filesCommitted: 0, + }; + } + + // Stage non-ignored modified files + await git.add(filesToStage); // Commit const commitMessage = `feat: ${taskName}\n\nAutomated commit by Ralphy agent ${agentNum}`; diff --git a/cli/src/execution/sandbox.test.ts b/cli/src/execution/sandbox.test.ts new file mode 100644 index 00000000..f523c8d3 --- /dev/null +++ b/cli/src/execution/sandbox.test.ts @@ -0,0 +1,314 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { + existsSync, + mkdirSync, + rmSync, + symlinkSync, + utimesSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + createSandbox, + getModifiedFiles, + verifySandboxIsolation, +} from "./sandbox.ts"; + +const TEST_BASE = join(tmpdir(), "ralphy-sandbox-test"); + +function makeTestDir(): string { + const id = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + const dir = join(TEST_BASE, id); + mkdirSync(dir, { recursive: true }); + return dir; +} + +describe("getModifiedFiles", () => { + let originalDir: string; + let sandboxDir: string; + + beforeEach(() => { + originalDir = makeTestDir(); + sandboxDir = makeTestDir(); + }); + + afterEach(() => { + rmSync(TEST_BASE, { recursive: true, force: true }); + }); + + it("should detect a new file in the sandbox", async () => { + writeFileSync(join(sandboxDir, "new-file.ts"), "export const x = 1;"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).toContain("new-file.ts"); + }); + + it("should detect a modified file by size change", async () => { + const original = "export const x = 1;"; + const changed = "export const x = 42; // updated"; + + writeFileSync(join(originalDir, "file.ts"), original); + writeFileSync(join(sandboxDir, "file.ts"), changed); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).toContain("file.ts"); + }); + + it("should detect a modified file by mtime change", async () => { + const content = "export const x = 1;"; + writeFileSync(join(originalDir, "file.ts"), content); + writeFileSync(join(sandboxDir, "file.ts"), content); + + // Force different mtime + const past = new Date(2000, 0, 1); + utimesSync(join(originalDir, "file.ts"), past, past); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).toContain("file.ts"); + }); + + it("should not report unchanged files", async () => { + const content = "export const x = 1;"; + writeFileSync(join(originalDir, "file.ts"), content); + writeFileSync(join(sandboxDir, "file.ts"), content); + + // Match timestamps + const ts = new Date(2024, 0, 1); + utimesSync(join(originalDir, "file.ts"), ts, ts); + utimesSync(join(sandboxDir, "file.ts"), ts, ts); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).not.toContain("file.ts"); + }); + + it("should detect new files in nested directories", async () => { + const nestedDir = join(sandboxDir, "src", "utils"); + mkdirSync(nestedDir, { recursive: true }); + writeFileSync(join(nestedDir, "helper.ts"), "export const h = 1;"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).toContain(join("src", "utils", "helper.ts")); + }); + + describe("skips gitignored entries", () => { + it("should skip .DS_Store at the root", async () => { + writeFileSync(join(sandboxDir, ".DS_Store"), "binary junk"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).not.toContain(".DS_Store"); + }); + + it("should skip .DS_Store in nested directories", async () => { + const nested = join(sandboxDir, "packages", "foo"); + mkdirSync(nested, { recursive: true }); + writeFileSync(join(nested, ".DS_Store"), "binary junk"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + const hasDSStore = modified.some((f) => f.includes(".DS_Store")); + expect(hasDSStore).toBe(false); + }); + + it("should skip Thumbs.db", async () => { + writeFileSync(join(sandboxDir, "Thumbs.db"), "binary junk"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).not.toContain("Thumbs.db"); + }); + + it("should skip nested node_modules directories", async () => { + const nestedNM = join(sandboxDir, "packages", "gateway", "node_modules", "lodash"); + mkdirSync(nestedNM, { recursive: true }); + writeFileSync(join(nestedNM, "index.js"), "module.exports = {};"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + const hasNodeModules = modified.some((f) => f.includes("node_modules")); + expect(hasNodeModules).toBe(false); + }); + + it("should skip dist directories at any depth", async () => { + const nestedDist = join(sandboxDir, "packages", "lib", "dist"); + mkdirSync(nestedDist, { recursive: true }); + writeFileSync(join(nestedDist, "index.js"), "built output"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + const hasDist = modified.some((f) => f.includes("dist")); + expect(hasDist).toBe(false); + }); + + it("should skip .tsbuildinfo files", async () => { + writeFileSync(join(sandboxDir, "tsconfig.tsbuildinfo"), "{}"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + const hasTsBuildInfo = modified.some((f) => f.includes(".tsbuildinfo")); + expect(hasTsBuildInfo).toBe(false); + }); + + it("should skip __pycache__ directories", async () => { + const pycache = join(sandboxDir, "src", "__pycache__"); + mkdirSync(pycache, { recursive: true }); + writeFileSync(join(pycache, "mod.cpython-312.pyc"), "bytecode"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + const hasPycache = modified.some((f) => f.includes("__pycache__")); + expect(hasPycache).toBe(false); + }); + }); + + it("should skip symlinked directories", async () => { + // Create a real directory in original and symlink it in sandbox + const realDir = join(originalDir, "node_modules"); + mkdirSync(realDir, { recursive: true }); + writeFileSync(join(realDir, "pkg.json"), "{}"); + symlinkSync(realDir, join(sandboxDir, "node_modules"), "junction"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + const hasNodeModules = modified.some((f) => f.includes("node_modules")); + expect(hasNodeModules).toBe(false); + }); + + it("should still detect legitimate source changes alongside ignored files", async () => { + // Legitimate change + writeFileSync(join(sandboxDir, "app.ts"), "console.log('hello');"); + + // Ignored entries + writeFileSync(join(sandboxDir, ".DS_Store"), "junk"); + writeFileSync(join(sandboxDir, "tsconfig.tsbuildinfo"), "{}"); + + const modified = await getModifiedFiles(sandboxDir, originalDir); + + expect(modified).toContain("app.ts"); + expect(modified).not.toContain(".DS_Store"); + expect(modified).not.toContain("tsconfig.tsbuildinfo"); + expect(modified).toHaveLength(1); + }); +}); + +describe("createSandbox", () => { + let originalDir: string; + let sandboxDir: string; + + beforeEach(() => { + originalDir = makeTestDir(); + sandboxDir = join(TEST_BASE, `sandbox-${Date.now()}`); + }); + + afterEach(() => { + rmSync(TEST_BASE, { recursive: true, force: true }); + }); + + it("should create sandbox directory", async () => { + writeFileSync(join(originalDir, "index.ts"), "export {};"); + + const result = await createSandbox({ + originalDir, + sandboxDir, + agentNum: 1, + }); + + expect(result.sandboxDir).toBe(sandboxDir); + expect(existsSync(sandboxDir)).toBe(true); + }); + + it("should copy source files into sandbox", async () => { + writeFileSync(join(originalDir, "index.ts"), "export const x = 1;"); + writeFileSync(join(originalDir, "README.md"), "# Hello"); + + await createSandbox({ + originalDir, + sandboxDir, + agentNum: 1, + }); + + expect(existsSync(join(sandboxDir, "index.ts"))).toBe(true); + expect(existsSync(join(sandboxDir, "README.md"))).toBe(true); + }); + + it("should copy nested directories recursively", async () => { + mkdirSync(join(originalDir, "src", "utils"), { recursive: true }); + writeFileSync(join(originalDir, "src", "utils", "helper.ts"), "export {};"); + + await createSandbox({ + originalDir, + sandboxDir, + agentNum: 1, + }); + + expect(existsSync(join(sandboxDir, "src", "utils", "helper.ts"))).toBe(true); + }); + + it("should symlink specified directories instead of copying", async () => { + const nmDir = join(originalDir, "test_deps"); + mkdirSync(nmDir, { recursive: true }); + writeFileSync(join(nmDir, "package.json"), "{}"); + + await createSandbox({ + originalDir, + sandboxDir, + agentNum: 1, + symlinkDirs: ["test_deps"], + }); + + const { lstatSync } = await import("node:fs"); + const stat = lstatSync(join(sandboxDir, "test_deps")); + expect(stat.isSymbolicLink()).toBe(true); + }); + + it("should return correct counts", async () => { + writeFileSync(join(originalDir, "file1.ts"), "a"); + writeFileSync(join(originalDir, "file2.ts"), "b"); + + const result = await createSandbox({ + originalDir, + sandboxDir, + agentNum: 1, + symlinkDirs: [], + }); + + expect(result.filesCopied).toBe(2); + expect(result.symlinksCreated).toBe(0); + }); +}); + +describe("verifySandboxIsolation", () => { + let sandboxDir: string; + let originalDir: string; + + beforeEach(() => { + originalDir = makeTestDir(); + sandboxDir = makeTestDir(); + }); + + afterEach(() => { + rmSync(TEST_BASE, { recursive: true, force: true }); + }); + + it("should return true when specified dirs are symlinked", () => { + const realDir = join(originalDir, "deps"); + mkdirSync(realDir, { recursive: true }); + symlinkSync(realDir, join(sandboxDir, "deps"), "junction"); + + const result = verifySandboxIsolation(sandboxDir, ["deps"]); + + expect(result).toBe(true); + }); + + it("should return true when symlink dirs do not exist in sandbox", () => { + const result = verifySandboxIsolation(sandboxDir, ["nonexistent"]); + + expect(result).toBe(true); + }); +}); diff --git a/cli/src/execution/sandbox.ts b/cli/src/execution/sandbox.ts index d27e76c9..a43748f7 100644 --- a/cli/src/execution/sandbox.ts +++ b/cli/src/execution/sandbox.ts @@ -267,6 +267,33 @@ export function verifySandboxIsolation(sandboxDir: string, symlinkDirs: string[] * Get list of files modified in the sandbox compared to original. * Uses file modification time comparison. */ +/** + * Directory and file names that should never be reported as modified. + * These are commonly generated by the OS or build tools and are always gitignored. + */ +const ALWAYS_SKIP_NAMES = new Set([ + ".DS_Store", + "Thumbs.db", + "node_modules", + "dist", + ".cache", + "__pycache__", + ".tsbuildinfo", +]); + +/** + * File extensions that should never be reported as modified. + */ +const ALWAYS_SKIP_EXTENSIONS = new Set([".tsbuildinfo"]); + +function shouldSkipEntry(name: string): boolean { + if (ALWAYS_SKIP_NAMES.has(name)) return true; + for (const ext of ALWAYS_SKIP_EXTENSIONS) { + if (name.endsWith(ext)) return true; + } + return false; +} + export async function getModifiedFiles( sandboxDir: string, originalDir: string, @@ -289,6 +316,10 @@ export async function getModifiedFiles( const topLevel = relPath.split(sep)[0]; if (symlinkDirs.includes(topLevel)) return; + // Skip commonly gitignored files and directories at any depth + const name = relPath.split(sep).pop() ?? ""; + if (shouldSkipEntry(name)) return; + if (stat.isDirectory()) { const items = readdirSync(sandboxPath); for (const item of items) { @@ -315,6 +346,9 @@ export async function getModifiedFiles( const itemStat = lstatSync(itemPath); if (itemStat.isSymbolicLink()) continue; + // Skip commonly gitignored entries at the top level too + if (shouldSkipEntry(item)) continue; + if (itemStat.isDirectory()) { scanDir(item); } else if (itemStat.isFile()) {