mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(worktrees): stop CLI-created worktrees flashing into a filtered sidebar (#30194)
* fix(worktrees): keep a CLI-created worktree out of listings until its metadata is saved A watcher-driven listing during `git worktree add` surfaced the new checkout as a bare external row. The host's default-terminal request then made that stale row active, so it slipped past Hide sleeping and Hide CLI-created until the next refresh hid it (~1s flash). * fix(worktrees): hold canonical checkout paths during creation
This commit is contained in:
@@ -41,6 +41,7 @@ import {
|
||||
import { readAllWorktreeMetaForRepo } from '../../../persistence/host-qualified-worktree-meta'
|
||||
import { classifyWorktreeScanFailure } from '../../../../shared/worktree-scan-failure'
|
||||
import { scanUntilNotOvertaken } from './overtaken-scan-rerun'
|
||||
import { withoutInFlightWorktreeCreates } from '../../../worktree-create-in-flight'
|
||||
|
||||
export async function listDetectedWorktreesForCapturedRepo(
|
||||
store: Store,
|
||||
@@ -123,7 +124,10 @@ export async function listDetectedWorktreesForCapturedRepo(
|
||||
return abortedResult() ?? null
|
||||
}
|
||||
const { gitWorktrees, fresh: freshScan, sideEffectToken, metadataPrune, hygieneDue } = scan
|
||||
const localRows = await withUnregisteredRemovalCheckouts(repo.id, gitWorktrees)
|
||||
const localRows = await withUnregisteredRemovalCheckouts(
|
||||
repo.id,
|
||||
withoutInFlightWorktreeCreates(repo.id, gitWorktrees)
|
||||
)
|
||||
const aborted = abortedResult()
|
||||
if (aborted) {
|
||||
return aborted
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
withUnregisteredRemovalCheckouts,
|
||||
type PendingWorktreeRemovals
|
||||
} from '../../../worktree-removal-listing'
|
||||
import { withoutInFlightWorktreeCreates } from '../../../worktree-create-in-flight'
|
||||
|
||||
const WORKTREE_LIST_ALL_CONCURRENCY = 8
|
||||
|
||||
@@ -128,7 +129,10 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo
|
||||
}
|
||||
loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`)
|
||||
const metadata = metadataForRepo(repo)
|
||||
const rows = await withUnregisteredRemovalCheckouts(repo.id, scan.gitWorktrees)
|
||||
const rows = await withUnregisteredRemovalCheckouts(
|
||||
repo.id,
|
||||
withoutInFlightWorktreeCreates(repo.id, scan.gitWorktrees)
|
||||
)
|
||||
const worktrees = buildDetectedGitWorktrees(store, repo, rows, metadata)
|
||||
.filter((worktree) => worktree.visible)
|
||||
.map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata))
|
||||
@@ -196,7 +200,10 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo
|
||||
}
|
||||
loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`)
|
||||
const metadata = allMeta ?? readAllWorktreeMetaForRepo(store, repo)
|
||||
const rows = await withUnregisteredRemovalCheckouts(repo.id, scan.gitWorktrees)
|
||||
const rows = await withUnregisteredRemovalCheckouts(
|
||||
repo.id,
|
||||
withoutInFlightWorktreeCreates(repo.id, scan.gitWorktrees)
|
||||
)
|
||||
const worktrees = buildDetectedGitWorktrees(store, repo, rows, metadata)
|
||||
.filter((worktree) => worktree.visible)
|
||||
.map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata))
|
||||
|
||||
@@ -92,6 +92,7 @@ import {
|
||||
type WorktreeCreateTimingRecorder
|
||||
} from '../worktree-create-timing'
|
||||
import type { SparsePreset } from '../../shared/worktree/create-types'
|
||||
import { withoutInFlightWorktreeCreates } from '../worktree-create-in-flight'
|
||||
|
||||
const worktreePath = resolve('/worktrees', 'app')
|
||||
|
||||
@@ -184,6 +185,29 @@ beforeEach(() => {
|
||||
readSparsePresets.mockReturnValue([WEB_PRESET])
|
||||
})
|
||||
|
||||
describe('runtime create listing hold', () => {
|
||||
const rows = [{ path: worktreePath }]
|
||||
|
||||
it('holds the checkout during Git creation and releases it once metadata is saved', async () => {
|
||||
mocks.consume.mockImplementation(async () => {
|
||||
expect(withoutInFlightWorktreeCreates('repo-1', rows)).toEqual([])
|
||||
return { status: 'hit', result: {}, rearm: mocks.rearm }
|
||||
})
|
||||
mocks.resolveShared.mockImplementation(async () => {
|
||||
expect(withoutInFlightWorktreeCreates('repo-1', rows)).toBe(rows)
|
||||
return []
|
||||
})
|
||||
await createWorktree()
|
||||
expect(mocks.resolveShared).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('releases the checkout when Git creation fails', async () => {
|
||||
mocks.consume.mockRejectedValue(new Error('checkout failed'))
|
||||
await expect(createWorktree()).rejects.toThrow('checkout failed')
|
||||
expect(withoutInFlightWorktreeCreates('repo-1', rows)).toBe(rows)
|
||||
})
|
||||
})
|
||||
|
||||
describe('runtime nested-workspaces layout', () => {
|
||||
const addedPath = (): string => String(mocks.add.mock.calls[0]?.[1])
|
||||
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
} from './runtime-local-git-worktree-create'
|
||||
import { resolveRuntimeLocalWorktreeCreateBase } from './runtime-local-worktree-create-base'
|
||||
import { materializeRuntimeLocalWorktree } from './runtime-local-worktree-materialization'
|
||||
import { holdInFlightWorktreeCreate } from '../worktree-create-in-flight'
|
||||
import { resolveRuntimeSetupDecision } from './runtime-local-worktree-setup'
|
||||
import { WorktreeCreateSetupChoiceRefusedError } from './worktree-create-setup-choice-refusal'
|
||||
import type { PreparationRearmHolder } from '../worktree-create-preparation'
|
||||
@@ -160,48 +161,60 @@ async function performRuntimeLocalWorktreeCreate<T>(args: RuntimeLocalWorktreeCr
|
||||
hostedReviewExecutionContext: args.hostedReviewExecutionContext
|
||||
})
|
||||
)
|
||||
const git = await createRuntimeLocalGitWorktree({
|
||||
request,
|
||||
repo,
|
||||
store,
|
||||
settings,
|
||||
base,
|
||||
workspaceRoot: computeWorkspaceRoot(repo.path, pathSettings),
|
||||
branchName: candidate.branchName,
|
||||
worktreePath: candidate.worktreePath,
|
||||
effectiveSanitizedName: candidate.effectiveSanitizedName,
|
||||
checkoutExistingBranch: candidate.checkoutExistingBranch,
|
||||
localWorktreeGitOptions: worktreeGitOptions,
|
||||
baseRefRefresh,
|
||||
rearm: args.rearm,
|
||||
timing: args.timing
|
||||
})
|
||||
const materialized = await materializeRuntimeLocalWorktree({
|
||||
request,
|
||||
repo,
|
||||
store,
|
||||
settings,
|
||||
created: git.created,
|
||||
remoteTrackingBase: base.remoteTrackingBase,
|
||||
sparseDirectories: git.sparseDirectories,
|
||||
configuredPushTarget: git.configuredPushTarget,
|
||||
checkoutExistingBranch: candidate.checkoutExistingBranch,
|
||||
baseBranch: base.baseBranch,
|
||||
branchName: candidate.branchName,
|
||||
effectiveRequestedName: candidate.effectiveRequestedName,
|
||||
requestedDisplayName: candidate.requestedDisplayName,
|
||||
displayNameKind: candidate.displayNameKind,
|
||||
effectiveSanitizedName: candidate.effectiveSanitizedName,
|
||||
effectiveCreatedWithAgent: args.createdWithAgent,
|
||||
localWorktreeGitOptions: worktreeGitOptions,
|
||||
onMetadataPersisted: args.onWorktreeMetadataPersisted,
|
||||
timing: args.timing
|
||||
})
|
||||
return {
|
||||
...materialized,
|
||||
worktreePath: candidate.worktreePath,
|
||||
created: git.created,
|
||||
addResult: git.addResult,
|
||||
...(base.baseFallback ? { baseFallback: base.baseFallback } : {})
|
||||
const releaseInFlightHold = await holdInFlightWorktreeCreate(
|
||||
repo.id,
|
||||
candidate.worktreePath,
|
||||
worktreeGitOptions
|
||||
)
|
||||
try {
|
||||
const git = await createRuntimeLocalGitWorktree({
|
||||
request,
|
||||
repo,
|
||||
store,
|
||||
settings,
|
||||
base,
|
||||
workspaceRoot: computeWorkspaceRoot(repo.path, pathSettings),
|
||||
branchName: candidate.branchName,
|
||||
worktreePath: candidate.worktreePath,
|
||||
effectiveSanitizedName: candidate.effectiveSanitizedName,
|
||||
checkoutExistingBranch: candidate.checkoutExistingBranch,
|
||||
localWorktreeGitOptions: worktreeGitOptions,
|
||||
baseRefRefresh,
|
||||
rearm: args.rearm,
|
||||
timing: args.timing
|
||||
})
|
||||
const materialized = await materializeRuntimeLocalWorktree({
|
||||
request,
|
||||
repo,
|
||||
store,
|
||||
settings,
|
||||
created: git.created,
|
||||
remoteTrackingBase: base.remoteTrackingBase,
|
||||
sparseDirectories: git.sparseDirectories,
|
||||
configuredPushTarget: git.configuredPushTarget,
|
||||
checkoutExistingBranch: candidate.checkoutExistingBranch,
|
||||
baseBranch: base.baseBranch,
|
||||
branchName: candidate.branchName,
|
||||
effectiveRequestedName: candidate.effectiveRequestedName,
|
||||
requestedDisplayName: candidate.requestedDisplayName,
|
||||
displayNameKind: candidate.displayNameKind,
|
||||
effectiveSanitizedName: candidate.effectiveSanitizedName,
|
||||
effectiveCreatedWithAgent: args.createdWithAgent,
|
||||
localWorktreeGitOptions: worktreeGitOptions,
|
||||
onMetadataPersisted: (worktree) => {
|
||||
releaseInFlightHold()
|
||||
return args.onWorktreeMetadataPersisted(worktree)
|
||||
},
|
||||
timing: args.timing
|
||||
})
|
||||
return {
|
||||
...materialized,
|
||||
worktreePath: candidate.worktreePath,
|
||||
created: git.created,
|
||||
addResult: git.addResult,
|
||||
...(base.baseFallback ? { baseFallback: base.baseFallback } : {})
|
||||
}
|
||||
} finally {
|
||||
releaseInFlightHold()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
LOCAL_EXECUTION_HOST_ID
|
||||
} from '../../../shared/execution-host'
|
||||
import { withUnregisteredRemovalCheckouts } from '../../worktree-removal-listing'
|
||||
import { withoutInFlightWorktreeCreates } from '../../worktree-create-in-flight'
|
||||
import { preserveFolderUpgradeWorktreePath } from '../../folder-upgrade-worktree-path'
|
||||
import type { RuntimeWorktreeScanCache } from '../orca-runtime-core'
|
||||
import { resolveWorktreeScanCacheTtlMs } from '../runtime-worktree-scan-cache'
|
||||
@@ -152,7 +153,13 @@ export abstract class RuntimeWorkspaceCatalog extends OrcaRuntimeWithReadKittyFi
|
||||
// checkout must carry the folder's locator here or its id loses metadata and terminals (#30150).
|
||||
const scan = { ...cached, worktrees: preserveFolderUpgradeWorktreePath(repo, cached.worktrees) }
|
||||
return scan.ok && getRepoExecutionHostId(repo) === LOCAL_EXECUTION_HOST_ID
|
||||
? { ok: true, worktrees: await withUnregisteredRemovalCheckouts(repo.id, scan.worktrees) }
|
||||
? {
|
||||
ok: true,
|
||||
worktrees: await withUnregisteredRemovalCheckouts(
|
||||
repo.id,
|
||||
withoutInFlightWorktreeCreates(repo.id, scan.worktrees)
|
||||
)
|
||||
}
|
||||
: scan
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { mkdir, mkdtemp, realpath, rm, symlink } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
holdInFlightWorktreeCreate,
|
||||
withoutInFlightWorktreeCreates
|
||||
} from './worktree-create-in-flight'
|
||||
|
||||
const rows = [{ path: '/repo' }, { path: '/ws/new' }, { path: '/ws/other' }]
|
||||
|
||||
describe('withoutInFlightWorktreeCreates', () => {
|
||||
it('omits a held checkout from its own repo until released', async () => {
|
||||
const release = await holdInFlightWorktreeCreate('repo-a', '/ws/new')
|
||||
expect(withoutInFlightWorktreeCreates('repo-a', rows)).toEqual([
|
||||
{ path: '/repo' },
|
||||
{ path: '/ws/other' }
|
||||
])
|
||||
expect(withoutInFlightWorktreeCreates('repo-b', rows)).toBe(rows)
|
||||
release()
|
||||
expect(withoutInFlightWorktreeCreates('repo-a', rows)).toBe(rows)
|
||||
})
|
||||
|
||||
it('keeps a second hold on the same path after the first releases', async () => {
|
||||
const releaseFirst = await holdInFlightWorktreeCreate('repo-a', '/ws/new')
|
||||
const releaseSecond = await holdInFlightWorktreeCreate('repo-a', '/ws/new/')
|
||||
releaseFirst()
|
||||
releaseFirst()
|
||||
expect(withoutInFlightWorktreeCreates('repo-a', rows)).toHaveLength(2)
|
||||
releaseSecond()
|
||||
expect(withoutInFlightWorktreeCreates('repo-a', rows)).toBe(rows)
|
||||
})
|
||||
|
||||
it('hides the real path Git lists under a symlinked workspace with missing parents', async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-create-hold-'))
|
||||
let release = () => {}
|
||||
try {
|
||||
const actual = join(root, 'actual')
|
||||
const alias = join(root, 'alias')
|
||||
await mkdir(actual)
|
||||
await symlink(actual, alias, 'junction')
|
||||
const requested = join(alias, 'nested', 'new')
|
||||
release = await holdInFlightWorktreeCreate('repo-alias', requested)
|
||||
await mkdir(requested, { recursive: true })
|
||||
const canonical = await realpath(requested)
|
||||
const listed = [{ path: requested }, { path: canonical }, { path: join(actual, 'other') }]
|
||||
expect(withoutInFlightWorktreeCreates('repo-alias', listed)).toEqual([listed[2]])
|
||||
expect(withoutInFlightWorktreeCreates('other-repo', listed)).toBe(listed)
|
||||
release()
|
||||
expect(withoutInFlightWorktreeCreates('repo-alias', listed)).toBe(listed)
|
||||
} finally {
|
||||
release()
|
||||
await rm(root, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,71 @@
|
||||
import { realpath } from 'node:fs/promises'
|
||||
import { basename, dirname, resolve } from 'node:path'
|
||||
import { toWindowsWslPath } from '../shared/wsl-paths'
|
||||
import { areWorktreePathsEqual } from './git/worktree-path-comparison'
|
||||
|
||||
type InFlightWorktreeCreate = { repoId: string; paths: string[] }
|
||||
|
||||
const inFlightCreates = new Set<InFlightWorktreeCreate>()
|
||||
|
||||
// Git lists real paths even when creation uses a symlink and the leaf does not exist yet.
|
||||
async function resolveCreatePathAlias(worktreePath: string): Promise<string> {
|
||||
let ancestor = worktreePath
|
||||
const missing: string[] = []
|
||||
for (;;) {
|
||||
try {
|
||||
return resolve(await realpath(ancestor), ...missing)
|
||||
} catch (error) {
|
||||
if (
|
||||
typeof error !== 'object' ||
|
||||
error === null ||
|
||||
!('code' in error) ||
|
||||
error.code !== 'ENOENT'
|
||||
) {
|
||||
return worktreePath
|
||||
}
|
||||
const parent = dirname(ancestor)
|
||||
if (parent === ancestor) {
|
||||
return worktreePath
|
||||
}
|
||||
missing.unshift(basename(ancestor))
|
||||
ancestor = parent
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Hide untagged checkouts until metadata makes sidebar filters reliable. */
|
||||
export async function holdInFlightWorktreeCreate(
|
||||
repoId: string,
|
||||
worktreePath: string,
|
||||
options: { wslDistro?: string } = {}
|
||||
): Promise<() => void> {
|
||||
const hostPath =
|
||||
process.platform === 'win32' && options.wslDistro && worktreePath.startsWith('/')
|
||||
? toWindowsWslPath(worktreePath, options.wslDistro)
|
||||
: worktreePath
|
||||
const entry: InFlightWorktreeCreate = {
|
||||
repoId,
|
||||
paths: [worktreePath, hostPath, await resolveCreatePathAlias(hostPath)]
|
||||
}
|
||||
inFlightCreates.add(entry)
|
||||
return () => {
|
||||
inFlightCreates.delete(entry)
|
||||
}
|
||||
}
|
||||
|
||||
export function withoutInFlightWorktreeCreates<T extends { path: string }>(
|
||||
repoId: string,
|
||||
rows: T[]
|
||||
): T[] {
|
||||
if (inFlightCreates.size === 0) {
|
||||
return rows
|
||||
}
|
||||
const held = [...inFlightCreates].filter((entry) => entry.repoId === repoId)
|
||||
if (held.length === 0) {
|
||||
return rows
|
||||
}
|
||||
return rows.filter(
|
||||
(row) =>
|
||||
!held.some((entry) => entry.paths.some((path) => areWorktreePathsEqual(row.path, path)))
|
||||
)
|
||||
}
|
||||
Reference in New Issue
Block a user