diff --git a/apps/server/src/features/projects/files/__tests__/file-service-unicode-paths.integration.test.ts b/apps/server/src/features/projects/files/__tests__/file-service-unicode-paths.integration.test.ts new file mode 100644 index 000000000..1d559f45f --- /dev/null +++ b/apps/server/src/features/projects/files/__tests__/file-service-unicode-paths.integration.test.ts @@ -0,0 +1,149 @@ +import "reflect-metadata"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import * as NodeFS from "node:fs"; +import * as NodePath from "node:path"; +import * as NodeOS from "node:os"; +import * as NodeChildProcess from "node:child_process"; +import { FileService } from "../file-service.js"; +import { RealGitExecutor } from "../../git/execution/real-git-executor.js"; + +const GIT_REPO_SETUP_TIMEOUT_MS = 30_000; + +/** + * Integration tests for FileService path handling against real git output. + * With Git's default core.quotePath=true, non-ASCII and special-character + * paths arrive C-quoted on newline-delimited output; the service must use + * `-z` output so listed paths round-trip into read and mention validation. + * Regression coverage for https://github.com/Mzeey-Empire/mcode/issues/1734. + */ + +/** Initializes a git repo in a temp directory with quotePath left at its default. */ +function createGitRepo(): string { + const tmpDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "mcode-files-unicode-")); + const git = (...args: string[]) => + NodeChildProcess.execFileSync("git", ["-C", tmpDir, ...args], { encoding: "utf8" }); + git("init", "-b", "main"); + git("config", "user.email", "test@mcode.test"); + git("config", "user.name", "Mcode Test"); + git("config", "commit.gpgSign", "false"); + git("config", "core.hooksPath", tmpDir); + git("config", "core.quotePath", "true"); + return tmpDir; +} + +function gitIn(root: string, ...args: string[]): void { + NodeChildProcess.execFileSync("git", ["-C", root, ...args], { encoding: "utf8" }); +} + +function makeService(root: string): FileService { + const workspaceRepo = { findById: () => ({ path: root }) }; + const threadRepo = { findById: () => null }; + const gitWorktrees = { resolveWorkingDir: () => root }; + return new FileService( + workspaceRepo as never, + threadRepo as never, + gitWorktrees as never, + new RealGitExecutor(), + { platform: process.platform } as never, + ); +} + +describe("FileService unicode paths (real git)", () => { + let service: FileService; + let root: string; + + beforeEach(() => { + root = createGitRepo(); + service = makeService(root); + }, GIT_REPO_SETUP_TIMEOUT_MS); + + afterEach(() => { + NodeFS.rmSync(root, { recursive: true, force: true }); + }); + + it("lists a tracked non-ASCII path verbatim and round-trips read + mention validation", async () => { + NodeFS.writeFileSync(NodePath.join(root, "café.ts"), "fixture content"); + gitIn(root, "add", "café.ts"); + gitIn(root, "commit", "-m", "add café"); + + const listed = await service.list("workspace-1"); + + expect(listed).toContain("café.ts"); + expect(service.read("workspace-1", "café.ts")).toBe("fixture content"); + for (const path of listed) { + expect(() => service.validateMentionPath("workspace-1", path)).not.toThrow(); + } + }); + + it("lists an untracked non-ASCII path verbatim", async () => { + NodeFS.writeFileSync(NodePath.join(root, "new üntracked.ts"), "untracked"); + + const listed = await service.list("workspace-1"); + + expect(listed).toContain("new üntracked.ts"); + expect(service.read("workspace-1", "new üntracked.ts")).toBe("untracked"); + }); + + it("lists names with spaces and non-ASCII names inside subdirectories", async () => { + NodeFS.mkdirSync(NodePath.join(root, "dir q"), { recursive: true }); + NodeFS.writeFileSync(NodePath.join(root, "dir q", "ünïcode.md"), "nested"); + NodeFS.writeFileSync(NodePath.join(root, "spaced name.ts"), "spaced"); + + const listed = await service.list("workspace-1"); + + expect(listed).toContain("dir q/ünïcode.md"); + expect(listed).toContain("spaced name.ts"); + }); + + it("keeps gitignore exclusions for non-ASCII names", async () => { + NodeFS.writeFileSync(NodePath.join(root, ".gitignore"), "*.log\n"); + NodeFS.writeFileSync(NodePath.join(root, "ignoré.log"), "ignored"); + + const listed = await service.list("workspace-1"); + + expect(listed).toContain(".gitignore"); + expect(listed).not.toContain("ignoré.log"); + }); + + it("reports unescaped paths in refresh changedPaths", async () => { + NodeFS.writeFileSync(NodePath.join(root, "base.ts"), "base"); + gitIn(root, "add", "base.ts"); + gitIn(root, "commit", "-m", "base"); + + await expect(service.refresh("workspace-1")).resolves.toBeNull(); + NodeFS.writeFileSync(NodePath.join(root, "café.ts"), "dirty"); + + await expect(service.refresh("workspace-1")).resolves.toEqual({ + changedPaths: ["café.ts"], + wholeWorkspace: false, + }); + }); + + it("reports the destination path for a renamed non-ASCII file", async () => { + NodeFS.writeFileSync(NodePath.join(root, "old name.ts"), "renamed"); + gitIn(root, "add", "old name.ts"); + gitIn(root, "commit", "-m", "base"); + + await expect(service.refresh("workspace-1")).resolves.toBeNull(); + gitIn(root, "mv", "old name.ts", "new näme.ts"); + + await expect(service.refresh("workspace-1")).resolves.toEqual({ + changedPaths: ["new näme.ts"], + wholeWorkspace: false, + }); + }); + + // POSIX filesystems allow newlines in filenames; NTFS does not. + it.skipIf(process.platform === "win32")( + "round-trips a filename containing a newline", + async () => { + const newlineName = "line\nbreak.ts"; + NodeFS.writeFileSync(NodePath.join(root, newlineName), "newline content"); + + const listed = await service.list("workspace-1"); + + expect(listed).toContain(newlineName); + expect(service.read("workspace-1", newlineName)).toBe("newline content"); + }, + ); +}); diff --git a/apps/server/src/features/projects/files/__tests__/file-service.test.ts b/apps/server/src/features/projects/files/__tests__/file-service.test.ts index a073762b1..521773fa3 100644 --- a/apps/server/src/features/projects/files/__tests__/file-service.test.ts +++ b/apps/server/src/features/projects/files/__tests__/file-service.test.ts @@ -28,9 +28,9 @@ describe("FileService.refresh", () => { it("baselines silently on the first call and reports only later deltas", async () => { const exec = vi .fn() - .mockResolvedValueOnce({ stdout: " M src/a.ts\n" }) - .mockResolvedValueOnce({ stdout: " M src/a.ts\n" }) - .mockResolvedValueOnce({ stdout: " M src/a.ts\n?? src/b.ts\n" }); + .mockResolvedValueOnce({ stdout: " M src/a.ts\0" }) + .mockResolvedValueOnce({ stdout: " M src/a.ts\0" }) + .mockResolvedValueOnce({ stdout: " M src/a.ts\0?? src/b.ts\0" }); const { service } = makeService({ exec }); await expect(service.refresh("workspace-1")).resolves.toBeNull(); @@ -41,16 +41,30 @@ describe("FileService.refresh", () => { }); expect(exec).toHaveBeenCalledTimes(3); expect(exec).toHaveBeenLastCalledWith( - ["status", "--porcelain", "--untracked-files=all"], + ["status", "--porcelain", "--untracked-files=all", "-z"], { cwd: "C:/workspace" }, ); }); + it("reports the destination path for rename entries in -z output", async () => { + const exec = vi + .fn() + .mockResolvedValueOnce({ stdout: "" }) + .mockResolvedValueOnce({ stdout: "R new name.ts\0old name.ts\0" }); + const { service } = makeService({ exec }); + + await expect(service.refresh("workspace-1")).resolves.toBeNull(); + await expect(service.refresh("workspace-1")).resolves.toEqual({ + changedPaths: ["new name.ts"], + wholeWorkspace: false, + }); + }); + it("tracks thread scopes independently", async () => { const exec = vi .fn() - .mockResolvedValueOnce({ stdout: " M src/a.ts\n" }) - .mockResolvedValueOnce({ stdout: " M src/a.ts\n" }); + .mockResolvedValueOnce({ stdout: " M src/a.ts\0" }) + .mockResolvedValueOnce({ stdout: " M src/a.ts\0" }); const { service } = makeService({ exec }); await expect(service.refresh("workspace-1", "thread-1")).resolves.toBeNull(); @@ -63,7 +77,7 @@ describe("FileService.refresh", () => { .fn() .mockResolvedValueOnce({ stdout: "" }) .mockResolvedValueOnce({ - stdout: Array.from({ length: 101 }, (_, i) => `?? dir/file-${i}.ts`).join("\n"), + stdout: Array.from({ length: 101 }, (_, i) => `?? dir/file-${i}.ts`).join("\0"), }); const { service } = makeService({ exec }); diff --git a/apps/server/src/features/projects/files/file-service.ts b/apps/server/src/features/projects/files/file-service.ts index 48c87e31c..76f68d005 100644 --- a/apps/server/src/features/projects/files/file-service.ts +++ b/apps/server/src/features/projects/files/file-service.ts @@ -33,19 +33,20 @@ export class FileService { /** * List files in a workspace, including both tracked and untracked files. - * Uses `git ls-files --cached --others --exclude-standard` to include - * untracked files that are not gitignored. + * Uses `git ls-files --cached --others --exclude-standard -z` to include + * untracked files that are not gitignored. The `-z` output is NUL-delimited + * and unquoted, so non-ASCII and whitespace-bearing names arrive verbatim. */ async list(workspaceId: string, threadId?: string): Promise { const cwd = this.resolveWorkingDir(workspaceId, threadId); try { const { stdout } = await this.gitExecutor.exec( - ["ls-files", "--cached", "--others", "--exclude-standard"], + ["ls-files", "--cached", "--others", "--exclude-standard", "-z"], { cwd }, ); return stdout - .split("\n") + .split("\0") .filter((line: string) => line.length > 0); } catch (err) { // Non-git folders have no ls-files source; a real repo failure still throws. @@ -78,17 +79,19 @@ export class FileService { let paths: string[]; try { const { stdout } = await this.gitExecutor.exec( - ["status", "--porcelain", "--untracked-files=all"], + ["status", "--porcelain", "--untracked-files=all", "-z"], { cwd }, ); - paths = parsePorcelainPaths(stdout); + paths = parsePorcelainZ(stdout); } catch { // Non-git folders fingerprint the same bounded listing `list` falls back to. if (NodeFS.existsSync(NodePath.join(cwd, ".git"))) return null; paths = listDirectoryTree(cwd); } - const fingerprint = [...paths].sort().join("\n"); + // Paths may legally contain "\n" on POSIX filesystems, so fingerprints + // join on the only separator git `-z` output can never embed in a name. + const fingerprint = [...paths].sort().join("\0"); const previous = this.statusFingerprints.get(scope); this.statusFingerprints.set(scope, fingerprint); if (previous === undefined || previous === fingerprint) return null; @@ -223,11 +226,23 @@ function assertFileSize(fullPath: string, relativePath: string): void { } } -/** Extracts the path from a `git status --porcelain` v1 line (`XY path` or `XY old -> new`). */ -function porcelainPath(line: string): string { - const raw = line.slice(3); - const renamed = raw.split(" -> ").at(-1) ?? raw; - return renamed.replace(/^"|"$/g, ""); +/** + * Parses `git status --porcelain -z` output into the current path of each entry. + * Entries are `XY ` NUL-terminated with no quoting; rename/copy entries + * append a second NUL field holding the source path, which is consumed so it + * is not reported as a separate path. + */ +function parsePorcelainZ(stdout: string): string[] { + const tokens = stdout.split("\0").filter((token) => token.length > 0); + const paths: string[] = []; + for (let i = 0; i < tokens.length; i += 1) { + const token = tokens[i]!; + if (token.length < 4) continue; + const status = token.slice(0, 2); + paths.push(token.slice(3)); + if (status.includes("R") || status.includes("C")) i += 1; + } + return paths; } /** Reports the symmetric difference between a stored fingerprint and the current path list. */ @@ -236,7 +251,7 @@ function diffFingerprints( paths: string[], ): { changedPaths: string[]; wholeWorkspace: boolean } { const current = new Set(paths); - const prior = new Set(previous.split("\n").filter((path) => path.length > 0)); + const prior = new Set(previous.split("\0").filter((path) => path.length > 0)); const delta = new Set(); for (const path of current) if (!prior.has(path)) delta.add(path); for (const path of prior) if (!current.has(path)) delta.add(path); @@ -245,13 +260,6 @@ function diffFingerprints( return { changedPaths: wholeWorkspace ? [] : changedPaths, wholeWorkspace }; } -function parsePorcelainPaths(stdout: string): string[] { - return stdout - .split("\n") - .filter((line) => line.length > 0) - .map(porcelainPath); -} - /** * Bounded recursive walk used only for non-git folders, where `git ls-files` * cannot provide an ignore-aware listing. Skips `.git` and `node_modules`