From 7a1d73576e43ad8a54ad372e27f4dd7c0d993237 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Fri, 9 Oct 2026 22:42:34 -0700 Subject: [PATCH] fix(cli): orca file open and diff open name their owner (#27154) * fix(cli): orca file open and diff open name their owner The desktop's own catalog resolves the worktree for files.open/files.openDiff, so the renderer notification now says so (null) instead of leaving the owner unset, which let the focused server claim a tab the renderer could not place yet. * fix(cli): keep a paired server's ownership on diff open A catalog row stamped runtime: names that server; null would route its diff through this desktop's git. --------- Co-authored-by: m4air --- ...untime-files-mobile-explorer-reads.test.ts | 78 +++++++++++++++++-- .../runtime-file-commands-constructor.ts | 30 +++++-- ...ui-bridge-clipboard-and-window-controls.ts | 4 +- .../ui-bridge-terminal-and-session-tabs.ts | 4 +- src/preload/api/ui-command-event-api.ts | 4 +- .../mobile-terminal-close-ipc-bridge.test.ts | 53 ++++++++++++- 6 files changed, 152 insertions(+), 21 deletions(-) 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 f12e89deeba..ee2a71fdac2 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 @@ -86,7 +86,7 @@ describe('RuntimeFileCommands', () => { } }) - it('opens source control diffs through the renderer host (inheriting active runtime env)', async () => { + it('opens source control diffs through the renderer host as owned by this desktop, not the focused server', async () => { const openDiff = vi.fn() const { commands } = createRuntimeFileCommands({ openDiff }) @@ -97,7 +97,7 @@ describe('RuntimeFileCommands', () => { '/repo/docs/readme.md', 'docs/readme.md', true, - undefined, + null, undefined ) expect(result).toEqual({ @@ -108,7 +108,7 @@ describe('RuntimeFileCommands', () => { }) }) - it('opens text files through the renderer host (inheriting active runtime env)', async () => { + it('opens text files through the renderer host as owned by this desktop, not the focused server', async () => { const openFile = vi.fn() const { commands } = createRuntimeFileCommands({ openFile }) resolveAuthorizedPathMock.mockResolvedValue('/repo/docs/readme.md') @@ -120,7 +120,7 @@ describe('RuntimeFileCommands', () => { 'wt-1', '/repo/docs/readme.md', 'docs/readme.md', - undefined, + null, undefined ) expect(result).toEqual({ @@ -143,7 +143,7 @@ describe('RuntimeFileCommands', () => { 'wt-1', '/repo/assets/logo.png', 'assets/logo.png', - undefined, + null, undefined ) expect(result).toEqual({ @@ -168,7 +168,7 @@ describe('RuntimeFileCommands', () => { 'wt-1', '/repo/docs/readme.md', 'docs/readme.md', - undefined, + null, 'all' ) expect(openDiff).toHaveBeenCalledWith( @@ -176,11 +176,73 @@ describe('RuntimeFileCommands', () => { '/repo/docs/readme.md', 'docs/readme.md', false, - undefined, + null, 'host' ) }) + it('names this desktop as the owner of SSH file and diff opens', async () => { + const openFile = vi.fn() + const openDiff = vi.fn() + const resolveRuntimeFileTarget = vi.fn(async () => ({ + worktree: { id: 'wt-1', repoId: 'repo-1', path: '/remote/repo' }, + executionHostId: 'ssh:ssh-1' + })) + const { commands } = createRuntimeFileCommands({ + openFile, + openDiff, + path: '/remote/repo', + resolveRuntimeFileTarget + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the open path only calls `stat`. + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: 1, mtime: 0 }) + } as never) + + await commands.openMobileFile('id:wt-1', 'docs/readme.md') + await commands.openMobileDiff('id:wt-1', 'docs/readme.md', false) + + expect(openFile).toHaveBeenCalledWith( + 'wt-1', + '/remote/repo/docs/readme.md', + 'docs/readme.md', + null, + undefined + ) + expect(openDiff).toHaveBeenCalledWith( + 'wt-1', + '/remote/repo/docs/readme.md', + 'docs/readme.md', + false, + null, + undefined + ) + }) + + it('names a paired server as the owner of a diff its catalog row is stamped with', async () => { + const openDiff = vi.fn() + const resolveRuntimeFileTarget = vi.fn(async () => ({ + worktree: { id: 'wt-1', repoId: 'repo-1', path: '/remote/repo' }, + executionHostId: 'runtime:env-a' + })) + const { commands } = createRuntimeFileCommands({ + openDiff, + path: '/remote/repo', + resolveRuntimeFileTarget + }) + + await commands.openMobileDiff('id:wt-1', 'docs/readme.md', true) + + expect(openDiff).toHaveBeenCalledWith( + 'wt-1', + '/remote/repo/docs/readme.md', + 'docs/readme.md', + true, + 'env-a', + undefined + ) + }) + it.each(['docs/example.pdf', 'dist/bundle.zip'])( 'opens binary %s in the desktop editor like the File Explorer does', async (relativePath) => { @@ -195,7 +257,7 @@ describe('RuntimeFileCommands', () => { 'wt-1', `/repo/${relativePath}`, relativePath, - undefined, + null, undefined ) expect(result).toEqual({ worktree: 'wt-1', relativePath, kind: 'binary', opened: true }) diff --git a/src/main/runtime/runtime-file-commands-constructor.ts b/src/main/runtime/runtime-file-commands-constructor.ts index 2efb8bacb03..7f3603a88b2 100644 --- a/src/main/runtime/runtime-file-commands-constructor.ts +++ b/src/main/runtime/runtime-file-commands-constructor.ts @@ -30,7 +30,7 @@ import { requireRuntimeFileProvider, runtimeFileRouteForTarget } from './runtime-file-command-target' -import type { ExecutionHostId } from '../../shared/execution-host' +import { parseExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import type { FileStat } from '../providers/types' export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithActiveRuntimeTextSearches { @@ -210,8 +210,13 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA const filePath = joinWorktreeRelativePath(worktree.path, relativePath) // Why: CLI/agents treat opened:true as success; stat first so missing paths and directories fail the RPC instead of opening a ghost tab. await this.assertOpenTargetIsFile(filePath, target) - // Why: the internal runtimeId isn't a valid env selector; pass undefined so openFile falls back to activeRuntimeEnvironmentId. - this.host.openFile(worktree.id, filePath, relativePath, undefined, navigation) + this.host.openFile( + worktree.id, + filePath, + relativePath, + runtimeOwnerForTarget(target), + navigation + ) return { worktree: worktree.id, relativePath, kind, opened: true } } @@ -243,7 +248,8 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA staged: boolean, navigation?: RuntimeNavigationTarget ): Promise { - const { worktree } = await this.host.resolveRuntimeFileTarget(worktreeSelector) + const target = await this.host.resolveRuntimeFileTarget(worktreeSelector) + const { worktree } = target if (!isSafeMobileRelativePath(relativePath)) { throw new Error('invalid_relative_path') } @@ -253,8 +259,20 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA ? 'markdown' : 'text' const filePath = joinWorktreeRelativePath(worktree.path, relativePath) - // Why: see openMobileFile; avoid stamping internal runtimeId as runtimeEnvironmentId. - this.host.openDiff(worktree.id, filePath, relativePath, staged, undefined, navigation) + this.host.openDiff( + worktree.id, + filePath, + relativePath, + staged, + runtimeOwnerForTarget(target), + navigation + ) return { worktree: worktree.id, relativePath, kind, opened: true } } } + +// Why: name the resolved owner; undefined would let the renderer fall back to the focused server. +function runtimeOwnerForTarget(target: { executionHostId: ExecutionHostId }): string | null { + const host = parseExecutionHostId(target.executionHostId) + return host?.kind === 'runtime' ? host.environmentId : null +} diff --git a/src/preload/api/ui-bridge-clipboard-and-window-controls.ts b/src/preload/api/ui-bridge-clipboard-and-window-controls.ts index 73b800c0e3d..6baa98ad6bb 100644 --- a/src/preload/api/ui-bridge-clipboard-and-window-controls.ts +++ b/src/preload/api/ui-bridge-clipboard-and-window-controls.ts @@ -23,7 +23,7 @@ export const uiClipboardAndWindowControlsApi = { filePath: string relativePath: string staged: boolean - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget }) => void ): (() => void) => { @@ -34,7 +34,7 @@ export const uiClipboardAndWindowControlsApi = { filePath: string relativePath: string staged: boolean - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget } ) => callback(data) 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 a9555fae256..fd2d00a86f6 100644 --- a/src/preload/api/ui-bridge-terminal-and-session-tabs.ts +++ b/src/preload/api/ui-bridge-terminal-and-session-tabs.ts @@ -227,7 +227,7 @@ export const uiTerminalAndSessionTabsApi = { worktreeId: string filePath: string relativePath: string - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget }) => void ): (() => void) => { @@ -237,7 +237,7 @@ export const uiTerminalAndSessionTabsApi = { worktreeId: string filePath: string relativePath: string - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget } ) => callback(data) diff --git a/src/preload/api/ui-command-event-api.ts b/src/preload/api/ui-command-event-api.ts index 02dce476eec..436c3160f43 100644 --- a/src/preload/api/ui-command-event-api.ts +++ b/src/preload/api/ui-command-event-api.ts @@ -232,7 +232,7 @@ export type UiCommandEventApi = { worktreeId: string filePath: string relativePath: string - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget }) => void ) => () => void @@ -242,7 +242,7 @@ export type UiCommandEventApi = { filePath: string relativePath: string staged: boolean - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget }) => void ) => () => void 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 f24a809d9f5..a0091a8fba0 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 @@ -41,7 +41,7 @@ type OpenFilePayload = { worktreeId: string filePath: string relativePath: string - runtimeEnvironmentId?: string + runtimeEnvironmentId?: string | null navigation?: RuntimeNavigationTarget } type OpenDiffPayload = OpenFilePayload & { staged: boolean } @@ -401,4 +401,55 @@ describe('runtime file opens on the host desktop', () => { expect(state.openFiles.find((file) => file.id === state.activeFileId)?.mode).toBe('diff') expect(state.pendingRevealWorktree?.worktreeId).toBe(BACKGROUND) }) + + describe('with another server focused', () => { + // A worktree the CLI just created, before this renderer's catalog lists it. + const FRESH = 'repo9::/repo9/fresh' + const FRESH_TS = { filePath: '/repo1/fresh/src/app.ts', relativePath: 'src/app.ts' } + + function focusServer(store: TestStore): void { + store.setState({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: owner routing reads only these fields. + settings: { activeRuntimeEnvironmentId: 'srv-focused' } as never, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: owner routing reads only the saved id. + runtimeEnvironments: [{ id: 'srv-focused' } as never] + }) + } + + it('keeps a desktop-owned file open on this desktop', () => { + const { openFile, store } = setup('editor') + focusServer(store) + + openFile({ worktreeId: FRESH, ...FRESH_TS, runtimeEnvironmentId: null, navigation: 'caller' }) + + const opened = store.getState().openFiles.find((file) => file.worktreeId === FRESH) + expect(opened?.runtimeEnvironmentId).toBeNull() + }) + + it('keeps a desktop-owned diff open on this desktop', () => { + const { openDiff, store } = setup('editor') + focusServer(store) + + openDiff({ + worktreeId: FRESH, + ...FRESH_TS, + staged: false, + runtimeEnvironmentId: null, + navigation: 'caller' + }) + + const opened = store.getState().openFiles.find((file) => file.worktreeId === FRESH) + expect(opened?.runtimeEnvironmentId).toBeNull() + }) + + it('an unnamed owner would hand the file to the focused server', () => { + const { openFile, store } = setup('editor') + focusServer(store) + + openFile({ worktreeId: FRESH, ...FRESH_TS, navigation: 'caller' }) + + const opened = store.getState().openFiles.find((file) => file.worktreeId === FRESH) + expect(opened?.runtimeEnvironmentId).toBe('srv-focused') + }) + }) })