diff --git a/packages/opencode/src/snapshot/index.ts b/packages/opencode/src/snapshot/index.ts index 1c1539090542..a9f741015c36 100644 --- a/packages/opencode/src/snapshot/index.ts +++ b/packages/opencode/src/snapshot/index.ts @@ -6,6 +6,7 @@ import { Global } from "../global" import z from "zod" import { Config } from "../config/config" import { Instance } from "../project/instance" +import { Filesystem } from "../util/filesystem" import { Scheduler } from "../scheduler" export namespace Snapshot { @@ -133,22 +134,43 @@ export namespace Snapshot { for (const item of patches) { for (const file of item.files) { if (files.has(file)) continue - log.info("reverting", { file, hash: item.hash }) - const result = await $`git --git-dir ${git} --work-tree ${Instance.worktree} checkout ${item.hash} -- ${file}` - .quiet() - .cwd(Instance.worktree) - .nothrow() + const relativePath = path.relative(Instance.worktree, file) + if (!Filesystem.contains(Instance.worktree, file) || path.isAbsolute(relativePath) || relativePath === "") { + log.warn("skipping file outside worktree", { file, relativePath, worktree: Instance.worktree }) + files.add(file) + continue + } + const gitPath = process.platform === "win32" ? relativePath.replaceAll("\\", "/") : relativePath + log.info("reverting", { file, gitPath, hash: item.hash }) + const result = + await $`git --git-dir ${git} --work-tree ${Instance.worktree} checkout ${item.hash} -- ${gitPath}` + .quiet() + .cwd(Instance.worktree) + .nothrow() if (result.exitCode !== 0) { - const relativePath = path.relative(Instance.worktree, file) const checkTree = - await $`git --git-dir ${git} --work-tree ${Instance.worktree} ls-tree ${item.hash} -- ${relativePath}` + await $`git --git-dir ${git} --work-tree ${Instance.worktree} ls-tree ${item.hash} -- ${gitPath}` .quiet() .cwd(Instance.worktree) .nothrow() if (checkTree.exitCode === 0 && checkTree.text().trim()) { - log.info("file existed in snapshot but checkout failed, keeping", { - file, - }) + log.info("checkout failed, trying git show fallback", { file, gitPath }) + const content = await $`git --git-dir ${git} show ${item.hash}:${gitPath}` + .quiet() + .cwd(Instance.worktree) + .nothrow() + if (content.exitCode === 0) { + const dir = path.dirname(file) + await fs.mkdir(dir, { recursive: true }) + await fs.writeFile(file, content.stdout) + log.info("restored file via git show", { file }) + } else { + log.warn("failed to restore file, keeping current version", { + file, + checkoutError: result.stderr.toString(), + showError: content.stderr.toString(), + }) + } } else { log.info("file did not exist in snapshot, deleting", { file }) await fs.unlink(file).catch(() => {}) diff --git a/packages/opencode/test/snapshot/snapshot.test.ts b/packages/opencode/test/snapshot/snapshot.test.ts index de58f4f85e67..6a4ae7e90d10 100644 --- a/packages/opencode/test/snapshot/snapshot.test.ts +++ b/packages/opencode/test/snapshot/snapshot.test.ts @@ -1,5 +1,6 @@ import { test, expect } from "bun:test" import { $ } from "bun" +import path from "path" import { Snapshot } from "../../src/snapshot" import { Instance } from "../../src/project/instance" import { tmpdir } from "../fixture/fixture" @@ -992,3 +993,324 @@ test("diffFull with whitespace changes", async () => { }, }) }) + +test("patch returns absolute paths and revert handles them correctly", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/newfile.txt`, "new content") + await Bun.write(`${tmp.path}/b.txt`, "modified content") + + const patch = await Snapshot.patch(before!) + + expect(patch.files.length).toBe(2) + for (const file of patch.files) { + expect(path.isAbsolute(file)).toBe(true) + expect(file.startsWith(tmp.path)).toBe(true) + } + + await Snapshot.revert([patch]) + + expect(await Bun.file(`${tmp.path}/newfile.txt`).exists()).toBe(false) + expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe(tmp.extra.bContent) + }, + }) +}) + +test("revert restores modified file content correctly", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + const originalA = tmp.extra.aContent + const originalB = tmp.extra.bContent + await Bun.write(`${tmp.path}/a.txt`, "COMPLETELY DIFFERENT CONTENT A") + await Bun.write(`${tmp.path}/b.txt`, "COMPLETELY DIFFERENT CONTENT B") + + expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe("COMPLETELY DIFFERENT CONTENT A") + expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe("COMPLETELY DIFFERENT CONTENT B") + + await Snapshot.revert([await Snapshot.patch(before!)]) + + expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(originalA) + expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe(originalB) + }, + }) +}) + +test("revert handles deeply nested files with absolute paths", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await $`mkdir -p ${tmp.path}/level1/level2/level3`.quiet() + await Bun.write(`${tmp.path}/level1/level2/level3/existing.txt`, "original nested content") + + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/level1/level2/level3/existing.txt`, "MODIFIED") + await Bun.write(`${tmp.path}/level1/level2/level3/new.txt`, "new nested file") + await Bun.write(`${tmp.path}/level1/level2/another.txt`, "another new file") + + const patch = await Snapshot.patch(before!) + + for (const file of patch.files) { + expect(path.isAbsolute(file)).toBe(true) + } + + await Snapshot.revert([patch]) + + expect(await Bun.file(`${tmp.path}/level1/level2/level3/existing.txt`).text()).toBe("original nested content") + expect(await Bun.file(`${tmp.path}/level1/level2/level3/new.txt`).exists()).toBe(false) + expect(await Bun.file(`${tmp.path}/level1/level2/another.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert handles files with spaces in names", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Bun.write(`${tmp.path}/file with spaces.txt`, "original spaces") + await Bun.write(`${tmp.path}/file-with-dashes.txt`, "original dashes") + + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/file with spaces.txt`, "MODIFIED spaces") + await Bun.write(`${tmp.path}/new file with spaces.txt`, "new file") + await $`mkdir -p "${tmp.path}/dir with spaces"`.quiet() + await Bun.write(`${tmp.path}/dir with spaces/nested file.txt`, "nested in space dir") + + await Snapshot.revert([await Snapshot.patch(before!)]) + + expect(await Bun.file(`${tmp.path}/file with spaces.txt`).text()).toBe("original spaces") + expect(await Bun.file(`${tmp.path}/file-with-dashes.txt`).text()).toBe("original dashes") + expect(await Bun.file(`${tmp.path}/new file with spaces.txt`).exists()).toBe(false) + expect(await Bun.file(`${tmp.path}/dir with spaces/nested file.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert handles multiple sequential patches correctly", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const snapshot1 = await Snapshot.track() + expect(snapshot1).toBeTruthy() + + await Bun.write(`${tmp.path}/a.txt`, "version 2") + await Bun.write(`${tmp.path}/new1.txt`, "new file 1") + + const patch1 = await Snapshot.patch(snapshot1!) + const snapshot2 = await Snapshot.track() + + await Bun.write(`${tmp.path}/a.txt`, "version 3") + await Bun.write(`${tmp.path}/new2.txt`, "new file 2") + + const patch2 = await Snapshot.patch(snapshot2!) + + expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe("version 3") + expect(await Bun.file(`${tmp.path}/new1.txt`).exists()).toBe(true) + expect(await Bun.file(`${tmp.path}/new2.txt`).exists()).toBe(true) + + await Snapshot.revert([patch2]) + expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe("version 2") + expect(await Bun.file(`${tmp.path}/new2.txt`).exists()).toBe(false) + + await Snapshot.revert([patch1]) + expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(tmp.extra.aContent) + expect(await Bun.file(`${tmp.path}/new1.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert with patches array containing multiple items", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const snapshot1 = await Snapshot.track() + expect(snapshot1).toBeTruthy() + + await Bun.write(`${tmp.path}/file1.txt`, "content 1") + const patch1 = await Snapshot.patch(snapshot1!) + + const snapshot2 = await Snapshot.track() + await Bun.write(`${tmp.path}/file2.txt`, "content 2") + const patch2 = await Snapshot.patch(snapshot2!) + + await Snapshot.revert([patch2, patch1]) + + expect(await Bun.file(`${tmp.path}/file1.txt`).exists()).toBe(false) + expect(await Bun.file(`${tmp.path}/file2.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert deletes file created after snapshot", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await $`mkdir -p ${tmp.path}/newdir/subdir`.quiet() + await Bun.write(`${tmp.path}/newdir/subdir/created.txt`, "created after snapshot") + + const patch = await Snapshot.patch(before!) + expect(patch.files).toContain(`${tmp.path}/newdir/subdir/created.txt`) + + await Snapshot.revert([patch]) + + expect(await Bun.file(`${tmp.path}/newdir/subdir/created.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert restores deleted file", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await $`rm ${tmp.path}/a.txt`.quiet() + expect(await Bun.file(`${tmp.path}/a.txt`).exists()).toBe(false) + + await Snapshot.revert([await Snapshot.patch(before!)]) + + expect(await Bun.file(`${tmp.path}/a.txt`).exists()).toBe(true) + expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(tmp.extra.aContent) + }, + }) +}) + +test("patch files relative path computation is correct", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/test.txt`, "test") + + const patch = await Snapshot.patch(before!) + + for (const file of patch.files) { + const relativePath = path.relative(tmp.path, file) + expect(relativePath).not.toContain("..") + expect(path.isAbsolute(relativePath)).toBe(false) + } + }, + }) +}) + +test("revert works when process cwd differs from worktree", async () => { + await using tmp = await bootstrap() + await using otherDir = await tmpdir({ git: false }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/modified.txt`, "modified content") + await Bun.write(`${tmp.path}/new.txt`, "new content") + + const patch = await Snapshot.patch(before!) + + const originalCwd = process.cwd() + try { + process.chdir(otherDir.path) + await Snapshot.revert([patch]) + } finally { + process.chdir(originalCwd) + } + + expect(await Bun.file(`${tmp.path}/modified.txt`).exists()).toBe(false) + expect(await Bun.file(`${tmp.path}/new.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert skips files outside worktree", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/safe.txt`, "safe content") + + const patch = await Snapshot.patch(before!) + patch.files.push("/tmp/outside-worktree.txt") + patch.files.push(`${tmp.path}/../escape-attempt.txt`) + + await Snapshot.revert([patch]) + + expect(await Bun.file(`${tmp.path}/safe.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert skips patch with empty relativePath", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/normal.txt`, "normal content") + + const patch = await Snapshot.patch(before!) + patch.files.push(tmp.path) + + await Snapshot.revert([patch]) + + expect(await Bun.file(`${tmp.path}/normal.txt`).exists()).toBe(false) + }, + }) +}) + +test("revert creates parent directory when using fallback", async () => { + await using tmp = await bootstrap() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await $`mkdir -p ${tmp.path}/deep/nested/dir`.quiet() + await Bun.write(`${tmp.path}/deep/nested/dir/file.txt`, "original content") + + const before = await Snapshot.track() + expect(before).toBeTruthy() + + await Bun.write(`${tmp.path}/deep/nested/dir/file.txt`, "modified content") + + const patch = await Snapshot.patch(before!) + + await $`rm -rf ${tmp.path}/deep`.quiet() + expect(await Bun.file(`${tmp.path}/deep/nested/dir/file.txt`).exists()).toBe(false) + + await Snapshot.revert([patch]) + + expect(await Bun.file(`${tmp.path}/deep/nested/dir/file.txt`).exists()).toBe(true) + expect(await Bun.file(`${tmp.path}/deep/nested/dir/file.txt`).text()).toBe("original content") + }, + }) +})