From cdeb993db3090f7a0ef5b9fdce4bc52003e182f8 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:36:42 -0700 Subject: [PATCH] fix(sidebar): stabilize cross-host project grouping --- .../sidebar/worktree-list-groups.test.ts | 137 +++++++++++++++++- .../sidebar/worktree-list-groups.ts | 62 +++++--- src/renderer/src/i18n/locales/en.json | 5 + .../src/store/slices/runtime-status.test.ts | 100 ++++++++++++- .../src/store/slices/runtime-status.ts | 87 ++++++++++- 5 files changed, 359 insertions(+), 32 deletions(-) diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts index 7d6dae4770c..c5a7f9606b2 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.test.ts @@ -765,7 +765,7 @@ describe('buildRows with pinned worktrees', () => { ) }) - it('splits all project setups when one host has duplicate checkouts', () => { + it('splits only the surface with duplicate checkouts', () => { const localRepoB: Repo = { ...repo, id: 'repo-local-b', @@ -822,10 +822,143 @@ describe('buildRows with pinned worktrees', () => { expect(headers.map((row) => row.key)).toEqual([ 'project:github:stablyai/orca::setup:repo-1', 'project:github:stablyai/orca::setup:repo-local-b', - 'project:github:stablyai/orca::setup:repo-remote' + 'project:github:stablyai/orca' ]) }) + it('counts projection twins for one directory as one checkout', () => { + const runtimeHostId = 'runtime:m2-air' + const runtimeRepo: Repo = { + ...remoteRepo, + id: 'repo-runtime', + path: '/home/alice/orca-runtime', + connectionId: undefined, + executionHostId: runtimeHostId + } + const runtimeWorktree: Worktree = { + ...remoteWorktree, + id: 'wt-runtime', + repoId: runtimeRepo.id, + path: '/home/alice/orca-runtime-feature' + } + const runtimeSetup: ProjectHostSetup = { + ...projectHostSetups[1]!, + id: runtimeRepo.id, + repoId: runtimeRepo.id, + path: runtimeRepo.path, + hostId: runtimeHostId, + executionHostId: runtimeHostId + } + const derivedSetup: ProjectHostSetup = { + ...projectHostSetups[1]!, + hostId: runtimeHostId, + connectionId: 'intel mac', + executionHostId: runtimeHostId + } + const authoritativeSetup: ProjectHostSetup = { + ...derivedSetup, + id: 'setup-authoritative', + path: `${derivedSetup.path}/`, + connectionId: null, + executionHostId: 'ssh:intel%20mac' + } + const grouping = { + projects: [{ ...project, sourceRepoIds: [remoteRepo.id, runtimeRepo.id] }], + projectHostSetups: [runtimeSetup, derivedSetup, authoritativeSetup] + } + + expect([ + getGroupKeyForWorktree( + 'repo', + remoteWorktree, + new Map([[remoteRepo.id, remoteRepo]]), + null, + undefined, + undefined, + grouping + ), + getGroupKeyForWorktree( + 'repo', + runtimeWorktree, + new Map([[runtimeRepo.id, runtimeRepo]]), + null, + undefined, + undefined, + grouping + ) + ]).toEqual(['project:github:stablyai/orca', 'project:github:stablyai/orca']) + }) + + it('keeps Git hosts grouped when folder setups share the project identity', () => { + const windowsHostId = 'runtime:windows-server' + const windowsRepo: Repo = { + ...repo, + id: 'repo-windows', + path: 'C:\\Users\\neil\\orca\\orca', + executionHostId: windowsHostId + } + const folderRepoA: Repo = { + ...repo, + id: 'folder-qa-a', + path: '/tmp/pr11751-folder-qa', + displayName: 'pr11751-folder-qa', + kind: 'folder' + } + const folderRepoB: Repo = { + ...folderRepoA, + id: 'folder-qa-b', + path: '/tmp/pr11767-runtime-folder', + displayName: 'pr11767-runtime-folder' + } + const setups: ProjectHostSetup[] = [ + projectHostSetups[0]!, + { ...projectHostSetups[0]!, id: 'projection-twin' }, + projectHostSetups[1]!, + { + ...projectHostSetups[0]!, + id: windowsRepo.id, + repoId: windowsRepo.id, + path: windowsRepo.path, + hostId: windowsHostId, + executionHostId: windowsHostId + }, + { + ...projectHostSetups[0]!, + id: folderRepoA.id, + repoId: folderRepoA.id, + path: folderRepoA.path, + displayName: folderRepoA.displayName, + kind: 'folder' + }, + { + ...projectHostSetups[0]!, + id: folderRepoB.id, + repoId: folderRepoB.id, + path: folderRepoB.path, + displayName: folderRepoB.displayName, + kind: 'folder' + } + ] + const repos = [repo, remoteRepo, windowsRepo, folderRepoA, folderRepoB] + const grouping = { + projects: [{ ...project, sourceRepoIds: repos.map((entry) => entry.id) }], + projectHostSetups: setups + } + + const groupKeys = repos.map((entry) => + getGroupKeyForWorktree( + 'repo', + { ...worktree, id: `wt-${entry.id}`, repoId: entry.id, path: entry.path }, + new Map([[entry.id, entry]]), + null, + undefined, + undefined, + grouping + ) + ) + expect(new Set(groupKeys)).toEqual(new Set(['project:github:stablyai/orca'])) + }) + it('keeps a provisioned runtime copy under the project header alongside a same-host checkout', () => { const runtimeRepoB: Repo = { ...repo, diff --git a/src/renderer/src/components/sidebar/worktree-list-groups.ts b/src/renderer/src/components/sidebar/worktree-list-groups.ts index cfc0955a398..777e9fcfbe6 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups.ts @@ -39,10 +39,14 @@ import { LOCAL_EXECUTION_HOST_ID, getRepoExecutionHostId, getWorktreeExecutionHostId, + toSshExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { parseWslUncPath } from '../../../../shared/wsl-paths' -import { isWindowsAbsolutePathLike } from '../../../../shared/cross-platform-path' +import { + isWindowsAbsolutePathLike, + normalizeRuntimePathForComparison +} from '../../../../shared/cross-platform-path' import { getCyclicProjectedWorktreeLineageIds, getLineageRenderInfo @@ -177,28 +181,43 @@ type WorktreeGroupEntry = { type ProjectGroupingIndex = { projectById: Map setupByRepoId: Map - projectIdsRequiringSetupGroups: Set + surfaceKeysRequiringSetupGroups: Set } const projectGroupingIndexCache = new WeakMap() -// Why: `provisioned` setups are ephemeral recipe-created runtime copies that -// nest under the project header; every other method is a real user checkout. See #5374. +// Why: provisioned and folder setups are not independent Git checkouts. function isDistinctUserCheckout(setup: ProjectHostSetup): boolean { - return setup.setupMethod !== 'provisioned' + return setup.setupMethod !== 'provisioned' && setup.kind !== 'folder' } +// Why: execution target and filesystem namespace independently identify a surface. function getProjectSetupSurfaceKey(setup: ProjectHostSetup): string { + return `${setup.projectId}::${setup.hostId}::${getExecutionSurface(setup)}::${getPathSurface(setup)}` +} + +function getExecutionSurface(setup: ProjectHostSetup): string { + const connectionId = setup.connectionId?.trim() + if (connectionId) { + return toSshExecutionHostId(connectionId) + } + return setup.executionHostId?.trim() || setup.hostId +} + +// Why: projection twins differ by row identity, not checkout directory. +function getCheckoutIdentity(setup: ProjectHostSetup): string { + return normalizeRuntimePathForComparison(setup.path.trim()) || setup.repoId || setup.id +} + +function getPathSurface(setup: ProjectHostSetup): string { const wslPath = parseWslUncPath(setup.path) if (wslPath) { - // Why: Windows host and WSL on one machine are separate execution surfaces; - // only duplicate checkouts within one surface make project grouping ambiguous. - return `${setup.projectId}::${setup.hostId}::wsl:${wslPath.distro.toLowerCase()}` + return `wsl:${wslPath.distro.toLowerCase()}` } if (isWindowsAbsolutePathLike(setup.path)) { - return `${setup.projectId}::${setup.hostId}::windows-host` + return 'windows-host' } - return `${setup.projectId}::${setup.hostId}::default` + return 'default' } function buildProjectGroupingIndex(model?: ProjectGroupingModel): ProjectGroupingIndex | null { @@ -215,9 +234,7 @@ function buildProjectGroupingIndex(model?: ProjectGroupingModel): ProjectGroupin projectGroupingIndexCache.set(model, null) return null } - // Count real user checkouts per host surface (provisioned copies excluded so - // they keep nesting); more than one on a surface makes that project ambiguous. - const checkoutsByProjectSurface = new Map() + const checkoutsByProjectSurface = new Map>() for (const setup of projectHostSetups) { if (!isDistinctUserCheckout(setup)) { continue @@ -225,21 +242,21 @@ function buildProjectGroupingIndex(model?: ProjectGroupingModel): ProjectGroupin const key = getProjectSetupSurfaceKey(setup) const existing = checkoutsByProjectSurface.get(key) if (existing) { - existing.count += 1 + existing.add(getCheckoutIdentity(setup)) } else { - checkoutsByProjectSurface.set(key, { projectId: setup.projectId, count: 1 }) + checkoutsByProjectSurface.set(key, new Set([getCheckoutIdentity(setup)])) } } - const projectIdsRequiringSetupGroups = new Set() - for (const { projectId, count } of checkoutsByProjectSurface.values()) { - if (count > 1) { - projectIdsRequiringSetupGroups.add(projectId) + const surfaceKeysRequiringSetupGroups = new Set() + for (const [surfaceKey, checkouts] of checkoutsByProjectSurface) { + if (checkouts.size > 1) { + surfaceKeysRequiringSetupGroups.add(surfaceKey) } } const index = { projectById: new Map(projects.map((project) => [project.id, project])), setupByRepoId: new Map(projectHostSetups.map((setup) => [setup.repoId, setup])), - projectIdsRequiringSetupGroups + surfaceKeysRequiringSetupGroups } projectGroupingIndexCache.set(model, index) return index @@ -268,11 +285,10 @@ function getProjectGroupingForRepo( } } if ( - projectIndex?.projectIdsRequiringSetupGroups.has(setup.projectId) && + projectIndex?.surfaceKeysRequiringSetupGroups.has(getProjectSetupSurfaceKey(setup)) && isDistinctUserCheckout(setup) ) { - // Why: independent user checkouts of one project on the same host surface - // can't be safely merged, so each keeps its own sidebar entry. See #5374. + // Why: only the ambiguous surface needs checkout-specific headers. return { key: `project:${project.id}::setup:${repoId}`, label: repo?.displayName ?? setup.displayName, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 98b6ff3d646..488fc93e22a 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -322,6 +322,11 @@ "f518e89aa5": "Project already exists in that profile", "f03ae7f27b": "Failed to transfer project" } + }, + "runtime": { + "status": { + "runtimeHostDisconnectedDescription": "Workspaces and terminals on this server are unavailable." + } } } }, diff --git a/src/renderer/src/store/slices/runtime-status.test.ts b/src/renderer/src/store/slices/runtime-status.test.ts index a82350f06e0..ac04c0d3e22 100644 --- a/src/renderer/src/store/slices/runtime-status.test.ts +++ b/src/renderer/src/store/slices/runtime-status.test.ts @@ -1,6 +1,8 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { create } from 'zustand' +import { toast } from 'sonner' import type { RuntimeStatus } from '../../../../shared/runtime-types' +import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' import { createCompatibleRuntimeStatusResponse } from '../../runtime/runtime-compatibility-test-fixture' import { callRuntimeRpc, @@ -12,6 +14,10 @@ import { getRuntimeEnvironmentConnectionGeneration } from './runtime-status' +vi.mock('sonner', () => ({ + toast: { warning: vi.fn(), dismiss: vi.fn() } +})) + function createSliceStore() { return create()((...a) => ({ ...createRuntimeStatusSlice(...(a as unknown as Parameters)) @@ -32,6 +38,22 @@ function makeStatus(overrides: Partial = {}): RuntimeStatus { } as RuntimeStatus } +function makeEnvironment( + overrides: Partial = {} +): PublicKnownRuntimeEnvironment { + return { + id: 'env-a', + name: 'Dev Box', + createdAt: 1, + updatedAt: 1, + lastUsedAt: null, + runtimeId: null, + endpoints: [{ id: 'ws-a', kind: 'websocket', label: 'WebSocket', endpoint: 'ws://x' }], + preferredEndpointId: 'ws-a', + ...overrides + } +} + function stubRuntimeEnvironmentApi({ getStatus = vi.fn(), list = vi.fn() @@ -50,6 +72,11 @@ function stubRuntimeEnvironmentApi({ return { getStatus, list } } +beforeEach(() => { + vi.mocked(toast.warning).mockReset() + vi.mocked(toast.dismiss).mockReset() +}) + afterEach(() => { vi.unstubAllGlobals() }) @@ -142,6 +169,77 @@ describe('runtime-status slice', () => { expect(map.get('env-a')).toEqual({ status: null, checkedAt: 5, connectionGeneration: 1 }) }) + it('does not toast when the first probe finds a saved server offline', () => { + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 1 }) + + expect(toast.warning).not.toHaveBeenCalled() + }) + + it('toasts once when a connected server becomes unavailable', () => { + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 3 }) + + expect(toast.warning).toHaveBeenCalledTimes(1) + expect(toast.warning).toHaveBeenCalledWith( + 'Dev Box disconnected', + expect.objectContaining({ + id: 'runtime-environment-disconnected:env-a', + description: 'Workspaces and terminals on this server are unavailable.', + action: expect.objectContaining({ label: 'Retry' }) + }) + ) + }) + + it('dismisses the disconnect toast when the server recovers', () => { + const store = createSliceStore() + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 3 }) + + expect(toast.dismiss).toHaveBeenCalledWith('runtime-environment-disconnected:env-a') + }) + + it('retries from the toast and re-surfaces it when the server is still offline', async () => { + const getStatus = vi.fn().mockRejectedValue(new Error('closed')) + stubRuntimeEnvironmentApi({ getStatus }) + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: null, checkedAt: 2 }) + const options = vi.mocked(toast.warning).mock.calls[0]?.[1] as unknown as { + action: { onClick: () => void } + } + + options.action.onClick() + + await vi.waitFor(() => { + expect(getStatus).toHaveBeenCalledWith({ selector: 'env-a', timeoutMs: 10_000 }) + expect(toast.warning).toHaveBeenCalledTimes(2) + }) + expect(vi.mocked(toast.warning).mock.calls[1]?.[1]?.id).toBe( + 'runtime-environment-disconnected:env-a' + ) + }) + + it('does not report removal or explicit status clearing as a disconnect', () => { + const store = createSliceStore() + store.setState({ runtimeEnvironments: [makeEnvironment()] }) + store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) + + store.getState().clearRuntimeEnvironmentStatus('env-a') + store.getState().setRuntimeEnvironments([]) + + expect(toast.warning).not.toHaveBeenCalled() + }) + it('clears a single environment entry', () => { const store = createSliceStore() store.getState().setRuntimeEnvironmentStatus('env-a', { status: makeStatus(), checkedAt: 1 }) diff --git a/src/renderer/src/store/slices/runtime-status.ts b/src/renderer/src/store/slices/runtime-status.ts index 03fdac1224f..2cd0d1cfbf0 100644 --- a/src/renderer/src/store/slices/runtime-status.ts +++ b/src/renderer/src/store/slices/runtime-status.ts @@ -1,4 +1,5 @@ import type { StateCreator } from 'zustand' +import { toast } from 'sonner' import type { AppState } from '../types' import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' import type { RuntimeStatus } from '../../../../shared/runtime-types' @@ -8,6 +9,7 @@ import { unwrapRuntimeRpcResult } from '@/runtime/runtime-rpc-client' import { replaceRuntimeEnvironmentRevisions } from '@/runtime/runtime-environment-revision' +import { translate } from '@/i18n/i18n' /** Live status for one saved runtime environment, as last observed by the * renderer. `status === null` records a probe that failed or timed out so the @@ -57,6 +59,65 @@ export type RuntimeStatusSlice = { } const connectionGenerationByEnvironment = new Map() +const activeRuntimeDisconnectedToasts = new Map() + +function getRuntimeDisconnectedToastId(environmentId: string): string { + return `runtime-environment-disconnected:${environmentId}` +} + +function showRuntimeDisconnectedToast(environmentId: string, getState: () => AppState): void { + const environment = getState().runtimeEnvironments.find((entry) => entry.id === environmentId) + const toastId = getRuntimeDisconnectedToastId(environmentId) + const activation = Symbol(toastId) + const title = environment?.name + ? translate( + 'auto.components.sidebar.WorktreeCard.runtimeHostDisconnectedNamed', + '{{hostName}} disconnected', + { hostName: environment.name } + ) + : translate( + 'auto.components.sidebar.WorktreeCard.runtimeHostDisconnected', + 'Server disconnected' + ) + activeRuntimeDisconnectedToasts.set(toastId, activation) + const clearActiveToast = (): void => { + if (activeRuntimeDisconnectedToasts.get(toastId) === activation) { + activeRuntimeDisconnectedToasts.delete(toastId) + } + } + toast.warning(title, { + id: toastId, + description: translate( + 'auto.store.slices.runtime.status.runtimeHostDisconnectedDescription', + 'Workspaces and terminals on this server are unavailable.' + ), + action: { + label: translate('browser.loadFailure.retry', 'Retry'), + onClick: () => { + void getState() + .refreshRuntimeEnvironmentStatus(environmentId) + .then((reachable) => { + const stillSaved = getState().runtimeEnvironments.some( + (entry) => entry.id === environmentId + ) + if (!reachable && stillSaved) { + showRuntimeDisconnectedToast(environmentId, getState) + } + }) + } + }, + onDismiss: clearActiveToast, + onAutoClose: clearActiveToast + }) +} + +function dismissRuntimeDisconnectedToast(environmentId: string): void { + const toastId = getRuntimeDisconnectedToastId(environmentId) + if (!activeRuntimeDisconnectedToasts.delete(toastId)) { + return + } + toast.dismiss?.(toastId) +} export function getRuntimeEnvironmentConnectionGeneration(environmentId: string): number { return connectionGenerationByEnvironment.get(environmentId) ?? 0 @@ -158,10 +219,12 @@ export const createRuntimeStatusSlice: StateCreator 0) { get().purgeStaleRuntimeHostState?.(retiredEnvironmentIds) + retiredEnvironmentIds.forEach(dismissRuntimeDisconnectedToast) } }, setRuntimeEnvironmentStatus: (environmentId, status) => { + const previous = get().runtimeStatusByEnvironmentId.get(environmentId) // Why: a non-null status proves the runtime just answered, so drop any stale // "offline" compat failure before this online transition fires the // reuse-flagged background refetches — a recovered host must re-probe. @@ -170,7 +233,6 @@ export const createRuntimeStatusSlice: StateCreator { const next = new Map(s.runtimeStatusByEnvironmentId) - const previous = next.get(environmentId) const connectionChanged = status.status !== null && (previous?.status == null || previous.status.runtimeId !== status.status.runtimeId) @@ -185,9 +247,15 @@ export const createRuntimeStatusSlice: StateCreator + clearRuntimeEnvironmentStatus: (environmentId) => { + dismissRuntimeDisconnectedToast(environmentId) set((s) => { advanceRuntimeEnvironmentConnectionGeneration(environmentId) if (!s.runtimeStatusByEnvironmentId.has(environmentId)) { @@ -196,11 +264,17 @@ export const createRuntimeStatusSlice: StateCreator + retainRuntimeEnvironmentStatuses: (environmentIds) => { + const keep = new Set(environmentIds) + for (const id of get().runtimeStatusByEnvironmentId.keys()) { + if (!keep.has(id)) { + dismissRuntimeDisconnectedToast(id) + } + } set((s) => { - const keep = new Set(environmentIds) let changed = false const next = new Map(s.runtimeStatusByEnvironmentId) for (const id of next.keys()) { @@ -210,7 +284,8 @@ export const createRuntimeStatusSlice: StateCreator { try {