mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
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.
This commit is contained in:
@@ -9,8 +9,11 @@ import { resolveGitDir } from './resolve-git-dir'
|
||||
|
||||
// Why: the git-status → existsSync race can miss a transient HEAD; fall back to 'unknown' for one poll cycle.
|
||||
// Why: detect rebase from rebase-merge/ or rebase-apply/ dirs (persist all steps), not REBASE_HEAD (partial, lingers → stale badge).
|
||||
export async function detectConflictOperation(worktreePath: string): Promise<GitConflictOperation> {
|
||||
const gitDir = await resolveGitDir(worktreePath)
|
||||
export async function detectConflictOperation(
|
||||
worktreePath: string,
|
||||
options: Pick<GitRuntimeOptions, 'wslDistro'> = {}
|
||||
): Promise<GitConflictOperation> {
|
||||
const gitDir = await resolveGitDir(worktreePath, options)
|
||||
const mergeHead = path.join(gitDir, 'MERGE_HEAD')
|
||||
const cherryPickHead = path.join(gitDir, 'CHERRY_PICK_HEAD')
|
||||
const rebaseMergeDir = path.join(gitDir, 'rebase-merge')
|
||||
|
||||
@@ -1,14 +1,25 @@
|
||||
import { readFile } from 'node:fs/promises'
|
||||
import * as path from 'node:path'
|
||||
import { resolveGitMetadataPath, resolveWorktreeHostPath } from '../../../shared/git-metadata-path'
|
||||
import { parseGitdirMarkerPayload } from '../../../shared/gitdir-marker-payload'
|
||||
import type { GitRuntimeOptions } from '../git-runtime-options'
|
||||
|
||||
export async function resolveGitDir(worktreePath: string): Promise<string> {
|
||||
const dotGitPath = path.join(worktreePath, '.git')
|
||||
export async function resolveGitDir(
|
||||
worktreePath: string,
|
||||
options: Pick<GitRuntimeOptions, 'wslDistro'> = {}
|
||||
): Promise<string> {
|
||||
// Why: git in a WSL distro reports the worktree in the guest namespace, but this read and the
|
||||
// pointer resolve below both run in the Windows main process, where that spelling names nothing.
|
||||
// A relative pointer (`worktree.useRelativePaths`) resolves against it too, so a guest-spelled
|
||||
// base would make Win32 resolve it drive-relative.
|
||||
// Null only for an empty path; the caller's spelling keeps the pre-existing fallback.
|
||||
const hostWorktreePath = resolveWorktreeHostPath(worktreePath, options) ?? worktreePath
|
||||
const dotGitPath = path.join(hostWorktreePath, '.git')
|
||||
|
||||
try {
|
||||
const gitDir = parseGitdirMarkerPayload(await readFile(dotGitPath, 'utf-8'))
|
||||
if (gitDir) {
|
||||
return path.resolve(worktreePath, gitDir)
|
||||
return resolveGitMetadataPath(hostWorktreePath, gitDir, options) ?? dotGitPath
|
||||
}
|
||||
} catch {
|
||||
// `.git` is likely a directory in a non-worktree checkout.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { existsSync } from 'node:fs'
|
||||
import { access } from 'node:fs/promises'
|
||||
import * as path from 'node:path'
|
||||
import type {
|
||||
GitConflictKind,
|
||||
@@ -78,10 +78,15 @@ async function getConflictCompatibilityStatus(
|
||||
return 'deleted'
|
||||
}
|
||||
|
||||
// Why async: on a WSL worktree this path is a `\\wsl.localhost\...` share, and a sync probe
|
||||
// per asymmetric conflict blocks the Electron main thread for a 9p round trip each.
|
||||
try {
|
||||
return existsSync(path.join(worktreePath, filePath)) ? 'modified' : 'deleted'
|
||||
} catch {
|
||||
// Why: on an fs check failure, 'modified' is safer — it keeps the row visible rather than falsely showing 'deleted'.
|
||||
await access(path.join(worktreePath, filePath))
|
||||
return 'modified'
|
||||
} catch (error) {
|
||||
// Why: only a definite "not there" reads as deleted; any other fs failure keeps the row visible
|
||||
// rather than falsely showing 'deleted'.
|
||||
const code = (error as NodeJS.ErrnoException).code
|
||||
return code === 'ENOENT' || code === 'ENOTDIR' ? 'deleted' : 'modified'
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
clearGitStatusLineStatsCacheKey,
|
||||
reuseOrRecomputeGitStatusLineStats
|
||||
} from '../../../shared/git-status-line-stats-cache'
|
||||
import { resolveWorktreeHostPath } from '../../../shared/git-metadata-path'
|
||||
import { gitOptionalLocksDisabledEnv, gitStreamStdout } from '../runner'
|
||||
import { findExistingWorktreeSymlinkPaths } from '../worktree-symlink-detection'
|
||||
import type { GetStatusOptions } from './get-status-options'
|
||||
@@ -80,14 +81,19 @@ function getStatusReadKey(worktreePath: string, options: GetStatusOptions): stri
|
||||
async function dropSharedSymlinkUntrackedEntries(
|
||||
worktreePath: string,
|
||||
entries: GitStatusEntry[],
|
||||
sharedLinkPaths: readonly string[]
|
||||
options: GetStatusOptions
|
||||
): Promise<void> {
|
||||
const sharedLinkPaths = options.sharedLinkPaths ?? []
|
||||
// Why: a clean tree has no untracked entries, so this costs nothing on the
|
||||
// common status-poll path — no syscall, no config read, no subprocess.
|
||||
if (sharedLinkPaths.length === 0 || !entries.some((entry) => entry.area === 'untracked')) {
|
||||
return
|
||||
}
|
||||
const sharedLinks = new Set(await findExistingWorktreeSymlinkPaths(worktreePath, sharedLinkPaths))
|
||||
const sharedLinks = new Set(
|
||||
await findExistingWorktreeSymlinkPaths(worktreePath, sharedLinkPaths, {
|
||||
wslDistro: options.wslDistro
|
||||
})
|
||||
)
|
||||
if (sharedLinks.size === 0) {
|
||||
return
|
||||
}
|
||||
@@ -112,7 +118,7 @@ async function runGetStatus(
|
||||
const limit = resolveGitStatusLimit(options.limit)
|
||||
|
||||
// Why: detectConflictOperation and git status are independent, so run them concurrently to save I/O latency.
|
||||
const conflictPromise = detectConflictOperation(worktreePath)
|
||||
const conflictPromise = detectConflictOperation(worktreePath, options)
|
||||
// Why: core.quotePath=false keeps non-ASCII paths as raw UTF-8, not octal escapes, so entry.path is readable and lookups match.
|
||||
const statusArgs = [
|
||||
'-c',
|
||||
@@ -167,6 +173,8 @@ async function runGetStatus(
|
||||
|
||||
const entries: GitStatusEntry[] = []
|
||||
const { head, branch, upstreamName, upstreamAheadBehind } = parser.branch
|
||||
// Why: git runs in the distro and answers in its namespace; the working-tree probes below run here.
|
||||
const hostWorktreePath = resolveWorktreeHostPath(worktreePath, options) ?? worktreePath
|
||||
|
||||
// Why: resolve deferred conflicts in Git's output order so the cap cannot hide
|
||||
// an early conflict behind ordinary rows that appeared later in the stream.
|
||||
@@ -177,14 +185,14 @@ async function runGetStatus(
|
||||
if (record.type === 'entry') {
|
||||
entries.push(record.entry)
|
||||
} else {
|
||||
const unmergedEntry = await parseUnmergedEntry(worktreePath, record.line)
|
||||
const unmergedEntry = await parseUnmergedEntry(hostWorktreePath, record.line)
|
||||
if (unmergedEntry) {
|
||||
entries.push(unmergedEntry)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
await dropSharedSymlinkUntrackedEntries(worktreePath, entries, options.sharedLinkPaths ?? [])
|
||||
await dropSharedSymlinkUntrackedEntries(worktreePath, entries, options)
|
||||
|
||||
if (statusSucceeded && !didHitLimit && shouldProbeEffectiveUpstreamStatus(branch, upstreamName)) {
|
||||
const branchName = getShortBranchName(branch)
|
||||
|
||||
@@ -147,4 +147,100 @@ describe('detectConflictOperation', () => {
|
||||
|
||||
await expect(detectConflictOperation('/repo')).resolves.toBe('unknown')
|
||||
})
|
||||
|
||||
// Both cases below assert the probed prefix rather than the joined string so they exercise
|
||||
// Win32 pointer resolution on every host, where `path.join` still uses the host separator.
|
||||
it('probes the drive spelling of a drvfs gitdir pointer on Windows', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockResolvedValue('gitdir: /mnt/c/Users/me/repo/.git/worktrees/feature\n')
|
||||
accessMock.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))
|
||||
|
||||
try {
|
||||
await expect(detectConflictOperation(String.raw`C:\Users\me\repo\feature`)).resolves.toBe(
|
||||
'unknown'
|
||||
)
|
||||
for (const [target] of accessMock.mock.calls) {
|
||||
expect(target).toContain(String.raw`C:\Users\me\repo\.git\worktrees\feature`)
|
||||
}
|
||||
expect(accessMock).toHaveBeenCalledTimes(4)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
// The worktree path itself can be guest-spelled (git in the distro reports it that way), so the
|
||||
// gitfile read has to be translated too or it ENOENTs before any pointer resolution happens.
|
||||
it('reads the gitfile at the host spelling of a guest-spelled worktree', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockResolvedValue('gitdir: /mnt/c/Users/me/repo/.git/worktrees/feature\n')
|
||||
accessMock.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))
|
||||
|
||||
try {
|
||||
await expect(detectConflictOperation('/mnt/c/Users/me/repo/feature')).resolves.toBe('unknown')
|
||||
expect(readFileMock).toHaveBeenCalledTimes(1)
|
||||
expect(readFileMock.mock.calls[0][0]).toContain(String.raw`C:\Users\me\repo\feature`)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
// A plain clone inside the distro has a `.git` directory, so the gitfile read fails and the
|
||||
// markers are probed under the worktree itself — that fallback needs the host spelling too.
|
||||
it('probes a directory .git under the distro share when the caller names one', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockRejectedValue(Object.assign(new Error('EISDIR'), { code: 'EISDIR' }))
|
||||
accessMock.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))
|
||||
|
||||
try {
|
||||
await expect(
|
||||
detectConflictOperation('/home/me/repo/feature', { wslDistro: 'Ubuntu' })
|
||||
).resolves.toBe('unknown')
|
||||
expect(readFileMock.mock.calls[0][0]).toContain(
|
||||
String.raw`\\wsl.localhost\Ubuntu\home\me\repo\feature`
|
||||
)
|
||||
for (const [target] of accessMock.mock.calls) {
|
||||
expect(target).toContain(String.raw`\\wsl.localhost\Ubuntu\home\me\repo\feature`)
|
||||
}
|
||||
expect(accessMock).toHaveBeenCalledTimes(4)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
// `git worktree repair --relative-paths` (2.48+) writes `gitdir: ../../..`, which the guest
|
||||
// spelling of the worktree would resolve drive-relative on Win32.
|
||||
it('resolves a relative gitdir pointer against the host spelling of the worktree', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockResolvedValue('gitdir: ../.git/worktrees/feature\n')
|
||||
accessMock.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))
|
||||
|
||||
try {
|
||||
await expect(detectConflictOperation('/mnt/c/Users/me/repo/feature')).resolves.toBe('unknown')
|
||||
for (const [target] of accessMock.mock.calls) {
|
||||
expect(target).toContain(String.raw`C:\Users\me\repo\.git\worktrees\feature`)
|
||||
}
|
||||
expect(accessMock).toHaveBeenCalledTimes(4)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('probes the distro UNC share when the caller names one', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockResolvedValue('gitdir: /home/me/repo/.git/worktrees/feature\n')
|
||||
accessMock.mockImplementation(async (target: string) =>
|
||||
target.includes(String.raw`\\wsl.localhost\Ubuntu\home\me\repo\.git\worktrees\feature`) &&
|
||||
target.endsWith('MERGE_HEAD')
|
||||
? undefined
|
||||
: Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))
|
||||
)
|
||||
|
||||
try {
|
||||
await expect(
|
||||
detectConflictOperation(String.raw`C:\Users\me\repo\feature`, { wslDistro: 'Ubuntu' })
|
||||
).resolves.toBe('merge')
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
+59
-16
@@ -15,8 +15,7 @@ const {
|
||||
readFileMock,
|
||||
statMock,
|
||||
rmMock,
|
||||
accessMock,
|
||||
existsSyncMock
|
||||
accessMock
|
||||
} = vi.hoisted(() => ({
|
||||
gitExecFileAsyncMock: vi.fn(),
|
||||
gitExecFileAsyncBufferMock: vi.fn(),
|
||||
@@ -26,8 +25,7 @@ const {
|
||||
readFileMock: vi.fn(),
|
||||
statMock: vi.fn(),
|
||||
rmMock: vi.fn(),
|
||||
accessMock: vi.fn(),
|
||||
existsSyncMock: vi.fn()
|
||||
accessMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('./runner', () =>
|
||||
@@ -49,11 +47,6 @@ vi.mock('fs/promises', () =>
|
||||
})
|
||||
)
|
||||
|
||||
// Why still here: unmerged-entry parsing probes the working tree through node:fs directly.
|
||||
vi.mock('fs', () => ({
|
||||
existsSync: existsSyncMock
|
||||
}))
|
||||
|
||||
vi.mock('../../shared/node-bounded-file-reader', async (importOriginal) =>
|
||||
createBoundedFileReaderModuleMock(await importOriginal<typeof BoundedFileReader>(), {
|
||||
readFileMock,
|
||||
@@ -71,7 +64,6 @@ describe('getStatus', () => {
|
||||
gitStreamOptionsMock.mockReset()
|
||||
lstatMock.mockReset()
|
||||
readFileMock.mockReset()
|
||||
existsSyncMock.mockReset()
|
||||
accessMock.mockReset()
|
||||
accessMock.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))
|
||||
// Why: untracked line counting stats a file before reading it; any
|
||||
@@ -130,11 +122,9 @@ describe('getStatus', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('falls back to modified when the filesystem existence check throws', async () => {
|
||||
it('falls back to modified when the working-tree probe fails for a non-absence reason', async () => {
|
||||
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
|
||||
existsSyncMock.mockImplementation(() => {
|
||||
throw new Error('stat failed')
|
||||
})
|
||||
accessMock.mockRejectedValue(Object.assign(new Error('EIO'), { code: 'EIO' }))
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout:
|
||||
'u AU N... 100644 100644 100644 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc src/new.ts\n'
|
||||
@@ -146,6 +136,61 @@ describe('getStatus', () => {
|
||||
expect(result.entries[0]?.conflictKind).toBe('added_by_us')
|
||||
})
|
||||
|
||||
// Why both cases normalize separators: git reports the worktree in the WSL guest namespace, and
|
||||
// the assertion is about which path is probed, not which separator this host's `path` emits.
|
||||
it('probes the conflict working tree through the distro spelling on Windows', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockResolvedValue('gitdir: /home/me/repo/.git/worktrees/feature\n')
|
||||
accessMock.mockImplementation(async (target: string) => {
|
||||
if (String(target).endsWith('new.ts')) {
|
||||
return undefined
|
||||
}
|
||||
throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' })
|
||||
})
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout:
|
||||
'u DU N... 100644 100644 100644 100644 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb cccccccccccccccccccccccccccccccccccccccc src/new.ts\n'
|
||||
})
|
||||
|
||||
try {
|
||||
const result = await getStatus('/home/me/repo/feature', { wslDistro: 'Ubuntu' })
|
||||
|
||||
const probed = accessMock.mock.calls.map(([target]) => String(target).replaceAll('\\', '/'))
|
||||
expect(probed).toContain('//wsl.localhost/Ubuntu/home/me/repo/feature/src/new.ts')
|
||||
expect(result.entries[0]?.status).toBe('modified')
|
||||
expect(result.entries[0]?.conflictKind).toBe('deleted_by_us')
|
||||
// The conflict-marker probes travel the same way.
|
||||
expect(
|
||||
probed.filter((target) =>
|
||||
target.startsWith('//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/feature/')
|
||||
)
|
||||
).toHaveLength(4)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('probes shared symlinks through the distro spelling on Windows', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
readFileMock.mockResolvedValue('gitdir: /home/me/repo/.git/worktrees/feature\n')
|
||||
lstatMock.mockResolvedValue({ isSymbolicLink: () => true })
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '? node_modules\n' })
|
||||
|
||||
try {
|
||||
const result = await getStatus('/home/me/repo/feature', {
|
||||
wslDistro: 'Ubuntu',
|
||||
sharedLinkPaths: ['node_modules']
|
||||
})
|
||||
|
||||
expect(String(lstatMock.mock.calls[0]?.[0]).replaceAll('\\', '/')).toContain(
|
||||
'//wsl.localhost/Ubuntu/home/me/repo/feature/node_modules'
|
||||
)
|
||||
expect(result.entries).toEqual([])
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('passes core.quotePath=false and round-trips UTF-8 paths', async () => {
|
||||
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
@@ -632,7 +677,6 @@ describe('getStatus', () => {
|
||||
|
||||
it('caps unmerged conflicts and keeps the visible conflict rows', async () => {
|
||||
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
|
||||
existsSyncMock.mockReturnValue(true)
|
||||
const lines = [
|
||||
'u UU S... 160000 160000 160000 160000 aa bb cc vendor/submodule',
|
||||
...Array.from(
|
||||
@@ -655,7 +699,6 @@ describe('getStatus', () => {
|
||||
|
||||
it('keeps an early conflict ahead of later ordinary rows at the cap', async () => {
|
||||
readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
|
||||
existsSyncMock.mockReturnValue(true)
|
||||
const lines = [
|
||||
'? before.ts',
|
||||
'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts',
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { readFileMock, statMock } = vi.hoisted(() => ({
|
||||
readFileMock: vi.fn(),
|
||||
statMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('fs/promises', () => ({
|
||||
readFile: readFileMock,
|
||||
stat: statMock
|
||||
}))
|
||||
|
||||
import { readWorktreeDiffStamp } from './source-control/worktree-diff-stamp'
|
||||
|
||||
const slashed = (value: unknown): string => String(value).replaceAll('\\', '/')
|
||||
const missing = () => Object.assign(new Error('ENOENT'), { code: 'ENOENT' })
|
||||
|
||||
// Why this file: git in a WSL distro reports the worktree as `/mnt/c/...`, and the gitdir resolve
|
||||
// reaches it on its own. If the working-tree read did not travel the same way, the stamp would be
|
||||
// built from a real HEAD and a permanently absent file — a settled diff that survives every edit.
|
||||
describe('readWorktreeDiffStamp on a drvfs-spelled WSL worktree', () => {
|
||||
beforeEach(() => {
|
||||
readFileMock.mockReset()
|
||||
statMock.mockReset()
|
||||
readFileMock.mockImplementation(async (target: string) => {
|
||||
const value = slashed(target)
|
||||
if (value === 'C:/repo/wt/.git') {
|
||||
return 'gitdir: /mnt/c/repo/.git/worktrees/wt\n'
|
||||
}
|
||||
// Detached HEAD, so the stamp needs no ref-store walk.
|
||||
if (value === 'C:/repo/.git/worktrees/wt/HEAD') {
|
||||
return `${'a'.repeat(40)}\n`
|
||||
}
|
||||
throw missing()
|
||||
})
|
||||
statMock.mockImplementation(async (target: string) =>
|
||||
slashed(target) === 'C:/repo/wt/src/a.ts'
|
||||
? { mtimeMs: 1_000, size: 12, ino: 7 }
|
||||
: Promise.reject(missing())
|
||||
)
|
||||
})
|
||||
|
||||
it('stamps the working-tree file through the same host spelling as the gitdir', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
|
||||
try {
|
||||
const stamp = await readWorktreeDiffStamp('/mnt/c/repo/wt', 'src/a.ts', true)
|
||||
|
||||
expect(stamp).not.toBeNull()
|
||||
expect(statMock.mock.calls.map(([target]) => slashed(target))).toContain(
|
||||
'C:/repo/wt/src/a.ts'
|
||||
)
|
||||
// The working-tree component is what moves when the user edits, so it has to be present.
|
||||
expect(stamp?.newestMtimeMs).toBe(1_000)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,6 @@
|
||||
import { lstat } from 'node:fs/promises'
|
||||
import { resolve } from 'node:path'
|
||||
import { resolveWorktreeHostPath } from '../../shared/git-metadata-path'
|
||||
|
||||
// Why this is a leaf module rather than part of ipc/worktree-symlinks: status
|
||||
// and review-creation need only the read-only "is this a symlink" question, and
|
||||
@@ -32,10 +33,19 @@ export function getSafeRelativePath(rawPath: string): SafeRelativePathResult {
|
||||
return { safe: true, rel }
|
||||
}
|
||||
|
||||
export type WorktreeSymlinkDetectionOptions = {
|
||||
/** Distro that spelled `worktreePath`, for a Windows host reopening a guest path. */
|
||||
wslDistro?: string
|
||||
}
|
||||
|
||||
export async function findExistingWorktreeSymlinkPaths(
|
||||
worktreePath: string,
|
||||
paths: readonly string[]
|
||||
paths: readonly string[],
|
||||
options: WorktreeSymlinkDetectionOptions = {}
|
||||
): Promise<string[]> {
|
||||
// Why: git in a WSL distro reports the worktree in the guest namespace, but this lstat runs in
|
||||
// the Windows main process, where that spelling names nothing.
|
||||
const hostWorktreePath = resolveWorktreeHostPath(worktreePath, options) ?? worktreePath
|
||||
const symlinkPaths: string[] = []
|
||||
for (const rawPath of paths) {
|
||||
const safePath = getSafeRelativePath(rawPath)
|
||||
@@ -43,7 +53,7 @@ export async function findExistingWorktreeSymlinkPaths(
|
||||
continue
|
||||
}
|
||||
try {
|
||||
if ((await lstat(resolve(worktreePath, safePath.rel))).isSymbolicLink()) {
|
||||
if ((await lstat(resolve(hostWorktreePath, safePath.rel))).isSymbolicLink()) {
|
||||
symlinkPaths.push(safePath.rel)
|
||||
}
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
handlers,
|
||||
store,
|
||||
REPO_PATH,
|
||||
WORKTREE_FEATURE_PATH,
|
||||
detectConflictOperationMock,
|
||||
resetFilesystemIpcMocks
|
||||
} from './filesystem-test-harness'
|
||||
|
||||
const getLocalGitOptionsForRegisteredWorktreeMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('electron', async () => (await import('./filesystem-test-harness')).electronMock)
|
||||
vi.mock('fs/promises', async () => (await import('./filesystem-test-harness')).fsPromisesMock)
|
||||
vi.mock(
|
||||
'../wsl-unc-delete',
|
||||
async () => (await import('./filesystem-test-harness')).wslUncDeleteMock
|
||||
)
|
||||
vi.mock(
|
||||
'../crash-reporting/crash-breadcrumb-store',
|
||||
async () => (await import('./filesystem-test-harness')).crashBreadcrumbMock
|
||||
)
|
||||
vi.mock(
|
||||
'../local-downloaded-folder-promotion',
|
||||
async () => (await import('./filesystem-test-harness')).folderPromotionMock
|
||||
)
|
||||
vi.mock(
|
||||
'../git/status',
|
||||
async () => (await import('./filesystem-test-harness')).gitStatusModuleMock
|
||||
)
|
||||
vi.mock(
|
||||
'../git/check-ignored-paths',
|
||||
async () => (await import('./filesystem-test-harness')).gitIgnoredPathsMock
|
||||
)
|
||||
vi.mock('../git/worktree', async () => (await import('./filesystem-test-harness')).gitWorktreeMock)
|
||||
vi.mock(
|
||||
'../providers/ssh-filesystem-dispatch',
|
||||
async () => (await import('./filesystem-test-harness')).sshFilesystemDispatchMock
|
||||
)
|
||||
vi.mock(
|
||||
'../providers/ssh-git-dispatch',
|
||||
async () => (await import('./filesystem-test-harness')).sshGitDispatchMock
|
||||
)
|
||||
vi.mock('./local-worktree-runtime-options', () => ({
|
||||
getLocalGitOptionsForRegisteredWorktree: getLocalGitOptionsForRegisteredWorktreeMock,
|
||||
getLocalGitOptionsForRepo: vi.fn(() => ({})),
|
||||
getLocalRepoForRegisteredWorktree: vi.fn(() => undefined)
|
||||
}))
|
||||
|
||||
import { registerFilesystemHandlers } from './filesystem'
|
||||
import {
|
||||
registerWorktreeRootsForRepo,
|
||||
invalidateAuthorizedRootsCache
|
||||
} from './registered-worktree-roots-cache'
|
||||
|
||||
// Why: `git:conflictOperation` reads the worktree's `.git` pointer directly, so it has to run in
|
||||
// the same host namespace as that worktree's git — a WSL project answers in the guest namespace.
|
||||
describe('git:conflictOperation local routing', () => {
|
||||
beforeEach(() => {
|
||||
resetFilesystemIpcMocks()
|
||||
invalidateAuthorizedRootsCache()
|
||||
getLocalGitOptionsForRegisteredWorktreeMock.mockReset()
|
||||
getLocalGitOptionsForRegisteredWorktreeMock.mockReturnValue({ wslDistro: 'Ubuntu' })
|
||||
detectConflictOperationMock.mockResolvedValue('merge')
|
||||
})
|
||||
|
||||
it("probes with the registered worktree's local git options", async () => {
|
||||
registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH])
|
||||
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
await expect(
|
||||
handlers.get('git:conflictOperation')!(null, { worktreePath: WORKTREE_FEATURE_PATH })
|
||||
).resolves.toBe('merge')
|
||||
|
||||
expect(getLocalGitOptionsForRegisteredWorktreeMock).toHaveBeenCalledWith(
|
||||
store,
|
||||
WORKTREE_FEATURE_PATH,
|
||||
WORKTREE_FEATURE_PATH
|
||||
)
|
||||
expect(detectConflictOperationMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -28,6 +28,7 @@ export const realpathMock: IpcMock = vi.fn()
|
||||
export const lstatMock: IpcMock = vi.fn()
|
||||
export const commitChangesMock: IpcMock = vi.fn()
|
||||
export const getStatusMock: IpcMock = vi.fn()
|
||||
export const detectConflictOperationMock: IpcMock = vi.fn()
|
||||
export const abortMergeMock: IpcMock = vi.fn()
|
||||
export const abortRebaseMock: IpcMock = vi.fn()
|
||||
export const getDiffMock: IpcMock = vi.fn()
|
||||
@@ -88,6 +89,7 @@ export const folderPromotionMock = {
|
||||
export const gitStatusModuleMock = {
|
||||
commitChanges: commitChangesMock,
|
||||
getStatus: getStatusMock,
|
||||
detectConflictOperation: detectConflictOperationMock,
|
||||
abortMerge: abortMergeMock,
|
||||
abortRebase: abortRebaseMock,
|
||||
getDiff: getDiffMock,
|
||||
|
||||
@@ -223,7 +223,12 @@ export function registerFilesystemGitStatusHandlers(context: FilesystemHandlerCo
|
||||
return provider.detectConflictOperation(args.worktreePath)
|
||||
}
|
||||
const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store)
|
||||
return detectConflictOperation(worktreePath)
|
||||
const gitOptions = getLocalGitOptionsForRegisteredWorktree(
|
||||
store,
|
||||
args.worktreePath,
|
||||
worktreePath
|
||||
)
|
||||
return detectConflictOperation(worktreePath, gitOptions)
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as GitStatusModule from '../git/status'
|
||||
|
||||
const detectConflictOperationMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('../git/status', async () => ({
|
||||
...(await vi.importActual<typeof GitStatusModule>('../git/status')),
|
||||
detectConflictOperation: detectConflictOperationMock
|
||||
}))
|
||||
|
||||
import { RuntimeGitStatusCommands } from './runtime-git-status-commands'
|
||||
|
||||
// Why: the conflict badge the runtime RPC serves is read from the worktree's `.git` pointer, so it
|
||||
// must run in the same host namespace as the target's git — a WSL target's paths are guest-spelled.
|
||||
describe('getRuntimeGitConflictOperation', () => {
|
||||
beforeEach(() => {
|
||||
detectConflictOperationMock.mockReset()
|
||||
detectConflictOperationMock.mockResolvedValue('merge')
|
||||
})
|
||||
|
||||
it("probes with the target's local git options", async () => {
|
||||
const commands = new RuntimeGitStatusCommands({
|
||||
resolveRuntimeGitTarget: async () => ({
|
||||
worktree: { path: '/home/me/repo/feature' },
|
||||
localGitOptions: { wslDistro: 'Ubuntu' }
|
||||
})
|
||||
} as never)
|
||||
|
||||
await expect(commands.getRuntimeGitConflictOperation('id:wt-1')).resolves.toBe('merge')
|
||||
|
||||
expect(detectConflictOperationMock).toHaveBeenCalledWith('/home/me/repo/feature', {
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -116,7 +116,7 @@ export class RuntimeGitStatusCommands {
|
||||
}
|
||||
return provider.detectConflictOperation(target.worktree.path)
|
||||
}
|
||||
return detectConflictOperation(target.worktree.path)
|
||||
return detectConflictOperation(target.worktree.path, localGitOptionsForTarget(target))
|
||||
}
|
||||
|
||||
async checkoutRuntimeGitBranch(
|
||||
|
||||
@@ -268,7 +268,7 @@ export async function hasUncommittedChanges(
|
||||
if (records.length === 0) {
|
||||
return false
|
||||
}
|
||||
return await anyRecordIsUserDirt(repoPath, records, options.sharedLinkPaths ?? [])
|
||||
return await anyRecordIsUserDirt(repoPath, records, options)
|
||||
}
|
||||
|
||||
/** True when any record is real user work rather than a shared symlink Orca put
|
||||
@@ -280,14 +280,21 @@ export async function hasUncommittedChanges(
|
||||
async function anyRecordIsUserDirt(
|
||||
worktreePath: string,
|
||||
records: readonly PorcelainV1Record[],
|
||||
sharedLinkPaths: readonly string[]
|
||||
options: HostedReviewExecutionOptions
|
||||
): Promise<boolean> {
|
||||
const sharedLinkPaths = options.sharedLinkPaths ?? []
|
||||
if (sharedLinkPaths.length === 0 || !records.some((record) => record.xy === '??')) {
|
||||
return true
|
||||
}
|
||||
// Why: only entries that are configured AND really symlinks are excluded, so a
|
||||
// regular file the user created at a configured name still blocks creation.
|
||||
const sharedLinks = new Set(await findExistingWorktreeSymlinkPaths(worktreePath, sharedLinkPaths))
|
||||
// Why the distro: git ran in the guest, so an untranslated lstat fails here and this
|
||||
// fail-closed check would block review creation over Orca's own symlink.
|
||||
const sharedLinks = new Set(
|
||||
await findExistingWorktreeSymlinkPaths(worktreePath, sharedLinkPaths, {
|
||||
wslDistro: getHostedReviewLocalGitOptions(options).wslDistro
|
||||
})
|
||||
)
|
||||
return records.some((record) => record.xy !== '??' || !sharedLinks.has(record.path))
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { gitExecFileAsyncMock, findExistingWorktreeSymlinkPathsMock } = vi.hoisted(() => ({
|
||||
gitExecFileAsyncMock: vi.fn(),
|
||||
findExistingWorktreeSymlinkPathsMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('../github/gh-utils', () => ({
|
||||
acquire: vi.fn(),
|
||||
release: vi.fn(),
|
||||
ghExecFileAsync: vi.fn(),
|
||||
gitExecFileAsync: gitExecFileAsyncMock
|
||||
}))
|
||||
vi.mock('../git/worktree-symlink-detection', () => ({
|
||||
findExistingWorktreeSymlinkPaths: findExistingWorktreeSymlinkPathsMock
|
||||
}))
|
||||
|
||||
import { hasUncommittedChanges } from './hosted-review-creation-git-state'
|
||||
|
||||
// Why: git ran inside the distro and answered in its namespace, so the fail-closed shared-symlink
|
||||
// check must lstat the host spelling — otherwise it never recognises Orca's own symlink and blocks
|
||||
// review creation on a permanently "dirty" WSL worktree.
|
||||
describe('hasUncommittedChanges shared-symlink probe', () => {
|
||||
beforeEach(() => {
|
||||
gitExecFileAsyncMock.mockReset()
|
||||
findExistingWorktreeSymlinkPathsMock.mockReset()
|
||||
gitExecFileAsyncMock.mockResolvedValue({ stdout: '?? node_modules\0', stderr: '' })
|
||||
findExistingWorktreeSymlinkPathsMock.mockResolvedValue(['node_modules'])
|
||||
})
|
||||
|
||||
it('passes the configured distro through to the probe', async () => {
|
||||
await expect(
|
||||
hasUncommittedChanges('/home/me/repo/feature', null, {
|
||||
localGitExecOptions: { wslDistro: 'Ubuntu' },
|
||||
sharedLinkPaths: ['node_modules']
|
||||
})
|
||||
).resolves.toBe(false)
|
||||
|
||||
expect(findExistingWorktreeSymlinkPathsMock).toHaveBeenCalledWith(
|
||||
'/home/me/repo/feature',
|
||||
['node_modules'],
|
||||
{ wslDistro: 'Ubuntu' }
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -47,6 +47,18 @@ describe('resolveGitMetadataPath', () => {
|
||||
).toBe(String.raw`\\wsl.localhost\Ubuntu\home\me\repo\.git`)
|
||||
})
|
||||
|
||||
// Git for Windows spells a UNC gitdir with forward slashes; that is already a host path, and
|
||||
// translating it would produce `\\wsl.localhost\Ubuntu\\wsl.localhost\Ubuntu\...`.
|
||||
it('keeps a forward-slash UNC pointer verbatim rather than re-prefixing the share', () => {
|
||||
expect(
|
||||
resolveGitMetadataPath(
|
||||
String.raw`\\wsl.localhost\Ubuntu\home\me\repo\feature`,
|
||||
'//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/feature',
|
||||
{ platform: 'win32', wslDistro: 'Ubuntu' }
|
||||
)
|
||||
).toBe('//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/feature')
|
||||
})
|
||||
|
||||
it('lets the distro encoded by a WSL UNC base outrank the caller-named one', () => {
|
||||
expect(
|
||||
resolveGitMetadataPath(
|
||||
@@ -168,4 +180,43 @@ describe('resolveWorktreeHostPath', () => {
|
||||
it.each(['', ' '])('has no spelling for an empty worktree path %j', (worktreePath) => {
|
||||
expect(resolveWorktreeHostPath(worktreePath, { platform: 'win32' })).toBeNull()
|
||||
})
|
||||
|
||||
it('maps a drvfs worktree to its drive spelling on a Windows host', () => {
|
||||
expect(resolveWorktreeHostPath('/mnt/c/Users/me/repo/feature', { platform: 'win32' })).toBe(
|
||||
String.raw`C:\Users\me\repo\feature`
|
||||
)
|
||||
})
|
||||
|
||||
it('maps a guest worktree through a caller-named distro', () => {
|
||||
expect(
|
||||
resolveWorktreeHostPath('/home/me/repo/feature', { platform: 'win32', wslDistro: 'Ubuntu' })
|
||||
).toBe(String.raw`\\wsl.localhost\Ubuntu\home\me\repo\feature`)
|
||||
})
|
||||
|
||||
it('keeps a guest worktree verbatim on Windows when no distro names it', () => {
|
||||
expect(resolveWorktreeHostPath('/home/me/repo/feature', { platform: 'win32' })).toBe(
|
||||
'/home/me/repo/feature'
|
||||
)
|
||||
})
|
||||
|
||||
it('ignores a caller-named distro on a POSIX host', () => {
|
||||
expect(
|
||||
resolveWorktreeHostPath('/home/me/repo/feature', { platform: 'linux', wslDistro: 'Ubuntu' })
|
||||
).toBe('/home/me/repo/feature')
|
||||
expect(
|
||||
resolveWorktreeHostPath('/mnt/c/repo/feature', { platform: 'darwin', wslDistro: 'Ubuntu' })
|
||||
).toBe('/mnt/c/repo/feature')
|
||||
})
|
||||
|
||||
// `//x` is a host UNC spelling, not a guest path: translating it would prepend a second share.
|
||||
// A relative path is deliberately absent: the wrapper resolves one against the cwd.
|
||||
it.each([
|
||||
String.raw`C:\Users\me\repo\feature`,
|
||||
String.raw`\\wsl.localhost\Ubuntu\home\me\repo\feature`,
|
||||
'//wsl.localhost/Ubuntu/home/me/repo/feature'
|
||||
])('leaves a non-guest worktree path untouched: %j', (worktreePath) => {
|
||||
expect(resolveWorktreeHostPath(worktreePath, { platform: 'win32', wslDistro: 'Ubuntu' })).toBe(
|
||||
worktreePath
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { posix, win32 } from 'node:path'
|
||||
import { parseWslUncPath, toWindowsWslDrivePath, toWindowsWslPath } from './wsl-paths'
|
||||
|
||||
// Why the single-leading-slash rule: only `/x` is a guest-namespace path. `//x` is already a host
|
||||
// UNC spelling that Win32 opens as-is, and translating it would prepend a second share prefix.
|
||||
// Same guard `resolveWslRepoWorktreeBasePath` uses for the same ambiguity.
|
||||
const GUEST_ROOTED_PATH = /^\/(?!\/)/
|
||||
|
||||
export type GitMetadataPathOptions = {
|
||||
/** Host that reads the pointer back. Defaults to the current process platform. */
|
||||
platform?: NodeJS.Platform
|
||||
@@ -26,7 +31,7 @@ export function resolveGitMetadataPath(
|
||||
if (!value) {
|
||||
return null
|
||||
}
|
||||
if (value.startsWith('/')) {
|
||||
if (GUEST_ROOTED_PATH.test(value)) {
|
||||
const translated = translateGuestPointer(value, basePath, platform, options.wslDistro)
|
||||
if (translated) {
|
||||
return translated
|
||||
@@ -61,7 +66,9 @@ function translateGuestPointer(
|
||||
}
|
||||
|
||||
/**
|
||||
* The reading host's spelling of a worktree *directory*.
|
||||
* The reading host's spelling of a worktree *directory*. Git inside WSL answers in the guest
|
||||
* namespace, so a Windows host reopening one of those paths needs the drvfs drive or the distro's
|
||||
* UNC share.
|
||||
*
|
||||
* A directory is not a pointer: `resolveGitMetadataPath` trims because a gitfile payload carries a
|
||||
* trailing newline, but a directory name may legally begin or end with whitespace on POSIX. Keep
|
||||
|
||||
Reference in New Issue
Block a user