From d8305fa016ac1a1d3137b1d71213fd0c72432d8e Mon Sep 17 00:00:00 2001 From: Hona <10430890+Hona@users.noreply.github.com> Date: Thu, 8 Oct 2026 04:55:46 +0000 Subject: [PATCH 1/2] fix(core): diff path selections through a private index to avoid ENAMETOOLONG --- packages/core/src/git.ts | 72 +++++++++++++++++++++++++++++++--- packages/core/test/git.test.ts | 52 ++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts index 224cd801cc54..d898b74ac392 100644 --- a/packages/core/src/git.ts +++ b/packages/core/src/git.ts @@ -298,6 +298,9 @@ const layer = Layer.effect( yield* operation("reset", repository.worktree, ["reset", "--hard", revision]) }) + // Never pass an unbounded path list as arguments: Windows caps a command line at + // 32,767 characters (ENAMETOOLONG). Feed paths through stdin instead, e.g. + // `--pathspec-from-file=- --pathspec-file-nul`, `--stdin`, or `update-index --index-info`. const repositoryArgs = (repository: Repository, args: string[]) => [ "--git-dir", repository.gitDirectory, @@ -513,6 +516,7 @@ const layer = Layer.effect( /** * Three batched invocations over the tree pair instead of three per file. An * explicit empty selection diffs nothing; an absent one diffs every changed path. + * A selected path also selects everything beneath it, like a literal pathspec. * Patch output is capped like VCS diffs: files past the cap get an empty patch. */ const treeDiff = Effect.fn("Git.tree.diff")(function* (input: { @@ -523,17 +527,64 @@ const layer = Layer.effect( paths?: readonly RelativePath[] }) { if (input.paths?.length === 0) return [] - const args = ["--no-renames", input.from, input.to, "--", ...(input.paths ?? [])] + if (!input.paths) return yield* diffFiles(input.repository, ["--no-renames", input.from, input.to], input.context) + // Never put the selection on the command line: a large revert selects thousands of + // files and the spawn fails with ENAMETOOLONG on Windows. `git diff` has no + // `--pathspec-from-file`, so stage only the selected changes of `to` over `from` + // in a private index (paths go through stdin) and diff that index against `from`. + const selected = new Set(input.paths) + const entries = nuls( + (yield* repositoryOperation("diff", input.repository, [ + "diff", + "--raw", + "-z", + "--no-abbrev", + "--no-renames", + input.from, + input.to, + ])).text, + ).flatMap((meta, index, records) => { + const file = records[index + 1] + if (index % 2 !== 0 || !file || !isSelected(selected, file)) return [] + // `: `; a deletion has mode 000000. + const [, mode, , oid] = meta.slice(1).split(" ") + return [`${mode === "000000" ? "0" : mode} ${oid}\t${file}\0`] + }) + if (!entries.length) return [] + const index = path.join(input.repository.gitDirectory, `opencode-diff-${crypto.randomUUID()}.index`) + const env = { GIT_INDEX_FILE: index } + return yield* Effect.gen(function* () { + yield* repositoryOperation("diff", input.repository, ["read-tree", input.from], { env }) + yield* repositoryOperation("diff", input.repository, ["update-index", "-z", "--index-info"], { + env, + stdin: entries.join(""), + }) + return yield* diffFiles(input.repository, ["--cached", "--no-renames", input.from], input.context, env) + }).pipe( + Effect.ensuring( + Effect.forEach([index, `${index}.lock`], (file) => fs.remove(file, { force: true }).pipe(Effect.ignore), { + discard: true, + }), + ), + ) + }) + + const diffFiles = Effect.fnUntraced(function* ( + repository: Repository, + args: string[], + context = 3, + env?: Record, + ) { // Patch headers have no -z form: unquoted paths keep chunksByFile matching non-ASCII names. const [names, numbers, patch] = yield* Effect.all( [ - repositoryOperation("diff", input.repository, ["diff", "--name-status", "-z", ...args]), - repositoryOperation("diff", input.repository, ["diff", "--numstat", "-z", ...args]), + repositoryOperation("diff", repository, ["diff", "--name-status", "-z", ...args], { env }), + repositoryOperation("diff", repository, ["diff", "--numstat", "-z", ...args], { env }), repositoryOperation( "diff", - input.repository, - ["-c", "core.quotepath=false", "diff", "--no-ext-diff", `--unified=${input.context ?? 3}`, ...args], - { maxOutputBytes: VcsPatch.MAX_TOTAL_PATCH_BYTES }, + repository, + ["-c", "core.quotepath=false", "diff", "--no-ext-diff", `--unified=${context}`, ...args], + { env, maxOutputBytes: VcsPatch.MAX_TOTAL_PATCH_BYTES }, ), ], { concurrency: 3 }, @@ -744,6 +795,15 @@ function execute(cwd: string, proc: AppProcess.Interface, args: string[]) { ) } +/** Whether `file` or one of its parent directories is in the selection. */ +function isSelected(selected: ReadonlySet, file: string) { + if (selected.has(".")) return true + for (let end = file.length; end > 0; end = file.lastIndexOf("/", end - 1)) { + if (selected.has(file.slice(0, end))) return true + } + return false +} + /** Split NUL-terminated git output into its records. */ function nuls(text: string) { return text.split("\0").filter(Boolean) diff --git a/packages/core/test/git.test.ts b/packages/core/test/git.test.ts index 39702b62d33e..4a938beb4a86 100644 --- a/packages/core/test/git.test.ts +++ b/packages/core/test/git.test.ts @@ -235,6 +235,58 @@ describe("Git trees", () => { }), ) + it.live("diffs a path selection longer than any platform command line", () => + Effect.gen(function* () { + const root = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (dir) => Effect.promise(() => dir[Symbol.asyncDispose]()), + ) + yield* Effect.promise(async () => { + await initRepo(root.path) + await fs.mkdir(path.join(root.path, "dir", "nested"), { recursive: true }) + await Bun.write(path.join(root.path, "changed.txt"), "before\n") + await Bun.write(path.join(root.path, "removed.txt"), "removed\n") + await Bun.write(path.join(root.path, "dir", "nested", "file.txt"), "before\n") + }) + const git = yield* Git.Service + const repository = yield* git.repo.discover(AbsolutePath.make(root.path)) + if (!repository) throw new Error("Repository not found") + const before = yield* git.tree.capture({ repository, scopes: [RelativePath.make(".")] }) + yield* Effect.promise(async () => { + await Bun.write(path.join(root.path, "changed.txt"), "after\n") + await fs.rm(path.join(root.path, "removed.txt")) + await Bun.write(path.join(root.path, "dir", "nested", "file.txt"), "after\n") + await Bun.write(path.join(root.path, "first.txt"), "first\n") + await Bun.write(path.join(root.path, "last.txt"), "last\n") + await Bun.write(path.join(root.path, "unselected.txt"), "unselected\n") + }) + const after = yield* git.tree.capture({ repository, scopes: [RelativePath.make(".")] }) + // About 3 MB of paths: past Windows' 32K command line and Linux's 2 MB ARG_MAX. + const missing = Array.from({ length: 30_000 }, (_, index) => + RelativePath.make(`missing/${String(index).padStart(6, "0")}/${"x".repeat(80)}.txt`), + ) + const paths = ["first.txt", "changed.txt", "removed.txt", "dir", ...missing, "last.txt"].map((file) => + RelativePath.make(file), + ) + + const diffs = yield* git.tree.diff({ repository, from: before, to: after, paths }) + expect(diffs.map((item) => [item.file, item.status, item.additions, item.deletions])).toEqual([ + ["changed.txt", "modified", 1, 1], + ["dir/nested/file.txt", "modified", 1, 1], + ["first.txt", "added", 1, 0], + ["last.txt", "added", 1, 0], + ["removed.txt", "deleted", 0, 1], + ]) + expect(diffs[0]?.patch).toContain("-before\n+after\n") + expect(diffs[2]?.patch).toContain("+first\n") + expect(diffs[4]?.patch).toContain("-removed\n") + expect(yield* git.tree.diff({ repository, from: before, to: after, paths: missing })).toEqual([]) + // The private index is removed and the repository's own index is untouched. + expect((yield* Effect.promise(() => fs.readdir(repository.gitDirectory))).filter((file) => file.startsWith("opencode-diff-"))).toEqual([]) + expect(yield* git.tree.write(repository)).toBe(after) + }), + ) + it.live("captures, compares, previews, and restores scoped trees", () => Effect.gen(function* () { const root = yield* Effect.acquireRelease( From ed7f7e5c97458ce93eab0932b1a94c095567e060 Mon Sep 17 00:00:00 2001 From: Hona <10430890+Hona@users.noreply.github.com> Date: Thu, 8 Oct 2026 05:35:42 +0000 Subject: [PATCH 2/2] fix(core): match tree diff selections with pathspec semantics --- packages/core/src/git.ts | 42 ++++++++++++++++++++---------- packages/core/test/git.test.ts | 47 +++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts index d898b74ac392..d673b7b8ddb0 100644 --- a/packages/core/src/git.ts +++ b/packages/core/src/git.ts @@ -532,8 +532,10 @@ const layer = Layer.effect( // files and the spawn fails with ENAMETOOLONG on Windows. `git diff` has no // `--pathspec-from-file`, so stage only the selected changes of `to` over `from` // in a private index (paths go through stdin) and diff that index against `from`. - const selected = new Set(input.paths) - const entries = nuls( + // Normalize like a pathspec: `./a` and `a//b` mean `a` and `a/b`; a trailing slash keeps + // its directory-only meaning. + const selected = new Set(input.paths.map((file) => path.posix.normalize(file))) + const changes = nuls( (yield* repositoryOperation("diff", input.repository, [ "diff", "--raw", @@ -545,21 +547,30 @@ const layer = Layer.effect( ])).text, ).flatMap((meta, index, records) => { const file = records[index + 1] - if (index % 2 !== 0 || !file || !isSelected(selected, file)) return [] + if (index % 2 !== 0 || !file) return [] // `: `; a deletion has mode 000000. - const [, mode, , oid] = meta.slice(1).split(" ") - return [`${mode === "000000" ? "0" : mode} ${oid}\t${file}\0`] + const [before, mode, , oid] = meta.slice(1).split(" ") + if (!isSelected(selected, file, before === "160000" || mode === "160000")) return [] + return [{ file, entry: `${mode === "000000" ? "0" : mode} ${oid}\t${file}\0` }] }) - if (!entries.length) return [] + if (!changes.length) return [] + const chosen = new Set(changes.map((change) => change.file)) const index = path.join(input.repository.gitDirectory, `opencode-diff-${crypto.randomUUID()}.index`) const env = { GIT_INDEX_FILE: index } return yield* Effect.gen(function* () { yield* repositoryOperation("diff", input.repository, ["read-tree", input.from], { env }) yield* repositoryOperation("diff", input.repository, ["update-index", "-z", "--index-info"], { env, - stdin: entries.join(""), + stdin: changes.map((change) => change.entry).join(""), }) - return yield* diffFiles(input.repository, ["--cached", "--no-renames", input.from], input.context, env) + // Staging a path over a file/directory conflict drops the other side from the index, + // so the index diff can include unselected deletions; keep only the selection. + return (yield* diffFiles( + input.repository, + ["--cached", "--no-renames", input.from], + input.context, + env, + )).filter((entry) => chosen.has(entry.file)) }).pipe( Effect.ensuring( Effect.forEach([index, `${index}.lock`], (file) => fs.remove(file, { force: true }).pipe(Effect.ignore), { @@ -795,11 +806,16 @@ function execute(cwd: string, proc: AppProcess.Interface, args: string[]) { ) } -/** Whether `file` or one of its parent directories is in the selection. */ -function isSelected(selected: ReadonlySet, file: string) { - if (selected.has(".")) return true - for (let end = file.length; end > 0; end = file.lastIndexOf("/", end - 1)) { - if (selected.has(file.slice(0, end))) return true +/** + * Whether `file` or one of its parent directories is in the normalized selection. Like a + * pathspec, a trailing slash selects only a directory or a submodule (gitlink) entry. + */ +function isSelected(selected: ReadonlySet, file: string, gitlink: boolean) { + if (selected.has(".") || selected.has("./") || selected.has(file)) return true + if (gitlink && selected.has(`${file}/`)) return true + for (let end = file.lastIndexOf("/"); end > 0; end = file.lastIndexOf("/", end - 1)) { + const directory = file.slice(0, end) + if (selected.has(directory) || selected.has(`${directory}/`)) return true } return false } diff --git a/packages/core/test/git.test.ts b/packages/core/test/git.test.ts index 4a938beb4a86..67f73fee8efb 100644 --- a/packages/core/test/git.test.ts +++ b/packages/core/test/git.test.ts @@ -235,6 +235,47 @@ describe("Git trees", () => { }), ) + it.live("selects tree diff paths with pathspec semantics", () => + Effect.gen(function* () { + const root = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (dir) => Effect.promise(() => dir[Symbol.asyncDispose]()), + ) + yield* Effect.promise(async () => { + await initRepo(root.path) + await Bun.write(path.join(root.path, "swap"), "file\n") + await Bun.write(path.join(root.path, "plain"), "before\n") + }) + const git = yield* Git.Service + const repository = yield* git.repo.discover(AbsolutePath.make(root.path)) + if (!repository) throw new Error("Repository not found") + const before = yield* git.tree.capture({ repository, scopes: [RelativePath.make(".")] }) + yield* Effect.promise(async () => { + await fs.rm(path.join(root.path, "swap")) + await Bun.write(path.join(root.path, "swap", "child.txt"), "child\n") + await Bun.write(path.join(root.path, "plain"), "after\n") + }) + const after = yield* git.tree.capture({ repository, scopes: [RelativePath.make(".")] }) + const select = (...paths: string[]) => + git.tree + .diff({ repository, from: before, to: after, paths: paths.map((file) => RelativePath.make(file)) }) + .pipe(Effect.map((diffs) => diffs.map((item) => [item.file, item.status]))) + + // Staging the child over the replaced file must not report the unselected file deletion. + expect(yield* select("swap/child.txt")).toEqual([["swap/child.txt", "added"]]) + expect(yield* select("./swap//child.txt", "plain")).toEqual([ + ["plain", "modified"], + ["swap/child.txt", "added"], + ]) + // A trailing slash selects directories only, not the file that was replaced. + expect(yield* select("swap/", "plain/")).toEqual([["swap/child.txt", "added"]]) + expect(yield* select("swap")).toEqual([ + ["swap", "deleted"], + ["swap/child.txt", "added"], + ]) + }), + ) + it.live("diffs a path selection longer than any platform command line", () => Effect.gen(function* () { const root = yield* Effect.acquireRelease( @@ -282,7 +323,11 @@ describe("Git trees", () => { expect(diffs[4]?.patch).toContain("-removed\n") expect(yield* git.tree.diff({ repository, from: before, to: after, paths: missing })).toEqual([]) // The private index is removed and the repository's own index is untouched. - expect((yield* Effect.promise(() => fs.readdir(repository.gitDirectory))).filter((file) => file.startsWith("opencode-diff-"))).toEqual([]) + expect( + (yield* Effect.promise(() => fs.readdir(repository.gitDirectory))).filter((file) => + file.startsWith("opencode-diff-"), + ), + ).toEqual([]) expect(yield* git.tree.write(repository)).toBe(after) }), )