From c102d8a42ccc51e317cf9b5e0433af439bd319a7 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Wed, 4 Nov 2026 09:45:59 +0100 Subject: [PATCH] fix(git): the untracked count matches what Changes lists The status bar counted everything git status reports, while Changes leaves out .DS_Store and everything under a heavy folder, so a Mac folder could show "1 untracked" over an empty list. An untracked .DS_Store no longer appears in status. Unignored heavy folders, and folders holding nothing listable, stay untracked in the tree but are named in GitStatus.unlisted and not counted. Plain files named like a heavy folder (target, venv) are now listed. DESIGN.md section 26 has the findings and fixes. Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 17 ++++++ src/client/git.ts | 5 +- src/server/git.ts | 44 ++++++++++++-- src/server/index.ts | 9 +++ test/client.git.test.ts | 10 +++ test/servergit.changes.test.ts | 108 +++++++++++++++++++++++++++++++++ 6 files changed, 186 insertions(+), 7 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index f759eb5..d337e7d 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1065,3 +1065,20 @@ on v1.10.6 and about one in four alone, because the server sometimes never ended The delay is measured, not derived: the cause is inside Bun. The looped test is what will notice if a Bun upgrade changes this. + +## 26. Round 17: "1 untracked" with nothing in Changes + +The status bar counted untracked files from `git status`, while Changes leaves out `.DS_Store` and +everything under a heavy folder, so a Mac folder could show `main* 1 untracked` over an empty list. + +| Finding | Fix | +| --- | --- | +| An untracked `.DS_Store` counted, made the branch dirty and could colour its folder, though the tree and Changes both hide it | `compute()` drops those `??` records. A tracked `.DS_Store` is a real change and stays | +| An unignored heavy folder (`target/`) counted with no row in Changes, but the tree does show it, so its files must stay untracked there (badge, gutter) | The record stays and is named in `GitStatus.unlisted`, which the client leaves out of the count | +| Status folds an untracked folder into one `dir/` record, so a folder holding only a `.DS_Store` or a `node_modules` counted too | `markUnlistedDirs` asks `ls-files` about those folders, only when there are some. It runs on every status, so `-x` skips heavy folders and `--directory` stops at the first real file: 9 ms instead of 54 ms on a 50,000-file untracked folder | +| The heavy-name test ran on the whole path and on the last segment: a plain file named `target` or `venv` was missing from Changes and search | `isHeavyFile` (`index.ts`) tests folders only, plus `.DS_Store`. Status tests the root-relative path, so a viewed folder inside `target/` is not affected by the name above it | +| A folder whose name is not UTF-8 decodes lossily and would match nothing as a pathspec, reading as empty | Such names are not checked and keep counting | + +`FileIndex.apply` keeps its own test: its watcher paths can be the heavy folder itself, which must +not restart a build. A nested repository still counts as one untracked folder with no row in +Changes: status cannot tell it from a plain folder, and it does exist. diff --git a/src/client/git.ts b/src/client/git.ts index c53bc47..75e6ea6 100644 --- a/src/client/git.ts +++ b/src/client/git.ts @@ -14,6 +14,8 @@ export interface GitStatus { head: { hash: string; subject: string; author: string; time: number } | null; /** The viewed folder is itself git-ignored: everything in it is ignored (git lists nothing inside). */ rootIgnored?: boolean; + /** `files` keys with no row in Changes: shown as untracked in the tree, not counted. */ + unlisted?: string[]; } /** Display status, VS Code style: Modified, Added, Untracked, Deleted, Renamed, Conflict, Ignored. */ @@ -69,13 +71,14 @@ export class GitState { this.prefixes = new Map(); this.dirs.clear(); this.counts = { changed: 0, staged: 0, untracked: 0, conflicts: 0 }; + const unlisted = new Set(s.unlisted); for (const [raw, xy] of Object.entries(s.files ?? {})) { const g = describe(xy); const p = nfc(raw); if (p.endsWith("/")) this.prefixes.set(p.slice(0, -1), g); else this.files.set(p, g); if (g.letter === "I") continue; - if (g.letter === "U") this.counts.untracked++; + if (g.letter === "U") this.counts.untracked += unlisted.has(raw) ? 0 : 1; else if (g.letter === "C") this.counts.conflicts++; else this.counts.changed++; if (xy[0] !== "." && xy[0] !== "?" && !CONFLICT.has(xy)) this.counts.staged++; diff --git a/src/server/git.ts b/src/server/git.ts index dce3ad0..c15abc5 100644 --- a/src/server/git.ts +++ b/src/server/git.ts @@ -1,5 +1,5 @@ import { watch, type FSWatcher } from "node:fs"; -import { HEAVY_DIRS } from "./index.ts"; +import { HEAVY_DIRS, isHeavyFile } from "./index.ts"; import { existsSync, lstatSync, readFileSync } from "node:fs"; import path from "node:path"; import { looksBinary, openRegular } from "./files.ts"; @@ -40,6 +40,8 @@ export interface GitStatus { files: Record; /** The viewed folder is itself git-ignored (git lists only the folder, so `files` has no "!!" entries). */ rootIgnored: boolean; + /** `files` keys that are really untracked but have no row in Changes: the tree shows them, the count does not. */ + unlisted: string[]; head: { hash: string; subject: string; author: string; time: number } | null; } @@ -80,7 +82,6 @@ export interface Changes { subjects: string[]; } -/** Most entries a Changes listing carries (tracked diffs and untracked files each). */ export interface HistoryEntry { oid: string; /** First parent; null for a root commit. */ @@ -96,7 +97,10 @@ export interface HistoryEntry { status: "M" | "A" | "D" | "R" | "C" | "T"; } +/** Most entries a Changes listing carries (tracked diffs and untracked files each). */ export const MAX_CHANGES = 1000; +/** Above this many untracked folders in one status, they are counted without checking what they hold. */ +const MAX_UNTRACKED_DIR_CHECKS = 1000; /** Commits listed by File History at most. */ export const MAX_HISTORY = 500; /** Untracked files whose lines are counted (the rest show no counts). */ @@ -270,7 +274,7 @@ export class Git { this.fresh(); const r = await this.git(["ls-files", "-z", "--cached", "--others", "--exclude-standard", "--deduplicate", "--", "."]); if (r.code !== 0) return null; - return new TextDecoder().decode(r.out).split("\0").filter((f) => f && !f.split("/").some((seg) => HEAVY_DIRS.has(seg))); + return new TextDecoder().decode(r.out).split("\0").filter((f) => f && !isHeavyFile(f)); } /** Current status; concurrent callers share one `git status` run. */ @@ -294,7 +298,7 @@ export class Git { ]); const s: GitStatus = { repo: true, branch: null, detached: false, oid: null, upstream: null, ahead: 0, behind: 0, - state: this.opState(), stash: 0, files: {}, rootIgnored: false, head: null, + state: this.opState(), stash: 0, files: {}, rootIgnored: false, unlisted: [], head: null, }; if (stash.code === 0) s.stash = Number(new TextDecoder().decode(stash.out).trim()) || 0; if (head.code === 0) { @@ -312,7 +316,13 @@ export class Git { } if (!p.startsWith(this.prefix)) return; const rel = p.slice(this.prefix.length); - if (rel) s.files[rel] = xy; + if (!rel) return; + if (xy === "??" && isHeavyFile(rel)) { + // The tree hides .DS_Store, so it must not colour a folder either; heavy folders it shows. + if (rel === ".DS_Store" || rel.endsWith("/.DS_Store")) return; + s.unlisted.push(rel); + } + s.files[rel] = xy; }; for (let i = 0; i < recs.length; i++) { const r = recs[i]!; @@ -343,9 +353,31 @@ export class Git { else if (kind === "?") add(r.slice(2), "??"); else if (kind === "!") add(r.slice(2), "!!"); } + await this.markUnlistedDirs(s); return s; } + /** + * Status folds an untracked folder into one "dir/" record, even when all it holds is what Changes + * leaves out (a lone .DS_Store, a node_modules). Such a record would count with nothing to list. + */ + private async markUnlistedDirs(s: GitStatus) { + // A name that is not UTF-8 does not survive decoding: as a pathspec it would match nothing. + const dirs = Object.keys(s.files).filter((p) => p.endsWith("/") && s.files[p] === "??" && !s.unlisted.includes(p) && !p.includes("\uFFFD")); + if (!dirs.length || dirs.length > MAX_UNTRACKED_DIR_CHECKS) return; + // Runs on every status: -x never enters a heavy folder, --directory stops at the first real file. + // A name in the viewed folder's own path stays out: -x tests the folders above the pathspec too. + const above = this.prefix.split("/"); + const skip = [...HEAVY_DIRS].filter((name) => !above.includes(name)).flatMap((name) => ["-x", name === ".DS_Store" ? name : `${name}/`]); + const r = await this.git(["--literal-pathspecs", "ls-files", "-z", "--others", "--exclude-standard", "--directory", "--no-empty-directory", ...skip, "--", ...dirs]); + if (r.code !== 0) return; + const kept = new Set(); + for (const f of new TextDecoder().decode(r.out).split("\0")) { + for (let i = f.indexOf("/"); i >= 0; i = f.indexOf("/", i + 1)) kept.add(f.slice(0, i + 1)); + } + for (const d of dirs) if (!kept.has(d)) s.unlisted.push(d); + } + private opState(): string | null { // Operation markers are per worktree: only this worktree's git dir, never the common dir // (a merge in the main checkout must not show as "merging" in a linked worktree). @@ -556,7 +588,7 @@ export class Git { const all = new TextDecoder() .decode(r.out) .split("\0") - .filter((f) => f && !f.endsWith("/") && !f.split("/").some((seg) => HEAVY_DIRS.has(seg))); + .filter((f) => f && !f.endsWith("/") && !isHeavyFile(f)); const paths = all.slice(0, MAX_CHANGES); const files: ChangeEntry[] = paths.map((p) => ({ path: p, status: "?", added: null, removed: null, binary: false })); // A few files at a time through one 64 KiB buffer each: never 200 MiB of buffers per request. diff --git a/src/server/index.ts b/src/server/index.ts index b83cfb0..298931d 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -8,6 +8,15 @@ export const HEAVY_DIRS = new Set([ ".pytest_cache", ".gradle", ".terraform", ".DS_Store", "target", ".zig-cache", "zig-out", ]); +/** + * A file left out of listings: anything under a heavy folder, and .DS_Store itself. A plain file + * that only shares a folder's name (`target`, `venv`) is listed. "dir/" is a folder. + */ +export function isHeavyFile(p: string): boolean { + const segs = p.split("/"); + return segs.pop() === ".DS_Store" || segs.some((seg) => HEAVY_DIRS.has(seg)); +} + const MAX_FILES = 200_000; /** diff --git a/test/client.git.test.ts b/test/client.git.test.ts index b10898d..b990c11 100644 --- a/test/client.git.test.ts +++ b/test/client.git.test.ts @@ -27,6 +27,16 @@ describe("GitState", () => { expect(g.counts).toEqual({ changed: 3, staged: 2, untracked: 1, conflicts: 1 }); }); + test("unlisted records stay untracked in the tree but do not count", () => { + const g = new GitState(); + g.set({ ...st({ "target/": "??", "lib/pkg/": "??", "a.txt": "??" }), unlisted: ["target/", "lib/pkg/"] }); + expect(g.counts.untracked).toBe(1); + expect(g.file("target/foo.rs")).toMatchObject({ letter: "U" }); + expect(g.dir("lib")).toBe("U"); + g.set({ ...st({ "target/": "??" }), unlisted: ["target/"] }); + expect(g.counts).toEqual({ changed: 0, staged: 0, untracked: 0, conflicts: 0 }); + }); + test("directories take the strongest status inside; prefixes cover descendants", () => { const g = new GitState(); g.set(st({ "src/a/x.ts": ".M", "src/a/y.ts": "??", "src/b/z.ts": "UU", "build/": "!!", "tmp/": "??" })); diff --git a/test/servergit.changes.test.ts b/test/servergit.changes.test.ts index e81df2d..e71b046 100644 --- a/test/servergit.changes.test.ts +++ b/test/servergit.changes.test.ts @@ -1,5 +1,6 @@ import { afterAll, beforeAll, expect, test } from "bun:test"; import { mkdtemp, mkdir, realpath, rm, writeFile, rename } from "node:fs/promises"; +import { mkdirSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { Git, isOid, MAX_CHANGES } from "../src/server/git.ts"; @@ -286,3 +287,110 @@ test("the list is capped and reports truncation; untracked symlinks are not read const lc = await lg.changes("head"); expect(lc.files).toEqual([{ path: "escape", status: "?", added: null, removed: null, binary: false }]); }); + +test("status counts as untracked exactly what Changes lists", async () => { + const r = path.join(dir, "hidden-untracked"); + await mkdir(path.join(r, "sub", "inner"), { recursive: true }); + sh(["init", "-q", "-b", "main"], r); + await writeFile(path.join(r, ".gitignore"), "ignored/\n"); + await writeFile(path.join(r, "sub", "s.txt"), "s\n"); + await mkdir(path.join(r, "lib", "target"), { recursive: true }); + await writeFile(path.join(r, "lib", "target", "t.txt"), "t\n"); + sh(["add", "."], r); + sh(["commit", "-qm", "init"], r); + + const put = async (rel: string) => { + await mkdir(path.dirname(path.join(r, rel)), { recursive: true }); + await writeFile(path.join(r, rel), "x\n"); + }; + // Metadata is gone altogether; folders that hold nothing listable stay for the tree, uncounted. + for (const f of [".DS_Store", "sub/.DS_Store", "only/.DS_Store", "only/deep/.DS_Store", "target/out.o", "pkg/node_modules/m/i.js", "ignored/x"]) await put(f); + let g = await git(r); + expect((await g.changes("head")).files).toEqual([]); + let s = await g.status(); + expect(s.files).toEqual({ "ignored/": "!!", "only/": "??", "pkg/": "??", "target/": "??" }); + expect(s.unlisted.sort()).toEqual(["only/", "pkg/", "target/"]); + + // Real files around them count, a folder holding one is one record, special characters are literal. + await put("real.txt"); + await put("mixed/.DS_Store"); + await put("mixed/new file.txt"); + await put("glob[1]*/keep.txt"); + g = await git(r); + expect((await g.changes("head")).files.map((f) => f.path).sort()).toEqual(["glob[1]*/keep.txt", "mixed/new file.txt", "real.txt"]); + s = await g.status(); + expect(s.files).toMatchObject({ "glob[1]*/": "??", "mixed/": "??", "real.txt": "??" }); + expect(s.unlisted.sort()).toEqual(["only/", "pkg/", "target/"]); + + // A plain file that only shares a heavy folder's name is an ordinary file, alone or in a new folder. + await put("venv"); + await put("plain/target"); + g = await git(r); + expect((await g.changes("head")).files.map((f) => f.path)).toEqual(expect.arrayContaining(["venv", "plain/target"])); + s = await g.status(); + expect(s.files).toMatchObject({ venv: "??", "plain/": "??" }); + expect(s.unlisted).not.toContain("venv"); + expect(s.unlisted).not.toContain("plain/"); + + // A nested repository really is there: it counts, though Changes has no row for it. + await put("nested/f.txt"); + sh(["init", "-q"], path.join(r, "nested")); + g = await git(r); + s = await g.status(); + expect(s.files["nested/"]).toBe("??"); + expect(s.unlisted).not.toContain("nested/"); + await rm(path.join(r, "nested"), { recursive: true }); + + // Viewing a subfolder: same rule, root-relative records. + await put("sub/only/.DS_Store"); + await put("sub/kept/k.txt"); + g = await git(path.join(r, "sub")); + s = await g.status(); + expect(s.files).toEqual({ "kept/": "??", "only/": "??" }); + expect(s.unlisted).toEqual(["only/"]); + + // Viewing a folder inside a heavy-named one: the name above the root does not matter. + await put("lib/target/new.txt"); + await put("lib/target/d/x.txt"); + g = await git(path.join(r, "lib", "target")); + expect((await g.changes("head")).files.map((f) => f.path).sort()).toEqual(["d/x.txt", "new.txt"]); + s = await g.status(); + expect(s.files).toEqual({ "d/": "??", "new.txt": "??" }); + expect(s.unlisted).toEqual([]); + + // A .DS_Store that is tracked is a real change and stays visible. + sh(["add", "-f", "sub/.DS_Store"], r); + sh(["commit", "-qm", "track it"], r); + await writeFile(path.join(r, "sub", ".DS_Store"), "changed\n"); + g = await git(path.join(r, "sub")); + expect((await g.status()).files[".DS_Store"]).toBe(".M"); +}); + +test("an untracked folder with a name that is not UTF-8 still counts (Linux)", async () => { + const r = path.join(dir, "latin1"); + await mkdir(r); + sh(["init", "-q", "-b", "main"], r); + const named = Buffer.concat([Buffer.from(r + "/caf"), Buffer.from([0xe9])]); + try { + mkdirSync(named); + } catch { + return; // APFS refuses such names + } + writeFileSync(Buffer.concat([named, Buffer.from("/f.txt")]), "x\n"); + const s = await (await git(r)).status(); + expect(Object.keys(s.files)).toEqual(["caf�/"]); + expect(s.unlisted).toEqual([]); +}); + +test("past the check limit, untracked folders are counted unchecked", async () => { + const r = path.join(dir, "many-dirs"); + await mkdir(r); + sh(["init", "-q", "-b", "main"], r); + for (let i = 0; i <= 1000; i++) { + await mkdir(path.join(r, `d${i}`)); + await writeFile(path.join(r, `d${i}`, ".DS_Store"), ""); + } + const s = await (await git(r)).status(); + expect(Object.keys(s.files)).toHaveLength(1001); + expect(s.unlisted).toEqual([]); +});