diff --git a/src/main/git/source-control/status-read.ts b/src/main/git/source-control/status-read.ts index 276bf26e646..11779d8af3a 100644 --- a/src/main/git/source-control/status-read.ts +++ b/src/main/git/source-control/status-read.ts @@ -18,7 +18,10 @@ import { findExistingWorktreeSymlinkPaths } from '../worktree-symlink-detection' import type { GetStatusOptions } from './get-status-options' import { statusReadLeaseOwner } from './git-read-cache-invalidation' import { detectConflictOperation } from './git-conflict-operation' -import { parseUnmergedEntry } from '../../../shared/git-status-conflict-entries' +import { + parseUnmergedEntry, + resolveUnmergedStatusRecords +} from '../../../shared/git-status-conflict-entries' import { getEffectiveUpstreamStatusCacheKey } from './effective-upstream-status-cache' import { getShortBranchName, @@ -176,16 +179,37 @@ async function runGetStatus( // 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: a record only pushes one entry, so the cap can never break before index `limit`; prefetching + // exactly that prefix keeps the probe count identical to a serial read on a truncated status. + const resolvableUnmergedEnd = didHitLimit + ? Math.min(parser.statusRecords.length, limit) + : parser.statusRecords.length + // Why: skip the prefetch entirely for the conflict-free poll so the common path allocates nothing extra. + const resolvedUnmerged = + parser.unmergedLines.length > 0 + ? await resolveUnmergedStatusRecords( + hostWorktreePath, + parser.statusRecords, + resolvableUnmergedEnd + ) + : undefined + // 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. - for (const record of parser.statusRecords) { + for (const [index, record] of parser.statusRecords.entries()) { if (didHitLimit && entries.length >= limit) { break } if (record.type === 'entry') { entries.push(record.entry) } else { - const unmergedEntry = await parseUnmergedEntry(hostWorktreePath, record.line) + const prefetched = resolvedUnmerged?.[index] + if (prefetched?.ok === false) { + throw prefetched.error + } + const unmergedEntry = prefetched + ? prefetched.entry + : await parseUnmergedEntry(hostWorktreePath, record.line) if (unmergedEntry) { entries.push(unmergedEntry) } diff --git a/src/main/ipc/worktrees/listing/detected-provider-listing-meta-index.test.ts b/src/main/ipc/worktrees/listing/detected-provider-listing-meta-index.test.ts new file mode 100644 index 00000000000..4bf55e97492 --- /dev/null +++ b/src/main/ipc/worktrees/listing/detected-provider-listing-meta-index.test.ts @@ -0,0 +1,131 @@ +/** + * The SSH worktree-meta index is only ever read via `metaIndex.get(repo.id)` on the disconnected + * fallbacks, so a connected listing must not pay `parseWorktreeId` over the whole host snapshot. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/repo-types' +import type { Store } from '../../../persistence/loading-store/store' +import type * as SshWorktreeFallbackModule from './ssh-worktree-fallback' + +const { getSshGitProviderMock, indexBuildSpy } = vi.hoisted(() => ({ + getSshGitProviderMock: vi.fn(), + indexBuildSpy: vi.fn() +})) + +vi.mock('../../../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: getSshGitProviderMock, + requireSshGitProvider: getSshGitProviderMock, + getSshGitProviderGeneration: () => 1 +})) + +vi.mock('./ssh-worktree-fallback', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + // Both builders are counted: the point is that NO index is built on the connected path. + createSshWorktreeMetaIndex: (...args: Parameters) => { + indexBuildSpy('all-hosts', ...args) + return actual.createSshWorktreeMetaIndex(...args) + }, + createSshWorktreeMetaIndexForRepo: ( + ...args: Parameters + ) => { + indexBuildSpy('repo-scoped', ...args) + return actual.createSshWorktreeMetaIndexForRepo(...args) + } + } +}) + +const { listDetectedWorktreesForCapturedRepo } = await import('./detected-provider-listing') + +const repo = { + id: 'repo-1', + path: '/home/user/repo', + displayName: 'repo', + connectionId: 'conn-1' +} as Repo + +const worktreeId = `${repo.id}::/home/user/feature` + +function createStore(): Store { + const rows: Record = { + [worktreeId]: { instanceId: 'instance-1' }, + // Other repos' rows share the host snapshot; only this repo's bucket is ever read back. + 'repo-2::/home/user/other': { instanceId: 'instance-2' } + } + return { + getRepos: () => [repo], + getRepo: () => repo, + getSettings: () => ({}), + getProjectHostSetups: () => [], + getAllWorktreeLineage: () => ({}), + getAllWorktreeMeta: () => rows, + getWorktreeMeta: (id: string) => rows[id], + setWorktreeMeta: vi.fn() + } as unknown as Store +} + +describe('SSH worktree meta index construction', () => { + beforeEach(() => { + indexBuildSpy.mockClear() + getSshGitProviderMock.mockReset() + }) + + it('does not build the index when the provider answers', async () => { + const provider = { + listWorktrees: vi.fn().mockResolvedValue([ + { path: repo.path, head: 'a', branch: 'main', isBare: false, isMainWorktree: true }, + { + path: '/home/user/feature', + head: 'b', + branch: 'feature', + isBare: false, + isMainWorktree: false + } + ]) + } + + const result = await listDetectedWorktreesForCapturedRepo( + createStore(), + repo, + () => true, + provider as never + ) + + expect(result).toMatchObject({ authoritative: true, source: 'git' }) + expect(indexBuildSpy).not.toHaveBeenCalled() + }) + + it('builds the index once when no provider is available', async () => { + const result = await listDetectedWorktreesForCapturedRepo( + createStore(), + repo, + () => true, + undefined + ) + + expect(result).toMatchObject({ authoritative: false, source: 'metadata-fallback' }) + expect(indexBuildSpy).toHaveBeenCalledTimes(1) + expect(indexBuildSpy).toHaveBeenCalledWith('all-hosts', expect.anything()) + expect( + (result as { worktrees: { id: string }[] }).worktrees.map((worktree) => worktree.id) + ).toEqual([worktreeId]) + }) + + it('builds the index once when the provider listing fails', async () => { + const provider = { listWorktrees: vi.fn().mockRejectedValue(new Error('relay down')) } + + const result = await listDetectedWorktreesForCapturedRepo( + createStore(), + repo, + () => true, + provider as never + ) + + expect(result).toMatchObject({ authoritative: false, source: 'metadata-fallback' }) + expect(indexBuildSpy).toHaveBeenCalledTimes(1) + expect( + (result as { worktrees: { id: string }[] }).worktrees.map((worktree) => worktree.id) + ).toEqual([worktreeId]) + }) +}) diff --git a/src/main/ipc/worktrees/listing/detected-provider-listing.ts b/src/main/ipc/worktrees/listing/detected-provider-listing.ts index 51a0618ba66..25c08d262fb 100644 --- a/src/main/ipc/worktrees/listing/detected-provider-listing.ts +++ b/src/main/ipc/worktrees/listing/detected-provider-listing.ts @@ -10,7 +10,8 @@ import type { ListDesktopLineageForHostArgs } from '../../../../shared/host-line import { buildDetectedGitWorktrees, createSshWorktreeMetaIndex, - listDisconnectedSshWorktrees + listDisconnectedSshWorktrees, + type SshWorktreeMetaIndex } from './ssh-worktree-fallback' import { buildDisconnectedDetectedWorktrees, @@ -42,9 +43,11 @@ export async function listDetectedWorktreesForCapturedRepo( const allMeta = isFolderRepo(repo) ? undefined : readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) - const sshWorktreeMetaIndex = repo.connectionId - ? createSshWorktreeMetaIndex(Object.entries(allMeta ?? {})) - : new Map() + // Why: only the disconnected fallbacks read this, so keep parseWorktreeId over the whole host snapshot + // off the connected path entirely. + let cachedSshWorktreeMetaIndex: SshWorktreeMetaIndex | undefined + const sshWorktreeMetaIndex = (): SshWorktreeMetaIndex => + (cachedSshWorktreeMetaIndex ??= createSshWorktreeMetaIndex(Object.entries(allMeta ?? {}))) try { let gitWorktrees: GitWorktreeInfo[] @@ -86,7 +89,7 @@ export async function listDetectedWorktreesForCapturedRepo( if (!isCurrent()) { return null } - const worktrees = listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex) + const worktrees = listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex()) return { repoId: repo.id, authoritative: false, @@ -158,7 +161,7 @@ export async function listDetectedWorktreesForCapturedRepo( err ) if (repo.connectionId) { - const worktrees = listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex) + const worktrees = listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex()) return { repoId: repo.id, authoritative: false, diff --git a/src/main/ipc/worktrees/listing/detected-worktree-classification.test.ts b/src/main/ipc/worktrees/listing/detected-worktree-classification.test.ts new file mode 100644 index 00000000000..6a25d696c9b --- /dev/null +++ b/src/main/ipc/worktrees/listing/detected-worktree-classification.test.ts @@ -0,0 +1,234 @@ +/** + * Guards the single-classification contract of `buildDetectedGitWorktrees`: every visible worktree + * used to be run through `mergeWorktree` + `toDetectedWorktree` twice per catalog pass. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/repo-types' +import type { Store } from '../../../persistence/loading-store/store' +import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' +import type { GitWorktreeInfo } from '../../../../shared/worktree/types' +import type * as NodeCryptoModule from 'node:crypto' +import type * as OwnershipModule from '../../../../shared/worktree/ownership' + +const { toDetectedWorktreeSpy } = vi.hoisted(() => ({ toDetectedWorktreeSpy: vi.fn() })) + +vi.mock('../../../../shared/worktree/ownership', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + toDetectedWorktree: (args: Parameters[0]) => { + toDetectedWorktreeSpy(args) + return actual.toDetectedWorktree(args) + } + } +}) + +vi.mock('node:crypto', async (importOriginal) => ({ + ...(await importOriginal()), + randomUUID: () => 'fixed-instance-id' +})) + +const { buildDetectedGitWorktrees } = await import('./ssh-worktree-fallback') +const { getProjectHostSetupWorktreeMeta } = + await import('../../../../shared/project-host-setup-lookup') +const { mergeWorktree } = await import('../../worktree-logic') +const { resolveWorktreeMetaWithDiscoveryBackfill } = await import('./worktree-discovery-metadata') +const ownership = await import('../../../../shared/worktree/ownership') +const { projectResolvedWorktreeLineage } = + await import('../../../../shared/resolved-worktree-lineage') +const { createWorktreeVisibilitySourceMatcher, resolveCustomWorktreeVisibilitySources } = + await import('../../../../shared/worktree/visibility-sources') +const { resolveConfiguredWorktreeBasePaths } = + await import('../../../../shared/worktree/configured-worktree-base-path') +const { dedupeWorktreesByPath } = await import('../../worktree-path-comparison') +const { readWorktreeMetaForHost } = + await import('../../../persistence/host-qualified-worktree-meta') +const { getRepoOwnedWorktreeMeta } = await import('../../../worktree-metadata-ownership') +const { getRepoExecutionHostId } = await import('../../../../shared/execution-host') + +const repo: Repo = { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 +} as Repo + +const ownershipMeta = getProjectHostSetupWorktreeMeta([], repo) + +function gitWorktree(path: string): GitWorktreeInfo { + return { + path, + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } +} + +/** Fully settled metadata: discovery backfill has nothing to write, so it hands the same object back. */ +function settledMeta(overrides: Partial = {}): WorktreeMeta { + return { + ...ownershipMeta, + instanceId: 'instance-settled', + orcaCreatedAt: 1, + lastActivityAt: 5, + ...overrides + } as WorktreeMeta +} + +function createStore(meta: Record, repos: Repo[] = [repo]) { + const rows = { ...meta } + return { + getRepos: () => repos, + getSettings: () => ({ workspaceDir: '/workspace', nestWorkspaces: true }), + getProjectHostSetups: () => [], + getAllWorktreeLineage: () => ({}), + getAllWorktreeMeta: () => rows, + getWorktreeMeta: (id: string) => rows[id], + getWorktreeMetaForHost: (id: string, hostId: string) => + rows[id]?.hostId === hostId ? rows[id] : undefined, + getAllWorktreeMetaForHost: () => rows, + setWorktreeMeta: (id: string, patch: Partial) => { + rows[id] = { ...rows[id], ...patch } as WorktreeMeta + return rows[id] + }, + setWorktreeMetaForHost: (id: string, hostId: string, patch: Partial) => { + rows[id] = { ...rows[id], ...patch, hostId } as WorktreeMeta + return rows[id] + } + } as unknown as Store +} + +/** The pre-change implementation, verbatim, as the equivalence oracle. */ +function buildDetectedGitWorktreesTwoPass( + store: Store, + target: Repo, + gitWorktrees: GitWorktreeInfo[], + allMetaOverride?: Record +) { + const settings = store.getSettings() + const knownOrcaLayouts = ownership.buildKnownOrcaWorkspaceLayouts(settings, target) + const isLegacyRepoForVisibility = ownership.isLegacyRepoForExternalWorktreeVisibility(target) + const liveWorktrees = dedupeWorktreesByPath(gitWorktrees.filter((info) => !info.prunable)) + const worktreeVisibilitySourceMatcher = createWorktreeVisibilitySourceMatcher( + [target.path, ...liveWorktrees.map((worktree) => worktree.path)], + resolveCustomWorktreeVisibilitySources(target, settings.worktreeVisibilityDefaults), + resolveConfiguredWorktreeBasePaths(target) + ) + const allMeta = allMetaOverride ?? store.getAllWorktreeMeta?.() + const repoOwnerCount = store.getRepos().filter((candidate) => candidate.id === target.id).length + const detectedRows = liveWorktrees.map((info) => { + const worktreeId = `${target.id}::${info.path}` + const legacyMeta = store.getWorktreeMeta?.(worktreeId) + const metaById = allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {}) + let meta = + readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(target)) ?? + getRepoOwnedWorktreeMeta(target, worktreeId, metaById, repoOwnerCount) + const worktree = mergeWorktree(target.id, info, meta, target.displayName) + const detected = ownership.toDetectedWorktree({ + repo: target, + worktree, + meta, + settings, + knownOrcaLayouts, + isLegacyRepoForVisibility, + worktreeVisibilitySourceMatcher + }) + if (!detected.visible) { + return detected + } + meta = resolveWorktreeMetaWithDiscoveryBackfill( + store, + target, + worktreeId, + allMeta, + repoOwnerCount + ) + return ownership.toDetectedWorktree({ + repo: target, + worktree: mergeWorktree(target.id, info, meta, target.displayName), + meta, + settings, + knownOrcaLayouts, + isLegacyRepoForVisibility, + worktreeVisibilitySourceMatcher + }) + }) + return projectResolvedWorktreeLineage(detectedRows, store.getAllWorktreeLineage?.() ?? {}) +} + +describe('buildDetectedGitWorktrees classification passes', () => { + beforeEach(() => { + toDetectedWorktreeSpy.mockClear() + // Discovery backfill stamps lastActivityAt from the clock; freeze it so equivalence is deterministic. + vi.spyOn(Date, 'now').mockReturnValue(1_700_000_000_000) + }) + + it('classifies each visible worktree once per catalog pass, not twice', () => { + const paths = ['/workspace/one', '/workspace/two', '/workspace/three'] + const meta = Object.fromEntries( + paths.map((path) => [`${repo.id}::${path}`, settledMeta({ displayName: path })]) + ) + const store = createStore(meta) + + const detected = buildDetectedGitWorktrees(store, repo, paths.map(gitWorktree), meta) + + expect(detected).toHaveLength(3) + expect(detected.every((row) => row.visible)).toBe(true) + expect(toDetectedWorktreeSpy).toHaveBeenCalledTimes(paths.length) + }) + + it('reads the locator-keyed metadata row only when no host snapshot is available', () => { + const worktreeId = `${repo.id}::/workspace/one` + const meta = { [worktreeId]: settledMeta() } + const store = createStore(meta) + const legacyReads = vi.spyOn(store, 'getWorktreeMeta') + + buildDetectedGitWorktrees(store, repo, [gitWorktree('/workspace/one')], meta) + expect(legacyReads).not.toHaveBeenCalled() + + // Partial stores (compatibility shapes) expose no snapshot, so the locator-keyed lookup must still run. + const partialStore = createStore(meta) as Partial + delete partialStore.getAllWorktreeMeta + delete partialStore.getAllWorktreeMetaForHost + delete partialStore.getWorktreeMetaForHost + const partialLegacyReads = vi.spyOn(partialStore as Store, 'getWorktreeMeta') + + const rows = buildDetectedGitWorktrees( + partialStore as Store, + repo, + [gitWorktree('/workspace/one')], + undefined + ) + expect(partialLegacyReads).toHaveBeenCalledWith(worktreeId) + expect(rows[0]).toMatchObject({ id: worktreeId, lastActivityAt: 5 }) + }) + + it.each([ + ['settled metadata', () => settledMeta()], + ['metadata needing discovery backfill', () => ({ orcaCreatedAt: 1 }) as WorktreeMeta], + ['no metadata at all', () => undefined] + ])('emits a catalog deep-equal to the two-pass build for %s', (_label, makeMeta) => { + const worktreeId = `${repo.id}::/workspace/one` + const seed = makeMeta() + const build = (fn: typeof buildDetectedGitWorktrees) => + fn( + createStore(seed ? { [worktreeId]: seed } : {}), + repo, + [gitWorktree('/workspace/one'), gitWorktree('/workspace/hidden-external')], + seed ? { [worktreeId]: seed } : {} + ) + + expect(build(buildDetectedGitWorktrees)).toEqual(build(buildDetectedGitWorktreesTwoPass)) + }) + + it('emits a catalog deep-equal to the two-pass build for a folder-style listing with no host snapshot', () => { + const worktreeId = `${repo.id}::/workspace/one` + const seed = settledMeta() + const build = (fn: typeof buildDetectedGitWorktrees) => + fn(createStore({ [worktreeId]: seed }), repo, [gitWorktree('/workspace/one')], undefined) + + expect(build(buildDetectedGitWorktrees)).toEqual(build(buildDetectedGitWorktreesTwoPass)) + }) +}) diff --git a/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts b/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts index 7ec2cac1fc9..5c734d8bcd9 100644 --- a/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts +++ b/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts @@ -155,9 +155,10 @@ export function buildDetectedGitWorktrees( const repoOwnerCount = store.getRepos().filter((candidate) => candidate.id === repo.id).length const detected = liveWorktrees.map((gitWorktree) => { const worktreeId = `${repo.id}::${gitWorktree.path}` - const legacyMeta = store.getWorktreeMeta?.(worktreeId) + // Why: the locator-keyed row is only a stand-in for a missing host snapshot, so don't read it when we have one. + const legacyMeta = allMeta === undefined ? store.getWorktreeMeta?.(worktreeId) : undefined const metaById = allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {}) - let meta = + const meta = readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo)) ?? getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) const worktree = mergeWorktree(repo.id, gitWorktree, meta, repo.displayName) @@ -174,17 +175,21 @@ export function buildDetectedGitWorktrees( return detected } - meta = resolveWorktreeMetaWithDiscoveryBackfill( + const backfilledMeta = resolveWorktreeMetaWithDiscoveryBackfill( store, repo, worktreeId, allMeta, repoOwnerCount ) + // Why: backfill hands back the same object when it wrote nothing, and both builders are pure over it. + if (backfilledMeta === meta) { + return detected + } return toDetectedWorktree({ repo, - worktree: mergeWorktree(repo.id, gitWorktree, meta, repo.displayName), - meta, + worktree: mergeWorktree(repo.id, gitWorktree, backfilledMeta, repo.displayName), + meta: backfilledMeta, settings, knownOrcaLayouts, isLegacyRepoForVisibility, diff --git a/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts b/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts index 6af4d45c3cd..b17677ddfcc 100644 --- a/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts +++ b/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts @@ -40,8 +40,9 @@ export function resolveWorktreeMetaWithDiscoveryBackfill( repoOwnerCount = store.getRepos().filter((candidate) => candidate.id === repo.id).length ): WorktreeMeta { const executionHostId = getRepoExecutionHostId(repo) - const legacyMeta = store.getWorktreeMeta?.(worktreeId) const allMeta = allMetaOverride ?? store.getAllWorktreeMeta?.() + // Why: the locator-keyed row is only a stand-in for a missing snapshot, so don't read it when we have one. + const legacyMeta = allMeta === undefined ? store.getWorktreeMeta?.(worktreeId) : undefined const existing = readWorktreeMetaForHost(store, worktreeId, executionHostId) ?? getRepoOwnedWorktreeMeta( diff --git a/src/shared/git-status-conflict-entries.test.ts b/src/shared/git-status-conflict-entries.test.ts new file mode 100644 index 00000000000..35f8e4685c5 --- /dev/null +++ b/src/shared/git-status-conflict-entries.test.ts @@ -0,0 +1,212 @@ +/** + * `u` records for asymmetric conflict kinds each cost an `fs.access`. Resolving them concurrently + * must not move a row, change how many probes run, or change which error wins. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { StatusPorcelainRecord } from './git-status-porcelain-parser' +import type { GitStatusEntry } from './git-status-types' +import type * as NodeFsPromisesModule from 'node:fs/promises' + +const { accessMock } = vi.hoisted(() => ({ accessMock: vi.fn() })) + +vi.mock('node:fs/promises', async (importOriginal) => ({ + ...(await importOriginal()), + access: accessMock +})) + +/** + * `parseUnmergedEntry` only rethrows when reading `error.code` itself throws, so this is how a + * record is made to reject rather than degrade to 'modified'. + */ +function rejectionEscapingTheErrnoGuard(failure: Error): unknown { + return new Proxy( + {}, + { + get: () => { + throw failure + } + } + ) +} + +const { parseUnmergedEntry, resolveUnmergedStatusRecords } = + await import('./git-status-conflict-entries') + +const WORKTREE = '/repo' + +function unmergedLine(xy: string, filePath: string): string { + return `u ${xy} N... 100644 100644 100644 100644 aaa bbb ccc ${filePath}` +} + +function entryRecord(path: string): StatusPorcelainRecord { + return { type: 'entry', entry: { path, status: 'modified', area: 'unstaged' } } +} + +/** Mixes both conflict families: UU/AA return without I/O, AU/UA/DU/UD each probe the filesystem. */ +const MIXED_RECORDS: StatusPorcelainRecord[] = [ + { type: 'unmerged', line: unmergedLine('UU', 'both-modified.ts') }, + entryRecord('plain-one.ts'), + { type: 'unmerged', line: unmergedLine('AU', 'added-by-us.ts') }, + { type: 'unmerged', line: unmergedLine('AA', 'both-added.ts') }, + { type: 'unmerged', line: unmergedLine('UA', 'added-by-them.ts') }, + entryRecord('plain-two.ts'), + { type: 'unmerged', line: unmergedLine('DU', 'deleted-by-us.ts') }, + { type: 'unmerged', line: unmergedLine('UD', 'deleted-by-them.ts') }, + { type: 'unmerged', line: unmergedLine('160000', 'submodule-ignored.ts') } +] + +/** The pre-change consumption: one `await` per record, in Git's output order. */ +async function collectSerially( + records: readonly StatusPorcelainRecord[] +): Promise<(GitStatusEntry | null)[]> { + const collected: (GitStatusEntry | null)[] = [] + for (const record of records) { + collected.push( + record.type === 'entry' ? record.entry : await parseUnmergedEntry(WORKTREE, record.line) + ) + } + return collected +} + +async function collectConcurrently( + records: readonly StatusPorcelainRecord[] +): Promise<(GitStatusEntry | null)[]> { + const resolved = await resolveUnmergedStatusRecords(WORKTREE, records, records.length) + return records.map((record, index) => { + if (record.type === 'entry') { + return record.entry + } + const settled = resolved[index] + if (settled?.ok === false) { + throw settled.error + } + return settled?.entry ?? null + }) +} + +describe('resolveUnmergedStatusRecords', () => { + beforeEach(() => { + accessMock.mockReset() + }) + + it('produces the same rows, in the same order, as the serial read', async () => { + accessMock.mockImplementation((target: string) => + target.includes('deleted-by') + ? Promise.reject(Object.assign(new Error('x'), { code: 'ENOENT' })) + : Promise.resolve(undefined) + ) + + const serial = await collectSerially(MIXED_RECORDS) + const concurrent = await collectConcurrently(MIXED_RECORDS) + + expect(concurrent).toEqual(serial) + expect(concurrent.map((entry) => entry?.path ?? null)).toEqual([ + 'both-modified.ts', + 'plain-one.ts', + 'added-by-us.ts', + 'both-added.ts', + 'added-by-them.ts', + 'plain-two.ts', + 'deleted-by-us.ts', + 'deleted-by-them.ts', + null + ]) + expect(concurrent.map((entry) => entry?.status ?? null)).toEqual([ + 'modified', + 'modified', + 'modified', + 'modified', + 'modified', + 'modified', + 'deleted', + 'deleted', + null + ]) + }) + + it('runs the same number of filesystem probes as the serial read', async () => { + accessMock.mockResolvedValue(undefined) + await collectSerially(MIXED_RECORDS) + const serialProbes = accessMock.mock.calls.length + + accessMock.mockClear() + await collectConcurrently(MIXED_RECORDS) + + // Only the four asymmetric kinds probe; UU/AA/submodule never touch the filesystem. + expect(serialProbes).toBe(4) + expect(accessMock.mock.calls.length).toBe(serialProbes) + }) + + it('never exceeds the concurrency bound', async () => { + let inFlight = 0 + let peak = 0 + accessMock.mockImplementation(async () => { + inFlight += 1 + peak = Math.max(peak, inFlight) + await new Promise((resolve) => setTimeout(resolve, 0)) + inFlight -= 1 + }) + const records = Array.from({ length: 50 }, (_, index) => ({ + type: 'unmerged' as const, + line: unmergedLine('AU', `conflict-${index}.ts`) + })) + + await resolveUnmergedStatusRecords(WORKTREE, records, records.length) + + expect(peak).toBeGreaterThan(1) + expect(peak).toBeLessThanOrEqual(8) + }) + + it('resolves only the requested prefix, leaving later records for the caller', async () => { + accessMock.mockResolvedValue(undefined) + + const resolved = await resolveUnmergedStatusRecords(WORKTREE, MIXED_RECORDS, 4) + + expect(resolved[0]).toEqual({ + ok: true, + entry: expect.objectContaining({ path: 'both-modified.ts' }) + }) + expect(resolved[1]).toBeUndefined() + expect(resolved[6]).toBeUndefined() + expect(accessMock.mock.calls.length).toBe(1) + }) + + it('surfaces the earliest failing record, exactly as the serial read did', async () => { + const firstFailure = new Error('first failure') + const secondFailure = new Error('second failure') + accessMock.mockImplementation((target: string) => { + if (target.endsWith('added-by-them.ts')) { + return Promise.reject(rejectionEscapingTheErrnoGuard(firstFailure)) + } + if (target.endsWith('deleted-by-us.ts')) { + return Promise.reject(rejectionEscapingTheErrnoGuard(secondFailure)) + } + return Promise.resolve(undefined) + }) + + await expect(collectSerially(MIXED_RECORDS)).rejects.toBe(firstFailure) + await expect(collectConcurrently(MIXED_RECORDS)).rejects.toBe(firstFailure) + }) + + it('does not surface a failure the serial read would never have reached', async () => { + const lateFailure = new Error('late failure') + accessMock.mockImplementation((target: string) => + target.endsWith('deleted-by-them.ts') + ? Promise.reject(rejectionEscapingTheErrnoGuard(lateFailure)) + : Promise.resolve(undefined) + ) + + // The caller stops before that record, so the captured rejection is simply never replayed. + const resolved = await resolveUnmergedStatusRecords( + WORKTREE, + MIXED_RECORDS, + MIXED_RECORDS.length + ) + const consumedPrefix = MIXED_RECORDS.slice(0, 7).map((record, index) => + record.type === 'entry' ? record.entry : resolved[index] + ) + + expect(consumedPrefix).not.toContainEqual({ ok: false, error: lateFailure }) + expect(resolved[7]).toEqual({ ok: false, error: lateFailure }) + }) +}) diff --git a/src/shared/git-status-conflict-entries.ts b/src/shared/git-status-conflict-entries.ts index a50d389e2bb..8071d8292d0 100644 --- a/src/shared/git-status-conflict-entries.ts +++ b/src/shared/git-status-conflict-entries.ts @@ -1,8 +1,52 @@ import { access } from 'node:fs/promises' import * as path from 'node:path' import type { GitConflictKind, GitFileStatus, GitStatusEntry } from './git-status-types' +import type { StatusPorcelainRecord } from './git-status-porcelain-parser' import { decodeGitCQuotedPath } from './git-cquoted-path' +const UNMERGED_ENTRY_RESOLVE_CONCURRENCY = 8 + +/** A settled `parseUnmergedEntry` result, replayed at the record index it belongs to. */ +export type ResolvedUnmergedEntry = + | { ok: true; entry: GitStatusEntry | null } + | { ok: false; error: unknown } + +/** + * Resolve the deferred `u` records in `records[0, end)` with bounded concurrency, keyed by record + * index. Asymmetric conflict kinds each cost an `fs.access`, which is a 9p/network round trip on a + * WSL or remote worktree, and a big rebase makes hundreds of them — serialising those stalls every + * status poll for the life of the conflict. + * + * Ordering is untouched: results stay at their own index, so the caller still consumes Git's output + * order, and a rejection is replayed only if the caller actually reaches that record. + */ +export async function resolveUnmergedStatusRecords( + worktreePath: string, + records: readonly StatusPorcelainRecord[], + end: number +): Promise<(ResolvedUnmergedEntry | undefined)[]> { + const resolved: (ResolvedUnmergedEntry | undefined)[] = [] + let nextIndex = 0 + const resolveNext = async (): Promise => { + while (nextIndex < end) { + const index = nextIndex + nextIndex += 1 + const record = records[index] + if (record?.type !== 'unmerged') { + continue + } + try { + resolved[index] = { ok: true, entry: await parseUnmergedEntry(worktreePath, record.line) } + } catch (error) { + resolved[index] = { ok: false, error } + } + } + } + const workerCount = Math.min(UNMERGED_ENTRY_RESOLVE_CONCURRENCY, end) + await Promise.all(Array.from({ length: workerCount }, () => resolveNext())) + return resolved +} + export async function parseUnmergedEntry( worktreePath: string, line: string