From 62878c3b0c8a64519fa219b0282da9b64d83c540 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Fri, 9 Oct 2026 13:50:36 -0700 Subject: [PATCH] fix(folders): folder workspaces can answer "paired server" (#26958) * fix(folders): folder workspaces can answer "paired server" * fix(folders): name a server folder's nested target only when its id is unambiguous * fix(folders): read the folder group only when the answer needs it --------- Co-authored-by: m4air Co-authored-by: m4air --- .../file-explorer-operation-owner.test.ts | 80 +++++++++++++++++++ .../hooks/automation-dispatch-workspace.ts | 2 +- .../src/lib/folder-workspace-connection.ts | 15 +++- .../worktree-host-connection-phase.test.ts | 32 ++++++++ src/shared/automation-list-scope.ts | 8 +- .../folder-workspace-execution-host.test.ts | 38 ++++++--- src/shared/folder-workspace-execution-host.ts | 44 ++++++---- 7 files changed, 187 insertions(+), 32 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/file-explorer-operation-owner.test.ts diff --git a/src/renderer/src/components/right-sidebar/file-explorer-operation-owner.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-operation-owner.test.ts new file mode 100644 index 00000000000..f99a057f329 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/file-explorer-operation-owner.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it, vi } from 'vitest' +import type { ExecutionHostId } from '../../../../shared/execution-host' +import type { FolderWorkspace } from '../../../../shared/folder-workspace-types' +import type { ProjectGroup } from '../../../../shared/project-group-types' +import { getFileExplorerOperationOwnerFromState } from './file-explorer-operation-owner' + +vi.mock('@/store', () => ({ useAppStore: { getState: vi.fn() } })) + +function folder( + projectGroupId: string, + executionHostId: ExecutionHostId, + connectionId: string | null +): FolderWorkspace { + return { + id: 'same-folder', + projectGroupId, + name: 'Notes', + folderPath: '/notes', + executionHostId, + connectionId, + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0, + createdAt: 0, + updatedAt: 0 + } +} + +function group( + id: string, + executionHostId: ExecutionHostId, + connectionId: string | null +): ProjectGroup { + return { + id, + name: id, + parentPath: '/', + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + connectionId, + executionHostId, + createdAt: 0, + updatedAt: 0 + } +} + +describe('getFileExplorerOperationOwnerFromState', () => { + it("never hands a selected local folder the same-id server folder's nested SSH target", () => { + const owner = getFileExplorerOperationOwnerFromState( + { + settings: null, + repos: [], + worktreesByRepo: {}, + detectedWorktreesByRepo: {}, + restoredRuntimeHostIdByWorkspaceSessionKey: {}, + activeWorktreeId: 'folder:same-folder', + activeWorkspaceExecutionHostId: 'local', + // The server's copy comes first, as a catalog merge can order it. + folderWorkspaces: [ + folder('remote-group', 'runtime:env-a', 'nested-box'), + folder('local-group', 'local', null) + ], + projectGroups: [ + group('remote-group', 'runtime:env-a', 'nested-box'), + group('local-group', 'local', null) + ] + }, + 'folder:same-folder' + ) + + expect(owner).toEqual({ kind: 'local' }) + }) +}) diff --git a/src/renderer/src/hooks/automation-dispatch-workspace.ts b/src/renderer/src/hooks/automation-dispatch-workspace.ts index f8980a56108..ea2be2708de 100644 --- a/src/renderer/src/hooks/automation-dispatch-workspace.ts +++ b/src/renderer/src/hooks/automation-dispatch-workspace.ts @@ -80,7 +80,7 @@ export async function prepareAutomationDispatchWorkspace(args: { folderWorkspaceHost && automationWorktree ? folderWorkspaceConnectionId ? toSshExecutionHostId(folderWorkspaceConnectionId) - : folderWorkspaceHost.kind === 'local' + : folderWorkspaceHost.kind === 'local' || folderWorkspaceHost.kind === 'runtime' ? getResolvedExecutionHostIdForWorktree(state, automationWorktree.id) : null : null diff --git a/src/renderer/src/lib/folder-workspace-connection.ts b/src/renderer/src/lib/folder-workspace-connection.ts index b0840fc7e9f..5c9caed34c5 100644 --- a/src/renderer/src/lib/folder-workspace-connection.ts +++ b/src/renderer/src/lib/folder-workspace-connection.ts @@ -20,8 +20,17 @@ export function getFolderWorkspaceConnectionId( folderWorkspaceId: string ): string | null | undefined { const host = resolveFolderWorkspaceHost(state, folderWorkspaceId) - if (host.kind === 'ssh') { - return host.targetId + switch (host.kind) { + case 'ssh': + return host.targetId + // Why the server's own target: like a project row there (`getRepoSshConnectionId`), so its + // SSH state is read inside that server rather than reading as local. + case 'runtime': + return host.sshTargetId + case 'local': + return null + case 'missing': + case 'ambiguous': + return undefined } - return host.kind === 'local' ? null : undefined } diff --git a/src/renderer/src/lib/worktree-host-connection-phase.test.ts b/src/renderer/src/lib/worktree-host-connection-phase.test.ts index 0a6478dd526..0b266705a02 100644 --- a/src/renderer/src/lib/worktree-host-connection-phase.test.ts +++ b/src/renderer/src/lib/worktree-host-connection-phase.test.ts @@ -197,6 +197,38 @@ describe('selectWorktreeHostConnectionPhase', () => { }) }) + it("reads a paired server's folder workspace on its nested target like a project there", () => { + const state = makeState({ + // As fetched from the server: its own SSH target, stamped with the server's host. + folderWorkspaces: [ + { + id: 'fw-runtime', + projectGroupId: 'group-runtime', + folderPath: '/srv/notes', + connectionId: 'ssh-nested', + executionHostId: 'runtime:env-a' + } + ], + projectGroups: [ + { + id: 'group-runtime', + kind: 'folder', + connectionId: 'ssh-nested', + executionHostId: 'runtime:env-a' + } + ], + runtimeStatusByEnvironmentId: new Map([ + ['env-a', { status: { runtimeId: 'runtime-a' }, checkedAt: 1 }] + ]) + }) + + expect(selectWorktreeHostConnectionPhase(state, 'folder:fw-runtime')).toMatchObject({ + phase: 'unverifiable', + targetId: 'ssh-nested', + environmentId: 'env-a' + }) + }) + it('names a new connection on every reconnect', () => { const connected = makeSshState('connected') const reconnected = makeSshState('connected', { diff --git a/src/shared/automation-list-scope.ts b/src/shared/automation-list-scope.ts index 0e540356483..d3ac1085802 100644 --- a/src/shared/automation-list-scope.ts +++ b/src/shared/automation-list-scope.ts @@ -14,6 +14,7 @@ import type { Automation } from './automations-types' import type { AutomationUsageSummary } from './automation-usage-summary' +import type { FolderWorkspaceHost } from './folder-workspace-execution-host' import { getAutomationRunRepoId } from './automation-run-identity' import { parseExecutionHostId } from './execution-host' import { sanitizeSshTargetGeneration } from './ssh-target-generation' @@ -67,9 +68,7 @@ export const AUTOMATION_ORPHAN_ISSUES = { /** Where a record's workspace executes; `unpinned` leaves the repo to decide. */ export type AutomationWorkspaceHost = | { kind: 'unpinned' } - | { kind: 'local' } - | { kind: 'ssh'; targetId: string } - | { kind: 'ambiguous' } + | Exclude export type AutomationProjectionContext = { /** The process that owns this store; runtime scheduling markers are local only on a runtime. */ @@ -164,6 +163,9 @@ export function projectAutomationSelector( if (workspaceHost.kind === 'ambiguous') { return orphan(AUTOMATION_ORPHAN_ISSUES.workspaceHostAmbiguous) } + if (workspaceHost.kind === 'runtime') { + return orphan(AUTOMATION_ORPHAN_ISSUES.scheduledElsewhere) + } // A pinned workspace is where the run actually goes, so the row belongs to that host, not Self. if (workspaceHost.kind === 'ssh') { return workspacePinSelector(automation, workspaceHost.targetId, context) diff --git a/src/shared/folder-workspace-execution-host.test.ts b/src/shared/folder-workspace-execution-host.test.ts index e1af0a7920e..d3633a6593e 100644 --- a/src/shared/folder-workspace-execution-host.test.ts +++ b/src/shared/folder-workspace-execution-host.test.ts @@ -273,15 +273,35 @@ describe('folder workspace execution host', () => { expect(resolved).toEqual({ kind: 'ssh', targetId: 'nested-box' }) }) - it('still answers local for a runtime pin, which the type cannot express otherwise', () => { - const resolved = resolveFolderWorkspaceHost( - state({ - folderWorkspaces: [workspace({ executionHostId: 'runtime:env-1' })] - }), - 'fw-1' - ) - - expect(resolved).toEqual({ kind: 'local' }) + it("answers a runtime pin with its server, paired with that server's own SSH target", () => { + expect( + resolveFolderWorkspaceHost( + state({ folderWorkspaces: [workspace({ executionHostId: 'runtime:env-1' })] }), + 'fw-1' + ) + ).toEqual({ kind: 'runtime', environmentId: 'env-1', sshTargetId: null }) + expect( + resolveFolderWorkspaceHost( + state({ + folderWorkspaces: [ + workspace({ executionHostId: 'runtime:env-1', connectionId: 'server-box' }) + ] + }), + 'fw-1' + ) + ).toEqual({ kind: 'runtime', environmentId: 'env-1', sshTargetId: 'server-box' }) + // A same-id copy on another host leaves the selected one unknown here, so no target is named. + expect( + resolveFolderWorkspaceHost( + state({ + folderWorkspaces: [ + workspace({ executionHostId: 'runtime:env-1', connectionId: 'server-box' }), + workspace({ executionHostId: 'local' }) + ] + }), + 'fw-1' + ) + ).toEqual({ kind: 'runtime', environmentId: 'env-1', sshTargetId: null }) }) // The candidate FILTER decides which rows reach the resolver, and it read `repo.connectionId` raw diff --git a/src/shared/folder-workspace-execution-host.ts b/src/shared/folder-workspace-execution-host.ts index 0aee75de543..def6c4d34e2 100644 --- a/src/shared/folder-workspace-execution-host.ts +++ b/src/shared/folder-workspace-execution-host.ts @@ -29,6 +29,8 @@ export type FolderWorkspaceHost = | { kind: 'missing' } | { kind: 'local' } | { kind: 'ssh'; targetId: string } + // A paired server's folder; `sshTargetId` is that server's own target, addressable only with it. + | { kind: 'runtime'; environmentId: string; sshTargetId: string | null } | { kind: 'ambiguous' } /** Reads a stored connection id: blank is local, since no write path normalizes it. */ @@ -46,8 +48,7 @@ export function normalizeConnectionId(value: string | null | undefined): string * reading raw in either place drops the row before the resolver can classify it. * * A non-SSH host falls back to the raw field so a `runtime:` row keeps contributing its nested - * target exactly as it does today. That target is not this client's to dial, but changing it is a - * separate defect with its own reasoning — see the note in `resolveFolderWorkspaceHost`. + * target, which is not this client's to dial. */ function getRepoScopeConnectionId(repo: Repo): string | null { const host = parseExecutionHostId(getRepoExecutionHostId(repo)) @@ -118,22 +119,33 @@ export function resolveFolderWorkspaceHost( if (!workspace) { return { kind: 'missing' } } + const readScopeConnectionId = (): string | null => + normalizeConnectionId( + workspace.connectionId ?? + state.projectGroups.find((entry) => entry.id === workspace.projectGroupId)?.connectionId + ) const explicitHost = parseExecutionHostId(workspace.executionHostId) - if (explicitHost) { - // A `runtime:` workspace deliberately answers `local`, and `FolderWorkspaceHost` has no runtime - // variant to answer with instead. That omission is known: a runtime environment's own server - // normalizes its work to `local`, and the nested SSH target on such a row is addressable only as - // the pair (environmentId, targetId) — handing it to this client's SSH table would dial a - // same-named box in the wrong namespace. Widening the type is its own change, not an oversight - // here. - return explicitHost.kind === 'ssh' - ? { kind: 'ssh', targetId: explicitHost.targetId } - : { kind: 'local' } + switch (explicitHost?.kind) { + case 'ssh': + return { kind: 'ssh', targetId: explicitHost.targetId } + case 'runtime': { + // Why: another host may hold a folder with this id, and this lookup cannot tell which one is + // selected, so the server's target is named only when the id is unambiguous. + const isOnlyOwner = state.folderWorkspaces.every( + (entry) => entry.id !== folderWorkspaceId || entry === workspace + ) + return { + kind: 'runtime', + environmentId: explicitHost.environmentId, + sshTargetId: isOnlyOwner ? readScopeConnectionId() : null + } + } + case 'local': + return { kind: 'local' } + case undefined: + break } - const scopeConnectionId = normalizeConnectionId( - workspace.connectionId ?? - state.projectGroups.find((entry) => entry.id === workspace.projectGroupId)?.connectionId - ) + const scopeConnectionId = readScopeConnectionId() const candidateRepos = findFolderWorkspaceCandidateRepos(state, folderWorkspaceId) let hasLocalRepo = false const connectionIds = new Set()