diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 9b6fcef5b..e1bb431c7 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -105,16 +105,14 @@ import { CREATE_WORKSPACE_QUICK_ACTION_ID } from '@/components/cmd-j/quick-actions' import { buildWorktreeChecksReviewIndex } from '@/components/cmd-j/worktree-checks-review-index' +import { resolvePaletteFocusRestoreTarget } from '@/components/cmd-j/palette-focus-restore-target' import { selectWorktreePaletteCacheInputs } from '@/components/cmd-j/worktree-palette-cache-inputs' import { getRepoHostIdentity } from '@/store/slices/repo-host-identity' import { getComposerEligibleRepos, resolveComposerGitRepoId } from '@/lib/new-workspace-composer-repo' -import { - lookupGitHubWorkItemByOwnerRepoForSource, - lookupGitHubWorkItemForSource -} from '@/lib/github-work-item-source-lookup' +import { lookupGitHubWorkItemForSource } from '@/lib/github-work-item-source-lookup' import type { SettingsNavTarget } from '@/lib/settings-navigation-types' import { getHostDisplayLabelOverrides } from '../../../shared/host-setting-overrides' import { isRuntimeOwnedSshTargetId } from '../../../shared/execution-host' @@ -433,6 +431,11 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { ) const previousBrowserPageIdRef = useRef(null) const previousBrowserFocusTargetRef = useRef<'webview' | 'address-bar'>('webview') + // Why: the exact element focused before Cmd+J opened (e.g. the terminal + // textarea the user was typing in) so Escape restores it precisely instead + // of the first matching surface in the DOM, which may be a background + // worktree's mounted-but-hidden terminal. + const previousFocusElementRef = useRef(null) const activeGroupSnapshotRef = useRef(null) const wasVisibleRef = useRef(false) const skipRestoreFocusRef = useRef(false) @@ -1177,6 +1180,13 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { document.activeElement.closest('[data-orca-browser-address-bar="true"]') ? 'address-bar' : 'webview' + // Why: same timing constraint — capture the pre-dialog focus target now + // so Escape can return focus to the exact input the user left (excluding + // document.body, which isn't a meaningful restore target). + previousFocusElementRef.current = + document.activeElement instanceof HTMLElement && document.activeElement !== document.body + ? document.activeElement + : null skipRestoreFocusRef.current = false setQuery('') setSelectedItemId('') @@ -1243,24 +1253,19 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { useEffect(() => cancelFallbackFocusFrames, [cancelFallbackFocusFrames]) - const focusFallbackSurface = useCallback(() => { - cancelFallbackFocusFrames() - fallbackFocusOuterFrameRef.current = requestAnimationFrame(() => { - fallbackFocusOuterFrameRef.current = null - fallbackFocusInnerFrameRef.current = requestAnimationFrame(() => { - fallbackFocusInnerFrameRef.current = null - const xterm = document.querySelector('.xterm-helper-textarea') as HTMLElement | null - if (xterm) { - xterm.focus() - return - } - const monaco = document.querySelector('.monaco-editor textarea') as HTMLElement | null - if (monaco) { - monaco.focus() - } + const focusFallbackSurface = useCallback( + (preferredTarget?: HTMLElement | null) => { + cancelFallbackFocusFrames() + fallbackFocusOuterFrameRef.current = requestAnimationFrame(() => { + fallbackFocusOuterFrameRef.current = null + fallbackFocusInnerFrameRef.current = requestAnimationFrame(() => { + fallbackFocusInnerFrameRef.current = null + resolvePaletteFocusRestoreTarget(preferredTarget ?? null)?.focus({ preventScroll: true }) + }) }) - }) - }, [cancelFallbackFocusFrames]) + }, + [cancelFallbackFocusFrames] + ) const requestBrowserFocus = useCallback( (detail: { pageId: string; target: 'webview' | 'address-bar' }) => { @@ -1294,7 +1299,10 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { return } if (previousWorktreeIdRef.current) { - focusFallbackSurface() + // Why: dismissing Cmd+J should return to whatever the user was doing — + // restore the exact previously-focused surface (e.g. the terminal they + // were typing in) rather than an arbitrary first match. + focusFallbackSurface(previousFocusElementRef.current) } }, [closeModal, focusFallbackSurface, requestBrowserFocus] @@ -1535,13 +1543,11 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { // Case 1: user pasted a GH issue/PR URL. if (ghLink) { - const { slug, number } = ghLink + const { number } = ghLink const state = useAppStore.getState() // Why: the existing-worktree check only needs the issue/PR number, which - // is repo-agnostic on the worktree meta side. We don't currently cache a - // repo-slug map, so slug-matching against a specific repo happens - // implicitly when we pick a repo for the `gh workItem` lookup below. + // is repo-agnostic on the worktree meta side. const matches = allWorktrees.filter( (w) => !w.isArchived && (w.linkedIssue === number || w.linkedPR === number) ) @@ -1553,73 +1559,21 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { return } - // Resolve via gh.workItem: prefer the active repo, else the first eligible. + // Why: hand the raw URL to the composer's name field so it runs the same + // cross-project detection as Cmd+N — surfacing the "Switch project?" + // dialog when the URL targets a different project. Pre-resolving here + // against an arbitrary repo silently linked cross-project items to the + // wrong project and skipped that prompt. Seed the active repo so the + // field compares the URL against the project the user is currently in. const eligibleRepos = state.repos.filter((r) => isGitRepoKind(r)) const repoForLookup = (state.activeRepoId && eligibleRepos.find((r) => r.id === state.activeRepoId)) || eligibleRepos[0] - if (!repoForLookup) { - openComposer({ prefilledName: trimmed }) - return - } - - prefetchCreateWorkspaceBaseForComposer(repoForLookup.id) - const sourceContext = buildTaskSourceContextFromRepo({ - provider: 'github', - projectId: repoForLookup.id, - repo: repoForLookup - }) - // Why: awaiting inside the user gesture would leave the palette open - // indefinitely on slow networks. Close immediately and populate the - // composer once the lookup returns. - const lookupToken = createLookupGuard.start() - preserveCreateLookupOnCloseRef.current = true - recordFeatureInteraction('cmd-j-create-workspace') - closeModal() - void lookupGitHubWorkItemByOwnerRepoForSource({ - repoPath: repoForLookup.path, - repoId: repoForLookup.id, - sourceContext, - owner: slug.owner, - repo: slug.repo, - number, - type: ghLink.type - }) - .then((item) => { - if (!createLookupGuard.isCurrent(lookupToken)) { - return - } - const data: Record = { initialRepoId: repoForLookup.id } - if (item) { - const linkedWorkItem: LinkedWorkItemSummary = { - type: item.type, - number: item.number, - title: item.title, - url: item.url - } - data.linkedWorkItem = linkedWorkItem - data.prefilledName = - getLinkedWorkItemWorkspaceName(linkedWorkItem)?.seedName ?? - getLinkedWorkItemSuggestedName({ title: item.title }) - } else { - // Fallback: we couldn't resolve the URL, just seed the name. - data.prefilledName = `${slug.owner}-${slug.repo}-${number}` - } - queueMicrotask(() => - openModal('new-workspace-composer', { ...data, telemetrySource: 'command_palette' }) - ) - }) - .catch(() => { - if (!createLookupGuard.isCurrent(lookupToken)) { - return - } - queueMicrotask(() => - openModal('new-workspace-composer', { - initialRepoId: repoForLookup.id, - telemetrySource: 'command_palette' - }) - ) - }) + openComposer( + repoForLookup + ? { prefilledName: trimmed, initialRepoId: repoForLookup.id } + : { prefilledName: trimmed } + ) return } diff --git a/src/renderer/src/components/cmd-j/palette-focus-restore-target.test.ts b/src/renderer/src/components/cmd-j/palette-focus-restore-target.test.ts new file mode 100644 index 000000000..76b4f28eb --- /dev/null +++ b/src/renderer/src/components/cmd-j/palette-focus-restore-target.test.ts @@ -0,0 +1,60 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it } from 'vitest' +import { resolvePaletteFocusRestoreTarget } from './palette-focus-restore-target' + +afterEach(() => { + document.body.innerHTML = '' +}) + +function addTerminal(id: string): HTMLTextAreaElement { + const textarea = document.createElement('textarea') + textarea.className = 'xterm-helper-textarea' + textarea.dataset.terminal = id + document.body.appendChild(textarea) + return textarea +} + +describe('resolvePaletteFocusRestoreTarget', () => { + it('returns the exact previously-focused element when it is still connected', () => { + // Two terminals mounted (e.g. a background worktree comes first in the DOM); + // the user was typing in the second one before opening Cmd+J. + addTerminal('background') + const active = addTerminal('active') + + expect(resolvePaletteFocusRestoreTarget(active)).toBe(active) + }) + + it('falls back to the first terminal when the previous element is gone', () => { + const first = addTerminal('first') + const detached = document.createElement('textarea') + detached.className = 'xterm-helper-textarea' + // Never appended → not connected (e.g. its pane unmounted while Cmd+J was open). + + expect(detached.isConnected).toBe(false) + expect(resolvePaletteFocusRestoreTarget(detached)).toBe(first) + }) + + it('falls back to the editor textarea when no preferred target and no terminal exist', () => { + const editor = document.createElement('div') + editor.className = 'monaco-editor' + const textarea = document.createElement('textarea') + editor.appendChild(textarea) + document.body.appendChild(editor) + + expect(resolvePaletteFocusRestoreTarget(null)).toBe(textarea) + }) + + it('prefers the terminal over the editor when both are present and nothing was captured', () => { + const terminal = addTerminal('only') + const editor = document.createElement('div') + editor.className = 'monaco-editor' + editor.appendChild(document.createElement('textarea')) + document.body.appendChild(editor) + + expect(resolvePaletteFocusRestoreTarget(null)).toBe(terminal) + }) + + it('returns null when nothing is focusable', () => { + expect(resolvePaletteFocusRestoreTarget(null)).toBeNull() + }) +}) diff --git a/src/renderer/src/components/cmd-j/palette-focus-restore-target.ts b/src/renderer/src/components/cmd-j/palette-focus-restore-target.ts new file mode 100644 index 000000000..3d739d8b0 --- /dev/null +++ b/src/renderer/src/components/cmd-j/palette-focus-restore-target.ts @@ -0,0 +1,19 @@ +// Why: when Cmd+J closes it must hand focus back to whatever the user was +// doing. Prefer the exact element focused before the palette opened (e.g. the +// specific terminal textarea they were typing in); the querySelector fallbacks +// grab the first match in the DOM, which can be a background worktree's +// mounted-but-hidden terminal rather than the visible one. +export function resolvePaletteFocusRestoreTarget( + preferredTarget: HTMLElement | null, + doc: Document = document +): HTMLElement | null { + if (preferredTarget && preferredTarget.isConnected) { + return preferredTarget + } + const xterm = doc.querySelector('.xterm-helper-textarea') + if (xterm instanceof HTMLElement) { + return xterm + } + const monaco = doc.querySelector('.monaco-editor textarea') + return monaco instanceof HTMLElement ? monaco : null +} diff --git a/src/renderer/src/components/worktree-jump-palette-source-context-boundary.test.ts b/src/renderer/src/components/worktree-jump-palette-source-context-boundary.test.ts index ac60d86d2..31bdaf65d 100644 --- a/src/renderer/src/components/worktree-jump-palette-source-context-boundary.test.ts +++ b/src/renderer/src/components/worktree-jump-palette-source-context-boundary.test.ts @@ -13,14 +13,21 @@ function sourceBetween(startPattern: string, endPattern: string): string { } describe('WorktreeJumpPalette source-context boundaries', () => { - it('resolves typed GitHub issue/PR entries through the lookup repo source host', () => { - expect(source).toContain('buildTaskSourceContextFromRepo') - + it('defers pasted GitHub URL resolution to the composer so cross-project detection runs', () => { + // Why: pasting a cross-project URL must surface the same "Switch project?" + // prompt as Cmd+N. The palette hands the raw URL to the composer's name + // field instead of pre-resolving it against an arbitrary repo, which + // silently linked cross-project items to the wrong project. const githubLinkSection = sourceBetween( - 'void lookupGitHubWorkItemByOwnerRepoForSource({', + '// Case 1: user pasted a GH issue/PR URL.', '// Case 2: user typed a raw issue number.' ) - expect(githubLinkSection).toContain('sourceContext') + expect(githubLinkSection).toContain('prefilledName: trimmed') + expect(githubLinkSection).not.toContain('lookupGitHubWorkItemByOwnerRepoForSource') + }) + + it('resolves typed raw issue/PR numbers through the lookup repo source host', () => { + expect(source).toContain('buildTaskSourceContextFromRepo') const rawNumberSection = sourceBetween( 'void lookupGitHubWorkItemForSource({',