diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index d9d5334016b..02c0d0939f1 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -334,6 +334,7 @@ "runtimeScopeForbiddenTitle": "This connection has limited (mobile) access", "runtimeScopeForbiddenDescription": "Workspaces are unavailable on a mobile-scope pairing. Reconnect using the browser access link from Settings → Runtime Environments → Share this Orca server.", "a17f4d2e93": "Could not update this workspace.", + "preservedBranchCleanupHostAmbiguous": "Multiple preserved branch cleanups are pending for \"{{value0}}\"; specify the host.", "metadata": { "worktree": { "meta": { 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 5f1b3d10886..d007192d18a 100644 --- a/src/renderer/src/store/slices/worktrees-lineage-state.test.ts +++ b/src/renderer/src/store/slices/worktrees-lineage-state.test.ts @@ -701,4 +701,63 @@ describe('worktree lineage state', () => { expect(store.getState().worktreeLineageById).toEqual({}) expect(store.getState().worktreesByRepo.repo1?.[0]).toEqual(updatedChild) }) + + // The sidebar's remove-parent-link action awaits updateWorktreeLineage without a catch, + // so every path out of it must resolve rather than reject. + it('skips the lineage update for a genuinely ambiguous owner instead of rejecting', async () => { + 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) + + await expect( + store.getState().updateWorktreeLineage(worktreeId, { noParent: true }) + ).resolves.toBeUndefined() + + expect(mockApi.worktrees.updateLineage).not.toHaveBeenCalled() + }) + + it('resolves when the update fails and the recovery lineage refresh fails too', async () => { + const lineage = makeLineage() + const store = createLocalLineageTestStore(lineage) + mockApi.worktrees.updateLineage.mockRejectedValueOnce(new Error('unnest failed')) + mockApi.worktrees.listLineage.mockRejectedValueOnce(new Error('refresh failed')) + vi.spyOn(console, 'error').mockImplementation(() => {}) + + await expect( + store.getState().updateWorktreeLineage(lineage.worktreeId, { noParent: true }) + ).resolves.toBeUndefined() + }) + + it('rethrows the original assign failure when the recovery refresh fails', async () => { + const lineage = makeLineage() + const store = createLocalLineageTestStore(lineage) + mockApi.worktrees.updateLineage.mockRejectedValueOnce(new Error('stale parent')) + mockApi.worktrees.listLineage.mockRejectedValueOnce(new Error('refresh failed')) + vi.spyOn(console, 'error').mockImplementation(() => {}) + + // A failed recovery refresh must not replace the cause the caller toasts. + await expect( + store.getState().assignWorktreeParent(lineage.worktreeId, { + parentWorktreeId: lineage.parentWorktreeId + }) + ).rejects.toThrow('stale parent') + }) }) diff --git a/src/renderer/src/store/slices/worktrees/metadata/worktree-lineage-actions.ts b/src/renderer/src/store/slices/worktrees/metadata/worktree-lineage-actions.ts index cbfe0da9cb5..ae40e7eba2e 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/worktree-lineage-actions.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/worktree-lineage-actions.ts @@ -7,7 +7,23 @@ import { refreshWorktreeLineageForSettings, setWorktreeLineageForRuntime } from './worktree-lineage-refresh' -import { settingsForWorktreeOwner } from '../listing/worktree-owner-settings' +import { + settingsForWorktreeOwner, + trySettingsForWorktreeOwner, + warnAmbiguousOwnerOnce +} from '../listing/worktree-owner-settings' + +// Why: this runs inside a catch, so letting the refresh reject would replace the failure it recovers from. +async function refreshWorktreeLineageBestEffort( + ownerSettings: AppState['settings'], + set: WorktreeSliceSet +): Promise { + try { + await refreshWorktreeLineageForSettings(ownerSettings, set) + } catch (err) { + console.error('Failed to refresh worktree lineage after a failed write:', err) + } +} export function createFetchWorktreeLineage( set: WorktreeSliceSet, @@ -43,7 +59,13 @@ export function createUpdateWorktreeLineage( get: WorktreeSliceGet ): WorktreeSlice['updateWorktreeLineage'] { return async (worktreeId, args) => { - const ownerSettings = settingsForWorktreeOwner(get(), worktreeId) + // Why: this action never rejects — the sidebar's remove-parent-link caller awaits it without a catch, + // so an ambiguous owner is a skip rather than an unhandled rejection. + const ownerSettings = trySettingsForWorktreeOwner(get(), worktreeId) + if (!ownerSettings) { + warnAmbiguousOwnerOnce(worktreeId, 'worktree lineage update') + return + } try { applyWorktreeLineageUpdate( set, @@ -52,7 +74,7 @@ export function createUpdateWorktreeLineage( ) } catch (err) { console.error('Failed to update worktree lineage:', err) - await refreshWorktreeLineageForSettings(ownerSettings, set) + await refreshWorktreeLineageBestEffort(ownerSettings, set) } } } @@ -71,7 +93,8 @@ export function createAssignWorktreeParent( ) } catch (err) { console.error('Failed to assign worktree parent:', err) - await refreshWorktreeLineageForSettings(ownerSettings, set) + // Unlike the update path this rethrows, so the recovery refresh must not mask the original cause. + await refreshWorktreeLineageBestEffort(ownerSettings, set) throw err } } diff --git a/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts b/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts index 88264944161..d00df69e8c2 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/force-delete-preserved-branch.ts @@ -44,9 +44,15 @@ export function createForceDeletePreservedBranch( throw new Error(`No preserved branch cleanup is pending for "${branchName}".`) } // Ambiguous route: deleting against the active runtime could hit the wrong host's branch. + // Localized because it surfaces in the toast below; the throw above mirrors a main-process + // message verbatim (orca-runtime.ts, ipc/worktrees.ts) and must stay in sync with it. if (!retainedTarget && matchingRetainedTargets.length > 1) { throw new Error( - `Multiple preserved branch cleanups are pending for "${branchName}"; specify the host.` + translate( + 'auto.store.slices.worktrees.preservedBranchCleanupHostAmbiguous', + 'Multiple preserved branch cleanups are pending for "{{value0}}"; specify the host.', + { value0: branchName } + ) ) } const cleanupHostId = options?.hostId ?? retainedTarget?.cleanup.hostId diff --git a/src/renderer/src/store/slices/worktrees/teardown/orphaned-runtime-ssh-project-purge.ts b/src/renderer/src/store/slices/worktrees/teardown/orphaned-runtime-ssh-project-purge.ts index cfa5d063173..428479f8623 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/orphaned-runtime-ssh-project-purge.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/orphaned-runtime-ssh-project-purge.ts @@ -9,10 +9,11 @@ export async function purgeOrphanedRuntimeSshProjects( if (destroyedSshTargetIds.length === 0) { return } - // Drop blanks so a repo with no connectionId never matches below. - const destroyedTargetIds = new Set(destroyedSshTargetIds.filter((id) => id !== '')) + // Drop blanks once, before both lookups, so a repo with no connectionId never matches below. + const purgeableSshTargetIds = destroyedSshTargetIds.filter((id) => id !== '') + const destroyedTargetIds = new Set(purgeableSshTargetIds) const destroyedHostIds = new Set( - destroyedSshTargetIds.map((id) => toSshExecutionHostId(id)) + purgeableSshTargetIds.map((id) => toSshExecutionHostId(id)) ) const orphanedSetupIds = get() .projectHostSetups.filter((setup) => destroyedHostIds.has(setup.hostId))