From 23cdb306befd554424fb64f870bd56c6ef2a0152 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:21:32 -0700 Subject: [PATCH] fix(worktrees): resolve a two-host project by the worktree's own host (#10634) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A project registered on both a local host and an SSH host permanently poisoned every one of its workspaces with "Workspace identity is ambiguous across hosts. Refresh projects and try again." Refresh could never help: nothing was stale, both host setups were valid and intentional. The error survived restarts. The ambiguity was manufactured. `resolveExactWorktreeRoute` starts from a worktree that already carries exactly one `hostId`, then throws that away and asks `resolveIndexedRepoOperationRoute` which host owns the *repo* — a question with two right answers once a project spans hosts. Only the project spans hosts; each worktree never does. Route resolution now filters repo setups to the ones matching the worktree's own host before looking for a transport, so a two-host project resolves as cleanly as a one-host project. Genuine ambiguity still returns `ambiguous`. Second half: the error escaped as an *uncaught renderer error* because passive background paths — unread marking, activity bumps — called a helper that threw. Those callers now degrade: `trySettingsForWorktreeOwner` returns null, the passive update is skipped with a warning, and local state stays consistent. Explicit user actions still surface the error. --- .../src/lib/worktree-operation-route.test.ts | 20 ++++ .../src/lib/worktree-operation-route.ts | 29 +++++- .../src/store/slices/worktrees.test.ts | 64 ++++++++++++ src/renderer/src/store/slices/worktrees.ts | 97 +++++++++++-------- 4 files changed, 169 insertions(+), 41 deletions(-) diff --git a/src/renderer/src/lib/worktree-operation-route.test.ts b/src/renderer/src/lib/worktree-operation-route.test.ts index acea0c918fe..fe965d9aad0 100644 --- a/src/renderer/src/lib/worktree-operation-route.test.ts +++ b/src/renderer/src/lib/worktree-operation-route.test.ts @@ -77,6 +77,26 @@ describe('resolveWorktreeOperationRouteResult', () => { ).toEqual({ kind: 'ambiguous' }) }) + it('resolves a host-stamped SSH worktree when its project also exists locally (#10634)', () => { + expect( + resolveWorktreeOperationRouteResult( + { + repos: [ + { id: 'repo-1', executionHostId: 'local' }, + { id: 'repo-1', connectionId: 'ssh-1', executionHostId: 'ssh:ssh-1' } + ], + worktreesByRepo: { + 'repo-1': [worktree('ssh:ssh-1')] + } + }, + WORKTREE_ID + ) + ).toEqual({ + kind: 'resolved', + route: { executionHostId: 'ssh:ssh-1', runtimeEnvironmentId: null } + }) + }) + it('deduplicates identical projections from the same HUB', () => { expect( resolveWorktreeOperationRouteResult( diff --git a/src/renderer/src/lib/worktree-operation-route.ts b/src/renderer/src/lib/worktree-operation-route.ts index 2028686e60d..04468c5e995 100644 --- a/src/renderer/src/lib/worktree-operation-route.ts +++ b/src/renderer/src/lib/worktree-operation-route.ts @@ -2,6 +2,7 @@ import type { AppState } from '@/store/types' import { getRepoExecutionHostId, parseExecutionHostId, + toSshExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' @@ -71,6 +72,31 @@ function addRoute( routes.set(JSON.stringify(route), route) } +function resolveRepoRouteForSshOwner( + repos: WorktreeOperationRouteState['repos'], + owner: WorktreeOperationOwnerRecord +): WorktreeOperationRouteResolution { + if (!repos || !owner.hostId) { + return { kind: 'missing' } + } + const routes = new Map() + for (const repo of repos) { + if (repo.id !== owner.repoId) { + continue + } + const connectionHostId = repo.connectionId ? toSshExecutionHostId(repo.connectionId) : null + if (getRepoExecutionHostId(repo) !== owner.hostId && connectionHostId !== owner.hostId) { + continue + } + addRoute(routes, routeForOwner({ hostId: getRepoExecutionHostId(repo) })) + } + const route = routes.values().next().value + if (routes.size === 1 && route) { + return { kind: 'resolved', route } + } + return routes.size > 1 ? { kind: 'ambiguous' } : { kind: 'missing' } +} + function resolveExactWorktreeRoute( state: WorktreeOperationRouteState, owner: WorktreeOperationOwnerRecord @@ -82,7 +108,8 @@ function resolveExactWorktreeRoute( if (route.runtimeEnvironmentId || parseExecutionHostId(route.executionHostId)?.kind !== 'ssh') { return { kind: 'resolved', route } } - const repoRoute = resolveIndexedRepoOperationRoute(state.repos, owner.repoId) + // Recover an optional HUB transport only from the repo setup matching the worktree's SSH host. + const repoRoute = resolveRepoRouteForSshOwner(state.repos, owner) if (repoRoute.kind === 'ambiguous') { return repoRoute } diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 4f8587db486..2c38abb4ec2 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -6058,6 +6058,70 @@ describe('worktree unread (show-until-interact)', () => { ) }) + it('routes multi-host project unread persistence by the worktree host (#10634)', () => { + const store = createTestStore() + const wt = makeWorktree({ + id: 'repo-shared::/home/user/wt', + repoId: 'repo-shared', + path: '/home/user/wt', + hostId: 'ssh:ssh-1' + }) + store.setState({ + repos: [ + { id: 'repo-shared', path: '/local', displayName: 'Local', badgeColor: '#000', addedAt: 0 }, + { + id: 'repo-shared', + path: '/home/user/repo', + displayName: 'SSH', + badgeColor: '#111', + addedAt: 1, + connectionId: 'ssh-1' + } + ], + worktreesByRepo: { 'repo-shared': [wt] } + } as Partial) + + expect(() => store.getState().markWorktreeUnread(wt.id)).not.toThrow() + + expect(store.getState().worktreesByRepo['repo-shared'][0].isUnread).toBe(true) + expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith( + expect.objectContaining({ + worktreeId: wt.id, + updates: expect.objectContaining({ isUnread: true }) + }) + ) + }) + + it('keeps unread state local instead of throwing for genuinely ambiguous owners (#10634)', () => { + const store = createTestStore() + const worktreeId = 'repo-shared::/same/path' + store.setState({ + settings: { activeRuntimeEnvironmentId: 'hub-c' } as never, + worktreesByRepo: { + 'repo-shared': [ + makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'ssh:ssh-a', + runtimeOwnerEnvironmentId: 'hub-a' + }), + makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'ssh:ssh-b', + runtimeOwnerEnvironmentId: 'hub-b' + }) + ] + } + } as Partial) + + expect(() => store.getState().markWorktreeUnread(worktreeId)).not.toThrow() + + expect(store.getState().worktreesByRepo['repo-shared'][0].isUnread).toBe(true) + expect(mockApi.worktrees.updateMeta).not.toHaveBeenCalled() + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + it('clearWorktreeUnread clears isUnread and persists the change', async () => { const store = createTestStore() const wt = makeWorktree({ diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index b10674821b6..f15ac3e6fc6 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -888,7 +888,7 @@ function settingsForKnownRepoOwner( : ({ activeRuntimeEnvironmentId: null } as AppState['settings']) } -function settingsForWorktreeOwner( +function trySettingsForWorktreeOwner( state: Pick< AppState, | 'repos' @@ -903,14 +903,46 @@ function settingsForWorktreeOwner( | 'removedRuntimeEnvironmentIds' >, worktreeId: string -) { +): AppState['settings'] | null { const route = resolveWorktreeOperationRoute(state, worktreeId) if (!route) { - throw new Error(WORKTREE_REMOVAL_AMBIGUOUS_ERROR) + return null } return settingsForWorktreeOperationRoute(state.settings, route) } +function settingsForWorktreeOwner( + state: Parameters[0], + worktreeId: string +) { + const settings = trySettingsForWorktreeOwner(state, worktreeId) + if (!settings) { + throw new Error(WORKTREE_REMOVAL_AMBIGUOUS_ERROR) + } + return settings +} + +function persistPassiveWorktreeMetaForOwner( + get: WorktreeSliceGet, + worktreeId: string, + updates: Partial, + errorLabel: string +): void { + const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId) + if (!ownerSettings) { + console.warn(`Skipped ${errorLabel}: workspace identity is ambiguous across hosts`, worktreeId) + return + } + void persistWorktreeMeta(ownerSettings, worktreeId, updates).catch((err) => { + if (isRuntimeSelectorNotFoundError(err)) { + void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) + return + } + console.error(`Failed to ${errorLabel}:`, err) + void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) + }) +} + async function listDetectedWorktreesForRepo( settings: AppState['settings'], repoId: string, @@ -4167,17 +4199,12 @@ export const createWorktreeSlice: StateCreator return } - void persistWorktreeMeta(settingsForWorktreeOwner(get(), worktreeId), worktreeId, { - isUnread: true, - lastActivityAt: now - }).catch((err) => { - if (isRuntimeSelectorNotFoundError(err)) { - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - return - } - console.error('Failed to persist unread worktree state:', err) - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - }) + persistPassiveWorktreeMetaForOwner( + get, + worktreeId, + { isUnread: true, lastActivityAt: now }, + 'persist unread worktree state' + ) }, observeTerminalGitHubPullRequestLink: (worktreeId, link) => { @@ -4291,16 +4318,12 @@ export const createWorktreeSlice: StateCreator return } - void persistWorktreeMeta(settingsForWorktreeOwner(get(), worktreeId), worktreeId, { - isUnread: false - }).catch((err) => { - if (isRuntimeSelectorNotFoundError(err)) { - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - return - } - console.error('Failed to persist cleared unread worktree state:', err) - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - }) + persistPassiveWorktreeMetaForOwner( + get, + worktreeId, + { isUnread: false }, + 'persist cleared unread worktree state' + ) }, bumpWorktreeActivity: (worktreeId) => { @@ -4367,7 +4390,11 @@ export const createWorktreeSlice: StateCreator return } - void persistWorktreeMeta(settingsForWorktreeOwner(get(), worktreeId), worktreeId, { + const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId) + if (!ownerSettings) { + return + } + void persistWorktreeMeta(ownerSettings, worktreeId, { lastActivityAt: now }).catch((err) => { if (isRuntimeSelectorNotFoundError(err)) { @@ -4804,22 +4831,12 @@ export const createWorktreeSlice: StateCreator void get().updateFolderWorkspace(workspaceScope.folderWorkspaceId, { isUnread: false }) return } - const updates: Partial = { - isUnread: false - } - - void persistWorktreeMeta( - settingsForWorktreeOwner(get(), worktreeId), + persistPassiveWorktreeMetaForOwner( + get, worktreeId, - updates - ).catch((err) => { - if (isRuntimeSelectorNotFoundError(err)) { - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - return - } - console.error('Failed to persist worktree activation state:', err) - void get().fetchWorktrees(getRepoIdFromWorktreeId(worktreeId)) - }) + { isUnread: false }, + 'persist worktree activation state' + ) } },