mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
946627f2cedb0d76a7b6026d3fbb459ff2cbfd50
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
9542b45d99 |
fix(wsl): resolve conflict and working-tree probes in the host path namespace (#17895)
Git running inside a WSL distro writes `.git` gitdir pointers, and answers
`status --porcelain`, in the guest namespace. Node reads both back in the
Windows main process, where `/mnt/c/repo/.git` resolves to `C:\mnt\c\repo\.git`
and `/home/me/wt` names nothing at all. Four fs probes were built on those
fabricated paths and always came back "absent":
- `detectConflictOperation`'s four marker probes, so merge/rebase/cherry-pick
badges silently went missing.
- `parseUnmergedEntry`'s compat existence check, so every `deleted_by_us` /
`added_by_them` conflict rendered as 'deleted' regardless of the working tree.
- `findExistingWorktreeSymlinkPaths`' `lstat` from status, so Orca's own shared
symlinks (node_modules and friends) showed as user changes.
- the same `lstat` from the hosted-review dirty preflight, which fails closed:
an unreadable shared symlink read as uncommitted work and blocked PR/MR
creation outright.
`resolveGitDir` computes the host spelling of the worktree once and uses it for
both the gitfile read and the pointer resolve, so a guest-spelled worktree path
is reached at all, and a relative pointer (`worktree.useRelativePaths`, git
2.48+) resolves against a spelling Win32 understands. The pointer itself now
goes through the already-landed `resolveGitMetadataPath`, and the function gains
an optional `{ wslDistro }` for a caller whose base path does not encode a
distro. `detectConflictOperation` forwards it, and the three callers that reach
it -- status-read, the runtime RPC, the `git:conflictOperation` IPC -- pass the
git options they already hold. The return type stays `Promise<string>`.
`resolveWorktreeHostPath` is the same rule applied to a worktree path, used by
status-read for the two working-tree probes and by the review preflight. Both it
and `resolveGitMetadataPath` now treat only a single-leading-slash path as guest
namespace: `//wsl.localhost/...` is already a host UNC spelling, and translating
it prepended a second share prefix.
`readWorktreeDiffStamp` needed the same one-namespace guarantee, since moving
translation inside `resolveGitDir` would otherwise make its HEAD and index real
while the working-tree stat stayed fabricated, letting a settled diff survive
every edit. #17896 landed that change first, so it is no longer in this diff;
its version is a superset and all four components already resolve from one
`hostWorktreePath`. What remains here is the `resolveGitDir` gitfile-pointer
fix that #17896 explicitly deferred, which `worktree-diff-stamp-host-paths.test.ts`
pins.
`getConflictCompatibilityStatus` moves from `existsSync` to async `access`, for
the same reason `detectConflictOperation` did: once these paths are real they
are `\\wsl.localhost\...` shares, and a sync probe per asymmetric conflict
blocks the Electron main thread for a 9p round trip on every status poll.
Per-platform delta:
- native Windows, no WSL: no behavioral change. Nothing here starts with a
single `/`, so no path is translated. An absolute pointer is now returned
verbatim rather than separator-normalized; every consumer re-joins or
normalizes it before use.
- macOS/Linux: no change. Guest-pointer translation is gated to win32, and a
caller-named distro is ignored off Windows.
- Windows + WSL: drvfs pointers and drvfs-spelled worktrees now resolve to their
drive spelling instead of `C:\mnt\...`; a non-drvfs guest path resolves
through the named distro's UNC share, or stays verbatim (ENOENT -> existing
fail-safe) when none is named.
- SSH/relay: none. Those paths return before any of this via the provider
branch; `src/relay/git-handler-status-ops.ts` keeps its own resolveGitDir.
- folder workspaces, GitLab: none. Neither is on these code paths.
|
||
 NeilandNeil
|
dff2ff0ec3 |
fix(git): read the diff working tree and stamp through the host path spelling (#17896)
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> |
||
|
Neil
|
a9babde9a3 |
refactor(git): accept a caller-named WSL distro on the metadata path resolver (#17804)
Two small changes to the Git metadata read path. Neither has a user-visible
effect on any platform except for a malformed `.git` gitfile, described below.
1. resolveGitMetadataPath's third parameter becomes an options object
`{ platform?, wslDistro? }`. A caller that knows which distro wrote a pointer
can now say so, where previously only a WSL UNC base path could. The distro
encoded in the base path still outranks the caller's, and translation only
happens when the reading host is win32, so a caller-named distro cannot make
a POSIX host fabricate a Windows path. The UNC-base branch is exempt from
that gate because that spelling only exists on Windows. Main's other
contracts are verbatim: never null for a non-empty pointer, and a drvfs
pointer keeps its drive spelling even when a distro is named. Both production
call sites (repo-git-marker-scan.ts) pass no options, so they are unchanged.
2. The `.git` gitfile marker parse moves into one shared function,
parseGitdirMarkerPayload: `gitdir:` at the start of the file, payload
trimmed, empty payload rejected — git's own read_gitfile_gently rule.
resolve-git-dir.ts and repo-git-marker-scan.ts both call it; the latter had a
near-identical private copy and is behaviorally identical after the swap
(verified across twelve marker spellings; the only divergence, a
whitespace-only payload, already resolved to null one call further down).
Main's `/^gitdir:\s*(.+)\s*$/m` in resolve-git-dir captured trailing padding
into the path and honored a `gitdir:` line anywhere in the file.
Per-platform delta: none on macOS, Linux, native Windows, WSL, SSH, relay, or
folder workspaces. The wslDistro option is inert; this change adds no caller.
For a malformed `.git` gitfile, padding is now stripped (strict improvement), a
whitespace-only payload falls back to `<worktree>/.git`, and a `gitdir:` line
that is not the first line is no longer honored — a narrowing, since main could
return a working gitdir there. All four resolveGitDir consumers already degrade
through a catch, so that case reports no sparse state / conflict operation /
diff stamp rather than failing.
Six other hand-rolled `gitdir:` parsers remain, including the relay's SSH copy;
converging them is its own change.
|
||
 NeilandNeil
|
7f63db7d7a |
fix(git): resolve WSL drvfs Git metadata pointers on a Windows host (#17790)
When Orca's runtime is a WSL distro but the repo sits on a Windows drive, git inside the distro writes `/mnt/c/...` into a worktree's `.git` gitfile and its `commondir`, while Orca reads those files back through Win32. `repo-git-marker-scan` returned the pointer verbatim, Windows read it as drive-relative `C:\mnt\c\...`, and the worktree was reported `invalid`. Move that resolver out of `repo-git-marker-scan` into `src/shared/git-metadata-path.ts` and give it exactly one new case: on win32, a drvfs pointer resolved against a base path that is not a WSL UNC path now gets its drive spelling. Every other base/pointer/platform combination is byte-identical to the deleted helper, verified differentially across a base x pointer x platform matrix — macOS, Linux and native Windows are unchanged. `toWindowsWslDrivePath` is factored out of `toWindowsWslPath` so the drvfs matcher has one home; `toWindowsWslPath` itself is unchanged for all inputs, including the line terminators JS `.` excludes (fuzzed 2M inputs, 0 divergences). This changes the marker scan's verdict only. `resolve-git-dir.ts` and the relay's own copy still `path.resolve` the same `/mnt/c/...` pointer in the Win32 namespace, so a worktree that is now accepted still degrades quietly in conflict detection, sparse-checkout detection, the diff stamp and worktree listing. Those parsers are deliberately untouched here; see the PR description. Co-authored-by: Neil <neil@example.com> |