mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 00:02:41 +00:00
Git can execute inside a WSL distro against a raw Linux worktree path while Node, on the Windows side, reads the same files back through Win32. `path.join( '/home/me/repo/feature', 'src/file.ts')` on win32 produces the drive-relative `\home\me\repo\feature\src\file.ts`, which resolves against whatever the current drive happens to be and almost always ENOENTs. The same mis-spelling hits the drvfs form, where `/mnt/c/repo` should read as `C:\repo`. Two consequences, both on the Node side only (git already works, because it gets the Linux path as its cwd and resolves it inside the distro): - getDiff's unstaged working-tree read missed, `readWorkingTreeFile` mapped ENOENT to `exists: false`, and an existing file rendered as DELETED in the diff view. - `readWorktreeDiffStamp` could not find `.git`, so the stamp was null, the settled diff cache neither hit nor stored, and every diff respawned `git show` - two `wsl.exe` spawns the cache exists specifically to avoid. Both now spell the worktree directory for the reading host first, via a new `resolveWorktreeHostPath` wrapper around the resolver that landed in #17804. The wrapper exists because `resolveGitMetadataPath` trims: a gitfile payload carries a trailing newline, but a directory name may legally begin or end with whitespace on POSIX, so the wrapper keeps the caller's spelling whenever the resolver only trimmed it. The stamp's opaque `value` still embeds the caller's original `worktreePath`, so settled-cache identity is byte-identical and no cache key moves. `readWorktreeDiffStamp` was already `Promise<WorktreeDiffStamp | null>` with one caller that treats null as a cache miss, so no new nullability enters the type system and the resolver's never-null-for-a-non-empty-pointer contract is untouched. The only unspellable input is an empty worktree path, handled locally as "not provably unchanged" in the stamp and as a read *failure* (not a proven deletion) in file-diff. What changes for users | Platform | Delta | |---|---| | macOS | No change. An absolute POSIX path is returned verbatim, including one whose directory name carries leading or trailing whitespace. | | Linux | No change. Same reason. | | Native Windows (no WSL) | No change. A `C:\...` or `\\server\share\...` path is already absolute for win32 and passes through verbatim. | | Windows + WSL, UNC worktree path (`\\wsl.localhost\Ubuntu\...`) | No change. Already absolute for win32; passes through verbatim. This is today's common case. | | Windows + WSL, drvfs worktree path (`/mnt/c/repo`) | Fixed. Reads as `C:\repo` instead of the drive-relative `\mnt\c\repo`. Needs no distro name. | | Windows + WSL, Linux worktree path with a named distro (`/home/me/repo`) | Fixed. Reads as `\\wsl.localhost\Ubuntu\home\me\repo`. The deleted-file misrender goes away and the diff cache starts hitting. | | Windows, POSIX path, no distro and not a drvfs mount | No change. Passes through verbatim, same ENOENT, same existing fallback. | | SSH | No change. `runtime-git-diff-commands.ts` and the `git:diff` IPC both route to `provider.getDiff` for a connection, so this local code is never reached. | | Relay / remote | No change. No RPC param, wire field, stream opcode, or published content is touched; the relay host runs the same local code and gets the same fix. | | Folder workspace (non-git) | No change. `.git` is absent either way, `resolveGitDir` returns the same fallback, and the stamp stays null exactly as today. | | GitLab / other providers | Not applicable. No provider-specific or review code is touched. | What this does NOT do - It does not fix `resolveGitDir` itself. For a drvfs repo whose worktree Orca already spells `C:\repo\feature`, the gitfile payload `gitdir: /mnt/c/repo/.git/ worktrees/feature` is still mis-resolved by `path.resolve` to `C:\mnt\c\repo\.git\...`, so the stamp still returns null in that shape. Separate change, separate PR; this one neither fixes nor regresses it. - It does not touch submodule path resolution. `resolveSubmoduleWorktreePath` is the path-escape guard and has a near-identical twin in the relay; changing it without escape tests on both is out of scope. - It does not change `readHeadComponent`'s `commondir` resolution. The relative `../..` git actually writes takes the identical `path.resolve` branch, and an absolute POSIX `commondir` under a WSL UNC `gitDir` already resolves correctly because the UNC root is `\\wsl.localhost\<distro>\`. - It does not reorder drvfs-before-UNC inside the shared resolver. That changes the identity of returned strings and needs a real Windows+WSL box. - It does not add any Git command, option, or version dependency. Costs and residual risk - One extra pure function call per diff read. No I/O added or removed on the unaffected paths. - Translation still trims. `resolveWorktreeHostPath` preserves whitespace only when no translation happened; a guest directory named `/home/me/repo ` loses its trailing space on a Windows reader. Reachable only on win32, where such a name is not addressable anyway, and the previous behavior for that shape was a drive-relative miss. - A relative worktree path (no caller passes one) is now resolved against the process cwd instead of joined relative to it. Same file in every case except a relative name that itself ends in whitespace. - `UNSPELLABLE_WORKING_TREE_READ`'s `exists`/`failed` fields are correct but not observable today: the stamp is null for the same input, so nothing can be cached and `reusable` cannot be read back. They are there so the branch stays right if `loadDiff` ever gains a second caller. The test pins the observable part - that no read lands on a cwd-relative path. - Every test here mocks `node:fs/promises` and spoofs `process.platform`. They prove which path string reaches `stat`/`readFile`, which is the right assertion, but none of this has executed against a real 9p mount on a Windows+WSL box and this repo's CI has no such runner. - Honest framing of the trigger: I could not demonstrate a mainline path that hands `getDiff` an untranslated POSIX worktree path on Windows today - `translateWslOutputPaths` UNC-translates worktree paths whenever a distro is known, `getWslHome` returns the UNC spelling, and `resolveWslRepoWorktreeBasePath` normalizes a configured Linux base. The drvfs case is the most plausible live one. Treat this as defense-in-depth that is a strict no-op on every configuration above except the two marked Fixed. Verification - `npx vitest run src/main/git src/shared/git-metadata-path.test.ts` -> 196 files / 2241 tests passed, 2 files and 5 tests skipped. One failure, `git-admission-storm-measurement.test.ts > reports bounded-concurrency before and after measurements` (ENOENT scandir on its own temp state dir), is pre-existing and environmental: it fails identically in isolation and spawns real git children without touching any changed module. - `npx vitest run src/main/git/status-diff-settled-cache.test.ts` -> 21/21 (16 pre-existing, 5 new). `npx vitest run src/shared/git-metadata-path.test.ts` -> 25/25 (19 pre-existing, 6 new cases across 3 new tests). - `npx oxfmt --write` then `npx oxlint` on all five changed files -> clean. Mutation checks - all eight production substitutions were reverted one at a time and the suite re-run. Each fails at least one test, and no new test survives its own mutation: | Reverted | Failing test | |---|---| | file-diff working-tree read -> `worktreePath` | reads the working tree through the host spelling instead of reporting a deletion; invalidates when the working tree file is edited under the host spelling | | stamp working-tree component -> `worktreePath` | invalidates when the working tree file is edited under the host spelling | | stamp `.gitmodules` stat -> `worktreePath` | invalidates when .gitmodules appears under the host spelling | | stamp `resolveGitDir` -> `worktreePath` | stamps through the host spelling so the second read does not respawn git | | `options` threading at the `readWorktreeDiffStamp` call | stamps through the host spelling...; invalidates when .gitmodules appears... | | wrapper's untrimmed preservation -> return the resolver's value | keeps whitespace that belongs to the directory name (both cases) | | `UNSPELLABLE_WORKING_TREE_READ` -> a cwd-relative `readWorkingTreeFile` | reads nothing relative to the cwd when the worktree path has no host spelling | | stamp's null early return -> `hostWorktreePath ?? worktreePath` | reads nothing relative to the cwd when the worktree path has no host spelling | The settled-cache tests seed the fake filesystem through the platform-bound `path` module rather than `path.win32`, so they assert real behavior on a POSIX CI host as well as on Windows and are not gated on the host platform. Co-authored-by: Neil <79079362+brennanb2025@users.noreply.github.com>
225 lines
8.3 KiB
TypeScript
225 lines
8.3 KiB
TypeScript
import * as path from 'node:path'
|
|
import type { GitDiffResult } from '../../../shared/git-diff-compare-types'
|
|
import { resolveWorktreeHostPath } from '../../../shared/git-metadata-path'
|
|
import { stableInFlightKey } from '../../../shared/in-flight-promise-dedupe'
|
|
import type { GitRuntimeOptions } from '../git-runtime-options'
|
|
import { gitRuntimeOptionsKey } from './git-runtime-options-cache-key'
|
|
import { gitDiffReadDedupe, settledDiffCache } from './git-read-cache-invalidation'
|
|
import { readWorktreeDiffStamp } from './worktree-diff-stamp'
|
|
import { buildDiffResult } from './diff-result'
|
|
import {
|
|
type GitBlobReadResult,
|
|
readGitBlobAtIndexPath,
|
|
readGitBlobAtOidPath,
|
|
readUnstagedLeftBlob,
|
|
readWorkingTreeFile
|
|
} from './git-blob-read'
|
|
import {
|
|
findContainingSubmodule,
|
|
listSubmodulePaths,
|
|
resolveSubmoduleWorktreePath
|
|
} from './submodule-paths'
|
|
import {
|
|
readGitlinkOidFromIndex,
|
|
readGitlinkOidFromTree,
|
|
readWorkingSubmoduleHead
|
|
} from './submodule-gitlink-oid'
|
|
import { buildSubmoduleInnerCommitRangeDiff, buildSubmodulePointerDiff } from './submodule-diff'
|
|
|
|
/**
|
|
* Get original and modified content for diffing a file.
|
|
*/
|
|
export async function getDiff(
|
|
worktreePath: string,
|
|
filePath: string,
|
|
staged: boolean,
|
|
compareAgainstHead = false,
|
|
options: GitRuntimeOptions = {}
|
|
): Promise<GitDiffResult> {
|
|
const readKey = stableInFlightKey([
|
|
'diff',
|
|
worktreePath,
|
|
filePath,
|
|
staged,
|
|
compareAgainstHead,
|
|
...gitRuntimeOptionsKey(options)
|
|
])
|
|
// Why: register the dedupe synchronously (before any await) so concurrent identical reads
|
|
// coalesce — including on the settled-cache lookup, which is itself I/O.
|
|
return gitDiffReadDedupe.run(readKey, () =>
|
|
loadDiffThroughSettledCache(
|
|
readKey,
|
|
worktreePath,
|
|
filePath,
|
|
staged,
|
|
compareAgainstHead,
|
|
options
|
|
)
|
|
)
|
|
}
|
|
|
|
/**
|
|
* Serve a settled diff when the git state it was built from is provably
|
|
* unchanged, otherwise read and — only if the read proved everything it touched
|
|
* — record it under the stamp taken *before* the read.
|
|
*
|
|
* Stamping first is what makes staleness impossible: anything that moves during
|
|
* or after the read leaves the stored stamp behind, so the next lookup misses.
|
|
*/
|
|
async function loadDiffThroughSettledCache(
|
|
readKey: string,
|
|
worktreePath: string,
|
|
filePath: string,
|
|
staged: boolean,
|
|
compareAgainstHead: boolean,
|
|
options: GitRuntimeOptions
|
|
): Promise<GitDiffResult> {
|
|
// Why before the stamp read: the stamp is itself several awaited stats, and a mutation that
|
|
// lands entirely inside that window would otherwise leave the fence covering only the git read.
|
|
const readGeneration = settledDiffCache.beginRead()
|
|
// A staged diff compares HEAD to the index, so the working tree is not one of its inputs.
|
|
const stamp = await readWorktreeDiffStamp(worktreePath, filePath, !staged, options)
|
|
const cached = settledDiffCache.get(readKey, stamp)
|
|
if (cached) {
|
|
return cached
|
|
}
|
|
const loaded = await loadDiff(worktreePath, filePath, staged, compareAgainstHead, options)
|
|
if (loaded.reusable) {
|
|
settledDiffCache.set(readKey, stamp, loaded.result, readGeneration)
|
|
}
|
|
return loaded.result
|
|
}
|
|
|
|
/**
|
|
* `reusable` is false when the result cannot be proven to describe the stamped
|
|
* state: a submodule route, whose inputs live in another repo and are stamped by
|
|
* that repo's own read, or a blob read that failed rather than proving absence.
|
|
*/
|
|
type LoadedDiff = { result: GitDiffResult; reusable: boolean }
|
|
|
|
async function loadDiff(
|
|
worktreePath: string,
|
|
filePath: string,
|
|
staged: boolean,
|
|
compareAgainstHead: boolean,
|
|
options: GitRuntimeOptions
|
|
): Promise<LoadedDiff> {
|
|
// Why: gitlink paths can't be read as blobs, so route submodule diffs explicitly (root → pointer, inner → recurse).
|
|
const submodulePaths = await listSubmodulePaths(worktreePath, options)
|
|
if (submodulePaths.length > 0) {
|
|
const matchedSubmodule = findContainingSubmodule(submodulePaths, filePath)
|
|
if (matchedSubmodule) {
|
|
// Why: validate the .gitmodules-derived path against the worktree boundary so a crafted one can't escape the repo.
|
|
const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, matchedSubmodule)
|
|
const normalizedFilePath = filePath.replace(/\\/g, '/').replace(/\/+$/, '')
|
|
if (normalizedFilePath === matchedSubmodule) {
|
|
return notReusable(
|
|
await buildSubmodulePointerDiff(
|
|
worktreePath,
|
|
matchedSubmodule,
|
|
staged,
|
|
compareAgainstHead,
|
|
options,
|
|
submoduleWorktreePath
|
|
)
|
|
)
|
|
}
|
|
const innerPath = normalizedFilePath.slice(matchedSubmodule.length + 1)
|
|
const fromOid = staged
|
|
? await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options)
|
|
: (await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options)) ||
|
|
(await readGitlinkOidFromTree(worktreePath, 'HEAD', matchedSubmodule, options))
|
|
const toOid = staged
|
|
? await readGitlinkOidFromIndex(worktreePath, matchedSubmodule, options)
|
|
: await readWorkingSubmoduleHead(submoduleWorktreePath, options)
|
|
// Why: a moved gitlink with a clean submodule worktree means the change is committed — diff the two commits.
|
|
if (fromOid && toOid && fromOid !== toOid) {
|
|
return notReusable(
|
|
await buildSubmoduleInnerCommitRangeDiff(
|
|
submoduleWorktreePath,
|
|
innerPath,
|
|
fromOid,
|
|
toOid,
|
|
options
|
|
)
|
|
)
|
|
}
|
|
// The inner read stamps and caches against the submodule's own repo state.
|
|
return notReusable(
|
|
await getDiff(submoduleWorktreePath, innerPath, staged, compareAgainstHead, options)
|
|
)
|
|
}
|
|
}
|
|
|
|
let originalContent = ''
|
|
let modifiedContent = ''
|
|
let originalIsBinary = false
|
|
let modifiedIsBinary = false
|
|
let modifiedDeleted = false
|
|
let readFailed = false
|
|
|
|
try {
|
|
if (staged) {
|
|
// Why concurrent: HEAD and the index are independent `git show` spawns.
|
|
// Only this branch qualifies — the unstaged left read chains index→HEAD.
|
|
const [leftBlob, rightBlob] = await Promise.all([
|
|
readGitBlobAtOidPath(worktreePath, 'HEAD', filePath, options),
|
|
readGitBlobAtIndexPath(worktreePath, filePath, options)
|
|
])
|
|
originalContent = leftBlob.content
|
|
originalIsBinary = leftBlob.isBinary
|
|
modifiedContent = rightBlob.content
|
|
modifiedIsBinary = rightBlob.isBinary
|
|
modifiedDeleted = !rightBlob.exists
|
|
readFailed = leftBlob.failed === true || rightBlob.failed === true
|
|
} else {
|
|
// The left chain (index→HEAD) is sequential within itself, but the working
|
|
// tree read is a plain fs read that does not depend on it.
|
|
// Git can run in the distro against a raw Linux worktree path while Node reads it through Win32.
|
|
const hostWorktreePath = resolveWorktreeHostPath(worktreePath, options)
|
|
const [leftBlob, workingTreeBlob] = await Promise.all([
|
|
compareAgainstHead
|
|
? readGitBlobAtOidPath(worktreePath, 'HEAD', filePath, options)
|
|
: readUnstagedLeftBlob(worktreePath, filePath, options),
|
|
hostWorktreePath
|
|
? readWorkingTreeFile(path.join(hostWorktreePath, filePath))
|
|
: Promise.resolve(UNSPELLABLE_WORKING_TREE_READ)
|
|
])
|
|
originalContent = leftBlob.content
|
|
originalIsBinary = leftBlob.isBinary
|
|
modifiedContent = workingTreeBlob.content
|
|
modifiedIsBinary = workingTreeBlob.isBinary
|
|
modifiedDeleted = !workingTreeBlob.exists
|
|
readFailed = leftBlob.failed === true || workingTreeBlob.failed === true
|
|
}
|
|
} catch {
|
|
// Fallback
|
|
readFailed = true
|
|
}
|
|
|
|
const result = buildDiffResult(
|
|
originalContent,
|
|
modifiedContent,
|
|
originalIsBinary,
|
|
modifiedIsBinary,
|
|
filePath
|
|
)
|
|
// Why: mark a proven deletion so previewers don't mistake a read failure's empty side for one.
|
|
if (result.kind === 'binary' && modifiedDeleted) {
|
|
return { result: { ...result, modifiedDeleted: true }, reusable: !readFailed }
|
|
}
|
|
return { result, reusable: !readFailed }
|
|
}
|
|
|
|
/** A worktree path with no host spelling is a read failure, never a proven deletion. */
|
|
const UNSPELLABLE_WORKING_TREE_READ: GitBlobReadResult = {
|
|
content: '',
|
|
isBinary: false,
|
|
exists: true,
|
|
failed: true
|
|
}
|
|
|
|
function notReusable(result: GitDiffResult): LoadedDiff {
|
|
return { result, reusable: false }
|
|
}
|