xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(worktrees): resolve a two-host project by the worktree's own host (#10634) (#10986) 

* 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.

* fix(worktrees): cover every passive path and warn once for ambiguous owners

Adversarial review found the routing fix sound but its coverage thin: only
markWorktreeUnread had an ambiguous-owner test, so restoring the throw in
clearWorktreeUnread or bumpWorktreeActivity would have reproduced the uncaught
renderer error with the suite still green. Both are now covered, verified by
mutation.

bumpWorktreeActivity also skipped silently where the other paths warned. It now
warns — but once per workspace, not per event: activity bumps fire on every PTY
event, so an unbounded warn would flood the console for exactly the users already
hitting this bug.

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin 2026-07-27 12:19:18 -07:00 committed by GitHub
parent 560f853a40
commit c140a51118
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 252 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/renderer/src/lib/worktree-operation-route.test.ts
View File

@ -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(

29
src/renderer/src/lib/worktree-operation-route.ts
View File

@ -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
}

134
src/renderer/src/store/slices/worktrees.test.ts
View File

@ -6058,6 +6058,140 @@ 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('never throws from any passive path for a genuinely ambiguous owner (#10634)', () => {
// Why every passive path, not just markWorktreeUnread: each one runs from a background
// notification, so any that still throws reproduces the uncaught renderer error.
const worktreeId = 'repo-shared::/same/path'
const ambiguousState = (): Partial<AppState> =>
({
settings: { activeRuntimeEnvironmentId: 'hub-c' } as never,
worktreesByRepo: {
'repo-shared': [
makeWorktree({
id: worktreeId,
repoId: 'repo-shared',
hostId: 'ssh:ssh-a',
runtimeOwnerEnvironmentId: 'hub-a',
isUnread: true
}),
makeWorktree({
id: worktreeId,
repoId: 'repo-shared',
hostId: 'ssh:ssh-b',
runtimeOwnerEnvironmentId: 'hub-b',
isUnread: true
})
]
}
}) as Partial<AppState>
for (const [label, run] of [
['clearWorktreeUnread', (s: AppState) => s.clearWorktreeUnread(worktreeId)],
['bumpWorktreeActivity', (s: AppState) => s.bumpWorktreeActivity(worktreeId)]
] as const) {
const store = createTestStore()
store.setState(ambiguousState())
mockApi.worktrees.updateMeta.mockClear()
expect(() => run(store.getState()), `${label} threw for an ambiguous owner`).not.toThrow()
expect(
mockApi.worktrees.updateMeta,
`${label} persisted to a guessed host`
).not.toHaveBeenCalled()
}
})
it('warns once per workspace rather than on every activity event (#10634)', () => {
// Why: activity bumps fire on every PTY event; an unbounded warn would flood the console.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
try {
const worktreeId = 'repo-spam::/same/path'
const store = createTestStore()
store.setState({
settings: { activeRuntimeEnvironmentId: 'hub-c' } as never,
worktreesByRepo: {
'repo-spam': [
makeWorktree({ id: worktreeId, repoId: 'repo-spam', hostId: 'ssh:ssh-a' }),
makeWorktree({ id: worktreeId, repoId: 'repo-spam', hostId: 'ssh:ssh-b' })
]
}
} as Partial<AppState>)
const before = warn.mock.calls.length
for (let i = 0; i < 5; i++) {
store.getState().bumpWorktreeActivity(worktreeId)
}
expect(warn.mock.calls.length - before).toBe(1)
} finally {
warn.mockRestore()
}
})
it('clearWorktreeUnread clears isUnread and persists the change', async () => {
const store = createTestStore()
const wt = makeWorktree({

110
src/renderer/src/store/slices/worktrees.ts
View File

@ -888,7 +888,7 @@ function settingsForKnownRepoOwner(
: ({ activeRuntimeEnvironmentId: null } as AppState['settings'])
}
function settingsForWorktreeOwner(
function trySettingsForWorktreeOwner(
state: Pick<
AppState,
| 'repos'
@ -903,14 +903,58 @@ 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
}
// Why: activity bumps fire on every PTY event, so an ambiguous workspace would warn continuously.
// One line per workspace is enough to diagnose it (#10634).
const ambiguousOwnerWarnedWorktreeIds = new Set<string>()
function warnAmbiguousOwnerOnce(worktreeId: string, errorLabel: string): void {
if (ambiguousOwnerWarnedWorktreeIds.has(worktreeId)) {
return
}
ambiguousOwnerWarnedWorktreeIds.add(worktreeId)
console.warn(`Skipped ${errorLabel}: workspace identity is ambiguous across hosts`, worktreeId)
}
function persistPassiveWorktreeMetaForOwner(
get: WorktreeSliceGet,
worktreeId: string,
updates: Partial<WorktreeMeta>,
errorLabel: string
): void {
const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId)
if (!ownerSettings) {
warnAmbiguousOwnerOnce(worktreeId, errorLabel)
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 +4211,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 +4330,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 +4402,12 @@ export const createWorktreeSlice: StateCreator<AppState, [], [], WorktreeSlice>
return
}
void persistWorktreeMeta(settingsForWorktreeOwner(get(), worktreeId), worktreeId, {
const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId)
if (!ownerSettings) {
warnAmbiguousOwnerOnce(worktreeId, 'persist worktree activity timestamp')
return
}
void persistWorktreeMeta(ownerSettings, worktreeId, {
lastActivityAt: now
}).catch((err) => {
if (isRuntimeSelectorNotFoundError(err)) {
@ -4804,22 +4844,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'
)
}
},