mirror of
https://github.com/stablyai/orca.git
synced 2026-10-08 08:02:32 +00:00
Neil
26dfa46aa9
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.
69 lines
2.4 KiB
TypeScript
69 lines
2.4 KiB
TypeScript
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' })
|
|
)
|
|
})
|
|
})
|