From b428def8f171cdc500f22a442047c104e3255a4d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:36:57 -0700 Subject: [PATCH] feat(cli): open files at a requested line and column (#28240) Accept positive safe-integer line and column flags through the existing file-open path. Reject invalid or incomplete positions before changing the editor. Bind deferred reveals to the requested workspace and tab, including floating panels, and clamp oversized positions without moving another editor or stealing late focus. Co-authored-by: Nikita Maketryuk --- docs/site/content/docs/cli/reference.mdx | 3 +- src/cli/command-scoped-flag-help.ts | 2 + src/cli/handlers/file-open-position.ts | 30 ++ src/cli/handlers/file.test.ts | 37 ++ src/cli/handlers/file.ts | 5 +- src/cli/root-help-text-secondary.ts | 2 +- src/cli/specs/file.ts | 7 +- ...ured-agent-session-permission-mode.test.ts | 24 ++ .../runtime/orca-runtime-file-commands.ts | 11 +- ...untime-files-mobile-explorer-reads.test.ts | 17 +- .../rpc/methods/files-open-navigation.test.ts | 37 ++ src/main/runtime/rpc/methods/files.ts | 7 +- .../runtime/runtime-file-command-access.ts | 7 +- src/main/runtime/runtime-file-command-host.ts | 4 +- src/main/runtime/runtime-notifier-contract.ts | 4 +- ...runtime-window-lifecycle-file-open.test.ts | 14 + src/main/window/runtime-window-lifecycle.ts | 5 +- .../ui-bridge-terminal-and-session-tabs.ts | 3 + src/preload/api/ui-command-event-api.ts | 2 + .../editor/EditorEditFileSurface.tsx | 11 +- .../src/components/editor/MonacoEditor.tsx | 35 +- .../editor/monaco-editor-mount-params.ts | 3 +- .../monaco-view-state-persistence.test.ts | 10 + .../editor/monaco-view-state-persistence.ts | 9 +- .../editor/use-monaco-editor-mount.ts | 36 +- .../use-monaco-reveal-scheduler.test.tsx | 74 ++++ .../editor/use-monaco-reveal-scheduler.ts | 9 +- .../mobile-terminal-close-ipc-bridge.test.ts | 328 +++++++++++++++++- .../mobile-terminal-close-ipc-bridge.ts | 18 +- .../ipc-events/runtime-file-position.test.ts | 197 +++++++++++ .../hooks/ipc-events/runtime-file-position.ts | 180 ++++++++++ ...s-close-routing-active-browser-tab.test.ts | 2 +- .../src/hooks/useIpcEvents-lifecycle.test.ts | 22 +- .../editor/focus/editor-focus-reveal.ts | 14 +- .../editor/types/pending-editor-reveal.ts | 3 + src/shared/rpc-contract/files-params.ts | 12 +- src/shared/runtime-file-contracts.ts | 5 + tests/e2e/cli-file-open-position.spec.ts | 205 +++++++++++ .../cli-floating-file-open-position.spec.ts | 206 +++++++++++ .../helpers/hidden-renderer-frame-capture.ts | 45 +++ tests/e2e/tab-hover-card.spec.ts | 239 ++++++------- 41 files changed, 1707 insertions(+), 177 deletions(-) create mode 100644 src/cli/handlers/file-open-position.ts create mode 100644 src/renderer/src/components/editor/use-monaco-reveal-scheduler.test.tsx create mode 100644 src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts create mode 100644 src/renderer/src/hooks/ipc-events/runtime-file-position.ts create mode 100644 tests/e2e/cli-file-open-position.spec.ts create mode 100644 tests/e2e/cli-floating-file-open-position.spec.ts create mode 100644 tests/e2e/helpers/hidden-renderer-frame-capture.ts diff --git a/docs/site/content/docs/cli/reference.mdx b/docs/site/content/docs/cli/reference.mdx index 46e995e0b47..f0e2061242c 100644 --- a/docs/site/content/docs/cli/reference.mdx +++ b/docs/site/content/docs/cli/reference.mdx @@ -201,11 +201,12 @@ For long output, use cursor reads. Save `nextCursor` from one stream read, then ```bash orca file open src/App.tsx --worktree active --json +orca file open src/App.tsx --line 42 --column 7 --worktree active --focus --json orca file diff src/App.tsx --staged --worktree active --json orca file open-changed --mode both --worktree active --json ``` -Paths are relative to the selected worktree. `open-changed` reads git status and opens changed files in edit, diff, or both modes. Tabs open without changing what you are looking at; pass `--focus` to be brought to them. +Paths are relative to the selected worktree. `--line` and `--column` are 1-based; a column requires a line. Positions are applied when you select the file, or immediately with `--focus`. Markdown switches to source view for positioning. Older hosts open the file without positioning. `open-changed` reads git status and opens changed files in edit, diff, or both modes. Tabs open without changing what you are looking at; pass `--focus` to be brought to them. ## Built-in browser diff --git a/src/cli/command-scoped-flag-help.ts b/src/cli/command-scoped-flag-help.ts index 0b34dd8cab6..01a00fa4872 100644 --- a/src/cli/command-scoped-flag-help.ts +++ b/src/cli/command-scoped-flag-help.ts @@ -40,6 +40,8 @@ const COMMAND_SCOPED_FLAG_HELP: Record> = { yes: '--yes Confirm stopping the server and unlinking it from this machine' }, 'file open': { + line: '--line Cursor line (1-based)', + column: '--column Cursor column (1-based; requires --line)', focus: FILE_OPEN_FOCUS_HELP }, 'file diff': { diff --git a/src/cli/handlers/file-open-position.ts b/src/cli/handlers/file-open-position.ts new file mode 100644 index 00000000000..3b99b3d5f48 --- /dev/null +++ b/src/cli/handlers/file-open-position.ts @@ -0,0 +1,30 @@ +import type { RuntimeFileOpenPosition } from '../../shared/runtime-file-contracts' +import { getOptionalPositiveIntegerFlag } from '../flags' +import { RuntimeClientError } from '../runtime-client' + +export function getFileOpenPosition( + flags: Map +): RuntimeFileOpenPosition | undefined { + for (const name of ['line', 'column']) { + if (flags.get(name) === '') { + throw new RuntimeClientError('invalid_argument', `Missing value for --${name}.`) + } + } + const line = getOptionalPositiveIntegerFlag(flags, 'line') + const column = getOptionalPositiveIntegerFlag(flags, 'column') + for (const [name, value] of [ + ['line', line], + ['column', column] + ] as const) { + if (value !== undefined && !Number.isSafeInteger(value)) { + throw new RuntimeClientError('invalid_argument', `Invalid positive integer for --${name}`) + } + } + if (line === undefined) { + if (column !== undefined) { + throw new RuntimeClientError('invalid_argument', '--column requires --line.') + } + return undefined + } + return column === undefined ? { line } : { line, column } +} diff --git a/src/cli/handlers/file.test.ts b/src/cli/handlers/file.test.ts index 016eaff6df5..9d8a134904b 100644 --- a/src/cli/handlers/file.test.ts +++ b/src/cli/handlers/file.test.ts @@ -88,6 +88,43 @@ describe('orca file CLI handlers', () => { expect(vi.mocked(console.log).mock.calls[0][0]).toBe('Opened src/App.tsx.') }) + it('forwards 1-based positions while preserving default background navigation', async () => { + queueFixtures( + callMock, + okFixture('open', { worktree: 'wt-1', relativePath: 'a.ts', kind: 'text', opened: true }) + ) + await main( + ['file', 'open', 'a.ts', '--worktree', 'id:wt-1', '--line', '42', '--column', '7'], + '/tmp/repo' + ) + expect(callMock).toHaveBeenCalledWith('files.open', { + worktree: 'id:wt-1', + relativePath: 'a.ts', + navigation: 'caller', + line: 42, + column: 7 + }) + }) + + it.each([ + ['--column', '7'], + ['--line', '0'], + ['--line', '-1'], + ['--line', '4.5'], + ['--line', 'x'], + ['--line', '9007199254740992'], + ['--line='], + ['--line'], + ['--line', '2', '--column='], + ['--line', '2', '--column', '0'] + ])('rejects invalid positioning %j before resolving a workspace', async (...flags) => { + await main(['file', 'open', 'a.ts', ...flags], '/tmp/repo') + expect(process.exitCode).toBe(1) + expect(callMock).not.toHaveBeenCalled() + expect(vi.mocked(console.error).mock.calls[0]?.[0]).toMatch(/--(?:line|column)/) + process.exitCode = undefined + }) + it('opens a staged diff for an explicit worktree without cwd inference', async () => { queueFixtures( callMock, diff --git a/src/cli/handlers/file.ts b/src/cli/handlers/file.ts index a7973d59be5..c0eb9ac7048 100644 --- a/src/cli/handlers/file.ts +++ b/src/cli/handlers/file.ts @@ -1,3 +1,4 @@ +import { getFileOpenPosition } from './file-open-position' import type { GitStatusEntry, GitStatusResult } from '../../shared/git-status-types' import type { RuntimeFileOpenResult, RuntimeWorktreeRecord } from '../../shared/runtime-types' import type { RuntimeNavigationTarget } from '../../shared/runtime-navigation' @@ -213,12 +214,14 @@ function formatFileDiff(result: RuntimeFileOpenResult): string { export const FILE_HANDLERS: Record = { 'file open': async (ctx) => { const path = getRequiredStringFlag(ctx.flags, 'path') + const position = getFileOpenPosition(ctx.flags) const worktree = await getFileWorktreeSelector(ctx) const relativePath = await resolveFilePath(ctx, worktree, path) const result = await ctx.client.call('files.open', { worktree, relativePath, - navigation: getFileOpenNavigation(ctx.flags) + navigation: getFileOpenNavigation(ctx.flags), + ...position }) requireOpened(result.result, relativePath) printResult(result, ctx.json, formatFileOpen) diff --git a/src/cli/root-help-text-secondary.ts b/src/cli/root-help-text-secondary.ts index e42ad51c5f1..a3b728141b3 100644 --- a/src/cli/root-help-text-secondary.ts +++ b/src/cli/root-help-text-secondary.ts @@ -56,7 +56,7 @@ export const ROOT_HELP_TEXT_SECONDARY = [ ' orca worktree set --worktree [--display-name ] [--issue ] [--pr ] [--linear-issue ] [--gitlab-issue ] [--gitlab-mr ] [--comment ] [--workspace-status ] [--unread|--read] [--parent-worktree |--no-parent] [--json]', ' orca worktree rm --worktree [--force] [--run-hooks] [--allow-failed-archive-hook] [--json]', ' orca worktree ps [--limit ] [--json]', - ' orca file open [--worktree ] [--focus] [--json]', + ' orca file open [--line [--column ]] [--worktree ] [--focus] [--json]', ' orca file diff [--staged] [--worktree ] [--focus] [--json]', ' orca file open-changed [--mode edit|diff|both] [--worktree ] [--focus] [--json]', ' orca terminal list [--worktree ] [--limit ] [--include-visual-layouts] [--json]', diff --git a/src/cli/specs/file.ts b/src/cli/specs/file.ts index 572671c49fb..cc4dca94081 100644 --- a/src/cli/specs/file.ts +++ b/src/cli/specs/file.ts @@ -6,15 +6,18 @@ export const FILE_COMMAND_SPECS: CommandSpec[] = [ path: ['file', 'open'], summary: 'Open a file as a tab in its worktree without changing your view; --focus brings you to it', - usage: 'orca file open [--worktree ] [--focus] [--json]', - allowedFlags: [...GLOBAL_FLAGS, 'path', 'worktree', 'focus'], + usage: + 'orca file open [--line [--column ]] [--worktree ] [--focus] [--json]', + allowedFlags: [...GLOBAL_FLAGS, 'path', 'line', 'column', 'worktree', 'focus'], positionalArgs: ['path'], notes: [ + '--line and --column use 1-based positions; --column requires --line. Without --focus, the position is applied when you select the file. Markdown opens in source view for positioning. Older hosts open the file without positioning.', 'The file opens as a tab in that worktree without changing what you are looking at, even in the worktree on screen. --focus brings you to it; agents should pass --focus only when the user asked to see the file.', 'The path may be relative to the selected worktree or an absolute path inside that worktree. When --worktree is omitted, local CLI calls infer the current Orca worktree from cwd.' ], examples: [ 'orca file open src/App.tsx', + 'orca file open src/App.tsx --line 42 --column 7 --focus', 'orca file open --path docs/readme.md --worktree active --focus' ] }, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-permission-mode.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-permission-mode.test.ts index 6b5616a53fb..66dbb918c23 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-permission-mode.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-permission-mode.test.ts @@ -6,6 +6,7 @@ import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-ses import { createRestTestRig, foundRestTestChat, + IDLE_MS, REST_TEST_CALLER as CALLER, REST_TEST_SESSION as SESSION, restTestSend, @@ -64,6 +65,29 @@ describe('a chat permission mode', () => { expect(rig.host.readStatusSummary(SESSION)?.permissionMode).toBe('full-access') }) + it('records a pick at rest with the host clock without starting an agent', async () => { + await foundRestTestChat(rig) + rig.clock.now += IDLE_MS + 1 + await vi.waitFor(() => expect(rig.store.getRecord(SESSION)?.lease.claimStatus).toBe('released')) + expect(rig.host.collaboratorsForTests().sessions.get(SESSION)?.child).toBeUndefined() + rig.clock.now += 123 + const pickedAt = rig.clock.now + const setOption = vi.spyOn(rig.host.deps.adapter, 'setOption') + + expect(await pick('full-access')).toMatchObject({ + ok: true, + value: { options: { permissionMode: 'full-access' } } + }) + expect(rig.store.getRecord(SESSION)).toMatchObject({ + permissionMode: 'full-access', + updatedAt: pickedAt + }) + expect(setOption).not.toHaveBeenCalled() + expect(rig.adapter.acquire).toHaveBeenCalledTimes(1) + await rig.restart() + expect((await rig.host.readOptions(SESSION)).current.permissionMode).toBe('full-access') + }) + it('keeps Full access through a model pick on the running agent and the next start', async () => { await foundRestTestChat(rig) await pick('full-access') diff --git a/src/main/runtime/orca-runtime-file-commands.ts b/src/main/runtime/orca-runtime-file-commands.ts index b504d6198f7..b5a229b6bb5 100644 --- a/src/main/runtime/orca-runtime-file-commands.ts +++ b/src/main/runtime/orca-runtime-file-commands.ts @@ -44,11 +44,18 @@ export class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreservedBranchC absolutePath }), resolveRuntimeGitTarget: (selector) => this.resolveRuntimeGitTarget(selector), - openFile: (worktreeId, filePath, relativePath, runtimeEnvironmentId, navigation) => { + openFile: (worktreeId, filePath, relativePath, runtimeEnvironmentId, navigation, position) => { if (!this.notifier?.openFile) { throw new Error('renderer_unavailable') } - this.notifier.openFile(worktreeId, filePath, relativePath, runtimeEnvironmentId, navigation) + this.notifier.openFile( + worktreeId, + filePath, + relativePath, + runtimeEnvironmentId, + navigation, + ...(position ? [position] : []) + ) }, openDiff: (worktreeId, filePath, relativePath, staged, runtimeEnvironmentId, navigation) => { if (!this.notifier?.openDiff) { diff --git a/src/main/runtime/orca-runtime-files-mobile-explorer-reads.test.ts b/src/main/runtime/orca-runtime-files-mobile-explorer-reads.test.ts index 403e928a255..683d56cc81f 100644 --- a/src/main/runtime/orca-runtime-files-mobile-explorer-reads.test.ts +++ b/src/main/runtime/orca-runtime-files-mobile-explorer-reads.test.ts @@ -1,3 +1,4 @@ +import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' import { SshHostNotServedError } from '../providers/ssh-host-not-served-error' import type * as FileSystem from 'node:fs/promises' @@ -127,6 +128,18 @@ describe('RuntimeFileCommands', () => { }) }) + it('passes file positioning to the renderer with the resolved owner', async () => { + const openFile = vi.fn() + const { commands } = createRuntimeFileCommands({ openFile }) + resolveAuthorizedPathMock.mockResolvedValue('/repo/a.ts') + statMock.mockResolvedValue({ isDirectory: () => false }) + await commands.openMobileFile('id:wt-1', 'a.ts', 'caller', { line: 12, column: 5 }) + expect(openFile).toHaveBeenCalledWith('wt-1', '/repo/a.ts', 'a.ts', null, 'caller', { + line: 12, + column: 5 + }) + }) + it('opens previewable images through the renderer host as an image tab', async () => { const openFile = vi.fn() const { commands } = createRuntimeFileCommands({ openFile }) @@ -290,7 +303,7 @@ describe('RuntimeFileCommands', () => { { name: 'linked-docs', isDirectory: false, isSymlink: true }, { name: 'README.md', isDirectory: false, isSymlink: false } ]) - expect(statMock).not.toHaveBeenCalledWith('/repo/linked-docs') + expect(statMock).not.toHaveBeenCalledWith(join('/repo', 'linked-docs')) }) it.each([ { setting: true, override: undefined, expected: true }, @@ -310,7 +323,7 @@ describe('RuntimeFileCommands', () => { ).resolves.toEqual([{ name: 'linked-docs', isDirectory: expected, isSymlink: true }]) expect(store.getSettings).toHaveBeenCalledTimes(override === undefined ? 1 : 0) if (expected) { - expect(statMock).toHaveBeenCalledWith('/repo/linked-docs') + expect(statMock).toHaveBeenCalledWith(join('/repo', 'linked-docs')) } else { expect(statMock).not.toHaveBeenCalled() } diff --git a/src/main/runtime/rpc/methods/files-open-navigation.test.ts b/src/main/runtime/rpc/methods/files-open-navigation.test.ts index d7d8809c512..faef34891c9 100644 --- a/src/main/runtime/rpc/methods/files-open-navigation.test.ts +++ b/src/main/runtime/rpc/methods/files-open-navigation.test.ts @@ -60,6 +60,43 @@ describe('file open RPC navigation', () => { expect(runtime.openMobileDiff).toHaveBeenCalledWith('id:wt-1', 'a.ts', true, undefined) }) + it('forwards optional positions, leaving diffs unchanged', async () => { + const { runtime, dispatcher } = createDispatcher() + const response = await dispatcher.dispatch( + makeRequest('files.open', { worktree: 'id:wt-1', relativePath: 'a.ts', line: 42, column: 7 }) + ) + expect(response).toMatchObject({ ok: true }) + expect(runtime.openMobileFile).toHaveBeenCalledWith('id:wt-1', 'a.ts', undefined, { + line: 42, + column: 7 + }) + await dispatcher.dispatch( + makeRequest('files.openDiff', { worktree: 'id:wt-1', relativePath: 'a.ts', line: 42 }) + ) + expect(runtime.openMobileDiff).toHaveBeenCalledWith('id:wt-1', 'a.ts', false, undefined) + }) + + it('rejects a column without a line', async () => { + const { runtime, dispatcher } = createDispatcher() + const response = await dispatcher.dispatch( + makeRequest('files.open', { worktree: 'id:wt-1', relativePath: 'a.ts', column: 7 }) + ) + expect(response).toMatchObject({ ok: false }) + expect(runtime.openMobileFile).not.toHaveBeenCalled() + }) + + it.each([0, -1, 1.5, '12', Infinity, Number.MAX_SAFE_INTEGER + 1])( + 'rejects invalid RPC positions %j', + async (line) => { + const { runtime, dispatcher } = createDispatcher() + const response = await dispatcher.dispatch( + makeRequest('files.open', { worktree: 'id:wt-1', relativePath: 'a.ts', line }) + ) + expect(response).toMatchObject({ ok: false }) + expect(runtime.openMobileFile).not.toHaveBeenCalled() + } + ) + it('rejects an unknown navigation target on file opens', async () => { const { runtime, dispatcher } = createDispatcher() diff --git a/src/main/runtime/rpc/methods/files.ts b/src/main/runtime/rpc/methods/files.ts index 7c49771448a..6d1ef332a7d 100644 --- a/src/main/runtime/rpc/methods/files.ts +++ b/src/main/runtime/rpc/methods/files.ts @@ -66,7 +66,12 @@ export const FILE_METHODS = [ permission: 'workspace', params: FileOpenTab, handler: async (params, { runtime }) => - runtime.openMobileFile(params.worktree, params.relativePath, params.navigation) + runtime.openMobileFile( + params.worktree, + params.relativePath, + params.navigation, + ...(params.line === undefined ? [] : [{ line: params.line, column: params.column }]) + ) }), defineMethod({ name: 'files.openDiff', diff --git a/src/main/runtime/runtime-file-command-access.ts b/src/main/runtime/runtime-file-command-access.ts index 433d055b63f..223e5be2c76 100644 --- a/src/main/runtime/runtime-file-command-access.ts +++ b/src/main/runtime/runtime-file-command-access.ts @@ -1,3 +1,4 @@ +import type { RuntimeFileOpenPosition } from '../../shared/runtime-file-contracts' import { RuntimeFileSearch } from './runtime-file-search' import { listFilesystemMarkdownDocuments } from '../providers/filesystem-markdown-listing' import type { MarkdownDocument } from '../../shared/filesystem-entry-types' @@ -58,7 +59,8 @@ export class RuntimeFileCommandAccess { async openMobileFile( worktreeSelector: string, relativePath: string, - navigation?: RuntimeNavigationTarget + navigation?: RuntimeNavigationTarget, + position?: RuntimeFileOpenPosition ): Promise { const target = await this.host.resolveRuntimeFileTarget(worktreeSelector) const { worktree } = target @@ -81,7 +83,8 @@ export class RuntimeFileCommandAccess { filePath, relativePath, runtimeOwnerForTarget(target), - navigation + navigation, + ...(position ? [position] : []) ) return { worktree: worktree.id, relativePath, kind, opened: true } } diff --git a/src/main/runtime/runtime-file-command-host.ts b/src/main/runtime/runtime-file-command-host.ts index f86bed0247c..a9c05470783 100644 --- a/src/main/runtime/runtime-file-command-host.ts +++ b/src/main/runtime/runtime-file-command-host.ts @@ -1,4 +1,5 @@ // @ts-nocheck -- mechanically split declarations. +import type { RuntimeFileOpenPosition } from '../../shared/runtime-file-contracts' import type { Store } from '../persistence' import type { ResolvedRuntimeFileTarget, @@ -54,7 +55,8 @@ export type RuntimeFileCommandHost = { filePath: string, relativePath: string, runtimeEnvironmentId?: string | null, - navigation?: RuntimeNavigationTarget + navigation?: RuntimeNavigationTarget, + position?: RuntimeFileOpenPosition ): void openDiff( worktreeId: string, diff --git a/src/main/runtime/runtime-notifier-contract.ts b/src/main/runtime/runtime-notifier-contract.ts index 340301e7bb9..5f12d134e08 100644 --- a/src/main/runtime/runtime-notifier-contract.ts +++ b/src/main/runtime/runtime-notifier-contract.ts @@ -1,3 +1,4 @@ +import type { RuntimeFileOpenPosition } from '../../shared/runtime-file-contracts' import type { AgentLaunchPaneVerdictEvent } from '../../shared/agent-launch-pane-verdict' import type { SleepingAgentLaunchConfig } from '../../shared/agent-session-resume' import type { @@ -126,7 +127,8 @@ export type RuntimeNotifier = { filePath: string, relativePath: string, runtimeEnvironmentId?: string | null, - navigation?: RuntimeNavigationTarget + navigation?: RuntimeNavigationTarget, + position?: RuntimeFileOpenPosition ): void openDiff?( worktreeId: string, diff --git a/src/main/window/runtime-window-lifecycle-file-open.test.ts b/src/main/window/runtime-window-lifecycle-file-open.test.ts index f901c6be83c..cbf24a225b2 100644 --- a/src/main/window/runtime-window-lifecycle-file-open.test.ts +++ b/src/main/window/runtime-window-lifecycle-file-open.test.ts @@ -66,9 +66,23 @@ describe('runtime window file-open notifications', () => { }) for (const [, payload] of send.mock.calls) { expect(payload).not.toHaveProperty('navigation') + expect(payload).not.toHaveProperty('position') } }) + it('adds position only when requested', () => { + const { notifier, send } = attachNotifier() + notifier.openFile?.('wt-1', '/repo/a.ts', 'a.ts', 'host-1', 'caller', { line: 12, column: 5 }) + expect(send).toHaveBeenCalledWith('ui:openFileFromMobile', { + worktreeId: 'wt-1', + filePath: '/repo/a.ts', + relativePath: 'a.ts', + runtimeEnvironmentId: 'host-1', + navigation: 'caller', + position: { line: 12, column: 5 } + }) + }) + it('forwards an explicit navigation target to the renderer', () => { const { notifier, send } = attachNotifier() diff --git a/src/main/window/runtime-window-lifecycle.ts b/src/main/window/runtime-window-lifecycle.ts index ca60b68f235..95acdabeff7 100644 --- a/src/main/window/runtime-window-lifecycle.ts +++ b/src/main/window/runtime-window-lifecycle.ts @@ -177,13 +177,14 @@ export function registerRuntimeWindowLifecycle( requestSessionTabCloseFromRenderer(mainWindow, tabId, worktreeId), moveSessionTab: (worktreeId: string, move: RuntimeMobileSessionTabMove) => send('ui:moveSessionTab', { worktreeId, ...move }), - openFile: (worktreeId, filePath, relativePath, runtimeEnvironmentId?, navigation?) => + openFile: (worktreeId, filePath, relativePath, runtimeEnvironmentId?, navigation?, position?) => send('ui:openFileFromMobile', { worktreeId, filePath, relativePath, runtimeEnvironmentId, - ...(navigation ? { navigation } : {}) + ...(navigation ? { navigation } : {}), + ...(position ? { position } : {}) }), openDiff: (worktreeId, filePath, relativePath, staged, runtimeEnvironmentId?, navigation?) => send('ui:openDiffFromMobile', { diff --git a/src/preload/api/ui-bridge-terminal-and-session-tabs.ts b/src/preload/api/ui-bridge-terminal-and-session-tabs.ts index 987499a6640..2af60a8707e 100644 --- a/src/preload/api/ui-bridge-terminal-and-session-tabs.ts +++ b/src/preload/api/ui-bridge-terminal-and-session-tabs.ts @@ -1,3 +1,4 @@ +import type { RuntimeFileOpenPosition } from '../../shared/runtime-file-contracts' import { ipcRenderer } from 'electron' import type { TerminalPaneSplitSource } from '../../shared/feature-education-telemetry' import type { TerminalTabCreateReply } from '../../shared/terminal-reveal-identity' @@ -232,6 +233,7 @@ export const uiTerminalAndSessionTabsApi = { relativePath: string runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget + position?: RuntimeFileOpenPosition }) => void ): (() => void) => { const listener = ( @@ -242,6 +244,7 @@ export const uiTerminalAndSessionTabsApi = { relativePath: string runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget + position?: RuntimeFileOpenPosition } ) => callback(data) ipcRenderer.on('ui:openFileFromMobile', listener) diff --git a/src/preload/api/ui-command-event-api.ts b/src/preload/api/ui-command-event-api.ts index 3fffee53269..d2d384345da 100644 --- a/src/preload/api/ui-command-event-api.ts +++ b/src/preload/api/ui-command-event-api.ts @@ -1,3 +1,4 @@ +import type { RuntimeFileOpenPosition } from '../../shared/runtime-file-contracts' import type { MarkdownDocument } from '../../shared/filesystem-entry-types' import type { PersistedUIState } from '../../shared/persisted-ui-state-types' import type { TuiAgent } from '../../shared/tui-agent' @@ -236,6 +237,7 @@ export type UiCommandEventApi = { relativePath: string runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget + position?: RuntimeFileOpenPosition }) => void ) => () => void onOpenDiffFromMobile: ( diff --git a/src/renderer/src/components/editor/EditorEditFileSurface.tsx b/src/renderer/src/components/editor/EditorEditFileSurface.tsx index c0eda2829ee..4e5a44fae76 100644 --- a/src/renderer/src/components/editor/EditorEditFileSurface.tsx +++ b/src/renderer/src/components/editor/EditorEditFileSurface.tsx @@ -232,17 +232,17 @@ export function EditorEditFileSurface({ markdownAnnotationsEnabled={markdownAnnotationsEnabled && isMarkdown} conflictDecorationsEnabled={activeFile.conflict?.conflictStatus === 'unresolved'} revealLine={ - matchesPendingEditorReveal(pendingEditorReveal, activeFile) + matchesPendingEditorReveal(pendingEditorReveal, activeFile, viewStateScopeId) ? pendingEditorReveal.line : undefined } revealColumn={ - matchesPendingEditorReveal(pendingEditorReveal, activeFile) + matchesPendingEditorReveal(pendingEditorReveal, activeFile, viewStateScopeId) ? pendingEditorReveal.column : undefined } revealMatchLength={ - matchesPendingEditorReveal(pendingEditorReveal, activeFile) + matchesPendingEditorReveal(pendingEditorReveal, activeFile, viewStateScopeId) ? pendingEditorReveal.matchLength : undefined } @@ -318,9 +318,10 @@ export function EditorEditFileSurface({ function matchesPendingEditorReveal( reveal: PendingEditorReveal | null, - file: Pick + file: Pick, + viewStateId: string ): reveal is PendingEditorReveal { - if (!reveal) { + if (!reveal || (reveal.viewStateId && reveal.viewStateId !== viewStateId)) { return false } return reveal.fileId ? reveal.fileId === file.id : reveal.filePath === file.filePath diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index 10cb1bb7c80..b955e01ba85 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -222,11 +222,36 @@ export default function MonacoEditor({ if (!revealLine || !editorRef.current) { return } - queueReveal(editorRef.current, revealLine, revealColumn ?? 1, revealMatchLength ?? 0, () => { - // Why: clear the pending payload only after the queued reveal runs, so navigation isn't lost if the editor unmounts first. - setPendingEditorReveal(null) - }) - }, [queueReveal, revealLine, revealColumn, revealMatchLength, setPendingEditorReveal]) + const reveal = useAppStore.getState().pendingEditorReveal + if (reveal?.viewStateId && (reveal.fileId !== fileId || reveal.viewStateId !== viewStateId)) { + return + } + queueReveal( + editorRef.current, + revealLine, + revealColumn ?? 1, + revealMatchLength ?? 0, + () => { + reveal?.onApplied?.() + // Why: a newer request may arrive during the editor-owned reveal frames. + if (useAppStore.getState().pendingEditorReveal === reveal) { + setPendingEditorReveal(null) + } + }, + reveal?.canApply + ? () => + useAppStore.getState().pendingEditorReveal === reveal && reveal.canApply?.() !== false + : undefined + ) + }, [ + queueReveal, + revealLine, + revealColumn, + revealMatchLength, + setPendingEditorReveal, + fileId, + viewStateId + ]) return (
void + onApplied?: () => void, + canApply?: () => boolean ) => void contentSync: { contentRef: MutableRefObject diff --git a/src/renderer/src/components/editor/monaco-view-state-persistence.test.ts b/src/renderer/src/components/editor/monaco-view-state-persistence.test.ts index a8001fdffb4..b1eec2dd430 100644 --- a/src/renderer/src/components/editor/monaco-view-state-persistence.test.ts +++ b/src/renderer/src/components/editor/monaco-view-state-persistence.test.ts @@ -58,6 +58,16 @@ describe('Monaco view state persistence', () => { expect(focus).toHaveBeenCalledOnce() }) + it('restores a sibling pane without claiming focus during a scoped reveal', () => { + scrollTopCache.set('file.ts::sibling', 320) + const pane = { setSelections: vi.fn(), setScrollTop: vi.fn(), focus: vi.fn() } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: restoreMonacoViewState accesses only these three stubbed editor methods. + const instance = pane as unknown as editor.IStandaloneCodeEditor + restoreMonacoViewState(instance, 'file.ts::sibling', () => false) + expect(pane.setScrollTop).toHaveBeenCalledWith(320) + expect(pane.focus).not.toHaveBeenCalled() + }) + it('defers selection caching until the lifecycle snapshot', () => { let emitCursorPosition: | ((event: { position: { lineNumber: number; column: number } }) => void) diff --git a/src/renderer/src/components/editor/monaco-view-state-persistence.ts b/src/renderer/src/components/editor/monaco-view-state-persistence.ts index 33c5a963923..83efa194a1f 100644 --- a/src/renderer/src/components/editor/monaco-view-state-persistence.ts +++ b/src/renderer/src/components/editor/monaco-view-state-persistence.ts @@ -42,7 +42,8 @@ export function installMonacoViewStateTracking(params: MonacoViewStateTrackingPa export function restoreMonacoViewState( editorInstance: editor.IStandaloneCodeEditor, - viewStateKey: string + viewStateKey: string, + canFocus?: () => boolean ): void { const savedSelections = editorSelectionCache.get(viewStateKey) const savedScrollTop = scrollTopCache.get(viewStateKey) @@ -55,9 +56,11 @@ export function restoreMonacoViewState( if (savedScrollTop !== undefined) { editorInstance.setScrollTop(savedScrollTop) } - editorInstance.focus() + if (canFocus?.() !== false) { + editorInstance.focus() + } }) - } else { + } else if (canFocus?.() !== false) { editorInstance.focus() } } diff --git a/src/renderer/src/components/editor/use-monaco-editor-mount.ts b/src/renderer/src/components/editor/use-monaco-editor-mount.ts index 3e355ffcefd..b794b41bd12 100644 --- a/src/renderer/src/components/editor/use-monaco-editor-mount.ts +++ b/src/renderer/src/components/editor/use-monaco-editor-mount.ts @@ -199,12 +199,38 @@ export function useMonacoEditorMount(params: MonacoEditorMountParams): OnMount { const revealMatchesEditor = reveal?.fileId ? reveal.fileId === fileId : reveal?.filePath === filePath - if (reveal && revealMatchesEditor) { - queueReveal(editorInstance, reveal.line, reveal.column, reveal.matchLength, () => { - useAppStore.getState().setPendingEditorReveal(null) - }) + if ( + reveal && + revealMatchesEditor && + (!reveal.viewStateId || reveal.viewStateId === viewStateId) + ) { + if (reveal.canApply?.() === false) { + restoreMonacoViewState(editorInstance, viewStateKey, () => false) + return + } + queueReveal( + editorInstance, + reveal.line, + reveal.column, + reveal.matchLength, + () => { + reveal.onApplied?.() + if (useAppStore.getState().pendingEditorReveal === reveal) { + useAppStore.getState().setPendingEditorReveal(null) + } + }, + reveal.canApply + ? () => + useAppStore.getState().pendingEditorReveal === reveal && + reveal.canApply?.() !== false + : undefined + ) } else { - restoreMonacoViewState(editorInstance, viewStateKey) + restoreMonacoViewState( + editorInstance, + viewStateKey, + reveal?.viewStateId && revealMatchesEditor ? () => false : undefined + ) } // Why: every mount path above focuses, so an explicit open handoff is already satisfied here. diff --git a/src/renderer/src/components/editor/use-monaco-reveal-scheduler.test.tsx b/src/renderer/src/components/editor/use-monaco-reveal-scheduler.test.tsx new file mode 100644 index 00000000000..1f2fac0fefc --- /dev/null +++ b/src/renderer/src/components/editor/use-monaco-reveal-scheduler.test.tsx @@ -0,0 +1,74 @@ +// @vitest-environment happy-dom +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { editor } from 'monaco-editor' +import { useMonacoRevealScheduler } from './use-monaco-reveal-scheduler' +import { performReveal } from './monaco-reveal' + +vi.mock('./monaco-reveal', () => ({ MAX_REVEAL_CONTENT_WAIT_FRAMES: 120, performReveal: vi.fn() })) + +const frames: FrameRequestCallback[] = [] +const model = { getLineCount: () => 400 } +// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the scheduler reads only getModel/getLineCount; performReveal is mocked. +const instance = { getModel: () => model } as unknown as editor.IStandaloneCodeEditor + +function flushFrames(): void { + while (frames.length) { + frames.shift()?.(0) + } +} + +describe('Monaco reveal application guard', () => { + beforeEach(() => { + vi.clearAllMocks() + frames.length = 0 + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => + frames.push(callback) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + }) + + it('does not focus or move the editor when its target becomes inactive before application', () => { + const { result, unmount } = renderHook(() => useMonacoRevealScheduler()) + let current = true + const applied = vi.fn() + act(() => result.current.queueReveal(instance, 300, 7, 0, applied, () => current)) + frames.shift()?.(0) + current = false + flushFrames() + expect(performReveal).not.toHaveBeenCalled() + expect(applied).not.toHaveBeenCalled() + unmount() + }) + + it('applies and acknowledges a still-current target, preserving legacy callers', () => { + const { result, unmount } = renderHook(() => useMonacoRevealScheduler()) + const applied = vi.fn() + act(() => result.current.queueReveal(instance, 300, 7, 0, applied)) + flushFrames() + expect(performReveal).toHaveBeenCalledWith( + instance, + 300, + 7, + 0, + expect.any(Function), + expect.any(Object), + expect.any(Object) + ) + expect(applied).toHaveBeenCalledOnce() + unmount() + }) + + it('checks the guard again after waiting for asynchronous file content', () => { + const { result, unmount } = renderHook(() => useMonacoRevealScheduler()) + let current = true + vi.spyOn(model, 'getLineCount').mockReturnValueOnce(1).mockReturnValue(400) + act(() => result.current.queueReveal(instance, 300, 7, 0, undefined, () => current)) + frames.shift()?.(0) + frames.shift()?.(0) + current = false + flushFrames() + expect(performReveal).not.toHaveBeenCalled() + unmount() + }) +}) diff --git a/src/renderer/src/components/editor/use-monaco-reveal-scheduler.ts b/src/renderer/src/components/editor/use-monaco-reveal-scheduler.ts index e1d7d961bba..66f18daf061 100644 --- a/src/renderer/src/components/editor/use-monaco-reveal-scheduler.ts +++ b/src/renderer/src/components/editor/use-monaco-reveal-scheduler.ts @@ -10,7 +10,8 @@ export type MonacoRevealScheduler = { line: number, column: number, matchLength: number, - onApplied?: () => void + onApplied?: () => void, + canApply?: () => boolean ) => void } @@ -47,7 +48,8 @@ export function useMonacoRevealScheduler(): MonacoRevealScheduler { line: number, column: number, matchLength: number, - onApplied?: () => void + onApplied?: () => void, + canApply?: () => boolean ) => { cancelScheduledReveal() let waitFrames = 0 @@ -58,6 +60,9 @@ export function useMonacoRevealScheduler(): MonacoRevealScheduler { revealInnerRafRef.current = requestAnimationFrame(() => { revealRafRef.current = null revealInnerRafRef.current = null + if (canApply?.() === false) { + return + } const modelLineCount = editorInstance.getModel()?.getLineCount() ?? 0 if (line > 1 && modelLineCount < line && waitFrames < MAX_REVEAL_CONTENT_WAIT_FRAMES) { // Why: fresh opens can mount an empty 1-line model before the async read; waiting stops the target line clamping to 1. diff --git a/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.test.ts b/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.test.ts index a0091a8fba0..24b78269925 100644 --- a/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.test.ts +++ b/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { RuntimeNavigationTarget } from '../../../../shared/runtime-navigation' +import { FLOATING_TERMINAL_WORKTREE_ID, getDefaultSettings } from '../../../../shared/constants' import { createTestStore, makeWorktree, seedStore } from '../../store/slices/store-test-helpers' import { createStoreSessionMockApi } from '../../store/slices/store-session-test-harness' import { buildMobileSessionTabSnapshots } from '@/runtime/sync-runtime-graph/mobile-session-snapshots' @@ -20,6 +21,8 @@ vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.f vi.mock('../../store', () => ({ useAppStore: { getState: () => testStore().getState(), + subscribe: (...args: Parameters['subscribe']>) => + testStore().subscribe(...args), setState: (...args: Parameters['setState']>) => testStore().setState(...args) } @@ -43,6 +46,7 @@ type OpenFilePayload = { relativePath: string runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget + position?: { line: number; column?: number } } type OpenDiffPayload = OpenFilePayload & { staged: boolean } type TestStore = ReturnType @@ -57,6 +61,7 @@ function setup(viewedSurface: 'editor' | 'terminal'): { openFile: (payload: OpenFilePayload) => void openDiff: (payload: OpenDiffPayload) => void store: TestStore + dispose: () => void } { const mockApi = createStoreSessionMockApi() const listeners: { @@ -105,12 +110,18 @@ function setup(viewedSurface: 'editor' | 'terminal'): { store.getState().setActiveTabType('terminal', VIEWED) } store.setState({ activeView: 'terminal', pendingRevealWorktree: null }) - registerMobileAndTerminalCloseIpcBridge([], vi.fn()) + const unsubs: (() => void)[] = [] + registerMobileAndTerminalCloseIpcBridge(unsubs, vi.fn()) const { openFile, openDiff } = listeners if (!openFile || !openDiff) { throw new Error('file-open listeners were not registered') } - return { openFile, openDiff, store } + return { + openFile, + openDiff, + store, + dispose: () => unsubs.forEach((unsubscribe) => unsubscribe()) + } } /** Everything the user can see or type into on the desktop. */ @@ -453,3 +464,316 @@ describe('runtime file opens on the host desktop', () => { }) }) }) + +describe('runtime file positioning', () => { + const frames: FrameRequestCallback[] = [] + beforeEach(() => { + frames.length = 0 + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => + frames.push(callback) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + }) + function flushFrames(): void { + while (frames.length) { + frames.shift()?.(0) + } + } + + it.each([undefined, { line: 17 }])( + 'preserves a visible floating editor when another file opens in the background (position %s)', + (position) => { + const { openFile, store } = setup('editor') + store.setState({ + settings: { ...getDefaultSettings('/tmp'), floatingTerminalEnabled: true }, + floatingWorkspacePanelOpen: true, + floatingWorkspacePath: '/tmp/floating' + }) + openFile({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + filePath: '/tmp/floating/first.ts', + relativePath: 'first.ts', + navigation: 'all' + }) + const before = activeEditorEntityId(store, FLOATING_TERMINAL_WORKTREE_ID) + openFile({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + filePath: '/tmp/floating/second.ts', + relativePath: 'second.ts', + navigation: 'caller', + position + }) + flushFrames() + expect(activeEditorEntityId(store, FLOATING_TERMINAL_WORKTREE_ID)).toBe(before) + expect(store.getState().pendingEditorReveal).toBeNull() + } + ) + + it('reveals a focused file after mounting, including its column', () => { + const { openFile, store } = setup('terminal') + openFile({ + worktreeId: VIEWED, + ...VIEWED_APP_TS, + navigation: 'all', + position: { line: 12, column: 5 } + }) + expect(store.getState().pendingEditorReveal).toBeNull() + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ + filePath: VIEWED_APP_TS.filePath, + fileId: VIEWED_APP_TS.filePath, + line: 12, + column: 5, + matchLength: 0 + }) + }) + + it('preserves a viewed terminal until the positioned background file is selected', () => { + const { openFile, store } = setup('terminal') + const before = screenState(store) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller', position: { line: 3 } }) + flushFrames() + expect(screenState(store)).toEqual(before) + expect(store.getState().pendingEditorReveal).toBeNull() + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find( + (candidate) => candidate.entityId === VIEWED_APP_TS.filePath + ) + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ + fileId: VIEWED_APP_TS.filePath, + line: 3, + column: 1 + }) + }) + + it('defers a viewed Markdown source switch until the user returns to its tab', () => { + const { openFile, store } = setup('editor') + store.getState().setMarkdownViewMode(VIEWED_FILE, 'rich') + openFile({ + worktreeId: VIEWED, + filePath: VIEWED_FILE, + relativePath: 'notes.md', + navigation: 'caller', + position: { line: 8 } + }) + flushFrames() + expect(store.getState().markdownViewMode[VIEWED_FILE]).toBe('rich') + expect(store.getState().pendingEditorReveal).toBeNull() + store.getState().createTab(VIEWED) + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find((candidate) => candidate.entityId === VIEWED_FILE) + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().markdownViewMode[VIEWED_FILE]).toBe('source') + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId: VIEWED_FILE, line: 8 }) + }) + + it('waits again if the user returns to a terminal before reveal frames run', () => { + const { openFile, store } = setup('terminal') + const terminal = store.getState().getActiveTab(VIEWED) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller', position: { line: 17 } }) + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find( + (candidate) => candidate.entityId === VIEWED_APP_TS.filePath + ) + store.getState().activateTab(tab?.id ?? '') + store.getState().activateTab(terminal?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ line: 17, viewStateId: tab?.id }) + }) + + it('invalidates a published reveal if navigation changes before Monaco applies it', () => { + const { openFile, store } = setup('terminal') + const terminal = store.getState().getActiveTab(VIEWED) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 17 } }) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal?.canApply?.()).toBe(true) + store.getState().activateTab(terminal?.id ?? '') + expect(reveal?.canApply?.()).toBe(false) + }) + + it('does not publish a reveal after its bridge is disposed', () => { + const { openFile, store, dispose } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 17 } }) + dispose() + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + }) + + it('invalidates an already-published reveal when its bridge is disposed', () => { + const { openFile, store, dispose } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 17 } }) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal?.canApply?.()).toBe(true) + dispose() + expect(reveal?.canApply?.()).toBe(false) + }) + + it('does not publish an old position after a close and reopen during queued frames', () => { + const { openFile, store } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 17 } }) + store.getState().closeFile(VIEWED_APP_TS.filePath) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all' }) + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + }) + + it('keeps the latest position for a repeated background open', () => { + const { openFile, store } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller', position: { line: 3 } }) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller', position: { line: 19 } }) + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find( + (candidate) => candidate.entityId === VIEWED_APP_TS.filePath + ) + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ line: 19 }) + }) + + it.each(['caller', 'all'] as const)( + 'cancels a queued position when the same file later opens without one (%s)', + (navigation) => { + const { openFile, store } = setup('terminal') + openFile({ + worktreeId: VIEWED, + ...VIEWED_APP_TS, + navigation: 'caller', + position: { line: 17 } + }) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation }) + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find( + (candidate) => candidate.entityId === VIEWED_APP_TS.filePath + ) + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + } + ) + + it('cancels a positioned open before publication when a plain open follows', () => { + const { openFile, store } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 17 } }) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller' }) + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + }) + + it('invalidates a published position when a plain open follows', () => { + const { openFile, store } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 17 } }) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal?.canApply?.()).toBe(true) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller' }) + expect(reveal?.canApply?.()).toBe(false) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all', position: { line: 23 } }) + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ line: 23 }) + expect(store.getState().pendingEditorReveal?.canApply?.()).toBe(true) + }) + + it('keeps another owner’s queued position when a same-path plain open follows', () => { + const { openFile, store } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller', position: { line: 3 } }) + openFile({ + worktreeId: BACKGROUND, + ...VIEWED_APP_TS, + runtimeEnvironmentId: 'remote-1', + navigation: 'caller', + position: { line: 19 } + }) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller' }) + store.getState().setActiveWorktree(BACKGROUND) + flushFrames() + const remote = store + .getState() + .openFiles.find((file) => file.runtimeEnvironmentId === 'remote-1') + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId: remote?.id, line: 19 }) + store.getState().setPendingEditorReveal(null) + store.getState().setActiveWorktree(VIEWED) + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find( + (candidate) => candidate.entityId === VIEWED_APP_TS.filePath + ) + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + }) + + it('discards positioning when its file closes', () => { + const { openFile, store } = setup('terminal') + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'caller', position: { line: 3 } }) + store.getState().closeFile(VIEWED_APP_TS.filePath) + openFile({ worktreeId: VIEWED, ...VIEWED_APP_TS, navigation: 'all' }) + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + }) + + it('isolates queued positions for the same path on different owners and workspaces', () => { + const { openFile, store } = setup('terminal') + openFile({ + worktreeId: VIEWED, + ...VIEWED_APP_TS, + runtimeEnvironmentId: null, + navigation: 'caller', + position: { line: 3 } + }) + openFile({ + worktreeId: BACKGROUND, + ...VIEWED_APP_TS, + runtimeEnvironmentId: 'remote-1', + navigation: 'caller', + position: { line: 19 } + }) + store.getState().setActiveWorktree(BACKGROUND) + flushFrames() + const remote = store + .getState() + .openFiles.find((file) => file.runtimeEnvironmentId === 'remote-1') + expect(remote?.id).not.toBe(VIEWED_APP_TS.filePath) + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId: remote?.id, line: 19 }) + store.getState().setPendingEditorReveal(null) + store.getState().setActiveWorktree(VIEWED) + const tab = store + .getState() + .unifiedTabsByWorktree[VIEWED]?.find( + (candidate) => candidate.entityId === VIEWED_APP_TS.filePath + ) + store.getState().activateTab(tab?.id ?? '') + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ + fileId: VIEWED_APP_TS.filePath, + line: 3 + }) + }) + + it('positions a background worktree only after it becomes visible', () => { + const { openFile, store } = setup('editor') + const before = screenState(store) + openFile({ worktreeId: BACKGROUND, ...APP_TS, navigation: 'caller', position: { line: 17 } }) + flushFrames() + expect(screenState(store)).toEqual(before) + expect(store.getState().pendingEditorReveal).toBeNull() + store.getState().setActiveWorktree(BACKGROUND) + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ + fileId: APP_TS.filePath, + line: 17 + }) + }) +}) diff --git a/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts index b60912006ea..a44f474a6ac 100644 --- a/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts @@ -1,3 +1,4 @@ +import { createRuntimeFilePositionQueue } from './runtime-file-position' import { applyClosedTerminalLeafNotice } from '@/components/terminal-pane/closed-terminal-leaf-notice' import { closeTerminalTab } from '@/components/terminal/terminal-tab-actions' import { detectLanguage } from '@/lib/language-detect' @@ -5,6 +6,8 @@ import { runSleepWorktree } from '@/components/sidebar/sleep-worktree-flow' import { buildWorkspaceSessionPayload } from '@/lib/workspace-session' import { persistWorkspaceSessionByHost } from '@/lib/workspace-session-host-persistence' import { revealWorkspaceSurface } from '@/lib/workspace-surface-reveal' +import { selectFloatingWorkspacePanelVisible } from '@/store/floating-workspace-panel-selector' +import { isFloatingWorkspaceId } from '../../../../shared/floating-workspace-worktree' import { useAppStore } from '../../store' import type { AppState } from '../../store/types' import type { EditorTabSelection } from '../../store/slices/editor/types/open-file' @@ -23,7 +26,10 @@ function openRuntimeEditorTab( ): void { const store = useAppStore.getState() if (navigation !== undefined && !navigationTargetsHost(navigation)) { - open(store, worktreeId === store.activeWorktreeId ? 'none' : 'background') + const isViewed = + worktreeId === store.activeWorktreeId || + (isFloatingWorkspaceId(worktreeId) && selectFloatingWorkspacePanelVisible(store)) + open(store, isViewed ? 'none' : 'background') return } revealWorkspaceSurface(store, worktreeId) @@ -35,13 +41,14 @@ export function registerMobileAndTerminalCloseIpcBridge( unsubs: (() => void)[], requestSleepingAgentWake: (worktreeId: string) => void ): void { + const positionFile = createRuntimeFilePositionQueue(unsubs) unsubs.push( window.api.ui.onOpenFileFromMobile( - ({ worktreeId, filePath, relativePath, runtimeEnvironmentId, navigation }) => { + ({ worktreeId, filePath, relativePath, runtimeEnvironmentId, navigation, position }) => { const basename = relativePath.split(/[\\/]/).pop() || relativePath - openRuntimeEditorTab(worktreeId, navigation, (store, selection) => + openRuntimeEditorTab(worktreeId, navigation, (store, selection) => { // Why: renderer owns tab creation so grouped order and markdown bridges share the desktop File Explorer's store path. - store.openFile( + const fileId = store.openFile( { filePath, relativePath, @@ -52,7 +59,8 @@ export function registerMobileAndTerminalCloseIpcBridge( }, { selection } ) - ) + positionFile(fileId, position, selection === 'focus') + }) } ) ) diff --git a/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts b/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts new file mode 100644 index 00000000000..2a61e0179ca --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/runtime-file-position.test.ts @@ -0,0 +1,197 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { FLOATING_TERMINAL_WORKTREE_ID, getDefaultSettings } from '../../../../shared/constants' +import { createTestStore, makeWorktree, seedStore } from '../../store/slices/store-test-helpers' +import { createStoreSessionMockApi } from '../../store/slices/store-session-test-harness' + +const { storeRef } = vi.hoisted(() => { + const storeRef: { current: ReturnType | null } = { current: null } + return { storeRef } +}) +vi.mock('../../store', () => ({ + useAppStore: { + getState: () => testStore().getState(), + subscribe: (...args: Parameters['subscribe']>) => + testStore().subscribe(...args) + } +})) +import { createRuntimeFilePositionQueue } from './runtime-file-position' + +const MAIN = 'repo1::/repo1/main' +const FLOATING = FLOATING_TERMINAL_WORKTREE_ID +const frames: FrameRequestCallback[] = [] +let unsubs: (() => void)[] = [] + +beforeEach(() => { + Object.defineProperty(window, 'api', { value: createStoreSessionMockApi(), configurable: true }) + frames.length = 0 + unsubs = [] + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => frames.push(callback)) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + document.body.innerHTML = + '
' +}) +afterEach(() => { + unsubs.forEach((unsubscribe) => unsubscribe()) + vi.unstubAllGlobals() + document.body.innerHTML = '' +}) +function flushFrames(): void { + while (frames.length) { + frames.shift()?.(0) + } +} +function testStore(): ReturnType { + if (!storeRef.current) { + throw new Error('Test store missing') + } + return storeRef.current +} +function setup(open = true) { + const store = createTestStore() + storeRef.current = store + seedStore(store, { + settings: { ...getDefaultSettings('/tmp'), floatingTerminalEnabled: true }, + floatingWorkspacePath: '/tmp/floating', + floatingWorkspacePanelOpen: open, + worktreesByRepo: { repo1: [makeWorktree({ id: MAIN, repoId: 'repo1', path: '/repo1/main' })] } + }) + store.getState().setActiveWorktree(MAIN) + const fileId = store.getState().openFile({ + filePath: '/tmp/floating/app.ts', + relativePath: 'app.ts', + worktreeId: FLOATING, + language: 'typescript', + runtimeEnvironmentId: null, + mode: 'edit' + }) + const position = createRuntimeFilePositionQueue(unsubs) + return { store, fileId, position } +} +function focus(surface: 'main' | 'floating'): void { + document.getElementById(surface)?.focus() +} + +it('reveals the selected floating editor without selecting its workspace in the main window', () => { + const { store, fileId, position } = setup() + position(fileId, { line: 300, column: 7 }, true) + flushFrames() + expect(store.getState().activeWorktreeId).toBe(MAIN) + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId, line: 300, column: 7 }) +}) + +it('waits for an asynchronously revealed floating panel', () => { + const { store, fileId, position } = setup(false) + position(fileId, { line: 300, column: 7 }, true) + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + store.setState({ floatingWorkspacePanelOpen: true }) + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId, line: 300, column: 7 }) +}) + +it('keeps a background floating position deferred until the panel owns keyboard input', () => { + const { store, fileId, position } = setup() + focus('main') + position(fileId, { line: 17 }, false) + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + focus('floating') + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId, line: 17 }) +}) + +it('does not publish after keyboard input moves from the panel to the main surface', () => { + const { store, fileId, position } = setup() + focus('floating') + position(fileId, { line: 17 }, true) + focus('main') + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() + focus('floating') + flushFrames() + expect(store.getState().pendingEditorReveal).toMatchObject({ fileId, line: 17 }) +}) + +it('cancels a published reveal after an outside click even if DOM focus does not change', () => { + const { store, fileId, position } = setup() + focus('main') + position(fileId, { line: 17 }, true) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal?.canApply?.()).toBe(true) + document.getElementById('main')?.dispatchEvent(new Event('pointerdown', { bubbles: true })) + expect(reveal?.canApply?.()).toBe(false) + expect(document.activeElement?.id).toBe('main') +}) + +it('does not reclaim input when an asynchronously revealed panel opens after an outside click', () => { + const { store, fileId, position } = setup(false) + position(fileId, { line: 17 }, true) + document.getElementById('main')?.dispatchEvent(new Event('pointerdown', { bubbles: true })) + store.setState({ floatingWorkspacePanelOpen: true }) + flushFrames() + expect(store.getState().pendingEditorReveal).toBeNull() +}) + +it('invalidates a published floating reveal when the panel closes or is disabled', () => { + const { store, fileId, position } = setup() + position(fileId, { line: 17 }, true) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal?.canApply?.()).toBe(true) + store.setState({ floatingWorkspacePanelOpen: false }) + expect(reveal?.canApply?.()).toBe(false) + store.setState({ floatingWorkspacePanelOpen: true }) + const settings = store.getState().settings + if (!settings) { + throw new Error('Settings missing') + } + store.setState({ settings: { ...settings, floatingTerminalEnabled: false } }) + expect(reveal?.canApply?.()).toBe(false) +}) + +it('keeps floating input ownership independent of an unrelated main workspace selection', () => { + const { store, fileId, position } = setup() + focus('floating') + position(fileId, { line: 300, column: 7 }, true) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal?.canApply?.()).toBe(true) + store.setState({ activeWorktreeId: 'other-main-workspace' }) + expect(document.activeElement?.id).toBe('floating') + expect(reveal?.canApply?.()).toBe(true) +}) + +it('scopes the floating reveal to its selected split pane', () => { + const { store, fileId, position } = setup() + const tab = store.getState().getActiveTab(FLOATING) + if (!tab) { + throw new Error('Floating editor missing') + } + const siblingGroupId = store.getState().createEmptySplitGroup(FLOATING, tab.groupId, 'right') + if (!siblingGroupId) { + throw new Error('Split missing') + } + store.getState().copyUnifiedTabToGroup(tab.id, siblingGroupId) + store.getState().focusGroup(FLOATING, tab.groupId) + position(fileId, { line: 300, column: 7 }, true) + flushFrames() + const reveal = store.getState().pendingEditorReveal + expect(reveal).toMatchObject({ fileId, viewStateId: tab.id }) + expect(reveal?.canApply?.()).toBe(true) + store.getState().focusGroup(FLOATING, siblingGroupId) + expect(reveal?.canApply?.()).toBe(false) +}) + +it('disposes floating DOM listeners and invalidates an already published reveal', () => { + const { store, fileId, position } = setup() + position(fileId, { line: 17 }, true) + flushFrames() + const reveal = store.getState().pendingEditorReveal + unsubs.forEach((unsubscribe) => unsubscribe()) + expect(reveal?.canApply?.()).toBe(false) + focus('floating') + flushFrames() + expect(store.getState().pendingEditorReveal).toBe(reveal) +}) diff --git a/src/renderer/src/hooks/ipc-events/runtime-file-position.ts b/src/renderer/src/hooks/ipc-events/runtime-file-position.ts new file mode 100644 index 00000000000..5852155a229 --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/runtime-file-position.ts @@ -0,0 +1,180 @@ +import type { RuntimeFileOpenPosition } from '../../../../shared/runtime-file-contracts' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { scheduleEditorLineReveal } from '@/store/slices/editor/focus/editor-focus-reveal' +import { selectFloatingWorkspacePanelVisible } from '@/store/floating-workspace-panel-selector' +import { + isEventTargetInsideFloatingWorkspacePanel, + isFloatingWorkspacePanelFocused +} from '@/lib/floating-workspace-terminal-actions' +import { useAppStore } from '../../store' +import type { AppState } from '../../store/types' + +function selectedEditorTab(state: AppState, worktreeId = state.activeWorktreeId) { + if ( + !worktreeId || + (worktreeId === FLOATING_TERMINAL_WORKTREE_ID + ? !selectFloatingWorkspacePanelVisible(state) + : worktreeId !== state.activeWorktreeId) + ) { + return undefined + } + const group = state.groupsByWorktree[worktreeId]?.find( + (candidate) => candidate.id === state.activeGroupIdByWorktree[worktreeId] + ) + return state.unifiedTabsByWorktree[worktreeId]?.find( + (tab) => tab.id === group?.activeTabId && tab.contentType === 'editor' + ) +} + +export function createRuntimeFilePositionQueue( + unsubs: (() => void)[] +): (fileId: string, position: RuntimeFileOpenPosition | undefined, focus: boolean) => void { + let alive = true + type Request = { + position: RuntimeFileOpenPosition + waitingForFloatingFocus: boolean + } + const pending = new Map() + + const reveal = (fileId: string, request: Request): void => { + const { position } = request + const store = useAppStore.getState() + const file = store.openFiles.find((candidate) => candidate.id === fileId) + const tab = file && selectedEditorTab(store, file.worktreeId) + if (!file || !tab || tab.entityId !== fileId || request.waitingForFloatingFocus) { + return + } + const canApply = (): boolean => + alive && + pending.get(fileId) === request && + !request.waitingForFloatingFocus && + (file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID || + useAppStore.getState().activeWorktreeId === store.activeWorktreeId) && + selectedEditorTab(useAppStore.getState(), file.worktreeId)?.id === tab.id + scheduleEditorLineReveal( + useAppStore.getState, + file.filePath, + position.line, + position.column, + fileId, + { + viewStateId: tab.id, + canApply, + beforePublish: () => { + if (file.language === 'markdown') { + useAppStore.getState().setMarkdownViewMode(fileId, 'source') + } + }, + onApplied: () => { + if (pending.get(fileId) === request) { + pending.delete(fileId) + } + } + } + ) + } + + if (typeof document !== 'undefined') { + const handleFloatingFocus = (event: Event): void => { + if (pending.size === 0) { + return + } + const state = useAppStore.getState() + if (!isEventTargetInsideFloatingWorkspacePanel(event.target)) { + for (const file of state.openFiles) { + if (file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + const request = pending.get(file.id) + if (request) { + request.waitingForFloatingFocus = true + } + } + } + return + } + const tab = selectedEditorTab(state, FLOATING_TERMINAL_WORKTREE_ID) + const request = tab && pending.get(tab.entityId) + if (tab && request?.waitingForFloatingFocus) { + request.waitingForFloatingFocus = false + reveal(tab.entityId, request) + } + } + document.addEventListener('focusin', handleFloatingFocus) + document.addEventListener('pointerdown', handleFloatingFocus, true) + unsubs.push(() => { + document.removeEventListener('focusin', handleFloatingFocus) + document.removeEventListener('pointerdown', handleFloatingFocus, true) + }) + } + + unsubs.push( + useAppStore.subscribe((state, previous) => { + if (pending.size === 0) { + return + } + if (state.openFiles !== previous.openFiles) { + const openIds = new Set(state.openFiles.map((file) => file.id)) + for (const fileId of pending.keys()) { + if (!openIds.has(fileId)) { + pending.delete(fileId) + } + } + } + if ( + state.activeWorktreeId === previous.activeWorktreeId && + state.groupsByWorktree === previous.groupsByWorktree && + state.activeGroupIdByWorktree === previous.activeGroupIdByWorktree && + selectFloatingWorkspacePanelVisible(state) === selectFloatingWorkspacePanelVisible(previous) + ) { + return + } + for (const worktreeId of new Set([state.activeWorktreeId, FLOATING_TERMINAL_WORKTREE_ID])) { + const tab = selectedEditorTab(state, worktreeId) + // A background open must wait for navigation, even if this file is already on screen. + if ( + !tab || + (tab.id === selectedEditorTab(previous, worktreeId)?.id && + (worktreeId === FLOATING_TERMINAL_WORKTREE_ID || + state.activeWorktreeId === previous.activeWorktreeId)) + ) { + continue + } + const request = pending.get(tab.entityId) + if (request) { + if ( + worktreeId === FLOATING_TERMINAL_WORKTREE_ID && + typeof document !== 'undefined' && + isFloatingWorkspacePanelFocused() + ) { + request.waitingForFloatingFocus = false + } + reveal(tab.entityId, request) + } + } + }), + () => { + alive = false + pending.clear() + } + ) + + return (fileId, position, focus) => { + if (!position) { + pending.delete(fileId) + return + } + const request = { + position, + waitingForFloatingFocus: + !focus && + useAppStore + .getState() + .openFiles.some( + (file) => file.id === fileId && file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID + ) + } + pending.set(fileId, request) + if (focus) { + reveal(fileId, request) + } + } +} diff --git a/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts b/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts index 23aae5bad77..1dcf3141536 100644 --- a/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts @@ -212,7 +212,7 @@ describe('useIpcEvents Close Tab on the active browser tab', () => { // Why: an open-but-empty floating panel is the ambient close fallback only; a guest-forwarded // source id names a main-workspace target and must not be swallowed by the panel toggle. it('closes the source workspace even while an empty floating panel is visible', async () => { - vi.stubGlobal('document', { querySelector: () => ({}) }) + vi.stubGlobal('document', Object.assign(new EventTarget(), { querySelector: () => ({}) })) const listenerRef: { current: CloseActiveTabListener | null } = { current: null } await useIpcEventsForCloseRouting({ closeActiveTabListenerRef: listenerRef, diff --git a/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts b/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts index 58b0098253e..f5e751ffefa 100644 --- a/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-lifecycle.test.ts @@ -251,6 +251,9 @@ describe('useIpcEvents App-lifetime lifecycle', () => { const cleanupOrder: string[] = [] const listeners = new Map() const storeSubscriptions: { active: boolean; cleanup: Mock }[] = [] + const documentEvents = new EventTarget() + const addDocumentListener = vi.spyOn(documentEvents, 'addEventListener') + const removeDocumentListener = vi.spyOn(documentEvents, 'removeEventListener') const setUpdateStatus = vi.fn() const storeState = new Proxy( createHarnessStoreState({ @@ -361,6 +364,7 @@ describe('useIpcEvents App-lifetime lifecycle', () => { }) const { installAppLifetimeIpcEvents } = await import('./ipc-events/app-lifetime-ipc-bridge') + vi.stubGlobal('document', documentEvents) const recordCleanupPhase = (phase: string): void => { cleanupOrder.push(phase) } @@ -469,7 +473,8 @@ describe('useIpcEvents App-lifetime lifecycle', () => { expect( [...listeners.values()].every((records) => records.filter((item) => item.active).length === 1) ).toBe(true) - expect(storeSubscriptions.filter((item) => item.active)).toHaveLength(3) + expect(storeSubscriptions.filter((item) => item.active)).toHaveLength(4) + expect(addDocumentListener.mock.calls.map(([type]) => type)).toEqual(['focusin', 'pointerdown']) firstCleanup() const ipcCleanupOrder = cleanupOrder @@ -481,11 +486,12 @@ describe('useIpcEvents App-lifetime lifecycle', () => { 'mobile.disposeHydration', 'store.unsubscribe.0', 'runtimeStore.unsubscribe', - 'store.unsubscribe.2', + 'store.unsubscribe.3', 'agentStore.unsubscribe' ]) // The background-removal bridge's row subscription, released with the rest of `unsubs`. expect(cleanupOrder).toContain('store.unsubscribe.1') + expect(cleanupOrder).toContain('store.unsubscribe.2') expect(cleanupOrder.indexOf('runtimeEnvironment.unsubscribe')).toBeGreaterThan( cleanupOrder.indexOf('ipc.ui.onMobileMarkdownRequest') ) @@ -500,6 +506,8 @@ describe('useIpcEvents App-lifetime lifecycle', () => { true ) expect(storeSubscriptions.filter((item) => item.active)).toHaveLength(0) + expect(storeSubscriptions.every((item) => item.cleanup.mock.calls.length === 1)).toBe(true) + expect(removeDocumentListener.mock.calls).toEqual(addDocumentListener.mock.calls) expect( [...listeners.values()].every((records) => records.every((item) => item.cleanup.mock.calls.length === 1) @@ -518,13 +526,21 @@ describe('useIpcEvents App-lifetime lifecycle', () => { expect( [...listeners.values()].every((records) => records.filter((item) => item.active).length === 1) ).toBe(true) - expect(storeSubscriptions.filter((item) => item.active)).toHaveLength(3) + expect(storeSubscriptions.filter((item) => item.active)).toHaveLength(4) + expect(addDocumentListener.mock.calls.map(([type]) => type)).toEqual([ + 'focusin', + 'pointerdown', + 'focusin', + 'pointerdown' + ]) secondCleanup() expect([...listeners.values()].every((records) => records.every((item) => !item.active))).toBe( true ) expect(storeSubscriptions.filter((item) => item.active)).toHaveLength(0) + expect(storeSubscriptions.every((item) => item.cleanup.mock.calls.length === 1)).toBe(true) + expect(removeDocumentListener.mock.calls).toEqual(addDocumentListener.mock.calls) expect( [...listeners.values()].every((records) => records.every((item) => item.cleanup.mock.calls.length === 1) diff --git a/src/renderer/src/store/slices/editor/focus/editor-focus-reveal.ts b/src/renderer/src/store/slices/editor/focus/editor-focus-reveal.ts index 44b5bb83e59..e5af7a2a73b 100644 --- a/src/renderer/src/store/slices/editor/focus/editor-focus-reveal.ts +++ b/src/renderer/src/store/slices/editor/focus/editor-focus-reveal.ts @@ -1,3 +1,4 @@ +import type { PendingEditorReveal } from '../types/pending-editor-reveal' import type { AppState } from '../../../types' import { EDITOR_FOCUS_REQUEST_TTL_MS } from '../types/pending-editor-reveal' @@ -44,19 +45,28 @@ export function scheduleEditorLineReveal( filePath: string, line: number, column?: number, - fileId?: string + fileId?: string, + options?: Pick & { + beforePublish?: () => void + } ): void { // Why: openFile may remount Monaco async; the reveal must land after remount or the old editor clears it. cancelPendingEditorLineRevealFrames() get().setPendingEditorReveal(null) requestTrackedEditorLineRevealFrame(() => { requestTrackedEditorLineRevealFrame(() => { + if (options?.canApply?.() === false) { + return + } + const { beforePublish, ...revealOptions } = options ?? {} + beforePublish?.() get().setPendingEditorReveal({ filePath, fileId, line, column: column ?? 1, - matchLength: 0 + matchLength: 0, + ...revealOptions }) }) }) diff --git a/src/renderer/src/store/slices/editor/types/pending-editor-reveal.ts b/src/renderer/src/store/slices/editor/types/pending-editor-reveal.ts index 8a4cce0a3bd..e5214428b2d 100644 --- a/src/renderer/src/store/slices/editor/types/pending-editor-reveal.ts +++ b/src/renderer/src/store/slices/editor/types/pending-editor-reveal.ts @@ -4,6 +4,9 @@ export type PendingEditorReveal = { line: number column: number matchLength: number + viewStateId?: string + canApply?: () => boolean + onApplied?: () => void } export type PendingEditorFocusRequest = { diff --git a/src/shared/rpc-contract/files-params.ts b/src/shared/rpc-contract/files-params.ts index e455d295be1..2b97cbef8a8 100644 --- a/src/shared/rpc-contract/files-params.ts +++ b/src/shared/rpc-contract/files-params.ts @@ -46,11 +46,19 @@ export const ResolveTerminalPath = WorktreeSelector.extend({ }) // Why: absent `navigation` (phones, older CLIs) keeps the original host switch; 'caller'/'clients' open without moving it. -export const FileOpenTab = FileOpen.extend({ +const FileOpenTarget = FileOpen.extend({ navigation: z.enum(RUNTIME_NAVIGATION_TARGETS).optional() }) -export const FileOpenDiff = FileOpenTab.extend({ +export const FileOpenTab = FileOpenTarget.extend({ + line: z.number().int().positive().max(Number.MAX_SAFE_INTEGER).optional(), + column: z.number().int().positive().max(Number.MAX_SAFE_INTEGER).optional() +}).refine((params) => params.column === undefined || params.line !== undefined, { + message: 'column requires line', + path: ['column'] +}) + +export const FileOpenDiff = FileOpenTarget.extend({ staged: z.boolean().optional() }) diff --git a/src/shared/runtime-file-contracts.ts b/src/shared/runtime-file-contracts.ts index d490ae95e31..0ff214b66ef 100644 --- a/src/shared/runtime-file-contracts.ts +++ b/src/shared/runtime-file-contracts.ts @@ -22,6 +22,11 @@ export type RuntimeFileOpenResult = { opened: boolean } +export type RuntimeFileOpenPosition = { + line: number + column?: number +} + export type RuntimeFileReadResult = { worktree: string relativePath: string diff --git a/tests/e2e/cli-file-open-position.spec.ts b/tests/e2e/cli-file-open-position.spec.ts new file mode 100644 index 00000000000..471ea8300e7 --- /dev/null +++ b/tests/e2e/cli-file-open-position.spec.ts @@ -0,0 +1,205 @@ +import { writeFileSync, rmSync, mkdirSync } from 'node:fs' +import path from 'node:path' +import { runProcess } from '@orca/process-host' +import { expect, test } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +// The fixture's hidden renderer is driven and captured through Playwright's CDP connection. +test('CLI file positioning preserves background navigation and reveals the requested line', async ({ + electronApp, + orcaPage +}, testInfo) => { + await waitForSessionReady(orcaPage) + const worktreeId = await waitForActiveWorktree(orcaPage) + const root = await orcaPage.evaluate( + (id) => + Object.values(window.__store?.getState().worktreesByRepo ?? {}) + .flat() + .find((worktree) => worktree.id === id)?.path ?? '', + worktreeId + ) + const userDataDir = await electronApp.evaluate(({ app }) => app.getPath('userData')) + const relativePath = 'cli-position-proof.ts' + const filePath = path.join(root, relativePath) + writeFileSync( + filePath, + Array.from({ length: 400 }, (_, index) => `const line${index + 1} = ${index + 1}`).join('\n') + ) + + const open = async (flags: string[]): Promise => { + const result = await runProcess({ + program: process.execPath, + args: [ + path.join(process.cwd(), 'out', 'cli', 'index.js'), + 'file', + 'open', + relativePath, + '--worktree', + `id:${worktreeId}`, + '--json', + ...flags + ], + env: { ...process.env, ORCA_USER_DATA_PATH: userDataDir }, + timeoutMs: 30_000 + }) + expect(result.code, result.stderr).toBe(0) + } + const cursor = () => + orcaPage.evaluate(() => { + const probe = window.__monacoEditorE2E + const snapshot = probe?.snapshot() + return { + line: snapshot?.selection?.positionLineNumber, + column: snapshot?.selection?.positionColumn, + visible: snapshot?.visibleRanges + } + }) + const proofDir = path.join(process.cwd(), 'validation-screenshots', 'file-position') + mkdirSync(proofDir, { recursive: true }) + try { + await open(['--focus']) + await expect(orcaPage.locator('.editor-header-path').first()).toContainText(relativePath) + await expect.poll(async () => (await cursor()).line).toBe(1) + await testInfo.attach('before-default-file-open.png', { + body: await orcaPage.screenshot({ path: path.join(proofDir, 'before.png') }), + contentType: 'image/png' + }) + await open(['--line', '300', '--column', '7']) + await orcaPage.evaluate( + () => + new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ) + ) + expect(await cursor()).toMatchObject({ line: 1, column: 1 }) + await open([]) + await orcaPage.evaluate((id) => { + const state = window.__store?.getState() + const tab = state?.getActiveTab(id) + if (!state || !tab) { + throw new Error('No active editor tab') + } + state.createTab(id) + state.activateTab(tab.id) + }, worktreeId) + await orcaPage.evaluate( + () => + new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ) + ) + expect(await orcaPage.evaluate(() => window.__store?.getState().pendingEditorReveal)).toBeNull() + expect(await cursor()).toMatchObject({ line: 1, column: 1 }) + await open(['--line', '300', '--column', '7', '--focus']) + await expect + .poll(async () => { + const position = await cursor() + return { line: position.line, column: position.column } + }) + .toEqual({ line: 300, column: 7 }) + await expect + .poll(async () => + (await cursor()).visible?.some( + (range) => range.startLineNumber <= 300 && range.endLineNumber >= 300 + ) + ) + .toBe(true) + await testInfo.attach('after-line-300-column-7.png', { + body: await orcaPage.screenshot({ path: path.join(proofDir, 'after.png') }), + contentType: 'image/png' + }) + await open(['--line', '1', '--focus']) + await expect.poll(async () => (await cursor()).line).toBe(1) + const split = await orcaPage.evaluate((id) => { + const state = window.__store?.getState() + const tab = state?.getActiveTab(id) + if (!state || !tab) { + throw new Error('No active editor tab') + } + const siblingGroupId = state.createEmptySplitGroup(id, tab.groupId, 'right') + if (!siblingGroupId) { + throw new Error('Split group was not created') + } + return { tabId: tab.id, groupId: tab.groupId, siblingGroupId } + }, worktreeId) + await orcaPage.evaluate( + () => + new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + ) + ) + await expect.poll(async () => (await cursor()).line).toBe(1) + const targetProbe = await orcaPage.evaluateHandle(() => window.__monacoEditorE2E) + await orcaPage.evaluate( + ({ id, split }) => { + const state = window.__store?.getState() + if (!state?.copyUnifiedTabToGroup(split.tabId, split.siblingGroupId)) { + throw new Error('Editor tab was not copied') + } + state.focusGroup(id, split.groupId) + }, + { id: worktreeId, split } + ) + await expect(orcaPage.locator('.monaco-editor:visible')).toHaveCount(2) + const siblingProbe = await orcaPage.evaluateHandle(() => window.__monacoEditorE2E) + expect(await targetProbe.evaluate((target, sibling) => target === sibling, siblingProbe)).toBe( + false + ) + await expect + .poll(() => siblingProbe.evaluate((probe) => probe?.snapshot().selection?.positionLineNumber)) + .toBe(1) + await orcaPage + .locator('.monaco-editor:visible') + .first() + .getByRole('textbox', { name: 'Editor content' }) + .focus() + await expect + .poll(() => + orcaPage.evaluate( + (id) => window.__store?.getState().activeGroupIdByWorktree[id], + worktreeId + ) + ) + .toBe(split.groupId) + await open(['--line', '300', '--column', '7', '--focus']) + await expect + .poll(() => + targetProbe.evaluate((probe) => { + const selection = probe?.snapshot().selection + return { line: selection?.positionLineNumber, column: selection?.positionColumn } + }) + ) + .toEqual({ line: 300, column: 7 }) + expect( + await siblingProbe.evaluate((probe) => { + const selection = probe?.snapshot().selection + return { line: selection?.positionLineNumber, column: selection?.positionColumn } + }) + ).toEqual({ line: 1, column: 1 }) + expect( + await orcaPage.evaluate( + (id) => window.__store?.getState().activeGroupIdByWorktree[id], + worktreeId + ) + ).toBe(split.groupId) + await expect + .poll(() => + targetProbe.evaluate((probe) => + probe + ?.snapshot() + .visibleRanges.some( + (range) => range.startLineNumber <= 300 && range.endLineNumber >= 300 + ) + ) + ) + .toBe(true) + await testInfo.attach('same-file-split-target-only.png', { + body: await orcaPage.screenshot({ path: path.join(proofDir, 'split.png') }), + contentType: 'image/png' + }) + await targetProbe.dispose() + await siblingProbe.dispose() + } finally { + rmSync(filePath, { force: true }) + } +}) diff --git a/tests/e2e/cli-floating-file-open-position.spec.ts b/tests/e2e/cli-floating-file-open-position.spec.ts new file mode 100644 index 00000000000..4304b3aea05 --- /dev/null +++ b/tests/e2e/cli-floating-file-open-position.spec.ts @@ -0,0 +1,206 @@ +import { mkdirSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { runProcess } from '@orca/process-host' +import { expect, test } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { startHiddenRendererFrameCapture } from './helpers/hidden-renderer-frame-capture' + +const FLOATING = 'global-floating-terminal' +const PANEL = '[data-floating-terminal-panel][aria-hidden="false"]' + +test('CLI positions floating files without changing the main workspace or reclaiming its input', async ({ + electronApp, + orcaPage +}, testInfo) => { + await waitForSessionReady(orcaPage) + const mainWorktreeId = await waitForActiveWorktree(orcaPage) + const mainRoot = await orcaPage.evaluate( + (id) => + Object.values(window.__store?.getState().worktreesByRepo ?? {}) + .flat() + .find((worktree) => worktree.id === id)?.path, + mainWorktreeId + ) + if (!mainRoot) { + throw new Error('Main workspace path unavailable') + } + const roots = { main: mainRoot, floating: path.join(mainRoot, 'floating-position-fixture') } + mkdirSync(roots.floating, { recursive: true }) + await electronApp.evaluate(({ dialog }, floatingPath) => { + dialog.showOpenDialog = async () => ({ + canceled: false, + filePaths: [floatingPath], + bookmarks: [] + }) + }, roots.floating) + await orcaPage.evaluate(async (floatingPath) => { + const store = window.__store + if (!store) { + throw new Error('Store unavailable') + } + const pickedPath = await window.api.app.pickFloatingWorkspaceDirectory() + if (pickedPath !== floatingPath) { + throw new Error('Floating directory fixture was not granted') + } + await store.getState().updateSettings({ + floatingTerminalEnabled: true, + floatingTerminalCwd: floatingPath + }) + store.setState({ floatingWorkspacePanelOpen: false }) + }, roots.floating) + const userDataDir = await electronApp.evaluate(({ app }) => app.getPath('userData')) + const relativePath = 'floating-position-proof.ts' + const emptyPath = 'floating-empty-position-proof.txt' + const mainFile = path.join(roots.main, relativePath) + const floatingFile = path.join(roots.floating, relativePath) + const floatingEmpty = path.join(roots.floating, emptyPath) + const lines = Array.from({ length: 400 }, (_, index) => `const line${index + 1} = ${index + 1}`) + mkdirSync(roots.floating, { recursive: true }) + writeFileSync(mainFile, 'const main = 1\n') + writeFileSync(floatingFile, lines.join('\n')) + writeFileSync(floatingEmpty, '') + const open = async (worktree: string, file: string, flags: string[]): Promise => { + const result = await runProcess({ + program: process.execPath, + args: [ + path.join(process.cwd(), 'out', 'cli', 'index.js'), + 'file', + 'open', + file, + '--worktree', + `id:${worktree}`, + '--json', + ...flags + ], + env: { ...process.env, ORCA_USER_DATA_PATH: userDataDir }, + timeoutMs: 30_000 + }) + expect(result.code, `${result.stderr}\n${result.stdout}`).toBe(0) + } + const pendingPosition = () => + orcaPage.evaluate(() => { + const reveal = window.__store?.getState().pendingEditorReveal + return reveal && { line: reveal.line, column: reveal.column, eligible: reveal.canApply?.() } + }) + const oversizedPosition = { + line: Number.MAX_SAFE_INTEGER, + column: Number.MAX_SAFE_INTEGER, + eligible: true + } + const resumeHiddenRendererFrames = async (): Promise => { + const browserWindow = await electronApp.browserWindow(orcaPage) + // Linux throttles hidden renderers to one frame per second, beyond this reveal's frame budget. + const state = await browserWindow.evaluate((window) => { + window.webContents.setBackgroundThrottling(false) + return { + throttled: window.webContents.getBackgroundThrottling(), + visible: window.isVisible(), + focused: window.isFocused() + } + }) + await browserWindow.dispose() + expect(state).toEqual({ throttled: false, visible: false, focused: false }) + } + let stopFrameCapture: (() => Promise) | undefined + try { + await open(mainWorktreeId, relativePath, ['--focus']) + await expect(orcaPage.locator('.monaco-editor:visible')).toHaveCount(1) + const mainProbe = await orcaPage.evaluateHandle(() => window.__monacoEditorE2E) + const mainInput = orcaPage + .locator('.monaco-editor:visible') + .first() + .getByRole('textbox', { name: 'Editor content' }) + await open(FLOATING, relativePath, ['--focus', '--line', '300', '--column', '7']) + const panel = orcaPage.locator(PANEL) + await expect(panel).toBeVisible() + await expect(panel.locator('.monaco-editor:visible')).toHaveCount(1) + const floatingProbe = await orcaPage.evaluateHandle(() => window.__monacoEditorE2E) + const cursor = () => + floatingProbe.evaluate((probe) => { + const selection = probe?.snapshot().selection + return { line: selection?.positionLineNumber, column: selection?.positionColumn } + }) + await expect.poll(cursor).toEqual({ line: 300, column: 7 }) + const floatingInput = panel.getByRole('textbox', { name: 'Editor content' }) + expect(await orcaPage.evaluate(() => window.__store?.getState().activeWorktreeId)).toBe( + mainWorktreeId + ) + expect( + await mainProbe.evaluate((probe) => probe?.snapshot().selection?.positionLineNumber) + ).toBe(1) + await open(FLOATING, emptyPath, ['--line', '1']) + await expect(panel.locator('.editor-header-path')).toContainText(relativePath) + expect(await cursor()).toEqual({ line: 300, column: 7 }) + await open(FLOATING, relativePath, ['--line', '17', '--column', '3']) + await orcaPage.waitForTimeout(200) + expect(await cursor()).toEqual({ line: 300, column: 7 }) + await mainInput.focus() + await floatingInput.focus() + await expect.poll(cursor).toEqual({ line: 17, column: 3 }) + + // An oversized line keeps the existing content-wait loop active while the user changes input. + await open(FLOATING, relativePath, [ + '--focus', + '--line', + String(Number.MAX_SAFE_INTEGER), + '--column', + String(Number.MAX_SAFE_INTEGER) + ]) + await expect.poll(pendingPosition).toEqual(oversizedPosition) + await mainInput.focus() + expect(await pendingPosition()).toEqual({ ...oversizedPosition, eligible: false }) + await resumeHiddenRendererFrames() + stopFrameCapture = await startHiddenRendererFrameCapture(orcaPage) + await orcaPage.waitForTimeout(3500) + expect(await cursor()).toEqual({ line: 17, column: 3 }) + await expect(mainInput).toBeFocused() + await floatingInput.focus() + await expect + .poll(cursor, { timeout: 8000 }) + .toEqual({ line: 400, column: lines[399].length + 1 }) + const proofDir = path.join(process.cwd(), 'validation-screenshots', 'file-position-floating') + mkdirSync(proofDir, { recursive: true }) + await testInfo.attach('floating-position-clamped.png', { + body: await orcaPage.screenshot({ path: path.join(proofDir, 'clamped.png') }), + contentType: 'image/png' + }) + await open(FLOATING, emptyPath, [ + '--focus', + '--line', + String(Number.MAX_SAFE_INTEGER), + '--column', + String(Number.MAX_SAFE_INTEGER) + ]) + await expect.poll(pendingPosition).toEqual(oversizedPosition) + await resumeHiddenRendererFrames() + await expect + .poll( + () => + orcaPage.evaluate(() => { + const selection = window.__monacoEditorE2E?.snapshot().selection + return { line: selection?.positionLineNumber, column: selection?.positionColumn } + }), + { timeout: 8000 } + ) + .toEqual({ line: 1, column: 1 }) + await expect + .poll(() => orcaPage.evaluate(() => window.__store?.getState().pendingEditorReveal)) + .toBeNull() + expect(await orcaPage.evaluate(() => window.__store?.getState().activeWorktreeId)).toBe( + mainWorktreeId + ) + expect( + await electronApp.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows().every((window) => !window.isVisible() && !window.isFocused()) + ) + ).toBe(true) + } finally { + try { + await stopFrameCapture?.() + } finally { + for (const file of [mainFile, floatingFile, floatingEmpty]) { + rmSync(file, { force: true }) + } + } + } +}) diff --git a/tests/e2e/helpers/hidden-renderer-frame-capture.ts b/tests/e2e/helpers/hidden-renderer-frame-capture.ts new file mode 100644 index 00000000000..41c427e9bb3 --- /dev/null +++ b/tests/e2e/helpers/hidden-renderer-frame-capture.ts @@ -0,0 +1,45 @@ +import type { Page } from '@playwright/test' +import { expect } from './orca-app' + +export async function startHiddenRendererFrameCapture(page: Page): Promise<() => Promise> { + const cdp = await page.context().newCDPSession(page) + let stopped = false + const handlePageClose = (): void => { + void stop().catch(() => {}) + } + const stop = async (): Promise => { + if (stopped) { + return + } + stopped = true + try { + await cdp.send('Page.stopScreencast') + } finally { + page.off('close', handlePageClose) + cdp.off('Page.screencastFrame', acknowledgeFrame) + await cdp.detach() + } + } + let hasFrame = false + const acknowledgeFrame = (frame: { sessionId: number }): void => { + hasFrame = true + void cdp.send('Page.screencastFrameAck', { sessionId: frame.sessionId }).catch(() => {}) + } + cdp.on('Page.screencastFrame', acknowledgeFrame) + page.once('close', handlePageClose) + try { + // Keep hidden renderers painting without revealing their native windows. + await cdp.send('Page.enable') + await cdp.send('Page.startScreencast', { + format: 'jpeg', + quality: 20, + maxWidth: 640, + maxHeight: 480 + }) + await expect.poll(() => hasFrame).toBe(true) + return stop + } catch (error) { + await stop().catch(() => {}) + throw error + } +} diff --git a/tests/e2e/tab-hover-card.spec.ts b/tests/e2e/tab-hover-card.spec.ts index 119a904af19..0b4d3581c53 100644 --- a/tests/e2e/tab-hover-card.spec.ts +++ b/tests/e2e/tab-hover-card.spec.ts @@ -1,6 +1,7 @@ -import type { CDPSession, JSHandle, Page } from '@playwright/test' +import type { JSHandle, Page } from '@playwright/test' import { expect, test } from './helpers/orca-app' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { startHiddenRendererFrameCapture } from './helpers/hidden-renderer-frame-capture' type CardFrame = { title: string @@ -11,25 +12,9 @@ type CardFrame = { states: string[] } type CardRecording = { stop: () => CardFrame[] } -type CardCapture = { recording: JSHandle; cdp: CDPSession } -async function startCardRecording(page: Page): Promise { - const cdp = await page.context().newCDPSession(page) - let hasFrame = false - cdp.on('Page.screencastFrame', (frame) => { - hasFrame = true - void cdp.send('Page.screencastFrameAck', { sessionId: frame.sessionId }).catch(() => {}) - }) - // Keep the hidden renderer painting so samples represent rendered frames. - await cdp.send('Page.enable') - await cdp.send('Page.startScreencast', { - format: 'jpeg', - quality: 20, - maxWidth: 640, - maxHeight: 480 - }) - await expect.poll(() => hasFrame).toBe(true) - const recording = await page.evaluateHandle(() => { +async function startCardRecording(page: Page): Promise> { + return page.evaluateHandle(() => { const frames: CardFrame[] = [] const started = performance.now() let nextFrame = 0 @@ -63,15 +48,14 @@ async function startCardRecording(page: Page): Promise { } } }) - return { recording, cdp } } -async function stopCardRecording({ recording, cdp }: CardCapture): Promise { - const frames = await recording.evaluate((recording) => recording.stop()) - await recording.dispose() - await cdp.send('Page.stopScreencast') - await cdp.detach() - return frames +async function stopCardRecording(recording: JSHandle): Promise { + try { + return await recording.evaluate((recording) => recording.stop()) + } finally { + await recording.dispose() + } } test('whole-tab hover cards slide immediately between neighboring tabs', async ({ @@ -95,109 +79,114 @@ test('whole-tab hover cards slide immediately between neighboring tabs', async ( const second = s.createTab(worktreeId) s.setTabCustomTitle(second.id, 'Review changes') }) - const first = orcaPage.locator('[data-testid="sortable-tab"]').first() - const second = orcaPage.locator('[data-testid="sortable-tab"]').nth(1) - const firstBox = await first.boundingBox() - const secondBox = await second.boundingBox() - if (!firstBox || !secondBox) { - throw new Error('Missing tab bounds') - } - const card = orcaPage.locator('[data-tab-hover-card]') - const cardTitle = card.locator('[data-tab-hover-card-title]') - - // The left padding and icon used to sit outside the terminal title's trigger. - await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) - await orcaPage.waitForTimeout(200) - await expect(card).toHaveCount(0) - await expect(cardTitle).toHaveText('Build and test the hover cards') - const program = card.locator('[data-tab-hover-card-program]') - await expect(program.locator('[data-shell-icon]')).toHaveCount(1) - await expect(program).toHaveText('Terminal') - const titleBox = await cardTitle.boundingBox() - const programBox = await program.boundingBox() - expect(programBox?.y).toBeGreaterThan((titleBox?.y ?? 0) + (titleBox?.height ?? 0)) - await orcaPage.waitForTimeout(200) - const browserWindow = await electronApp.browserWindow(orcaPage) await browserWindow.evaluate((window) => window.webContents.setBackgroundThrottling(false)) await browserWindow.dispose() - const recording = await startCardRecording(orcaPage) - await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) - await expect(cardTitle).toHaveText('Review changes', { timeout: 150 }) - await orcaPage.waitForTimeout(200) - const frames = await stopCardRecording(recording) - const firstTargetFrame = frames.findIndex((frame) => frame.title === 'Review changes') - expect(firstTargetFrame, JSON.stringify(frames)).toBeGreaterThan(0) - expect( - frames - .slice(0, firstTargetFrame) - .every((frame) => frame.title === 'Build and test the hover cards') - ).toBe(true) - expect(frames.slice(firstTargetFrame).every((frame) => frame.title === 'Review changes')).toBe( - true - ) - expect(frames.some((frame) => frame.sliding)).toBe(true) - expect(frames[firstTargetFrame]?.left).toBeLessThan(secondBox.x - 10) - await expect(cardTitle).toHaveText('Review changes') - await orcaPage.waitForTimeout(500) + const stopCapture = await startHiddenRendererFrameCapture(orcaPage) + try { + const first = orcaPage.locator('[data-testid="sortable-tab"]').first() + const second = orcaPage.locator('[data-testid="sortable-tab"]').nth(1) + const firstBox = await first.boundingBox() + const secondBox = await second.boundingBox() + if (!firstBox || !secondBox) { + throw new Error('Missing tab bounds') + } + const card = orcaPage.locator('[data-tab-hover-card]') + const cardTitle = card.locator('[data-tab-hover-card-title]') - await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) - await expect(cardTitle).toHaveText('Build and test the hover cards', { timeout: 150 }) - await orcaPage.waitForTimeout(600) - await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) - await expect(cardTitle).toHaveText('Review changes', { timeout: 150 }) + // The left padding and icon used to sit outside the terminal title's trigger. + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) + await orcaPage.waitForTimeout(200) + await expect(card).toHaveCount(0) + await expect(cardTitle).toHaveText('Build and test the hover cards') + const program = card.locator('[data-tab-hover-card-program]') + await expect(program.locator('[data-shell-icon]')).toHaveCount(1) + await expect(program).toHaveText('Terminal') + const titleBox = await cardTitle.boundingBox() + const programBox = await program.boundingBox() + expect(programBox?.y).toBeGreaterThan((titleBox?.y ?? 0) + (titleBox?.height ?? 0)) + await orcaPage.waitForTimeout(200) - const reversalRecording = await startCardRecording(orcaPage) - for (let index = 0; index < 12; index += 1) { - const target = index % 2 === 0 ? firstBox : secondBox - const title = index % 2 === 0 ? 'Build and test the hover cards' : 'Review changes' - await orcaPage.mouse.move(target.x + 3, target.y + target.height / 2) - await expect(cardTitle).toHaveText(title, { timeout: 150 }) - await orcaPage.waitForTimeout(30) - await expect(card).toHaveCount(1) + const recording = await startCardRecording(orcaPage) + await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) + await expect(cardTitle).toHaveText('Review changes', { timeout: 150 }) + await orcaPage.waitForTimeout(200) + const frames = await stopCardRecording(recording) + const firstTargetFrame = frames.findIndex((frame) => frame.title === 'Review changes') + expect(firstTargetFrame, JSON.stringify(frames)).toBeGreaterThan(0) + expect( + frames + .slice(0, firstTargetFrame) + .every((frame) => frame.title === 'Build and test the hover cards') + ).toBe(true) + expect(frames.slice(firstTargetFrame).every((frame) => frame.title === 'Review changes')).toBe( + true + ) + expect(frames.some((frame) => frame.sliding)).toBe(true) + expect(frames[firstTargetFrame]?.left).toBeLessThan(secondBox.x - 10) + await expect(cardTitle).toHaveText('Review changes') + await orcaPage.waitForTimeout(500) + + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) + await expect(cardTitle).toHaveText('Build and test the hover cards', { timeout: 150 }) + await orcaPage.waitForTimeout(600) + await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) + await expect(cardTitle).toHaveText('Review changes', { timeout: 150 }) + + const reversalRecording = await startCardRecording(orcaPage) + for (let index = 0; index < 12; index += 1) { + const target = index % 2 === 0 ? firstBox : secondBox + const title = index % 2 === 0 ? 'Build and test the hover cards' : 'Review changes' + await orcaPage.mouse.move(target.x + 3, target.y + target.height / 2) + await expect(cardTitle).toHaveText(title, { timeout: 150 }) + await orcaPage.waitForTimeout(30) + await expect(card).toHaveCount(1) + } + const reversalFrames = await stopCardRecording(reversalRecording) + expect( + reversalFrames.every((frame) => frame.count === 1), + JSON.stringify(reversalFrames) + ).toBe(true) + for (const title of ['Build and test the hover cards', 'Review changes']) { + expect(reversalFrames.some((frame) => frame.title === title && frame.sliding)).toBe(true) + } + + await first.locator('[data-tab-close-button]').hover() + await orcaPage.waitForTimeout(100) + await expect(cardTitle).toHaveText('Build and test the hover cards') + await expect(orcaPage.locator('[data-tab-hover-card], [data-tab-close-tooltip]')).toHaveCount(1) + await orcaPage.waitForTimeout(500) + await expect(cardTitle).toHaveText('Build and test the hover cards') + await expect(orcaPage.locator('[data-tab-hover-card], [data-tab-close-tooltip]')).toHaveCount(1) + await expect(orcaPage.locator('[data-tab-close-tooltip]')).toBeVisible() + await expect(card).toHaveCount(0) + await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) + await expect(cardTitle).toHaveText('Review changes') + + await orcaPage.emulateMedia({ reducedMotion: 'reduce' }) + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) + await expect(cardTitle).toHaveText('Build and test the hover cards') + expect(await card.evaluate((element) => element.getAnimations().length)).toBe(0) + await first.locator('[data-tab-close-button]').focus() + await expect(orcaPage.locator('[data-tab-close-tooltip]')).toBeVisible() + await expect(card).toHaveCount(0) + await first.focus() + await expect(cardTitle).toHaveText('Build and test the hover cards') + await expect(orcaPage.locator('[data-tab-close-tooltip]')).toHaveCount(0) + await first.press('Escape') + await expect(card).toHaveCount(0) + + await first.click({ position: { x: 3, y: firstBox.height / 2 } }) + await expect(card).toHaveCount(0) + + await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height + 100) + await orcaPage.waitForTimeout(400) + await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) + await orcaPage.waitForTimeout(200) + await expect(card).toHaveCount(0) + await expect(cardTitle).toHaveText('Review changes') + await orcaPage.waitForTimeout(500) + } finally { + await stopCapture() } - const reversalFrames = await stopCardRecording(reversalRecording) - expect( - reversalFrames.every((frame) => frame.count === 1), - JSON.stringify(reversalFrames) - ).toBe(true) - for (const title of ['Build and test the hover cards', 'Review changes']) { - expect(reversalFrames.some((frame) => frame.title === title && frame.sliding)).toBe(true) - } - - await first.locator('[data-tab-close-button]').hover() - await orcaPage.waitForTimeout(100) - await expect(cardTitle).toHaveText('Build and test the hover cards') - await expect(orcaPage.locator('[data-tab-hover-card], [data-tab-close-tooltip]')).toHaveCount(1) - await orcaPage.waitForTimeout(500) - await expect(cardTitle).toHaveText('Build and test the hover cards') - await expect(orcaPage.locator('[data-tab-hover-card], [data-tab-close-tooltip]')).toHaveCount(1) - await expect(orcaPage.locator('[data-tab-close-tooltip]')).toBeVisible() - await expect(card).toHaveCount(0) - await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) - await expect(cardTitle).toHaveText('Review changes') - - await orcaPage.emulateMedia({ reducedMotion: 'reduce' }) - await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height / 2) - await expect(cardTitle).toHaveText('Build and test the hover cards') - expect(await card.evaluate((element) => element.getAnimations().length)).toBe(0) - await first.locator('[data-tab-close-button]').focus() - await expect(orcaPage.locator('[data-tab-close-tooltip]')).toBeVisible() - await expect(card).toHaveCount(0) - await first.focus() - await expect(cardTitle).toHaveText('Build and test the hover cards') - await expect(orcaPage.locator('[data-tab-close-tooltip]')).toHaveCount(0) - await first.press('Escape') - await expect(card).toHaveCount(0) - - await first.click({ position: { x: 3, y: firstBox.height / 2 } }) - await expect(card).toHaveCount(0) - - await orcaPage.mouse.move(firstBox.x + 3, firstBox.y + firstBox.height + 100) - await orcaPage.waitForTimeout(400) - await orcaPage.mouse.move(secondBox.x + 3, secondBox.y + secondBox.height / 2) - await orcaPage.waitForTimeout(200) - await expect(card).toHaveCount(0) - await expect(cardTitle).toHaveText('Review changes') - await orcaPage.waitForTimeout(500) })