Describe the bug
git diff reports a file that exists in neither endpoint it is comparing.
When a path is staged and then removed from the working tree, git diff HEAD
compares HEAD against the working tree, and the path is absent from both. Real
git prints nothing. diffWith prints a header-only patch with no hunks, and
diffSummaryWith reports the path as an addition, so git diff --stat and
git diff --name-status list a file that was never added and does not exist.
The two surfaces also disagree with each other in a subtler way. diffWith
happens to emit a patch body, so a consumer diffing text sees a stray file
header. diffSummaryWith reports status: "A" with insertions: 0 and
deletions: 0, which reads as "an empty file was added".
Expected behavior
Nothing on every diff surface, matching git:
$ git init -b main . && echo base > base.txt && git add base.txt && git commit -m init
$ echo hello > added.txt && git add added.txt && rm added.txt
$ git diff HEAD
$ git diff HEAD --stat
$ git diff HEAD --name-status
$ git status --short
AD added.txt
git status is right to mention the path, because the index holds a version of
it. git diff HEAD is right to stay silent, because neither side it compares
contains it.
Steps to reproduce
Against 76d9e75. The failing test below is written in the style of the
existing packages/computer/src/git/diff.test.ts, using memfs with the real
isomorphic-git and the real diff package, so nothing in the path under test
is stubbed.
packages/computer/src/git/diff.repro.test.ts:
import { createPatch } from "diff";
import git from "isomorphic-git";
import { fs as memfs, vol } from "memfs";
import { beforeEach, describe, expect, it } from "vitest";
import { diffSummaryWith, diffWith, type IsomorphicGitDiffClient } from "./diff.js";
const DIR = "/repo";
const AUTHOR = { name: "test", email: "test@example.test" };
const isomorphicGit = git as unknown as IsomorphicGitDiffClient;
function deps() {
return {
git: isomorphicGit,
fs: memfs,
createPatch,
readFile: (path: string) => memfs.promises.readFile(path) as Promise<Uint8Array | string>,
dir: DIR,
};
}
// Commit a base file, stage a second file, then remove it from the
// working tree. Leaves the status matrix row [path, 0, 0, 3].
async function stageThenRemove(): Promise<void> {
await memfs.promises.mkdir(DIR, { recursive: true });
await git.init({ fs: memfs, dir: DIR, defaultBranch: "main" });
await memfs.promises.writeFile(`${DIR}/base.txt`, "base\n");
await git.add({ fs: memfs, dir: DIR, filepath: "base.txt" });
await git.commit({ fs: memfs, dir: DIR, message: "init", author: AUTHOR });
await memfs.promises.writeFile(`${DIR}/added.txt`, "hello\n");
await git.add({ fs: memfs, dir: DIR, filepath: "added.txt" });
await memfs.promises.unlink(`${DIR}/added.txt`);
}
describe("diff with a staged addition removed from the working tree", () => {
beforeEach(() => {
vol.reset();
});
it("reports the path as absent from both endpoints", async () => {
await stageThenRemove();
const rows = await git.statusMatrix({ fs: memfs, dir: DIR });
expect(rows).toContainEqual(["added.txt", 0, 0, 3]);
});
it("emits no patch, the way `git diff HEAD` does", async () => {
await stageThenRemove();
expect(await diffWith(deps())).toBe("");
});
it("omits the path from the summary, the way `git diff HEAD --stat` does", async () => {
await stageThenRemove();
expect(await diffSummaryWith(deps())).toEqual([]);
});
});
Run it:
npx vitest run src/git/diff.repro.test.ts --root packages/computer
The first assertion passes, which pins the input. The other two fail:
FAIL emits no patch, the way `git diff HEAD` does
expected 'Index: added.txt\n===============…' to be ''
FAIL omits the path from the summary, the way `git diff HEAD --stat` does
- []
+ [ { path: "added.txt", status: "A", insertions: 0, deletions: 0 } ]
Diagnosis
collectDiffEntries in packages/computer/src/git/diff.ts is the shared
traversal behind both diffWith and diffSummaryWith. For this state
statusMatrix returns ["added.txt", 0, 0, 3]:
| Column |
Value |
Effect in the collector |
headStatus |
0 |
oldText stays "" at diff.ts:178-181 |
workdirStatus |
0 |
not caught by the === 1 skip at diff.ts:175, so newText stays "" at diff.ts:182-183 |
stageStatus |
3 |
the index holds a version, which is why the row exists at all |
So the entry is pushed with oldText === newText === "" and status: "A".
diffWith's only guard is patch.trim().length > 0 at diff.ts:124. For two
identical strings, createPatch still returns a 106 character header:
Index: added.txt
===================================================================
--- added.txt
+++ added.txt
That is non-empty after trimming, so the guard passes and the phantom patch is
emitted. diffSummaryWith has no such guard at all and maps every collected
entry.
The ref-to-ref collector one function down already handles this correctly, at
diff.ts:224:
The working-tree collector is missing the same guard. One omission, two broken
surfaces.
Proposed fix
Add the guard to the shared collector so both surfaces inherit it. Guarding
inside diffWith alone would leave diffSummaryWith still reporting the
phantom entry, and guarding each caller separately would be two copies of a
check that belongs in the traversal.
--- a/packages/computer/src/git/diff.ts
+++ b/packages/computer/src/git/diff.ts
@@ -181,6 +181,13 @@ async function collectDiffEntries(opts: DiffWithDeps): Promise<DiffEntry[]> {
: "";
const newText =
workdirStatus === 2 ? await readWorkdirAsText(opts.readFile, dir, filepath) : "";
+ // Both endpoints agree, so there is nothing to render. A path
+ // staged and then removed from the working tree lands here: the
+ // status matrix still reports it because the index holds a
+ // version, but `git diff HEAD` compares HEAD to the working
+ // tree and the path is absent from both. Matches the same guard
+ // in the ref-to-ref collector below.
+ if (oldText === newText) continue;
// headStatus 0 -> not in the base -> added. workdirStatus 0
// -> gone from the working tree -> deleted. Otherwise it's a
// content change.
What this proves
- Before the change, the two behavior assertions above fail and the input
assertion passes.
- After the change,
npx vitest run src/git/ --root packages/computer reports
128 passing tests with no regressions across the existing git suite.
npx biome check is clean on both changed files.
src/git/cli.test.ts fails to collect in my environment with
Cannot find package '@cloudflare/dofs/testing'. That reproduces on a clean
checkout of main without a prior npm run build, so it is unrelated to this
change.
Compatibility
No public API change. DiffSummaryEntry and the patch string shape are
untouched, and no exported signature moves. The only behavior difference is
that output real git never produced stops appearing. Any consumer parsing
--stat or --name-status gets strictly fewer, and more accurate, rows.
The change touches one package and needs no changeset beyond a patch bump if
you want the fix in a release.
Environment
@cloudflare/computer at 0.1.0-alpha.1, repo at 76d9e75
- Node 22, npm workspaces
- Windows 11, though the reproduction is filesystem-independent: it runs
entirely on memfs and does not touch FUSE, Docker, or a container backend
The patch and the test are on a branch if that is easier to take directly:
https://github.com/Neal006/computer/tree/fix/diff-phantom-entry
CONTRIBUTING.md says to start from the problem rather than a pull request, so
I have not opened one. Happy to send it if you would like it, or to leave it
here for you to take, whichever is less work for you.
Describe the bug
git diffreports a file that exists in neither endpoint it is comparing.When a path is staged and then removed from the working tree,
git diff HEADcompares HEAD against the working tree, and the path is absent from both. Real
git prints nothing.
diffWithprints a header-only patch with no hunks, anddiffSummaryWithreports the path as an addition, sogit diff --statandgit diff --name-statuslist a file that was never added and does not exist.The two surfaces also disagree with each other in a subtler way.
diffWithhappens to emit a patch body, so a consumer diffing text sees a stray file
header.
diffSummaryWithreportsstatus: "A"withinsertions: 0anddeletions: 0, which reads as "an empty file was added".Expected behavior
Nothing on every diff surface, matching git:
git statusis right to mention the path, because the index holds a version ofit.
git diff HEADis right to stay silent, because neither side it comparescontains it.
Steps to reproduce
Against
76d9e75. The failing test below is written in the style of theexisting
packages/computer/src/git/diff.test.ts, using memfs with the realisomorphic-gitand the realdiffpackage, so nothing in the path under testis stubbed.
packages/computer/src/git/diff.repro.test.ts:Run it:
The first assertion passes, which pins the input. The other two fail:
Diagnosis
collectDiffEntriesinpackages/computer/src/git/diff.tsis the sharedtraversal behind both
diffWithanddiffSummaryWith. For this statestatusMatrixreturns["added.txt", 0, 0, 3]:headStatusoldTextstays""atdiff.ts:178-181workdirStatus=== 1skip atdiff.ts:175, sonewTextstays""atdiff.ts:182-183stageStatusSo the entry is pushed with
oldText === newText === ""andstatus: "A".diffWith's only guard ispatch.trim().length > 0atdiff.ts:124. For twoidentical strings,
createPatchstill returns a 106 character header:That is non-empty after trimming, so the guard passes and the phantom patch is
emitted.
diffSummaryWithhas no such guard at all and maps every collectedentry.
The ref-to-ref collector one function down already handles this correctly, at
diff.ts:224:The working-tree collector is missing the same guard. One omission, two broken
surfaces.
Proposed fix
Add the guard to the shared collector so both surfaces inherit it. Guarding
inside
diffWithalone would leavediffSummaryWithstill reporting thephantom entry, and guarding each caller separately would be two copies of a
check that belongs in the traversal.
What this proves
assertion passes.
npx vitest run src/git/ --root packages/computerreports128 passing tests with no regressions across the existing git suite.
npx biome checkis clean on both changed files.src/git/cli.test.tsfails to collect in my environment withCannot find package '@cloudflare/dofs/testing'. That reproduces on a cleancheckout of
mainwithout a priornpm run build, so it is unrelated to thischange.
Compatibility
No public API change.
DiffSummaryEntryand the patch string shape areuntouched, and no exported signature moves. The only behavior difference is
that output real git never produced stops appearing. Any consumer parsing
--stator--name-statusgets strictly fewer, and more accurate, rows.The change touches one package and needs no changeset beyond a patch bump if
you want the fix in a release.
Environment
@cloudflare/computerat0.1.0-alpha.1, repo at76d9e75entirely on memfs and does not touch FUSE, Docker, or a container backend
The patch and the test are on a branch if that is easier to take directly:
https://github.com/Neal006/computer/tree/fix/diff-phantom-entry
CONTRIBUTING.mdsays to start from the problem rather than a pull request, soI have not opened one. Happy to send it if you would like it, or to leave it
here for you to take, whichever is less work for you.