Skip to content

fix(server): changed files keep their real paths for renames and non-ASCII names #20175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
juliusmarminge merged 8 commits into from
Oct 19, 2026
Merged

fix(server): changed files keep their real paths for renames and non-ASCII names #20175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3450b0b
fix(server): changed files keep their real paths for renames and non-…
Atharvsinh-codez Oct 16, 2026
86680b5
Merge branch 'main' into fix/git-status-real-paths
Atharvsinh-codez Oct 16, 2026
f21194c
fix(server): stage selected files from the repository root
Atharvsinh-codez Oct 16, 2026
494bc68
Merge commit '952921f39acd7793187e2b14650f1cbc447f483d' into takeover…
juliusmarminge Oct 18, 2026
3a6fd15
refactor(server): simplify porcelain record lookup
juliusmarminge Oct 19, 2026
f7de77f
fix(web): open status files from the repository root
juliusmarminge Oct 19, 2026
d973a24
fix(server): the status root keeps trailing whitespace in its basename
juliusmarminge Oct 19, 2026
ad93c17
Merge commit 'c274a251510e4bb035f8479a7f1e96443cd2503c' into takeover…
juliusmarminge Oct 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 92 additions & 1 deletion apps/server/src/vcs/GitVcsDriverCore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ it.effect("uses stable diagnostics for every parsed non-repository command", ()

assert.deepStrictEqual(commands, [
{ args: ["rev-parse", "--git-path", "index"], lcAll: "C" },
{ args: ["status", "--porcelain=2", "--branch"], lcAll: "C" },
{ args: ["status", "--porcelain=2", "--branch", "-z"], lcAll: "C" },
{ args: ["rev-parse", "--abbrev-ref", "HEAD"], lcAll: "C" },
{ args: ["rev-parse", "--git-common-dir"], lcAll: "C" },
]);
Expand Down Expand Up @@ -2643,6 +2643,97 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => {
}),
);

it.effect("reports renamed and non-ASCII files by paths that can be committed", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
yield* writeTextFile(cwd, "src/old.ts", "one\ntwo\nthree\n");
yield* writeTextFile(cwd, "café notes.md", "draft\n");
yield* git(cwd, ["add", "."]);
yield* git(cwd, ["commit", "-m", "add files"]);
yield* git(cwd, ["mv", "src/old.ts", "src/new.ts"]);
yield* writeTextFile(cwd, "src/new.ts", "one\ntwo\nthree\nfour\n");
yield* writeTextFile(cwd, "café notes.md", "draft\nedit\n");
const driver = yield* GitVcsDriver.GitVcsDriver;

const status = yield* driver.statusDetails(cwd);

assert.deepEqual(status.workingTree.files, [
{ path: "café notes.md", insertions: 1, deletions: 0 },
{ path: "src/new.ts", insertions: 1, deletions: 0 },
{ path: "src/old.ts", insertions: 0, deletions: 0 },
]);
yield* driver.commit(cwd, "Commit selection", "", {
stage: { filePaths: status.workingTree.files.map((file) => file.path) },
});
assert.equal(yield* git(cwd, ["status", "--porcelain"]), "");
}),
);

it.effect("commits selected files from a nested project directory", () =>
Effect.gen(function* () {
const path = yield* Path.Path;
// A root basename ending in whitespace catches a root lookup that trims Git's output.
const cwd = path.join(yield* makeTmpDir(), "repo ");
yield* (yield* FileSystem.FileSystem).makeDirectory(cwd);
yield* initRepoWithCommit(cwd);
yield* writeTextFile(cwd, "nested/tracked.txt", "one\n");
yield* git(cwd, ["add", "."]);
yield* git(cwd, ["commit", "-m", "add nested file"]);
yield* writeTextFile(cwd, "nested/tracked.txt", "one\ntwo\n");
yield* writeTextFile(cwd, "nested/new.txt", "new\n");
yield* writeTextFile(cwd, "README.md", "# test\nleft out\n");
const nestedCwd = path.join(cwd, "nested");
const driver = yield* GitVcsDriver.GitVcsDriver;

const status = yield* driver.statusDetails(nestedCwd);

assert.deepEqual(status.workingTree.files, [
{ path: "nested/new.txt", insertions: 0, deletions: 0 },
{ path: "nested/tracked.txt", insertions: 1, deletions: 0 },
{ path: "README.md", insertions: 1, deletions: 0 },
]);
// The root is what the paths above are relative to, not the nested cwd, and it keeps
// the trailing space so the resolved editor target is the file that exists.
assert.equal(status.workingTree.rootPath, NodeFS.realpathSync.native(cwd));
assert.equal(
NodeFS.existsSync(path.join(status.workingTree.rootPath!, "nested", "new.txt")),
true,
);
yield* driver.commit(nestedCwd, "Commit nested selection", "", {
stage: { filePaths: ["nested/new.txt", "nested/tracked.txt"] },
});
assert.equal(yield* git(cwd, ["status", "--porcelain"]), "M README.md");
}),
);

