From d3475957f326f61d7032cf55d7cfe40770c24858 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:43:59 -0700 Subject: [PATCH] fix(routing): resolve unstamped local worktrees to the local host (STA-5683) (#16841) * fix(routing): resolve unstamped local worktrees * fix(routing): preserve remote worktree ownership * fix(routing): restore empty-catalog local fallback --- .../src/lib/terminal-worktree-route.test.ts | 19 +++ .../src/lib/worktree-operation-route.test.ts | 135 +++++++++++++++++- .../src/lib/worktree-operation-route.ts | 10 +- .../worktrees-fetch-owner-routing.test.ts | 30 +++- .../worktrees-fetch-remote-lineage.test.ts | 16 ++- .../slices/worktrees-lineage-state.test.ts | 18 ++- .../worktrees-remote-runtime-create.test.ts | 8 +- .../listing/worktree-host-ownership.ts | 5 +- 8 files changed, 222 insertions(+), 19 deletions(-) diff --git a/src/renderer/src/lib/terminal-worktree-route.test.ts b/src/renderer/src/lib/terminal-worktree-route.test.ts index 24255b61c02..85826d52207 100644 --- a/src/renderer/src/lib/terminal-worktree-route.test.ts +++ b/src/renderer/src/lib/terminal-worktree-route.test.ts @@ -65,6 +65,25 @@ describe('resolveTerminalWorktreeRoute', () => { }) }) + it('routes and tears down an unstamped local worktree despite an unrelated saved runtime', () => { + const ownerlessWorktree = { id: 'repo-1::/w', repoId: 'repo-1' } + const state = localState({ + repos: [{ id: 'repo-1' }], + worktreesByRepo: { 'repo-1': [ownerlessWorktree] }, + detectedWorktreesByRepo: { 'repo-1': { worktrees: [ownerlessWorktree] } }, + runtimeEnvironments: [{ id: 'saved-runtime' }], + settings: { activeRuntimeEnvironmentId: null } + } as unknown as Partial) + + expect(resolveTerminalWorktreeRoute(state, 'repo-1::/w')).toEqual({ + runtimeEnvironmentId: null + }) + expect(resolveTerminalHostOwnership(state, 'repo-1::/w', 'teardown')).toEqual({ + kind: 'local-or-ssh', + runtimeEnvironmentId: null + }) + }) + it('still fails a genuinely unknown/stale worktree closed', () => { expect(resolveTerminalWorktreeRoute(localState(), 'repo-9::/stale')).toBeNull() }) diff --git a/src/renderer/src/lib/worktree-operation-route.test.ts b/src/renderer/src/lib/worktree-operation-route.test.ts index 98878845e22..15e2cc11c6d 100644 --- a/src/renderer/src/lib/worktree-operation-route.test.ts +++ b/src/renderer/src/lib/worktree-operation-route.test.ts @@ -155,19 +155,128 @@ describe('resolveWorktreeOperationRouteResult', () => { expect(resolveWorktreeOperationRouteResult({}, WORKTREE_ID)).toEqual({ kind: 'missing' }) }) - it('fails a paired-client ownerless stale publication closed instead of routing it locally', () => { + it('routes the reported unstamped local shape with one saved runtime', () => { expect( resolveWorktreeOperationRouteResult( { repos: [{ id: 'repo-1' } as never], - runtimeEnvironments: [{ id: 'disconnected-hub' }], - worktreesByRepo: { 'repo-1': [worktree(undefined)] } + runtimeEnvironments: [{ id: 'saved-runtime' }], + runtimeEnvironmentCatalogHydrated: true, + settings: { activeRuntimeEnvironmentId: null } as never, + worktreesByRepo: { 'repo-1': [worktree(undefined)] }, + detectedWorktreesByRepo: { + 'repo-1': { worktrees: [worktree(undefined)] } + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'local', runtimeEnvironmentId: null } + }) + }) + + it('ignores the number of unrelated saved runtimes for unstamped local identity', () => { + expect( + resolveWorktreeOperationRouteResult( + { + repos: [{ id: 'repo-1' } as never], + runtimeEnvironments: [{ id: 'runtime-a' }, { id: 'runtime-b' }, { id: 'runtime-c' }], + runtimeEnvironmentCatalogHydrated: true, + settings: { activeRuntimeEnvironmentId: null } as never, + worktreesByRepo: { 'repo-1': [worktree(undefined)] }, + detectedWorktreesByRepo: { + 'repo-1': { worktrees: [worktree(undefined)] } + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'local', runtimeEnvironmentId: null } + }) + }) + + it('does not treat a repo row alone as positive local identity', () => { + expect( + resolveWorktreeOperationRouteResult( + { + repos: [{ id: 'repo-1' } as never], + runtimeEnvironments: [{ id: 'saved-runtime' }], + runtimeEnvironmentCatalogHydrated: true }, WORKTREE_ID ) ).toEqual({ kind: 'missing' }) }) + it('fails a paired-client ownerless stale publication closed instead of routing it locally', () => { + expect( + resolveWorktreeOperationRouteResult( + { + worktreesByRepo: { 'repo-1': [worktree(undefined)] }, + runtimeEnvironments: [{ id: 'saved-runtime' }], + runtimeEnvironmentCatalogHydrated: true + }, + WORKTREE_ID + ) + ).toEqual({ kind: 'missing' }) + }) + + it('keeps an unstamped row ambiguous when another owner names a different host', () => { + expect( + resolveWorktreeOperationRouteResult( + { + repos: [{ id: 'repo-1' } as never], + runtimeEnvironments: [{ id: 'saved-runtime' }], + runtimeEnvironmentCatalogHydrated: true, + worktreesByRepo: { + 'repo-1': [worktree('local'), worktree('runtime:hub-a')] + } + }, + WORKTREE_ID + ) + ).toEqual({ kind: 'ambiguous' }) + }) + + it('routes a focused runtime only when it is the single saved runtime', () => { + expect( + resolveWorktreeOperationRouteResult( + { + settings: { activeRuntimeEnvironmentId: 'hub-a' } as never, + runtimeEnvironments: [{ id: 'hub-a' }], + runtimeEnvironmentCatalogHydrated: true, + worktreesByRepo: { 'repo-1': [worktree(undefined)] } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'runtime:hub-a', runtimeEnvironmentId: 'hub-a' } + }) + }) + + it('ignores unrelated removed-runtime tombstones for positive local identity', () => { + expect( + resolveWorktreeOperationRouteResult( + { + repos: [{ id: 'repo-1' } as never], + runtimeEnvironments: [{ id: 'saved-runtime' }], + runtimeEnvironmentCatalogHydrated: true, + removedRuntimeEnvironmentIds: new Set(['removed-runtime']), + worktreesByRepo: { 'repo-1': [worktree(undefined)] }, + detectedWorktreesByRepo: { + 'repo-1': { worktrees: [worktree(undefined)] } + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'local', runtimeEnvironmentId: null } + }) + }) + it('fails ownerless rows closed until the saved-runtime catalog is hydrated', () => { expect( resolveWorktreeOperationRouteResult( @@ -198,6 +307,26 @@ describe('resolveWorktreeOperationRouteResult', () => { }) it('preserves ownerless local compatibility after an empty catalog hydrates', () => { + expect( + resolveWorktreeOperationRouteResult( + { + repos: [{ id: 'repo-1' } as never], + runtimeEnvironments: [], + runtimeEnvironmentCatalogHydrated: true, + worktreesByRepo: { 'repo-1': [worktree(undefined)] }, + detectedWorktreesByRepo: { + 'repo-1': { worktrees: [worktree(undefined)] } + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'local', runtimeEnvironmentId: null } + }) + }) + + it('preserves ownerless local compatibility before detected scan with no saved runtimes', () => { expect( resolveWorktreeOperationRouteResult( { diff --git a/src/renderer/src/lib/worktree-operation-route.ts b/src/renderer/src/lib/worktree-operation-route.ts index d0660426175..7a56a23c4af 100644 --- a/src/renderer/src/lib/worktree-operation-route.ts +++ b/src/renderer/src/lib/worktree-operation-route.ts @@ -192,9 +192,10 @@ export function resolveWorktreeOperationRouteResult( return explicitResolution } + const hasDetectedWorktree = hasIndexedDetectedWorktree(state.detectedWorktreesByRepo, worktreeId) const hasKnownWorktree = resolveIndexedWorktreeOwner(state.worktreesByRepo, worktreeId).kind !== 'missing' || - hasIndexedDetectedWorktree(state.detectedWorktreesByRepo, worktreeId) + hasDetectedWorktree const repoId = getRepoIdFromWorktreeId(worktreeId) const hasKnownRepo = state.repos?.some((repo) => repo.id === repoId) === true if (!hasKnownWorktree && !hasKnownRepo) { @@ -219,10 +220,11 @@ export function resolveWorktreeOperationRouteResult( } } } + // Why: no saved runtime can publish a remote ownerless row; otherwise current detected presence affirms identity under the stamped-writer invariant. const mayBeLegacyLocal = - (savedRuntimeIds === undefined || - (state.runtimeEnvironmentCatalogHydrated === true && savedRuntimeIds.length === 0)) && - (state.removedRuntimeEnvironmentIds?.size ?? 0) === 0 + savedRuntimeIds === undefined || + (state.runtimeEnvironmentCatalogHydrated === true && + (savedRuntimeIds.length === 0 || hasDetectedWorktree)) return mayBeLegacyLocal ? { kind: 'resolved', route: { executionHostId: 'local', runtimeEnvironmentId: null } } : { kind: 'missing' } diff --git a/src/renderer/src/store/slices/worktrees-fetch-owner-routing.test.ts b/src/renderer/src/store/slices/worktrees-fetch-owner-routing.test.ts index 9ffc48eade5..b92879e32bc 100644 --- a/src/renderer/src/store/slices/worktrees-fetch-owner-routing.test.ts +++ b/src/renderer/src/store/slices/worktrees-fetch-owner-routing.test.ts @@ -45,7 +45,7 @@ describe('fetchWorktrees', () => { clearHugeRepoWarningDismissalsForTests() }) - it('fetches worktrees from the active remote runtime environment', async () => { + it('stamps ownerless worktrees from an older active remote runtime before repos hydrate', async () => { const store = createTestStore() const remote = makeWorktree({ id: 'repo1::/remote/wt1', @@ -63,7 +63,15 @@ describe('fetchWorktrees', () => { await store.getState().fetchWorktrees('repo1') - expect(store.getState().worktreesByRepo.repo1).toEqual([remote]) + const expected = { + ...remote, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + } + expect(store.getState().worktreesByRepo.repo1).toEqual([expected]) + expect(store.getState().detectedWorktreesByRepo.repo1?.worktrees).toEqual([ + expect.objectContaining(expected) + ]) expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ selector: 'env-1', method: 'worktree.detectedList', @@ -581,12 +589,26 @@ describe('fetchWorktrees', () => { await store.getState().fetchWorktrees('repo1') - expect(store.getState().worktreesByRepo.repo1).toEqual([remote]) + expect(store.getState().worktreesByRepo.repo1).toEqual([ + { + ...remote, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + } + ]) expect(store.getState().detectedWorktreesByRepo.repo1).toMatchObject({ repoId: 'repo1', authoritative: true, source: 'session-fallback', - worktrees: [{ id: remote.id, ownership: 'orca-managed', visible: true }] + worktrees: [ + { + id: remote.id, + ownership: 'orca-managed', + visible: true, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + } + ] }) expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ selector: 'env-1', diff --git a/src/renderer/src/store/slices/worktrees-fetch-remote-lineage.test.ts b/src/renderer/src/store/slices/worktrees-fetch-remote-lineage.test.ts index ee30d3a9233..a7e4dff8a27 100644 --- a/src/renderer/src/store/slices/worktrees-fetch-remote-lineage.test.ts +++ b/src/renderer/src/store/slices/worktrees-fetch-remote-lineage.test.ts @@ -44,7 +44,9 @@ describe('fetchWorktrees', () => { id: 'repo1::/remote/wt1', repoId: 'repo1', path: '/remote/wt1', - branch: 'refs/heads/remote' + branch: 'refs/heads/remote', + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' }) const lineage = makeLineage({ worktreeId: initial.id }) const refreshed = { ...initial, lineage } @@ -100,7 +102,9 @@ describe('fetchWorktrees', () => { id: 'repo1::/remote/wt1', repoId: 'repo1', path: '/remote/wt1', - branch: 'refs/heads/remote' + branch: 'refs/heads/remote', + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' }) const staleLineage = makeLineage({ worktreeId: worktree.id, @@ -292,7 +296,13 @@ describe('fetchWorktrees', () => { await store.getState().fetchWorktrees('repo1') - expect(store.getState().worktreesByRepo.repo1).toEqual([refreshed]) + expect(store.getState().worktreesByRepo.repo1).toEqual([ + { + ...refreshed, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + } + ]) expect(store.getState().worktreeLineageById).toEqual({ [staleLineage.worktreeId]: staleLineage }) diff --git a/src/renderer/src/store/slices/worktrees-lineage-state.test.ts b/src/renderer/src/store/slices/worktrees-lineage-state.test.ts index d0d3587393c..288fce9596f 100644 --- a/src/renderer/src/store/slices/worktrees-lineage-state.test.ts +++ b/src/renderer/src/store/slices/worktrees-lineage-state.test.ts @@ -453,7 +453,11 @@ describe('worktree lineage state', () => { }) expect(mockApi.worktrees.updateLineage).not.toHaveBeenCalled() expect(store.getState().worktreeLineageById).toEqual({ [lineage.worktreeId]: lineage }) - expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual(updatedChild) + expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual({ + ...updatedChild, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + }) expect(store.getState().sortEpoch).toBe(4) }) @@ -537,7 +541,11 @@ describe('worktree lineage state', () => { }) expect(mockApi.worktrees.updateLineage).not.toHaveBeenCalled() expect(store.getState().worktreeLineageById).toEqual({ [lineage.worktreeId]: lineage }) - expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual(updatedChild) + expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual({ + ...updatedChild, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + }) expect(store.getState().sortEpoch).toBe(4) runtimeEnvironmentCall @@ -701,7 +709,11 @@ describe('worktree lineage state', () => { timeoutMs: 15_000 }) expect(store.getState().worktreeLineageById).toEqual({}) - expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual(updatedChild) + expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual({ + ...updatedChild, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + }) }) // An unresolvable owner route must reach the caller so the sidebar can toast it, rather than diff --git a/src/renderer/src/store/slices/worktrees-remote-runtime-create.test.ts b/src/renderer/src/store/slices/worktrees-remote-runtime-create.test.ts index de0d6101072..09b3374cf1e 100644 --- a/src/renderer/src/store/slices/worktrees-remote-runtime-create.test.ts +++ b/src/renderer/src/store/slices/worktrees-remote-runtime-create.test.ts @@ -90,7 +90,13 @@ describe('worktree remote runtime mutations', () => { timeoutMs: 10 * 60_000 }) expect(mockApi.worktrees.create).not.toHaveBeenCalled() - expect(store.getState().worktreesByRepo.repo1).toEqual([wt]) + expect(store.getState().worktreesByRepo.repo1).toEqual([ + { + ...wt, + hostId: 'runtime:env-1', + runtimeOwnerEnvironmentId: 'env-1' + } + ]) }) it('forwards generated-name provenance through paired-runtime create', async () => { diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-host-ownership.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-host-ownership.ts index d5fe96141b9..a70e6b28af0 100644 --- a/src/renderer/src/store/slices/worktrees/listing/worktree-host-ownership.ts +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-host-ownership.ts @@ -6,6 +6,7 @@ import { reuseEqualCatalogRows } from '../../worktree-catalog-reconciliation' import { getRepoIdFromWorktreeId } from '../../worktree-helpers' import { getRepoExecutionHostId, + getSettingsFocusedExecutionHostId, LOCAL_EXECUTION_HOST_ID, parseExecutionHostId, toSshExecutionHostId, @@ -60,7 +61,9 @@ export function repoHostId( if (repo) { return getRepoExecutionHostId(repo) } - return hostId && parseExecutionHostId(hostId) ? hostId : LOCAL_EXECUTION_HOST_ID + return hostId && parseExecutionHostId(hostId) + ? hostId + : getSettingsFocusedExecutionHostId(state.settings) } export function repoHasExactlyOneExecutionHostOwner(