mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(worktrees): retire runtime-host metadata a scan proved gone
A paired client's WorktreeMeta for a runtime host is exempt from gcStaleWorktreeMeta -- that GC skips any row that is not local on both the repo and the meta's hostId -- so a scan-proven removal is the only thing that ever retires one. Both halves of that path were gated to `ssh:`, so the client kept a row for every remote worktree it had ever seen and dropped none. The renderer already computed the removals for runtime hosts and purged its own in-memory state with them; only the persisted half bailed. Widen it, and the matching main-side handler, to runtime hosts. `OffHostExecutionHostId` names the set precisely: the hosts the local-only GC skips. Also require `source === 'git'` before retiring anything. `session-fallback` reports `authoritative: true` but is the truncated, visibility-filtered `worktree.list` reply from a host too old for `worktree.detectedList`; its omissions are no evidence a checkout is gone. That guard did not matter while this only ran the in-memory purge, and does now that it deletes rows. A repo that reaches its checkouts over a connection is still never condemned under a runtime host id -- the host that executes owns that verdict. Refs #17776
This commit is contained in:
@@ -1,7 +1,11 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types'
|
||||
import type { ProviderRequestId } from '../../shared/detected-worktree-provider-contract'
|
||||
import { LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId } from '../../shared/execution-host'
|
||||
import {
|
||||
LOCAL_EXECUTION_HOST_ID,
|
||||
toRuntimeExecutionHostId,
|
||||
toSshExecutionHostId
|
||||
} from '../../shared/execution-host'
|
||||
import { getSshProviderAuthority } from '../ssh/ssh-provider-authority'
|
||||
import {
|
||||
listWorktreesMock,
|
||||
@@ -443,7 +447,74 @@ describe('registerWorktreeHandlers', () => {
|
||||
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refuses to retire metadata for non-SSH hosts and unowned repos', async () => {
|
||||
// Runtime-host rows are exempt from gcStaleWorktreeMeta exactly as SSH ones are, so a paired
|
||||
// client needs this path to ever drop them (#17776).
|
||||
it('retires runtime-host metadata an authoritative scan proved gone', async () => {
|
||||
const runtimeHostId = toRuntimeExecutionHostId('env-1')
|
||||
const runtimeRepo = {
|
||||
id: 'repo-1',
|
||||
path: '/home/orca/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
executionHostId: runtimeHostId
|
||||
}
|
||||
const metaById: Record<string, ReturnType<typeof makeWorktreeMeta>> = {
|
||||
'repo-1::/home/orca/deleted': makeWorktreeMeta({ hostId: runtimeHostId }),
|
||||
'repo-1::/home/orca/other-host': makeWorktreeMeta({
|
||||
hostId: toSshExecutionHostId('target-a')
|
||||
})
|
||||
}
|
||||
store.getRepos.mockReturnValue([runtimeRepo])
|
||||
store.getProjectHostSetups.mockReturnValue([])
|
||||
store.getAllWorktreeMeta.mockReturnValue(metaById)
|
||||
store.removeWorktreeMeta.mockImplementation((worktreeId: string) => {
|
||||
delete metaById[worktreeId]
|
||||
})
|
||||
|
||||
const forgotten = await handlers['worktrees:forgetRemovedForExecutionHost'](null, {
|
||||
repoId: runtimeRepo.id,
|
||||
executionHostId: runtimeHostId,
|
||||
worktreeIds: ['repo-1::/home/orca/deleted', 'repo-1::/home/orca/other-host']
|
||||
})
|
||||
|
||||
// The row stamped to another host needs that host's own scan, not this one's.
|
||||
expect(forgotten).toEqual({ forgottenWorktreeIds: ['repo-1::/home/orca/deleted'] })
|
||||
expect(store.removeWorktreeMeta).toHaveBeenCalledExactlyOnceWith(
|
||||
'repo-1::/home/orca/deleted',
|
||||
runtimeHostId
|
||||
)
|
||||
})
|
||||
|
||||
// A repo that reaches its checkouts over SSH is not the runtime host's to condemn.
|
||||
it('refuses to retire a connection-backed repo under a runtime host id', async () => {
|
||||
const runtimeHostId = toRuntimeExecutionHostId('env-1')
|
||||
store.getRepos.mockReturnValue([
|
||||
{
|
||||
id: 'repo-1',
|
||||
path: '/home/orca/repo',
|
||||
displayName: 'repo',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
executionHostId: runtimeHostId,
|
||||
connectionId: 'target-a'
|
||||
}
|
||||
])
|
||||
store.getAllWorktreeMeta.mockReturnValue({
|
||||
'repo-1::/home/orca/deleted': makeWorktreeMeta({ hostId: runtimeHostId })
|
||||
})
|
||||
|
||||
expect(
|
||||
await handlers['worktrees:forgetRemovedForExecutionHost'](null, {
|
||||
repoId: 'repo-1',
|
||||
executionHostId: runtimeHostId,
|
||||
worktreeIds: ['repo-1::/home/orca/deleted']
|
||||
})
|
||||
).toEqual({ forgottenWorktreeIds: [] })
|
||||
expect(store.removeWorktreeMeta).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refuses to retire metadata for non-executing hosts and unowned repos', async () => {
|
||||
const sshRepo = {
|
||||
id: 'repo-1',
|
||||
path: '/remote/repo-a',
|
||||
|
||||
@@ -103,11 +103,20 @@ export function registerHostCatalogHandlers(context: WorktreeIpcContext): void {
|
||||
const requestedExecutionHostId = args?.executionHostId ?? 'ssh:'
|
||||
const worktreeIds = Array.isArray(args?.worktreeIds) ? args.worktreeIds : []
|
||||
const parsedHost = parseExecutionHostId(requestedExecutionHostId)
|
||||
if (parsedHost?.kind !== 'ssh' || worktreeIds.length === 0) {
|
||||
// Runtime hosts belong here for the same reason SSH ones do: their rows are exempt from
|
||||
// gcStaleWorktreeMeta, so a scan-proven removal is the only thing that ever retires them.
|
||||
if (
|
||||
(parsedHost?.kind !== 'ssh' && parsedHost?.kind !== 'runtime') ||
|
||||
worktreeIds.length === 0
|
||||
) {
|
||||
return nothingForgotten
|
||||
}
|
||||
const repo = findExactRepoOwner(store, args?.repoId ?? '', requestedExecutionHostId)
|
||||
if (!repo || repo.connectionId !== parsedHost.targetId) {
|
||||
// The connection must be the one the host id names, so a caller cannot retire a row belonging
|
||||
// to a repo that reaches its checkouts some other way.
|
||||
const connectionMatchesHost =
|
||||
parsedHost.kind === 'ssh' ? repo?.connectionId === parsedHost.targetId : !repo?.connectionId
|
||||
if (!repo || !connectionMatchesHost) {
|
||||
return nothingForgotten
|
||||
}
|
||||
// Why: a folder workspace's meta IS the workspace record, not a checkout row — gcStaleWorktreeMeta skips
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
// Why this file exists: a paired client's WorktreeMeta for a runtime host is exempt from
|
||||
// gcStaleWorktreeMeta (it skips any row that is not local on both the repo and the meta's hostId),
|
||||
// and `forgetPersistedWorktreeMetaForRemovals` used to bail for every non-SSH host. So the client
|
||||
// kept a row per remote worktree it had ever seen and dropped none (#17776).
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { AppState } from '../types'
|
||||
import { makeWorktree } from './worktrees-slice-test-fixtures'
|
||||
import { makeDetectedResult } from './worktrees-detected-listing-fixtures'
|
||||
import {
|
||||
createTestStore,
|
||||
forgetRemovedForExecutionHostMock,
|
||||
resetRemoteRuntimeMocks,
|
||||
resetWorktreeSliceModuleMemory,
|
||||
runtimeEnvironmentCall
|
||||
} from './worktrees-slice-test-harness'
|
||||
|
||||
const REPO_ID = 'repo-runtime'
|
||||
const HOST_ID = 'runtime:env-1'
|
||||
|
||||
const worktree = (path: string) =>
|
||||
makeWorktree({ id: `${REPO_ID}::${path}`, repoId: REPO_ID, path, hostId: HOST_ID })
|
||||
|
||||
const live = worktree('/home/orca/live')
|
||||
const deletedOnHost = worktree('/home/orca/deleted')
|
||||
|
||||
function seedClientWithBothRows(): ReturnType<typeof createTestStore> {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
settings: { activeRuntimeEnvironmentId: 'env-1' } as never,
|
||||
repos: [
|
||||
{
|
||||
id: REPO_ID,
|
||||
path: '/home/orca/repo',
|
||||
displayName: 'Runtime Repo',
|
||||
badgeColor: '#000',
|
||||
addedAt: 0,
|
||||
executionHostId: HOST_ID
|
||||
}
|
||||
],
|
||||
worktreesByRepo: { [REPO_ID]: [live, deletedOnHost] }
|
||||
} as Partial<AppState>)
|
||||
return store
|
||||
}
|
||||
|
||||
beforeEach(resetWorktreeSliceModuleMemory)
|
||||
|
||||
describe('runtime-host persisted metadata retirement', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
resetRemoteRuntimeMocks()
|
||||
})
|
||||
|
||||
it('retires metadata for rows an authoritative runtime-host scan proved gone', async () => {
|
||||
const store = seedClientWithBothRows()
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-detected',
|
||||
ok: true,
|
||||
result: makeDetectedResult(REPO_ID, [live]),
|
||||
_meta: { runtimeId: 'runtime-remote' }
|
||||
})
|
||||
|
||||
await store.getState().fetchWorktrees(REPO_ID, { executionHostId: HOST_ID })
|
||||
|
||||
expect(forgetRemovedForExecutionHostMock).toHaveBeenCalledExactlyOnceWith({
|
||||
repoId: REPO_ID,
|
||||
executionHostId: HOST_ID,
|
||||
worktreeIds: [deletedOnHost.id]
|
||||
})
|
||||
})
|
||||
|
||||
// A non-authoritative reply is a failed listing, not a report that a checkout is gone.
|
||||
it('retires nothing when the runtime host could not scan', async () => {
|
||||
const store = seedClientWithBothRows()
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-detected',
|
||||
ok: true,
|
||||
result: makeDetectedResult(REPO_ID, [live], {
|
||||
authoritative: false,
|
||||
source: 'metadata-fallback'
|
||||
}),
|
||||
_meta: { runtimeId: 'runtime-remote' }
|
||||
})
|
||||
|
||||
await store.getState().fetchWorktrees(REPO_ID, { executionHostId: HOST_ID })
|
||||
|
||||
expect(forgetRemovedForExecutionHostMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// `session-fallback` claims authoritative but is the truncated, visibility-filtered `worktree.list`
|
||||
// reply from a host too old for `worktree.detectedList`. Its omissions prove nothing.
|
||||
it('retires nothing from a legacy session-fallback listing', async () => {
|
||||
const store = seedClientWithBothRows()
|
||||
runtimeEnvironmentCall.mockResolvedValue({
|
||||
id: 'rpc-detected',
|
||||
ok: true,
|
||||
result: makeDetectedResult(REPO_ID, [live], { source: 'session-fallback' }),
|
||||
_meta: { runtimeId: 'runtime-remote' }
|
||||
})
|
||||
|
||||
await store.getState().fetchWorktrees(REPO_ID, { executionHostId: HOST_ID })
|
||||
|
||||
expect(forgetRemovedForExecutionHostMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
+6
-4
@@ -50,16 +50,18 @@ export function resetAuthoritativelyRemovedWorktreeMemoryForTests(): void {
|
||||
authoritativelyRemovedWorktreeIdsByHost.clear()
|
||||
}
|
||||
|
||||
// Why: SSH WorktreeMeta is exempt from gcStaleWorktreeMeta (persistence.ts:407,415) and outlives the remote
|
||||
// worktree, so a scan-proven removal must retire the metadata itself — otherwise the next launch's fallback
|
||||
// re-lists the deleted row before the host connects, and the in-memory suppression above is already gone.
|
||||
// Why: off-host WorktreeMeta is exempt from gcStaleWorktreeMeta -- it skips any row whose repo or hostId is
|
||||
// not local -- and outlives the remote worktree, so a scan-proven removal must retire the metadata itself.
|
||||
// Otherwise the next launch's fallback re-lists the deleted row before the host connects, and the in-memory
|
||||
// suppression above is already gone. Runtime hosts were excluded until #17776, which is why a paired client
|
||||
// accumulated a row per remote worktree it had ever seen and never dropped one.
|
||||
export function forgetPersistedWorktreeMetaForRemovals(
|
||||
repoId: string,
|
||||
hostId: ExecutionHostId,
|
||||
worktreeIds: readonly string[]
|
||||
): void {
|
||||
const parsedHost = parseExecutionHostId(hostId)
|
||||
if (worktreeIds.length === 0 || parsedHost?.kind !== 'ssh') {
|
||||
if (worktreeIds.length === 0 || (parsedHost?.kind !== 'ssh' && parsedHost?.kind !== 'runtime')) {
|
||||
return
|
||||
}
|
||||
const forget = window.api.worktrees.forgetRemovedForExecutionHost
|
||||
|
||||
@@ -254,7 +254,14 @@ export function mergeFetchedWorktrees(
|
||||
// Why: applied outside the updater so a repeated updater call cannot double-apply the removal memory.
|
||||
forgetAuthoritativelyRemovedWorktrees(args.hostId, authoritativelySeenIds)
|
||||
rememberAuthoritativelyRemovedWorktrees(args.hostId, authoritativelyRemovedIds)
|
||||
forgetPersistedWorktreeMetaForRemovals(args.repoId, args.hostId, authoritativelyRemovedIds)
|
||||
// Only a real scan retires persisted metadata. `session-fallback` also reports authoritative,
|
||||
// but it is the truncated, visibility-filtered `worktree.list` reply from a host too old for
|
||||
// `worktree.detectedList` -- its omissions are not evidence a checkout is gone.
|
||||
forgetPersistedWorktreeMetaForRemovals(
|
||||
args.repoId,
|
||||
args.hostId,
|
||||
args.refresh.result.source === 'git' ? authoritativelyRemovedIds : []
|
||||
)
|
||||
}
|
||||
return admitted
|
||||
}
|
||||
|
||||
@@ -41,9 +41,15 @@ export type HostQualifiedKnownWorktreeResult =
|
||||
executionHostId: SshExecutionHostId
|
||||
}
|
||||
|
||||
/**
|
||||
* Hosts whose persisted metadata a scan can retire: exactly those `gcStaleWorktreeMeta` skips,
|
||||
* because it only ever condemns rows that are local on both the repo and the meta's `hostId`.
|
||||
*/
|
||||
export type OffHostExecutionHostId = Extract<ExecutionHostId, `ssh:${string}` | `runtime:${string}`>
|
||||
|
||||
export type ForgetRemovedWorktreesForExecutionHostArgs = {
|
||||
repoId: string
|
||||
executionHostId: SshExecutionHostId
|
||||
executionHostId: OffHostExecutionHostId
|
||||
/** Ids an authoritative scan of this host proved gone — the only evidence that retires persisted metadata. */
|
||||
worktreeIds: readonly string[]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user