From f2bc45f16c3f6d7eb7ae70870d776b65e8b19202 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 6 Sep 2026 17:31:22 -0400 Subject: [PATCH] fix: import hidden worktrees before terminal activation (STA-6846) --- .../ipc-events/terminal-command-state.ts | 4 + .../terminal-presentation-ipc-bridge.ts | 12 +- .../ipc-events/terminal-request-ipc-bridge.ts | 12 +- .../terminal-worktree-visibility.test.ts | 205 ++++++++++++++++++ .../terminal-worktree-visibility.ts | 84 +++++++ 5 files changed, 313 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.test.ts create mode 100644 src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.ts diff --git a/src/renderer/src/hooks/ipc-events/terminal-command-state.ts b/src/renderer/src/hooks/ipc-events/terminal-command-state.ts index 83b06e15550..89e03d415fa 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-command-state.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-command-state.ts @@ -6,6 +6,7 @@ import type { TerminalLayoutSnapshot, TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types' +import { hasTerminalWorktreeRow, hiddenTerminalWorktreeError } from './terminal-worktree-visibility' import type { AppState } from '../../store/types' export function resolveTerminalPresentation(data: { @@ -36,6 +37,9 @@ export function focusTerminalInitiatedTab( } export function activateTerminalInitiatedWorktree(store: AppState, worktreeId: string): void { + if (!hasTerminalWorktreeRow(store, worktreeId)) { + throw hiddenTerminalWorktreeError() + } store.setActiveView('terminal') store.setActiveWorktree(worktreeId) store.markWorktreeVisited(worktreeId) diff --git a/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts index 47835ab529d..58538b1c1bf 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts @@ -1,3 +1,7 @@ +import { + ensureTerminalWorktreeVisible, + hasTerminalWorktreeRow +} from './terminal-worktree-visibility' import { requestBackgroundTerminalWorktreeMount } from '@/components/terminal/background-terminal-worktree-mount' import { hasRegisteredRuntimeTerminalTab } from '@/runtime/sync-runtime-graph' import { planMobileTerminalTabMount } from '@/lib/mobile-terminal-tab-mount' @@ -22,7 +26,7 @@ import { export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): void { unsubs.push( window.api.ui.onCreateTerminal( - ({ + async ({ requestId, worktreeId, command, @@ -46,7 +50,7 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v splitTelemetrySource }) => { try { - const store = useAppStore.getState() + let store = useAppStore.getState() const terminalPresentation = resolveTerminalPresentation({ presentation, activate, @@ -54,6 +58,10 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v }) const shouldActivate = terminalPresentation === 'focused' const shouldSurfaceOwner = terminalPresentation !== 'background' && surfaceOwner !== false + if (shouldActivate && !hasTerminalWorktreeRow(store, worktreeId)) { + await ensureTerminalWorktreeVisible(worktreeId) + store = useAppStore.getState() + } if (shouldActivate) { activateTerminalInitiatedWorktree(store, worktreeId) } diff --git a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts index 1fd3eed2039..f9e43cbe4e3 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts @@ -1,3 +1,7 @@ +import { + ensureTerminalWorktreeVisible, + hasTerminalWorktreeRow +} from './terminal-worktree-visibility' import { requestBackgroundTerminalWorktreeMount } from '@/components/terminal/background-terminal-worktree-mount' import { getConnectionIdFromState } from '@/lib/connection-context' import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode' @@ -13,9 +17,9 @@ import { export function registerTerminalRequestIpcBridge(unsubs: (() => void)[]): void { unsubs.push( - window.api.ui.onRequestTerminalCreate((data) => { + window.api.ui.onRequestTerminalCreate(async (data) => { try { - const store = useAppStore.getState() + let store = useAppStore.getState() const worktreeId = data.worktreeId ?? store.activeWorktreeId if (!worktreeId) { window.api.ui.replyTerminalCreate({ @@ -50,6 +54,10 @@ export function registerTerminalRequestIpcBridge(unsubs: (() => void)[]): void { const shouldActivate = terminalPresentation === 'focused' const shouldSurfaceOwner = terminalPresentation !== 'background' && data.surfaceOwner !== false + if (shouldActivate && !hasTerminalWorktreeRow(store, worktreeId)) { + await ensureTerminalWorktreeVisible(worktreeId) + store = useAppStore.getState() + } if (shouldActivate) { activateTerminalInitiatedWorktree(store, worktreeId) } diff --git a/src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.test.ts b/src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.test.ts new file mode 100644 index 00000000000..75fe793f960 --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.test.ts @@ -0,0 +1,205 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '../../store/types' +import { setupTerminalCreateSurfacing } from '../ipc-events-terminal-create-test-harness' + +beforeEach(() => { + vi.resetModules() + vi.unstubAllGlobals() +}) + +async function setup(hostId = 'local') { + const scenario = await setupTerminalCreateSurfacing(() => false) + const state = scenario.storeState as unknown as AppState + const worktreeId = 'repo-1::/hidden' + const hidden = { + id: worktreeId, + repoId: 'repo-1', + path: '/hidden', + hostId, + visible: false, + ownership: 'external' + } + Object.assign(state, { + repos: [ + { id: 'repo-1', executionHostId: hostId, importedExternalWorktreePaths: ['/existing'] } + ], + detectedWorktreesByRepo: { + 'repo-1': { authoritative: true, worktrees: [hidden] } + }, + // Why: detected rows are known too; only the visible catalog proves a sidebar row exists. + getKnownWorktreeById: () => hidden + }) + const updateRepo = vi.fn(async (_id, updates) => { + state.repos = state.repos.map((repo) => ({ ...repo, ...updates })) + return true + }) + const fetchWorktrees = vi.fn(async () => { + state.worktreesByRepo = { + ...state.worktreesByRepo, + 'repo-1': [...state.worktreesByRepo['repo-1'], hidden as never] + } + return true + }) + Object.assign(state, { updateRepo, fetchWorktrees }) + return { ...scenario, state, worktreeId, updateRepo, fetchWorktrees } +} + +describe.each(['reveal', 'create'] as const)('hidden worktree %s bridge', (bridge) => { + async function invoke(scenario: Awaited>, focused = true) { + const payload = { + requestId: 'request', + worktreeId: scenario.worktreeId, + presentation: focused ? ('focused' as const) : ('background' as const), + source: 'runtime-session' as const + } + await (bridge === 'reveal' + ? scenario.createTerminalListenerRef.current!(payload) + : scenario.requestTerminalCreateListenerRef.current!(payload)) + } + + it.each(['local', 'ssh:server', 'runtime:server'])( + 'imports on the %s owner before activating and replying', + async (hostId) => { + const s = await setup(hostId) + let finishRefresh!: () => void + s.fetchWorktrees.mockImplementationOnce(async () => { + await new Promise((resolve) => { + finishRefresh = resolve + }) + s.state.worktreesByRepo['repo-1'].push({ id: s.worktreeId } as never) + return true + }) + const pending = invoke(s) + await vi.waitFor(() => expect(s.fetchWorktrees).toHaveBeenCalled()) + expect(s.setActiveWorktree).not.toHaveBeenCalled() + expect(s.createTab).not.toHaveBeenCalled() + expect(s.replyTerminalCreate).not.toHaveBeenCalled() + finishRefresh() + await pending + expect(s.updateRepo).toHaveBeenCalledWith( + 'repo-1', + { + importedExternalWorktreePaths: ['/existing', '/hidden'], + externalWorktreeInboxBaselinePaths: ['/hidden'] + }, + { hostId } + ) + expect(s.fetchWorktrees).toHaveBeenCalledWith('repo-1', { + requireAuthoritative: true, + executionHostId: hostId + }) + expect(s.setActiveWorktree).toHaveBeenCalledWith(s.worktreeId) + expect(s.revealWorktreeInSidebar).toHaveBeenCalledWith(s.worktreeId) + expect(s.replyTerminalCreate).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'tab-new' }) + ) + } + ) + + it.each(['update', 'refresh', 'missing-row', 'unknown'])( + 'replies with an error without navigation when %s fails', + async (failure) => { + const s = await setup() + if (failure === 'update') { + s.updateRepo.mockResolvedValue(false) + } + if (failure === 'refresh') { + s.fetchWorktrees.mockResolvedValue(false) + } + if (failure === 'missing-row') { + s.fetchWorktrees.mockResolvedValue(true) + } + if (failure === 'unknown') { + s.state.detectedWorktreesByRepo = {} + } + await invoke(s) + expect(s.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'request', + error: expect.stringContaining('worktree_hidden') + }) + expect(s.setActiveView).not.toHaveBeenCalled() + expect(s.setActiveWorktree).not.toHaveBeenCalled() + expect(s.recordWorktreeVisit).not.toHaveBeenCalled() + expect(s.createTab).not.toHaveBeenCalled() + expect(s.revealWorktreeInSidebar).not.toHaveBeenCalled() + if (failure === 'refresh') { + expect(s.updateRepo).toHaveBeenLastCalledWith( + 'repo-1', + { + importedExternalWorktreePaths: ['/existing'], + externalWorktreeInboxBaselinePaths: [] + }, + { hostId: 'local' } + ) + } + } + ) + + it('keeps background requests hidden without activating', async () => { + const s = await setup() + await invoke(s, false) + expect(s.updateRepo).not.toHaveBeenCalled() + expect(s.setActiveWorktree).not.toHaveBeenCalled() + expect(s.createTab).toHaveBeenCalled() + }) + + it('accepts a folder workspace without importing it as a git worktree', async () => { + const s = await setup() + s.worktreeId = 'folder:folder-1' + s.state.folderWorkspaces = [{ id: 'folder-1' } as never] + await invoke(s) + expect(s.updateRepo).not.toHaveBeenCalled() + expect(s.setActiveWorktree).toHaveBeenCalledWith(s.worktreeId) + }) +}) + +describe('terminal worktree activation safety', () => { + it('rejects direct activation of a detected hidden row', async () => { + const s = await setup() + const { activateTerminalInitiatedWorktree } = await import('./terminal-command-state') + expect(() => activateTerminalInitiatedWorktree(s.state, s.worktreeId)).toThrow( + 'worktree_hidden' + ) + expect(s.setActiveView).not.toHaveBeenCalled() + expect(s.setActiveWorktree).not.toHaveBeenCalled() + }) + + it('coalesces simultaneous reveals of the same hidden worktree', async () => { + const s = await setup() + const { ensureTerminalWorktreeVisible } = await import('./terminal-worktree-visibility') + await Promise.all([ + ensureTerminalWorktreeVisible(s.worktreeId), + ensureTerminalWorktreeVisible(s.worktreeId) + ]) + expect(s.updateRepo).toHaveBeenCalledTimes(1) + expect(s.fetchWorktrees).toHaveBeenCalledTimes(1) + }) + + it('preserves both imports for simultaneous reveals in the same repo', async () => { + const s = await setup() + const second = { + ...s.state.detectedWorktreesByRepo['repo-1'].worktrees[0], + id: 'repo-1::/second', + path: '/second' + } + s.state.detectedWorktreesByRepo['repo-1'].worktrees.push(second) + s.fetchWorktrees.mockImplementation(async () => { + s.state.worktreesByRepo['repo-1'] = s.state.detectedWorktreesByRepo[ + 'repo-1' + ].worktrees.filter((row) => + s.state.repos[0].importedExternalWorktreePaths?.includes(row.path) + ) + return true + }) + const { ensureTerminalWorktreeVisible } = await import('./terminal-worktree-visibility') + await Promise.all([ + ensureTerminalWorktreeVisible(s.worktreeId), + ensureTerminalWorktreeVisible(second.id) + ]) + expect(s.state.repos[0].importedExternalWorktreePaths).toEqual([ + '/existing', + '/hidden', + '/second' + ]) + }) +}) diff --git a/src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.ts b/src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.ts new file mode 100644 index 00000000000..b46fab8a8db --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/terminal-worktree-visibility.ts @@ -0,0 +1,84 @@ +import type { AppState } from '../../store/types' +import { parseWorkspaceKey } from '../../../../shared/workspace-scope' +import { importNewExternalWorktreeInboxPaths } from '@/components/sidebar/new-external-worktrees-inbox-actions' +import { resolveWorktreeOperationRoute } from '@/lib/worktree-operation-route' +import { findRepoForHost, getRepoHostIdentity } from '@/store/slices/repo-host-identity' +import { findWorktreeById, getRepoIdFromWorktreeId } from '@/store/slices/worktree-helpers' +import { + worktreeHostMatchOptions, + worktreeMatchesHost +} from '@/store/slices/worktrees/listing/worktree-host-ownership' +import { useAppStore } from '../../store' + +const pendingImports = new Map>() + +export function hasTerminalWorktreeRow(state: AppState, worktreeId: string): boolean { + if (parseWorkspaceKey(worktreeId)?.type === 'folder') { + return Boolean(state.getKnownWorktreeById(worktreeId)) + } + return Boolean(findWorktreeById(state.worktreesByRepo, worktreeId)) +} + +export function hiddenTerminalWorktreeError(): Error { + return new Error('worktree_hidden: Terminal workspace could not be shown in the sidebar') +} + +export async function ensureTerminalWorktreeVisible(worktreeId: string): Promise { + const state = useAppStore.getState() + if (hasTerminalWorktreeRow(state, worktreeId)) { + return + } + const route = resolveWorktreeOperationRoute(state, worktreeId) + const hostId = route?.executionHostId + if (!hostId) { + throw hiddenTerminalWorktreeError() + } + const repoId = getRepoIdFromWorktreeId(worktreeId) + const repo = findRepoForHost(state.repos, repoId, { hostId }) + if (!repo) { + throw hiddenTerminalWorktreeError() + } + const scope = getRepoHostIdentity(repo) + // Why: concurrent reveals must merge imports against the preceding persisted update. + const previous = pendingImports.get(scope) ?? Promise.resolve() + const pending = previous + .catch(() => {}) + .then(async () => { + const current = useAppStore.getState() + if (hasTerminalWorktreeRow(current, worktreeId)) { + return + } + const targetRepo = findRepoForHost(current.repos, repoId, { hostId }) + const detected = current.detectedWorktreesByRepo[repoId]?.worktrees.filter( + (worktree) => + worktree.id === worktreeId && + worktreeMatchesHost(worktree, hostId, worktreeHostMatchOptions(current, repoId, hostId)) + ) + if (!targetRepo || detected?.length !== 1 || detected[0].visible) { + throw hiddenTerminalWorktreeError() + } + let imported = false + await importNewExternalWorktreeInboxPaths({ + projectId: repoId, + repo: targetRepo, + worktreePaths: [detected[0].path], + updateRepo: (id, updates) => current.updateRepo(id, updates, { hostId }), + fetchWorktrees: (id, options) => + current.fetchWorktrees(id, { ...options, executionHostId: hostId }), + setInboxState: (_id, status) => { + imported = status === null + } + }) + if (!imported || !hasTerminalWorktreeRow(useAppStore.getState(), worktreeId)) { + throw hiddenTerminalWorktreeError() + } + }) + pendingImports.set(scope, pending) + try { + await pending + } finally { + if (pendingImports.get(scope) === pending) { + pendingImports.delete(scope) + } + } +}