it.effect("reports the linked worktree as the root of a nested directory in it", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const worktreePath = yield* makeTmpDir("git-vcs-driver-linked-");
yield* initRepoWithCommit(cwd);
yield* writeTextFile(cwd, "nested/tracked.txt", "one\n");
yield* git(cwd, ["add", "."]);
yield* git(cwd, ["commit", "-m", "add nested file"]);
yield* git(cwd, ["worktree", "add", "-b", "feature/linked", worktreePath]);
yield* writeTextFile(worktreePath, "nested/tracked.txt", "one\ntwo\n");
yield* writeTextFile(worktreePath, "nested/new.txt", "new\n");
const nestedCwd = (yield* Path.Path).join(worktreePath, "nested");
const driver = yield* GitVcsDriver.GitVcsDriver;

const status = yield* driver.statusDetails(nestedCwd);

assert.deepEqual(status.workingTree.files, [
{ path: "nested/new.txt", insertions: 0, deletions: 0 },
{ path: "nested/tracked.txt", insertions: 1, deletions: 0 },
]);
assert.equal(
NodeFS.realpathSync.native(status.workingTree.rootPath!),
NodeFS.realpathSync.native(worktreePath),
);
}),
);

it.effect("reports default-branch delta separately from upstream delta", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
Expand Down
116 changes: 47 additions & 69 deletions apps/server/src/vcs/GitVcsDriverCore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,30 +185,6 @@ function parseBranchAb(value: string): { ahead: number; behind: number } {
};
}

function parseNumstatEntries(
stdout: string,
): Array<{ path: string; insertions: number; deletions: number }> {
const entries: Array<{ path: string; insertions: number; deletions: number }> = [];
for (const line of stdout.split(/\r?\n/g)) {
if (line.trim().length === 0) continue;
const [addedRaw, deletedRaw, ...pathParts] = line.split("\t");
const rawPath =
pathParts.length > 1 ? (pathParts.at(-1) ?? "").trim() : pathParts.join("\t").trim();
if (rawPath.length === 0) continue;
const added = Number.parseInt(addedRaw ?? "0", 10);
const deleted = Number.parseInt(deletedRaw ?? "0", 10);
const renameArrowIndex = rawPath.indexOf(" => ");
const normalizedPath =
renameArrowIndex >= 0 ? rawPath.slice(renameArrowIndex + " => ".length).trim() : rawPath;
entries.push({
path: normalizedPath.length > 0 ? normalizedPath : rawPath,
insertions: Number.isFinite(added) ? added : 0,
deletions: Number.isFinite(deleted) ? deleted : 0,
});
}
return entries;
}

