mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
fix(wsl): resolve sparse-checkout and diff-stamp gitdirs through the caller's distro (#17932)
Two main-process `resolveGitDir` call sites dropped the WSL distro their caller
already held, so they could only resolve a gitdir pointer whose spelling carries
its own translation: a `//wsl.localhost/<Distro>/...` base, or a `/mnt/<letter>`
drvfs pointer that maps to a drive letter on its own.
The layout that needs the third case is a repo inside the distro's filesystem
with its worktrees on the Windows drive. `git worktree list` reports the worktree
as `/mnt/c/wt/x`, which the listing translates to `C:\wt\x` — a base that no
longer names a distro — while the `.git` gitfile beside it points at
`/home/me/repo/.git/worktrees/x`, which has no drive to derive. Win32 then treats
that pointer as absolute and reads a path that names nothing:
- `detectSparseCheckout` stats `info/sparse-checkout` under the fabricated path,
always misses, and reports the worktree as non-sparse — no sparse badge, and
the file list claims files that are not on disk.
- `readWorktreeDiffStamp` reads HEAD under the same path, gets nothing, and
returns null. Null is the safe answer ("cannot prove unchanged"), but it
retires the settled-diff cache for every file in that worktree, so each diff
respawns Git.
Both callers already have the distro: the listing threads its
`GitWorktreeExecOptions` to `annotateSparseCheckoutStatus`, on through
`detectSparseCheckoutCached` (the annotation cache added by #17859) and its
background revalidation probe, and finally to `detectSparseCheckout`; and
`file-diff` already forwards its `GitRuntimeOptions` to `readWorktreeDiffStamp`,
which now forwards it to `resolveGitDir` as well.
`resolveGitMetadataPath` still prefers a UNC base's distro and still tries drvfs
before the caller-named distro, so nothing that resolved before resolves
differently.
The cache hop matters twice over. It is the only remaining caller of
`detectSparseCheckout`, so without threading it the fix would not reach the
probe at all. And the cache is where the bug turns sticky. #17859 keyed entries
on `repoPath` + `worktreePath` alone, on the reasoning that the distro is a
property of the repo and so every read for a given `repoPath` carries the same
one. That invariant does not hold. `listRepoWorktrees(repo)` is called with no
options at all from the filesystem-auth root rebuild
(`registered-worktree-roots-cache.ts`, reached from `ensureAuthorizedRootsCache`
on any auth check with a dirty cache) and from the local worktree-ownership
check in `filesystem-worktree-helpers.ts`. Both land on the *same* key as the
distro-carrying listing, because `translateWslOutputPaths` derives the distro
from the cwd spelling before falling back to `options.wslDistro`, so a
UNC-spelled repo path yields the identical `C:\...` worktree row either way.
Measured on Windows in one process, branch build: a distro-less read followed by
a distro-carrying read reported the sparse worktree as non-sparse both times.
So `wslDistro` now joins the cache key -- trimmed and lowercased, matching how
the rest of the codebase compares distro names, and appended last so the
repo-scoped prefix delete still matches every variant. The per-path invalidate
becomes a prefix delete for the same reason, dropping every distro variant of a
removed or moved worktree.
Keying on it closes both halves of the defect. A correct caller can no longer be
served an answer derived without the distro it supplied. And because the entry a
reader reaches is now selected by the same distro it would re-probe with,
`revalidateInBackground` can no longer re-derive a warm entry under weaker
options -- which mattered on its own: a distro-less reader crossing the
five-minute window would otherwise flip a correct `true` to `false`, and the
resulting change notification runs the registered invalidator, clearing the
whole repo's cache and re-probing every worktree cold, on a five-minute loop.
Cost of the extra key dimension is bounded by the number of distinct distros a
given repo is actually read under: one where a distro is threaded everywhere,
two while the distro-less callers above still exist. Entries are still
repo-scoped, and both clears already sweep by prefix.
Per-platform delta:
- macOS/Linux: no change. Guest-pointer translation is gated to win32 and a
caller-named distro is ignored off Windows; no caller supplies one there, so
the cache keys and probes exactly as before.
- native Windows, no WSL: no change. `wslDistro` is undefined, so the resolver
takes exactly the branches it took before and every read keys on the same
empty distro component, so the cache behaves exactly as it did.
- Windows + WSL, UNC-spelled worktree: no change. The base already names the
distro and outranks the caller's.
- Windows + WSL, drvfs-spelled worktree with a drvfs pointer: no change. The
drive-letter derivation still runs first.
- Windows + WSL, drvfs-spelled worktree with a non-drvfs pointer: the sparse
badge appears and the settled-diff cache starts hitting. Both previously
failed toward "not sparse" / "do not cache", so neither can now serve a stale
answer, and the distro-less listings no longer share the badge's cache entry.
- SSH/relay: none. Those paths return through the provider branch before
reaching either function.
- folder workspaces, GitLab: none. Neither is on these code paths.
Not in this change:
- `readRepoCommonDirFromDisk` (worktree-listing). Passing the distro there is
inert: a repo root's `.git` is a directory, so the gitfile-pointer branch never
runs, and when `repoPath` itself is guest-spelled the preceding `stat` already
fails — which no `resolveGitDir` option can fix.
- The two `findExistingWorktreeSymlinkPaths` calls on the removal paths. Both
receive `registeredWorktree.path` from `listWorktreesStrict`, which already
translates every row out of the guest namespace, so the distro would be a
no-op. The `removeWorktreeLinkedPaths` unlink beside them is untranslated too,
so a half-threaded fix would only move the refusal from Orca's preflight to
`git worktree remove`.
- An absolute `commondir` payload, which `resolveGitCommonDir` still resolves
untranslated. Git writes that file relative in the layouts above, and the
failure direction is unchanged.
- Giving the two distro-less `listRepoWorktrees(repo)` callers a distro. Neither
reads `isSparse` -- both use only `worktree.path` -- so the distro would buy
them nothing they consume, while resolving a project runtime inside the
filesystem-auth rebuild would put a call that throws on `repair-required`
behind a catch that skips the whole repo's authorized roots. The cache key
makes their reads harmless; skipping the annotation for callers that never
read it is a separate, larger change. The third no-options call in
`hosted-review.ts` is inside the `repo.connectionId` branch and returns
through the SSH provider, so it never reaches this cache.
This commit is contained in:
@@ -81,7 +81,9 @@ export async function readWorktreeDiffStamp(
|
||||
// Only an empty worktree path lands here, and nothing about it is provably unchanged.
|
||||
return null
|
||||
}
|
||||
const gitDir = await resolveGitDir(hostWorktreePath)
|
||||
// Why still pass options: the host spelling above only encodes the distro when it lands on a
|
||||
// UNC share, so a drvfs-spelled worktree needs it again to resolve a non-drvfs gitdir pointer.
|
||||
const gitDir = await resolveGitDir(hostWorktreePath, options)
|
||||
const [head, index, gitmodules, workingTree] = await Promise.all([
|
||||
readHeadComponent(gitDir),
|
||||
// Over-invalidates on purpose: git run outside Orca (a terminal `git status`/`git add`)
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { readFileMock, statMock } = vi.hoisted(() => ({
|
||||
readFileMock: vi.fn(),
|
||||
statMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node: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' })
|
||||
|
||||
// A worktree on the Windows drive whose repo lives in the distro's own filesystem: the worktree
|
||||
// resolves to a drive letter, so the gitdir pointer beside it — which is not drvfs — has no
|
||||
// drive to derive and needs the distro the diff read already carries.
|
||||
const HOST_WORKTREE = 'C:/wt/x'
|
||||
const GUEST_GIT_DIR = '/home/me/repo/.git/worktrees/x'
|
||||
const HOST_GIT_DIR = '//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/x'
|
||||
|
||||
describe('readWorktreeDiffStamp with a non-drvfs gitdir pointer', () => {
|
||||
beforeEach(() => {
|
||||
readFileMock.mockReset()
|
||||
statMock.mockReset()
|
||||
readFileMock.mockImplementation(async (target: string) => {
|
||||
const value = slashed(target)
|
||||
if (value === `${HOST_WORKTREE}/.git`) {
|
||||
return `gitdir: ${GUEST_GIT_DIR}\n`
|
||||
}
|
||||
// Detached HEAD, so the stamp needs no ref-store walk.
|
||||
if (value === `${HOST_GIT_DIR}/HEAD`) {
|
||||
return `${'a'.repeat(40)}\n`
|
||||
}
|
||||
throw missing()
|
||||
})
|
||||
statMock.mockImplementation(async (target: string) =>
|
||||
slashed(target) === `${HOST_WORKTREE}/src/a.ts`
|
||||
? { mtimeMs: 1_000, size: 12, ino: 7 }
|
||||
: Promise.reject(missing())
|
||||
)
|
||||
})
|
||||
|
||||
it('resolves the gitdir through the caller-named distro so the diff stays cacheable', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
|
||||
try {
|
||||
const stamp = await readWorktreeDiffStamp('/mnt/c/wt/x', 'src/a.ts', true, {
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
|
||||
// Null here means "cannot prove unchanged", which is what an unreadable HEAD produces —
|
||||
// correct, but it retires the settled-diff cache for every file in the worktree.
|
||||
expect(stamp).not.toBeNull()
|
||||
expect(stamp?.newestMtimeMs).toBe(1_000)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -243,7 +243,13 @@ describe('listWorktrees', () => {
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
expect(resolveGitDirMock).toHaveBeenCalledWith(featureWorktreePath)
|
||||
// The second argument is what carries the distro when the caller has one; this listing has
|
||||
// none, and the UNC repo path names the distro on its own.
|
||||
const gitDirCall = resolveGitDirMock.mock.calls.find(
|
||||
([probed]) => probed === featureWorktreePath
|
||||
)
|
||||
expect(gitDirCall).toBeDefined()
|
||||
expect(gitDirCall?.[1]?.wslDistro).toBeUndefined()
|
||||
// Why: the detection path must not spawn a git subprocess per worktree —
|
||||
// the perf regression in #1131 came from `git sparse-checkout list` firing
|
||||
// on every poll.
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const {
|
||||
readCheckedOutBranchRefMock,
|
||||
readFileMock,
|
||||
readRepoCommonDirFromGitMock,
|
||||
readRepoLocationMock,
|
||||
readWorktreeHeadOidMock,
|
||||
realpathMock,
|
||||
statMock
|
||||
} = vi.hoisted(() => ({
|
||||
readCheckedOutBranchRefMock: vi.fn(),
|
||||
readFileMock: vi.fn(),
|
||||
readRepoCommonDirFromGitMock: vi.fn(),
|
||||
readRepoLocationMock: vi.fn(),
|
||||
readWorktreeHeadOidMock: vi.fn(),
|
||||
realpathMock: vi.fn(),
|
||||
statMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node:fs/promises', () => ({
|
||||
readFile: readFileMock,
|
||||
realpath: realpathMock,
|
||||
stat: statMock
|
||||
}))
|
||||
// Only Git is stubbed; the sparse probe below runs for real so the distro has somewhere to matter.
|
||||
vi.mock('./worktree-list-reader', () => ({
|
||||
readCheckedOutBranchRef: readCheckedOutBranchRefMock,
|
||||
readRepoCommonDirFromGit: readRepoCommonDirFromGitMock,
|
||||
readRepoLocation: readRepoLocationMock,
|
||||
readTranslatedWorktreeGraph: vi.fn(),
|
||||
readWorktreeHeadOid: readWorktreeHeadOidMock,
|
||||
readWorktreeList: vi.fn()
|
||||
}))
|
||||
|
||||
import { describeCreatedWorktree } from './worktree-listing'
|
||||
|
||||
const slashed = (value: unknown): string => String(value).replaceAll('\\', '/')
|
||||
const missing = () => Object.assign(new Error('ENOENT'), { code: 'ENOENT' })
|
||||
|
||||
// The layout that needs the caller's distro: the repo lives in the distro, its worktrees on the
|
||||
// Windows drive. `git worktree list` reports `/mnt/c/wt/x`, which translates to a drive letter that
|
||||
// no longer names a distro, and the gitfile beside it points at a guest path with no drive to
|
||||
// derive — so only the distro the create ran under can resolve it.
|
||||
const REPO = '\\\\wsl.localhost\\Ubuntu\\home\\me\\repo'
|
||||
const GUEST_WORKTREE = '/mnt/c/wt/x'
|
||||
const HOST_WORKTREE = 'C:/wt/x'
|
||||
const GUEST_GIT_DIR = '/home/me/repo/.git/worktrees/x'
|
||||
const HOST_GIT_DIR = '//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/x'
|
||||
const HEAD_OID = 'a'.repeat(40)
|
||||
|
||||
describe('describeCreatedWorktree on a drvfs-spelled WSL worktree', () => {
|
||||
beforeEach(() => {
|
||||
readRepoLocationMock.mockReset()
|
||||
readRepoLocationMock.mockResolvedValue({
|
||||
topLevel: GUEST_WORKTREE,
|
||||
commonDir: '/home/me/repo/.git'
|
||||
})
|
||||
readRepoCommonDirFromGitMock.mockReset()
|
||||
readRepoCommonDirFromGitMock.mockResolvedValue('/home/me/repo/.git')
|
||||
readCheckedOutBranchRefMock.mockReset()
|
||||
readCheckedOutBranchRefMock.mockResolvedValue('refs/heads/feature')
|
||||
readWorktreeHeadOidMock.mockReset()
|
||||
readWorktreeHeadOidMock.mockResolvedValue(HEAD_OID)
|
||||
realpathMock.mockReset()
|
||||
realpathMock.mockRejectedValue(missing())
|
||||
readFileMock.mockReset()
|
||||
readFileMock.mockImplementation(async (target: string) => {
|
||||
const value = slashed(target)
|
||||
if (value === `${HOST_WORKTREE}/.git`) {
|
||||
return `gitdir: ${GUEST_GIT_DIR}\n`
|
||||
}
|
||||
// No `commondir`, so the gitdir is its own common dir and the config read stays in one
|
||||
// namespace — the pointer resolve is the only thing under test.
|
||||
if (value === `${HOST_GIT_DIR}/config`) {
|
||||
return '[core]\n\tsparseCheckout = true\n'
|
||||
}
|
||||
throw missing()
|
||||
})
|
||||
statMock.mockReset()
|
||||
statMock.mockImplementation(async (target: string) =>
|
||||
slashed(target) === `${HOST_GIT_DIR}/info/sparse-checkout`
|
||||
? { isFile: () => true, size: 12 }
|
||||
: Promise.reject(missing())
|
||||
)
|
||||
})
|
||||
|
||||
it('marks the recovered row sparse through the caller-named distro', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
|
||||
try {
|
||||
const described = await describeCreatedWorktree(REPO, 'C:\\wt\\x', 'feature', {
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
|
||||
expect(described?.path && slashed(described.path)).toBe(HOST_WORKTREE)
|
||||
expect(described?.isSparse).toBe(true)
|
||||
expect(statMock.mock.calls.map(([target]) => slashed(target))).toContain(
|
||||
`${HOST_GIT_DIR}/info/sparse-checkout`
|
||||
)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,68 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitWorktreeInfo } from '../../shared/worktree/types'
|
||||
|
||||
const { detectSparseCheckoutMock, readWorktreeListMock, readTranslatedWorktreeGraphMock } =
|
||||
vi.hoisted(() => ({
|
||||
detectSparseCheckoutMock: vi.fn(),
|
||||
readWorktreeListMock: vi.fn(),
|
||||
readTranslatedWorktreeGraphMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('./worktree-sparse-state', () => ({
|
||||
detectSparseCheckout: detectSparseCheckoutMock,
|
||||
resolveGitCommonDir: vi.fn()
|
||||
}))
|
||||
vi.mock('./worktree-list-reader', () => ({
|
||||
readCheckedOutBranchRef: vi.fn(),
|
||||
readRepoCommonDirFromGit: vi.fn(),
|
||||
readRepoLocation: vi.fn(),
|
||||
readTranslatedWorktreeGraph: readTranslatedWorktreeGraphMock,
|
||||
readWorktreeHeadOid: vi.fn(),
|
||||
readWorktreeList: readWorktreeListMock
|
||||
}))
|
||||
|
||||
import { __resetSparseCheckoutStateCacheForTests } from './worktree-sparse-checkout-cache'
|
||||
import { listWorktreesStrict, listWorktreesUnshared } from './worktree-listing'
|
||||
|
||||
// A WSL repo's sparse probe is pure `fs`, so it only reaches the right namespace if the listing
|
||||
// hands it the distro the git call already ran under.
|
||||
const ROW: GitWorktreeInfo = {
|
||||
path: 'C:\\wt\\x',
|
||||
head: 'a'.repeat(40),
|
||||
branch: 'refs/heads/feature',
|
||||
isBare: false,
|
||||
isMainWorktree: false
|
||||
}
|
||||
|
||||
describe('worktree listing sparse annotation', () => {
|
||||
beforeEach(() => {
|
||||
detectSparseCheckoutMock.mockReset()
|
||||
detectSparseCheckoutMock.mockResolvedValue(false)
|
||||
readWorktreeListMock.mockReset()
|
||||
readWorktreeListMock.mockResolvedValue([ROW])
|
||||
readTranslatedWorktreeGraphMock.mockReset()
|
||||
readTranslatedWorktreeGraphMock.mockResolvedValue([ROW])
|
||||
// The listing now reads through a repo-scoped cache; a warm entry would skip the probe.
|
||||
__resetSparseCheckoutStateCacheForTests()
|
||||
})
|
||||
|
||||
it('passes the listing distro to the strict-list sparse probe', async () => {
|
||||
await listWorktreesStrict('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo', { wslDistro: 'Ubuntu' })
|
||||
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledWith(
|
||||
'C:\\wt\\x',
|
||||
expect.objectContaining({ wslDistro: 'Ubuntu' })
|
||||
)
|
||||
})
|
||||
|
||||
it('passes the listing distro to the unshared-list sparse probe', async () => {
|
||||
await listWorktreesUnshared('\\\\wsl.localhost\\Ubuntu\\home\\me\\repo', {
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledWith(
|
||||
'C:\\wt\\x',
|
||||
expect.objectContaining({ wslDistro: 'Ubuntu' })
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -62,7 +62,7 @@ export async function listWorktreesUnshared(
|
||||
const visibleWorktrees = options.includeCreatePreparations
|
||||
? worktrees
|
||||
: worktrees.filter((worktree) => !isWorktreeCreatePreparation(worktree))
|
||||
return annotateSparseCheckoutStatus(repoPath, visibleWorktrees)
|
||||
return annotateSparseCheckoutStatus(repoPath, visibleWorktrees, options)
|
||||
} catch (err) {
|
||||
if (getErrorCode(err) === 'ENOENT') {
|
||||
try {
|
||||
@@ -94,12 +94,13 @@ export async function listWorktreesStrict(
|
||||
const visibleWorktrees = options.includeCreatePreparations
|
||||
? worktrees
|
||||
: worktrees.filter((worktree) => !isWorktreeCreatePreparation(worktree))
|
||||
return annotateSparseCheckoutStatus(repoPath, visibleWorktrees)
|
||||
return annotateSparseCheckoutStatus(repoPath, visibleWorktrees, options)
|
||||
}
|
||||
|
||||
async function annotateSparseCheckoutStatus(
|
||||
repoPath: string,
|
||||
worktrees: GitWorktreeInfo[]
|
||||
worktrees: GitWorktreeInfo[],
|
||||
options: GitWorktreeExecOptions = {}
|
||||
): Promise<GitWorktreeInfo[]> {
|
||||
const annotated = [...worktrees]
|
||||
let nextIndex = 0
|
||||
@@ -112,7 +113,7 @@ async function annotateSparseCheckoutStatus(
|
||||
if (!worktree || worktree.isBare || worktree.isSparse) {
|
||||
continue
|
||||
}
|
||||
const isSparse = await detectSparseCheckoutCached(repoPath, worktree.path)
|
||||
const isSparse = await detectSparseCheckoutCached(repoPath, worktree.path, options)
|
||||
if (isSparse) {
|
||||
annotated[index] = { ...worktree, isSparse }
|
||||
}
|
||||
@@ -255,15 +256,19 @@ export async function describeCreatedWorktree(
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
const [described] = await annotateSparseCheckoutStatus(repoPath, [
|
||||
{
|
||||
path: translateWorktreePath(created.topLevel, repoPath, options),
|
||||
head,
|
||||
branch: expectedRef,
|
||||
isBare: false,
|
||||
// `git worktree add` only ever produces a linked worktree.
|
||||
isMainWorktree: false
|
||||
}
|
||||
])
|
||||
const [described] = await annotateSparseCheckoutStatus(
|
||||
repoPath,
|
||||
[
|
||||
{
|
||||
path: translateWorktreePath(created.topLevel, repoPath, options),
|
||||
head,
|
||||
branch: expectedRef,
|
||||
isBare: false,
|
||||
// `git worktree add` only ever produces a linked worktree.
|
||||
isMainWorktree: false
|
||||
}
|
||||
],
|
||||
options
|
||||
)
|
||||
return described
|
||||
}
|
||||
|
||||
@@ -217,7 +217,7 @@ describe('invalidateSparseCheckoutState', () => {
|
||||
await detectSparseCheckoutCached('/repo', '/repo/wt-a')
|
||||
await detectSparseCheckoutCached('/repo', '/repo/wt-b')
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledTimes(1)
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledWith('/repo/wt-a')
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledWith('/repo/wt-a', {})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -250,3 +250,107 @@ describe('clearSparseCheckoutStateCache', () => {
|
||||
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
// Regression coverage for the live Windows+WSL sequence: a distro-less listing (filesystem-auth
|
||||
// root rebuild, worktree ownership checks) racing the real distro-carrying listing for the same
|
||||
// repo. Before the distro joined the cache key they shared one entry, so whichever ran first
|
||||
// decided the sparse badge for the whole reconcile window.
|
||||
describe('detectSparseCheckoutCached with a WSL distro', () => {
|
||||
// Mirrors the real probe: without the distro the gitdir pointer resolves to a fabricated Win32
|
||||
// path, the sparse-checkout stat misses, and the worktree reads as non-sparse.
|
||||
function detectOnlyWithDistro(distro: string): void {
|
||||
detectSparseCheckoutMock.mockImplementation(
|
||||
async (_worktreePath: string, options?: { wslDistro?: string }) =>
|
||||
options?.wslDistro === distro
|
||||
)
|
||||
}
|
||||
|
||||
it('does not serve a distro-carrying read an answer derived without that distro', async () => {
|
||||
detectOnlyWithDistro('Ubuntu')
|
||||
|
||||
expect(await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt')).toBe(false)
|
||||
expect(
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
).toBe(true)
|
||||
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('keeps a distro-carrying answer correct when a distro-less read follows it', async () => {
|
||||
detectOnlyWithDistro('Ubuntu')
|
||||
|
||||
expect(
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
).toBe(true)
|
||||
expect(await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt')).toBe(false)
|
||||
expect(
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
).toBe(true)
|
||||
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('treats distro spellings that name the same distro as one entry', async () => {
|
||||
detectSparseCheckoutMock.mockResolvedValue(true)
|
||||
|
||||
expect(
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
).toBe(true)
|
||||
expect(
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: ' ubuntu ' })
|
||||
).toBe(true)
|
||||
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not let a distro-less reader past the window revalidate a distro-carrying entry', async () => {
|
||||
const listener = vi.fn()
|
||||
onSparseCheckoutStateChanged(listener)
|
||||
const nowSpy = vi.spyOn(Date, 'now')
|
||||
try {
|
||||
detectOnlyWithDistro('Ubuntu')
|
||||
nowSpy.mockReturnValue(1_000)
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt')
|
||||
|
||||
// Past the window the distro-less caller re-probes its own entry, not the sparse one, so the
|
||||
// badge cannot blink off and fire the change listener that clears the whole repo's cache.
|
||||
nowSpy.mockReturnValue(1_000 + RECONCILE_WINDOW_MS + 1)
|
||||
expect(await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt')).toBe(false)
|
||||
await flushBackgroundRevalidation()
|
||||
|
||||
expect(listener).not.toHaveBeenCalled()
|
||||
expect(
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
).toBe(true)
|
||||
} finally {
|
||||
nowSpy.mockRestore()
|
||||
onSparseCheckoutStateChanged(undefined)
|
||||
}
|
||||
})
|
||||
|
||||
it('drops every distro variant of a path on invalidate, so a removed worktree leaves nothing behind', async () => {
|
||||
detectSparseCheckoutMock.mockResolvedValue(true)
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt')
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\wt', { wslDistro: 'Ubuntu' })
|
||||
await detectSparseCheckoutCached('C:\\repo', 'C:\\repo\\other')
|
||||
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(3)
|
||||
|
||||
invalidateSparseCheckoutState('C:\\repo', 'C:\\repo\\wt')
|
||||
|
||||
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(1)
|
||||
})
|
||||
|
||||
it('still caches normally with no distro anywhere, as on macOS/Linux and native Windows', async () => {
|
||||
detectSparseCheckoutMock.mockResolvedValue(true)
|
||||
|
||||
expect(await detectSparseCheckoutCached('/repo', '/repo/wt-a')).toBe(true)
|
||||
expect(await detectSparseCheckoutCached('/repo', '/repo/wt-a', {})).toBe(true)
|
||||
expect(await detectSparseCheckoutCached('/repo', '/repo/wt-a', { wslDistro: undefined })).toBe(
|
||||
true
|
||||
)
|
||||
|
||||
expect(detectSparseCheckoutMock).toHaveBeenCalledTimes(1)
|
||||
expect(__getSparseCheckoutStateCacheSizeForTests()).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import type { GitRuntimeOptions } from './git-runtime-options'
|
||||
import { canonicalWorktreePath } from './worktree-path-comparison'
|
||||
import { detectSparseCheckout } from './worktree-sparse-state'
|
||||
|
||||
@@ -24,6 +25,15 @@ import { detectSparseCheckout } from './worktree-sparse-state'
|
||||
// - App cold start: the map starts empty, so the first read is always a fresh detect.
|
||||
const SPARSE_CHECKOUT_CACHE_RECONCILE_INTERVAL_MS = 5 * 60_000
|
||||
|
||||
// Part of the cache key, not just a probe argument. A distro-less read of a WSL-hosted repo
|
||||
// resolves the gitdir pointer against a fabricated Win32 path and reports "not sparse"; several
|
||||
// callers (filesystem-auth root rebuild, worktree ownership checks) list a repo with no options at
|
||||
// all and would otherwise publish that wrong answer onto the entry the distro-carrying listing
|
||||
// reads. Keying on it also pins each entry's revalidation to the options that produced it, so the
|
||||
// background probe can never re-derive a warm entry under weaker options and flip it. Every field
|
||||
// here must be in the key; widening this type means widening `cacheKey`.
|
||||
type SparseCheckoutProbeOptions = Pick<GitRuntimeOptions, 'wslDistro'>
|
||||
|
||||
type SparseCheckoutCacheEntry = {
|
||||
isSparse: boolean
|
||||
cachedAt: number
|
||||
@@ -39,8 +49,25 @@ export type SparseCheckoutChangeListener = (
|
||||
const sparseCheckoutStateCache = new Map<string, SparseCheckoutCacheEntry>()
|
||||
let changeListener: SparseCheckoutChangeListener | undefined
|
||||
|
||||
function cacheKey(repoPath: string, worktreePath: string): string {
|
||||
return `${canonicalWorktreePath(repoPath)}\0${canonicalWorktreePath(worktreePath)}`
|
||||
// Distro last so the repo- and worktree-scoped prefix deletes below still match every variant.
|
||||
function cacheKey(
|
||||
repoPath: string,
|
||||
worktreePath: string,
|
||||
options: SparseCheckoutProbeOptions
|
||||
): string {
|
||||
return `${worktreeKeyPrefix(repoPath, worktreePath)}${options.wslDistro?.trim().toLowerCase() ?? ''}`
|
||||
}
|
||||
|
||||
function worktreeKeyPrefix(repoPath: string, worktreePath: string): string {
|
||||
return `${canonicalWorktreePath(repoPath)}\0${canonicalWorktreePath(worktreePath)}\0`
|
||||
}
|
||||
|
||||
function deleteKeysWithPrefix(prefix: string): void {
|
||||
for (const key of sparseCheckoutStateCache.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
sparseCheckoutStateCache.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Wired by the ipc/ layer to the shared worktrees-changed notification; last registration wins. */
|
||||
@@ -53,12 +80,13 @@ export function onSparseCheckoutStateChanged(
|
||||
/** Cached wrapper around {@link detectSparseCheckout}; see module doc for invalidation coverage. */
|
||||
export async function detectSparseCheckoutCached(
|
||||
repoPath: string,
|
||||
worktreePath: string
|
||||
worktreePath: string,
|
||||
options: SparseCheckoutProbeOptions = {}
|
||||
): Promise<boolean> {
|
||||
const key = cacheKey(repoPath, worktreePath)
|
||||
const key = cacheKey(repoPath, worktreePath, options)
|
||||
const cached = sparseCheckoutStateCache.get(key)
|
||||
if (!cached) {
|
||||
const isSparse = await detectSparseCheckout(worktreePath)
|
||||
const isSparse = await detectSparseCheckout(worktreePath, options)
|
||||
sparseCheckoutStateCache.set(key, { isSparse, cachedAt: Date.now() })
|
||||
return isSparse
|
||||
}
|
||||
@@ -66,8 +94,10 @@ export async function detectSparseCheckoutCached(
|
||||
return cached.isSparse
|
||||
}
|
||||
// Stale-while-revalidate: serve the still-cached value now and correct it in the background,
|
||||
// deduplicated so concurrent readers past the window don't each start their own probe.
|
||||
cached.revalidating ??= revalidateInBackground(key, repoPath, worktreePath, cached)
|
||||
// deduplicated so concurrent readers past the window don't each start their own probe. Whichever
|
||||
// reader wins the dedupe re-probes with the entry's own distro, because that distro is what
|
||||
// routed it to this key.
|
||||
cached.revalidating ??= revalidateInBackground(key, repoPath, worktreePath, cached, options)
|
||||
return cached.isSparse
|
||||
}
|
||||
|
||||
@@ -75,10 +105,11 @@ async function revalidateInBackground(
|
||||
key: string,
|
||||
repoPath: string,
|
||||
worktreePath: string,
|
||||
startingEntry: SparseCheckoutCacheEntry
|
||||
startingEntry: SparseCheckoutCacheEntry,
|
||||
options: SparseCheckoutProbeOptions
|
||||
): Promise<void> {
|
||||
try {
|
||||
const isSparse = await detectSparseCheckout(worktreePath)
|
||||
const isSparse = await detectSparseCheckout(worktreePath, options)
|
||||
// Identity guard against a race with an explicit invalidate/clear -- or a remove+recreate at
|
||||
// the same path that repopulates the key with a fresh cold read -- while this was in flight.
|
||||
// A `has()`/presence check can't tell "still mine" from "someone else's fresh value" sharing
|
||||
@@ -99,17 +130,12 @@ async function revalidateInBackground(
|
||||
|
||||
/** Drop one worktree's cached state; call when Orca itself removes or moves a worktree path. */
|
||||
export function invalidateSparseCheckoutState(repoPath: string, worktreePath: string): void {
|
||||
sparseCheckoutStateCache.delete(cacheKey(repoPath, worktreePath))
|
||||
deleteKeysWithPrefix(worktreeKeyPrefix(repoPath, worktreePath))
|
||||
}
|
||||
|
||||
/** Clear one repo's cached entries; wired to the shared worktree-change invalidator registry in ipc/. */
|
||||
export function clearSparseCheckoutStateCacheForRepo(repoPath: string): void {
|
||||
const prefix = `${canonicalWorktreePath(repoPath)}\0`
|
||||
for (const key of sparseCheckoutStateCache.keys()) {
|
||||
if (key.startsWith(prefix)) {
|
||||
sparseCheckoutStateCache.delete(key)
|
||||
}
|
||||
}
|
||||
deleteKeysWithPrefix(`${canonicalWorktreePath(repoPath)}\0`)
|
||||
}
|
||||
|
||||
/** Clear every cached entry; fallback for a change notification whose repo can't be resolved to a path. */
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { readFileMock, statMock } = vi.hoisted(() => ({
|
||||
readFileMock: vi.fn(),
|
||||
statMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node:fs/promises', () => ({ readFile: readFileMock, stat: statMock }))
|
||||
|
||||
import { detectSparseCheckout } from './worktree-sparse-state'
|
||||
|
||||
const slashed = (value: unknown): string => String(value).replaceAll('\\', '/')
|
||||
const missing = () => Object.assign(new Error('ENOENT'), { code: 'ENOENT' })
|
||||
|
||||
// The layout that needs the caller's distro: `git worktree list` reports a drvfs worktree, which
|
||||
// translates to a drive letter, so the base path no longer names the distro that wrote the gitdir
|
||||
// pointer — and that pointer is not itself drvfs, so there is nothing to derive a drive from.
|
||||
const HOST_WORKTREE = 'C:/wt/x'
|
||||
const GUEST_GIT_DIR = '/home/me/repo/.git/worktrees/x'
|
||||
const HOST_GIT_DIR = '//wsl.localhost/Ubuntu/home/me/repo/.git/worktrees/x'
|
||||
|
||||
describe('detectSparseCheckout on a drvfs-spelled WSL worktree', () => {
|
||||
beforeEach(() => {
|
||||
readFileMock.mockReset()
|
||||
statMock.mockReset()
|
||||
readFileMock.mockImplementation(async (target: string) => {
|
||||
const value = slashed(target)
|
||||
if (value === `${HOST_WORKTREE}/.git`) {
|
||||
return `gitdir: ${GUEST_GIT_DIR}\n`
|
||||
}
|
||||
// No `commondir`, so the gitdir is its own common dir and the config read stays in one
|
||||
// namespace — the pointer resolve above is the only thing under test.
|
||||
if (value === `${HOST_GIT_DIR}/config`) {
|
||||
return '[core]\n\tsparseCheckout = true\n'
|
||||
}
|
||||
throw missing()
|
||||
})
|
||||
statMock.mockImplementation(async (target: string) =>
|
||||
slashed(target) === `${HOST_GIT_DIR}/info/sparse-checkout`
|
||||
? { isFile: () => true, size: 12 }
|
||||
: Promise.reject(missing())
|
||||
)
|
||||
})
|
||||
|
||||
it('reads the pattern file through the caller-named distro', async () => {
|
||||
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||
|
||||
try {
|
||||
await expect(detectSparseCheckout('/mnt/c/wt/x', { wslDistro: 'Ubuntu' })).resolves.toBe(true)
|
||||
expect(statMock.mock.calls.map(([target]) => slashed(target))).toContain(
|
||||
`${HOST_GIT_DIR}/info/sparse-checkout`
|
||||
)
|
||||
} finally {
|
||||
platformSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -1,12 +1,18 @@
|
||||
import { readFile, stat } from 'node:fs/promises'
|
||||
import { isAbsolute, join, resolve } from 'node:path'
|
||||
import type { GitRuntimeOptions } from './git-runtime-options'
|
||||
import { resolveGitDir } from './status'
|
||||
|
||||
export async function detectSparseCheckout(worktreePath: string): Promise<boolean> {
|
||||
export async function detectSparseCheckout(
|
||||
worktreePath: string,
|
||||
// Why: git in a WSL distro reports the worktree, and writes its gitdir pointer, in the guest
|
||||
// namespace; without the distro this stats a path Win32 fabricates and reads "not sparse".
|
||||
options: Pick<GitRuntimeOptions, 'wslDistro'> = {}
|
||||
): Promise<boolean> {
|
||||
// Why: fs.stat the per-worktree gitdir's sparse-checkout pattern file instead of a per-poll `git sparse-checkout list` subprocess that regressed responsiveness (PR #1290);
|
||||
// this is the cheap fast-path gate before the enabled check below.
|
||||
try {
|
||||
const gitDir = await resolveGitDir(worktreePath)
|
||||
const gitDir = await resolveGitDir(worktreePath, options)
|
||||
const stats = await stat(join(gitDir, 'info', 'sparse-checkout'))
|
||||
if (!stats.isFile() || stats.size === 0) {
|
||||
return false
|
||||
|
||||
Reference in New Issue
Block a user