mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(worktrees): resolve a two-host project by the worktree's own host (#10634)
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.
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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<string, WorktreeOperationRoute>()
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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<AppState>)
|
||||
|
||||
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<AppState>)
|
||||
|
||||
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({
|
||||
|
||||
@@ -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<typeof trySettingsForWorktreeOwner>[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<WorktreeMeta>,
|
||||
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<AppState, [], [], WorktreeSlice>
|
||||
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<AppState, [], [], WorktreeSlice>
|
||||
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<AppState, [], [], WorktreeSlice>
|
||||
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<AppState, [], [], WorktreeSlice>
|
||||
void get().updateFolderWorkspace(workspaceScope.folderWorkspaceId, { isUnread: false })
|
||||
return
|
||||
}
|
||||
const updates: Partial<WorktreeMeta> = {
|
||||
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'
|
||||
)
|
||||
}
|
||||
},
|
||||
|
||||
|
||||
Reference in New Issue
Block a user