function parseReviewNumstat(stdout: string): ReviewDiffFileStat[] {
const fields = stdout.split("\0");
const files: ReviewDiffFileStat[] = [];
Expand All @@ -228,25 +204,20 @@ function parseReviewNumstat(stdout: string): ReviewDiffFileStat[] {
return files;
}

function parsePorcelainPath(line: string): string | null {
if (line.startsWith("? ") || line.startsWith("! ")) {
const simple = line.slice(2).trim();
return simple.length > 0 ? simple : null;
}

if (!(line.startsWith("1 ") || line.startsWith("2 ") || line.startsWith("u "))) {
return null;
}

const tabIndex = line.indexOf("\t");
if (tabIndex >= 0) {
const fromTab = line.slice(tabIndex + 1);
const [filePath] = fromTab.split("\t");
return filePath?.trim().length ? filePath.trim() : null;
}
// Space-separated fields precede the path in each porcelain v2 record type. The path itself
// may contain spaces, so it is everything after them.
const PORCELAIN_V2_FIELDS_BEFORE_PATH: Record<string, number> = {
"1": 8,
"2": 9,
u: 10,
"?": 1,
"!": 1,
};

const parts = line.trim().split(/\s+/g);
const filePath = parts.at(-1) ?? "";
function parsePorcelainPath(record: string): string | null {
const fieldsBeforePath = PORCELAIN_V2_FIELDS_BEFORE_PATH[record[0] ?? ""];
if (fieldsBeforePath === undefined) return null;
const filePath = record.split(" ").slice(fieldsBeforePath).join(" ");
return filePath.length > 0 ? filePath : null;
}

Expand Down Expand Up @@ -1347,7 +1318,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
],
{ concurrency: 2 },
);
const worktreeRootOutput = worktreeRootResult.stdout.trim();
// Git terminates --show-toplevel with exactly one "\n" on every platform (stdout is
// binary mode on Windows). Whitespace before it belongs to the path and must survive,
// because this root becomes the editor target for every repository-relative status path.
const worktreeRootOutput = worktreeRootResult.stdout.endsWith("\n")
? worktreeRootResult.stdout.slice(0, -1)
: worktreeRootResult.stdout;
const worktreeRoot =
worktreeRootResult.exitCode === 0 && worktreeRootOutput.length > 0
? path.normalize(
Expand Down Expand Up @@ -1891,6 +1867,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
"status",
"--porcelain=2",
"--branch",
"-z",

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stage NUL status paths from the worktree root

When cwd is a repository subdirectory (the supported nested-project case in GitVcsDriverCore.test.ts:1531), this added -z changes porcelain-v2 names from cwd-relative to repository-relative. After checking git status -h (-z, --[no-]null is documented as “terminate entries with NUL”), I reproduced from /repo/nested that the line form reports ? new.ts while this command reports ? nested/new.ts\0. stageCommitChanges still invokes git --literal-pathspecs add in /repo/nested, so once the user unchecks any file, the selected path resolves to /repo/nested/nested/new.ts and the commit fails with an unmatched pathspec; use a top-anchored literal pathspec (as diffCheckpoints does) or stage from the worktree root.

Useful? React with 👍 / 👎.

@Atharvsinh-codez Atharvsinh-codez Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, confirmed. From /repo/nested, git status --porcelain=2 prints ? new.ts but -z prints ? nested/new.ts. Numstat was already root-relative before this PR, so tracked edits in a nested project were already failing this way. The -z change would have made untracked files fail too.

Fixed in f21194c. Selected files are now staged with :(top,literal) pathspecs, the same way diffCheckpoints does it. I also added commits selected files from a nested project directory, which fails without the staging change and passes with it.

...(includeDivergence ? [] : ["--no-ahead-behind"]),
];
const indexResult = yield* executeGitWithStableDiagnostics(
Expand Down Expand Up @@ -1968,56 +1945,45 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
Effect.catchTags({ GitCommandError: () => Effect.succeed(null) }),
);
const statusCacheKey = repositoryPaths?.gitCommonDir;
const [numstatStdout, defaultBranch, hasPrimaryRemote] = yield* Effect.all(
const [numstatEntries, defaultBranch, hasPrimaryRemote] = yield* Effect.all(
[
// Plumbing, because porcelain `git diff` rewrites the index even with
// GIT_OPTIONAL_LOCKS=0. -M keeps porcelain's rename detection.
executeGitWithStableDiagnostics(
"GitVcsDriver.statusDetails.numstat",
cwd,
["diff-index", "-M", "--numstat", "HEAD", "--"],
["diff-index", "-M", "--numstat", "-z", "HEAD", "--"],
{ allowNonZeroExit: true },
).pipe(
Effect.flatMap((result) => {
if (result.exitCode === 0) return Effect.succeed(result.stdout);
if (result.exitCode === 0) return Effect.succeed(parseReviewNumstat(result.stdout));
if (isUnbornHeadStderr(result.stderr)) {
return Effect.map(
Effect.all([
runGitStdout("GitVcsDriver.statusDetails.numstat.unborn", cwd, [
"diff-files",
"--numstat",
"-z",
]),
runGitStdout("GitVcsDriver.statusDetails.numstat.unborn.staged", cwd, [
"diff",
"--cached",
"--numstat",
"-z",
]),
]),
([unstagedStdout, stagedStdout]) => {
const staged = parseNumstatEntries(stagedStdout);
const unstaged = parseNumstatEntries(unstagedStdout);
const map = new Map<string, { insertions: number; deletions: number }>();
for (const entry of [...staged, ...unstaged]) {
const existing = map.get(entry.path) ?? {
insertions: 0,
deletions: 0,
};
existing.insertions += entry.insertions;
existing.deletions += entry.deletions;
map.set(entry.path, existing);
}
return Array.from(map.entries())
.map(([p, s]) => `${s.insertions}\t${s.deletions}\t${p}`)
.join("\n");
},
([unstagedStdout, stagedStdout]) => [
...parseReviewNumstat(stagedStdout),
...parseReviewNumstat(unstagedStdout),
],
);
}
return Effect.fail(
new GitCommandError({
...gitCommandContext({
operation: "GitVcsDriver.statusDetails.numstat",
cwd,
args: ["diff-index", "-M", "--numstat", "HEAD", "--"],
args: ["diff-index", "-M", "--numstat", "-z", "HEAD", "--"],
}),
detail: "git diff-index HEAD --numstat failed.",
exitCode: result.exitCode,
Expand Down Expand Up @@ -2046,7 +2012,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
let hasWorkingTreeChanges = false;
const changedFilesWithoutNumstat = new Set<string>();

for (const line of statusStdout.split(/\r?\n/g)) {
const statusRecords = statusStdout.split("\0");
for (let index = 0; index < statusRecords.length; index++) {
const line = statusRecords[index]!;
if (line.startsWith("# branch.head ")) {
const value = line.slice("# branch.head ".length).trim();
refName = value.startsWith("(") ? null : value;
Expand All @@ -2064,10 +2032,16 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
behindCount = parsed.behind;
continue;
}
if (line.trim().length > 0 && !line.startsWith("#")) {
if (line.length > 0 && !line.startsWith("#")) {
hasWorkingTreeChanges = true;
const pathValue = parsePorcelainPath(line);
if (pathValue) changedFilesWithoutNumstat.add(pathValue);
// A rename record is followed by its original path as a separate record. Listing it
// keeps the deletion selectable when committing only some files.
if (line.startsWith("2 ")) {
const originalPath = statusRecords[++index];
if (originalPath) changedFilesWithoutNumstat.add(originalPath);
}
}
}

Expand All @@ -2092,10 +2066,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
: yield* computeAheadCountAgainstBase(cwd, refName).pipe(Effect.orElseSucceed(() => 0));
}

const numstatEntries = parseNumstatEntries(numstatStdout);
const fileStatMap = new Map<string, { insertions: number; deletions: number }>();
for (const entry of numstatEntries) {
fileStatMap.set(entry.path, { insertions: entry.insertions, deletions: entry.deletions });
const stat = fileStatMap.get(entry.path) ?? { insertions: 0, deletions: 0 };
stat.insertions += entry.additions;
stat.deletions += entry.deletions;
fileStatMap.set(entry.path, stat);
}

let insertions = 0;
Expand Down Expand Up @@ -2128,6 +2104,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
upstreamRef,
hasWorkingTreeChanges,
workingTree: {
...(repositoryPaths?.worktreeRoot ? { rootPath: repositoryPaths.worktreeRoot } : {}),
files,
insertions,
deletions,
Expand Down Expand Up @@ -2221,7 +2198,8 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
yield* runGit(
"GitVcsDriver.commit.addSelected",
cwd,
[...config, "--literal-pathspecs", "add", "-A", "--", ...filePaths],
// Status paths are repository-relative, while cwd can be a subdirectory.
[...config, "add", "-A", "--", ...filePaths.map((file) => `:(top,literal)${file}`)],
env ? { env } : {},
);
} else {
Expand Down
Loading
Loading