diff --git a/docs/reference/remote-wire-compatibility.md b/docs/reference/remote-wire-compatibility.md index b2bcb3c40f4e..9d5c204abc36 100644 --- a/docs/reference/remote-wire-compatibility.md +++ b/docs/reference/remote-wire-compatibility.md @@ -245,6 +245,24 @@ The cross-version suite derives the old client's list by removing this capabilit baseline's own list, per the rule above, so the downgrade stays exercised after a release ships it. +## Worked example: `files.open` of a PDF + +`files.open` used to classify `.pdf` as `binary` and return `opened: false` without opening a +tab. The desktop viewer now opens that file and the reply is `kind: 'pdf', opened: true`. The +frame shape is unchanged, but an already-installed mobile client treats `opened: true` as +"activate the synced file tab" and then `files.read`s it, which rejects the PDF. That is Rule 3. + +The reply stays behind `files.open.pdf-desktop.v1` +(`FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY`). The check is +`supportsFilesPdfDesktopOpen` in +`src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.ts`, applied only to +`files.open`. A client that advertises it, and any in-process caller, gets the desktop viewer. +A negotiated client that does not gets the previous answer: `kind: 'binary'`, `opened: false`, +and no tab. CLI and mobile advertise it from `remoteRuntimeClientCapabilities`. The phone +build that advertises it also skips activating a `kind: 'pdf'` tab +(`mobile/src/session/mobile-file-tap-open.ts`). Delete the gate once no supported release +lacks the capability. + ## Known debt: JSON-RPC errors drop Node's string code An error raised on an SSH host crosses the relay as JSON-RPC, and diff --git a/mobile/src/session/mobile-file-tap-open.test.ts b/mobile/src/session/mobile-file-tap-open.test.ts index 732db327e082..0dff83418198 100644 --- a/mobile/src/session/mobile-file-tap-open.test.ts +++ b/mobile/src/session/mobile-file-tap-open.test.ts @@ -140,6 +140,56 @@ describe('openMobileFileTap', () => { expect(switchSessionTab).toHaveBeenCalledWith(openedTab) }) + it('does not activate the synced tab after the desktop opens a PDF', async () => { + const client = createClient([ + ok({ + worktree: 'wt-1', + relativePath: 'docs/example.pdf', + absolutePath: '/repo/docs/example.pdf', + exists: true, + isDirectory: false, + openTarget: { + kind: 'worktree-file', + provider: 'local', + relativePath: 'docs/example.pdf', + absolutePath: '/repo/docs/example.pdf' + } + }), + ok({ opened: true, kind: 'pdf', relativePath: 'docs/example.pdf' }) + ]) + const switchSessionTab = vi.fn() + const scheduleDelayedAction = vi.fn() + + openMobileFileTap({ + client, + hostId: 'host-1', + worktreeId: 'wt-1', + pathText: 'docs/example.pdf', + line: null, + column: null, + pushPreviewRoute: vi.fn(), + openBrowser: vi.fn(), + triggerOpenFeedback: vi.fn(), + fetchSessionTabs: vi.fn(), + getSessionTabs: () => [{ id: 'tab-pdf', relativePath: 'docs/example.pdf' }], + getActiveSessionTabId: () => 'terminal-tab', + getActivationState: activeTerminalState, + switchSessionTab, + scheduleDelayedAction + }) + await Promise.resolve() + await Promise.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(client.sendRequest).toHaveBeenCalledWith( + 'files.open', + { worktree: 'id:wt-1', relativePath: 'docs/example.pdf' }, + { timeoutMs: 15_000 } + ) + expect(scheduleDelayedAction).not.toHaveBeenCalled() + expect(switchSessionTab).not.toHaveBeenCalled() + }) + it('opens a sibling terminal path through the resolved owning worktree', async () => { const client = createClient([ ok({ diff --git a/mobile/src/session/mobile-file-tap-open.ts b/mobile/src/session/mobile-file-tap-open.ts index 36f9251f618a..89556d98fcf4 100644 --- a/mobile/src/session/mobile-file-tap-open.ts +++ b/mobile/src/session/mobile-file-tap-open.ts @@ -187,6 +187,11 @@ async function openMobileFileTapAsync( reportOpenFailure(options) return } + // The desktop PDF viewer is already open. Activating the synced tab would + // files.read the PDF and fail with binary_file. + if (opened.value.kind === 'pdf') { + return + } scheduleOpenedWorktreeTabActivation(options, openedPath) } diff --git a/mobile/src/session/session-launch-reply-schema.ts b/mobile/src/session/session-launch-reply-schema.ts index 1b4c970e4562..b2fcad52f708 100644 --- a/mobile/src/session/session-launch-reply-schema.ts +++ b/mobile/src/session/session-launch-reply-schema.ts @@ -16,7 +16,12 @@ import { salvagedOptional } from '../../../src/shared/zod-salvage' * that declined the open. An incompatible reply reaches the same `reportOpenFailure` through * openMobileFileTap's own catch. */ -export const fileTapOpenedSchema = z.looseObject({ opened: z.boolean() }) +export const fileTapOpenedSchema = z.looseObject({ + opened: z.boolean(), + // Optional so an older host that only sends `opened` still activates the tab. + // `pdf` is the desktop viewer; the phone must not load that tab through files.read. + kind: z.string().optional() +}) /** * The browser tab a user opens from the tab strip. diff --git a/mobile/src/session/session-reply-schema.test.ts b/mobile/src/session/session-reply-schema.test.ts index 606c3153f743..dd46a2f10c87 100644 --- a/mobile/src/session/session-reply-schema.test.ts +++ b/mobile/src/session/session-reply-schema.test.ts @@ -203,6 +203,7 @@ describe('required members', () => { it('requires the open verdict the tap routes on', () => { expect(reads(fileTapOpenedSchema, { opened: true }).opened).toBe(true) + expect(reads(fileTapOpenedSchema, { opened: true, kind: 'pdf' }).kind).toBe('pdf') expect(refuses(fileTapOpenedSchema, {})).toBe(true) expect(refuses(fileTapOpenedSchema, { opened: 1 })).toBe(true) }) diff --git a/src/cli/handlers/file.test.ts b/src/cli/handlers/file.test.ts index d94587125303..c9e258b69ec1 100644 --- a/src/cli/handlers/file.test.ts +++ b/src/cli/handlers/file.test.ts @@ -88,6 +88,52 @@ describe('orca file CLI handlers', () => { expect(vi.mocked(console.log).mock.calls[0][0]).toBe('Opened src/App.tsx.') }) + it('rejects a direct open the runtime declines', async () => { + queueFixtures( + callMock, + worktreeListFixture([buildWorktree('/tmp/repo', 'feature')]), + okFixture('req_open', { + worktree: 'wt-1', + relativePath: 'dist/bundle.zip', + kind: 'binary', + opened: false + }) + ) + + await main(['file', 'open', 'dist/bundle.zip'], '/tmp/repo') + + expect(vi.mocked(console.log)).not.toHaveBeenCalled() + expect(vi.mocked(console.error).mock.calls[0][0]).toBe( + 'Did not open dist/bundle.zip: binary file.' + ) + expect(process.exitCode).toBe(1) + }) + + it('reports a declined direct open as unsupported-type in json', async () => { + queueFixtures( + callMock, + worktreeListFixture([buildWorktree('/tmp/repo', 'feature')]), + okFixture('req_open', { + worktree: 'wt-1', + relativePath: 'dist/bundle.zip', + kind: 'binary', + opened: false + }) + ) + + await main(['file', 'open', 'dist/bundle.zip', '--json'], '/tmp/repo') + + const output = JSON.parse(vi.mocked(console.log).mock.calls[0][0]) + expect(output).toMatchObject({ + ok: false, + error: { + code: 'unsupported-type', + message: 'Did not open dist/bundle.zip: binary file.' + } + }) + expect(process.exitCode).toBe(1) + }) + it('opens a staged diff for an explicit worktree without cwd inference', async () => { queueFixtures( callMock, @@ -135,6 +181,7 @@ describe('orca file CLI handlers', () => { expect(vi.mocked(console.log).mock.calls[0][0]).toBe( 'Did not open diff for assets/logo.png: binary file.' ) + expect(process.exitCode).toBeUndefined() }) it('rejects --worktree without a value before cwd inference or RPC calls', async () => { @@ -225,6 +272,8 @@ describe('orca file CLI handlers', () => { await main(['file', 'open-changed', '--worktree', 'id:wt-1', '--json'], '/tmp/elsewhere') const output = JSON.parse(vi.mocked(console.log).mock.calls[0][0]) + expect(output.ok).toBe(true) + expect(process.exitCode).toBeUndefined() expect(output.result.opened).toEqual([]) expect(output.result.skipped).toEqual([ { diff --git a/src/cli/handlers/file.ts b/src/cli/handlers/file.ts index 83be828c040c..dc1fd9a32e11 100644 --- a/src/cli/handlers/file.ts +++ b/src/cli/handlers/file.ts @@ -6,7 +6,7 @@ import { isWslUncPath, parseWslUncPath, toWindowsWslPath } from '../../shared/ws import type { CommandHandler, HandlerContext } from '../dispatch' import { getOptionalStringFlag, getRequiredStringFlag } from '../flags' import { printResult } from '../format' -import { RuntimeClientError } from '../runtime-client' +import { RuntimeClientError } from '../runtime/types' import { getOptionalWorktreeSelector, resolveCurrentWorktreeSelector } from '../selectors' type FileOpenMode = 'edit' | 'diff' @@ -213,6 +213,11 @@ export const FILE_HANDLERS: Record = { relativePath, navigation: getFileOpenNavigation(ctx.flags) }) + // Why: printResult keeps ok:true. Agents read that as success, so a binary the runtime + // declined must be a real error here. file diff and file open-changed keep the skipped row. + if (!result.result.opened) { + throw new RuntimeClientError('unsupported-type', formatFileOpen(result.result)) + } printResult(result, ctx.json, formatFileOpen) }, 'file diff': async (ctx) => { diff --git a/src/cli/runtime/websocket-transport.test.ts b/src/cli/runtime/websocket-transport.test.ts index 59a4102471ba..176ea40393d1 100644 --- a/src/cli/runtime/websocket-transport.test.ts +++ b/src/cli/runtime/websocket-transport.test.ts @@ -33,6 +33,7 @@ import { WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY } from '../../shared/protocol-version' import { AGENT_SESSION_BACKGROUND_TASK_CHILD_VIEWS_CAPABILITY } from '../../shared/agent-session-background-task-child-views-capability' +import { FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY } from '../../shared/files-pdf-desktop-open-capability' vi.mock('./launch', () => ({ launchOrcaApp: vi.fn() @@ -85,7 +86,8 @@ describe('CLI remote WebSocket transport', () => { WORKTREE_GITHUB_PR_SUPPRESSION_RUNTIME_CAPABILITY, WORKTREE_VISIBILITY_DEFAULTS_RUNTIME_CAPABILITY, WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY, - AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY + AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, + FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY ] }) ) 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 a1086ae85801..da6b6b4016e5 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 @@ -140,6 +140,45 @@ describe('RuntimeFileCommands', () => { ) }) + it('opens a PDF through the same renderer host as the desktop viewer', async () => { + const openFile = vi.fn() + const { commands } = createRuntimeFileCommands({ openFile }) + resolveAuthorizedPathMock.mockResolvedValue('/repo/docs/example.pdf') + statMock.mockResolvedValue({ isDirectory: () => false }) + + const result = await commands.openMobileFile('id:wt-1', 'docs/example.pdf') + + expect(openFile).toHaveBeenCalledWith( + 'wt-1', + '/repo/docs/example.pdf', + 'docs/example.pdf', + undefined, + undefined + ) + expect(result).toEqual({ + worktree: 'wt-1', + relativePath: 'docs/example.pdf', + kind: 'pdf', + opened: true + }) + }) + + it('answers a PDF as an unopened binary when the caller would activate the file tab', async () => { + const openFile = vi.fn() + const { commands } = createRuntimeFileCommands({ openFile }) + + const result = await commands.openMobileFile('id:wt-1', 'docs/example.pdf', undefined, false) + + expect(openFile).not.toHaveBeenCalled() + expect(statMock).not.toHaveBeenCalled() + expect(result).toEqual({ + worktree: 'wt-1', + relativePath: 'docs/example.pdf', + kind: 'binary', + opened: false + }) + }) + it('leaves non-previewable binaries unavailable on mobile', async () => { const openFile = vi.fn() const { commands } = createRuntimeFileCommands({ openFile }) @@ -156,6 +195,44 @@ describe('RuntimeFileCommands', () => { }) }) + it('rejects a directory named like a PDF without creating an editor tab', async () => { + const openFile = vi.fn() + const { commands } = createRuntimeFileCommands({ openFile }) + resolveAuthorizedPathMock.mockResolvedValue('/repo/docs/notes.pdf') + statMock.mockResolvedValue({ isDirectory: () => true }) + + await expect(commands.openMobileFile('id:wt-1', 'docs/notes.pdf')).rejects.toThrow( + "EISDIR: illegal operation on a directory, open '/repo/docs/notes.pdf'" + ) + expect(openFile).not.toHaveBeenCalled() + }) + + it('rejects a remote directory named like a PDF without creating an editor tab', async () => { + const openFile = 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, + path: '/remote/repo', + resolveRuntimeFileTarget + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the command only reads `stat`. + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'directory', size: 0, mtime: 0 }) + } as never) + + await expect(commands.openMobileFile('id:wt-1', 'docs/notes.pdf')).rejects.toThrow( + "EISDIR: illegal operation on a directory, open '/remote/repo/docs/notes.pdf'" + ) + expect(openFile).not.toHaveBeenCalled() + }) + it('rejects missing local files without creating an editor tab', async () => { const openFile = vi.fn() const { commands } = createRuntimeFileCommands({ openFile }) diff --git a/src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.test.ts b/src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.test.ts new file mode 100644 index 000000000000..fa5680026f02 --- /dev/null +++ b/src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it, vi } from 'vitest' +import { RpcDispatcher } from '../dispatcher' +import type { RpcRequest } from '../core' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY } from '../../../../shared/files-pdf-desktop-open-capability' +import { remoteRuntimeClientCapabilities } from '../../../../shared/remote-runtime-client-capabilities' +import { FILE_METHODS } from './files' +import { supportsFilesPdfDesktopOpen } from './files-open-pdf-desktop-capability' + +function makeRequest(params: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method: 'files.open', params } +} + +function createDispatcher(): { + runtime: { openMobileFile: ReturnType } + dispatcher: RpcDispatcher +} { + const runtime = { + getRuntimeId: () => 'test-runtime', + openMobileFile: vi.fn().mockResolvedValue({ + worktree: 'wt-1', + relativePath: 'docs/example.pdf', + kind: 'pdf', + opened: true + }) + } + const dispatcher = new RpcDispatcher({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: this handler only reads openMobileFile. + runtime: runtime as unknown as OrcaRuntimeService, + methods: FILE_METHODS + }) + return { runtime, dispatcher } +} + +describe('files.open PDF desktop capability', () => { + it('treats an in-process caller as this build', () => { + expect(supportsFilesPdfDesktopOpen({})).toBe(true) + }) + + it('requires the capability from a negotiated client', () => { + expect(supportsFilesPdfDesktopOpen({ clientKind: 'mobile', clientCapabilities: [] })).toBe( + false + ) + expect( + supportsFilesPdfDesktopOpen({ + clientKind: 'runtime', + clientCapabilities: [FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY] + }) + ).toBe(true) + }) + + it('is advertised by the CLI and mobile remote defaults', () => { + expect(remoteRuntimeClientCapabilities()).toContain(FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY) + }) + + it('opens a PDF for an in-process caller without a fourth argument', async () => { + const { runtime, dispatcher } = createDispatcher() + + await dispatcher.dispatch( + makeRequest({ worktree: 'id:wt-1', relativePath: 'docs/example.pdf' }) + ) + + expect(runtime.openMobileFile).toHaveBeenCalledWith('id:wt-1', 'docs/example.pdf', undefined) + }) + + it('keeps the old binary answer for a client that would activate the file tab', async () => { + const { runtime, dispatcher } = createDispatcher() + + await dispatcher.dispatch( + makeRequest({ worktree: 'id:wt-1', relativePath: 'docs/example.pdf' }), + { clientKind: 'mobile', clientCapabilities: [] } + ) + + expect(runtime.openMobileFile).toHaveBeenCalledWith( + 'id:wt-1', + 'docs/example.pdf', + undefined, + false + ) + }) + + it('opens a PDF for a client that advertises the desktop viewer', async () => { + const { runtime, dispatcher } = createDispatcher() + + await dispatcher.dispatch( + makeRequest({ worktree: 'id:wt-1', relativePath: 'docs/example.pdf' }), + { + clientKind: 'mobile', + clientCapabilities: [FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY] + } + ) + + expect(runtime.openMobileFile).toHaveBeenCalledWith('id:wt-1', 'docs/example.pdf', undefined) + }) +}) diff --git a/src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.ts b/src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.ts new file mode 100644 index 000000000000..e0443798ada2 --- /dev/null +++ b/src/main/runtime/rpc/methods/files-open-pdf-desktop-capability.ts @@ -0,0 +1,18 @@ +// Transitional: remove once no supported release lacks FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY. +// +// files.open used to classify a PDF as binary and return opened:false without opening a tab. +// An already-installed mobile client treats opened:true as "activate that tab and files.read it", +// which renders Binary preview unavailable. The desktop viewer stays behind this capability. + +import { FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY } from '../../../../shared/files-pdf-desktop-open-capability' +import type { RpcContext } from '../core' + +export function supportsFilesPdfDesktopOpen( + context: Pick +): boolean { + // An in-process caller is this build. Only a negotiated client can predate the viewer. + return ( + context.clientKind === undefined || + context.clientCapabilities?.includes(FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY) === true + ) +} diff --git a/src/main/runtime/rpc/methods/files.ts b/src/main/runtime/rpc/methods/files.ts index 15ddd47b335f..e8359f9886dd 100644 --- a/src/main/runtime/rpc/methods/files.ts +++ b/src/main/runtime/rpc/methods/files.ts @@ -1,4 +1,5 @@ import { defineMethod, defineStreamingMethod } from '../core' +import { supportsFilesPdfDesktopOpen } from './files-open-pdf-desktop-capability' import { runFileWatchStream } from './file-watch-stream-lifecycle' import { FILE_MUTATION_METHODS } from './files-mutation-methods' import { remoteFileContentBudget } from './files-remote-content-budget' @@ -58,8 +59,17 @@ export const FILE_METHODS = [ defineMethod({ name: 'files.open', params: FileOpenTab, - handler: async (params, { runtime }) => - runtime.openMobileFile(params.worktree, params.relativePath, params.navigation) + handler: async (params, context) => { + const pdfDesktopOpen = supportsFilesPdfDesktopOpen(context) + return pdfDesktopOpen + ? context.runtime.openMobileFile(params.worktree, params.relativePath, params.navigation) + : context.runtime.openMobileFile( + params.worktree, + params.relativePath, + params.navigation, + false + ) + } }), defineMethod({ name: 'files.openDiff', diff --git a/src/main/runtime/runtime-file-commands-constructor.ts b/src/main/runtime/runtime-file-commands-constructor.ts index ecd21b43994e..57ff9c01928f 100644 --- a/src/main/runtime/runtime-file-commands-constructor.ts +++ b/src/main/runtime/runtime-file-commands-constructor.ts @@ -27,6 +27,13 @@ import { resolveAuthorizedPath } from '../ipc/filesystem-auth' import { isENOENT } from '../ipc/filesystem-path-containment' import { runtimeFileRouteForTarget, type RuntimeFileRoute } from './runtime-file-command-target' +function isRuntimePreviewablePdfPath(relativePath: string): boolean { + const basename = basenameFromRelativePath(relativePath) + const dotIndex = basename.lastIndexOf('.') + // Why: `.pdf` is already a desktop preview (PdfViewer). A leading dot is not an extension. + return dotIndex > 0 && basename.slice(dotIndex).toLowerCase() === '.pdf' +} + export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithActiveRuntimeTextSearches { constructor(private readonly host: RuntimeFileCommandHost) { super() @@ -157,23 +164,31 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA async openMobileFile( worktreeSelector: string, relativePath: string, - navigation?: RuntimeNavigationTarget + navigation?: RuntimeNavigationTarget, + // Why: the RPC layer passes false for a negotiated client that still activates a file tab + // when opened is true. In-process callers and clients that advertise the capability omit it. + pdfDesktopOpen = true ): Promise { const target = await this.host.resolveRuntimeFileTarget(worktreeSelector) const { worktree } = target if (!isSafeMobileRelativePath(relativePath)) { throw new Error('invalid_relative_path') } - // Previewable images open like text (mobile renders via files.readPreview); other binaries stay unavailable on mobile. + // Previewable images open like text (mobile renders via files.readPreview). + // PDFs open the same desktop viewer the file explorer uses. Other binaries stay unavailable. const kind = isMobilePreviewableImagePath(relativePath) ? 'image' - : isMobileBinaryPath(relativePath) - ? 'binary' - : isMobileMarkdownPath(relativePath) - ? 'markdown' - : 'text' - if (kind === 'binary') { - return { worktree: worktree.id, relativePath, kind, opened: false } + : isRuntimePreviewablePdfPath(relativePath) + ? 'pdf' + : isMobileBinaryPath(relativePath) + ? 'binary' + : isMobileMarkdownPath(relativePath) + ? 'markdown' + : 'text' + // A client that predates the desktop PDF open classified .pdf as binary and did not open it. + // Sending opened:true would make that client files.read the new tab. + if (kind === 'binary' || (kind === 'pdf' && !pdfDesktopOpen)) { + return { worktree: worktree.id, relativePath, kind: 'binary', opened: false } } const filePath = joinWorktreeRelativePath(worktree.path, relativePath) // Why: CLI/agents treat opened:true as success; stat first so missing paths fail the RPC instead of opening a ghost tab. @@ -187,10 +202,12 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA filePath: string, route: RuntimeFileRoute ): Promise { + let stats: { isDirectory: () => boolean } try { - await (route.kind === 'ssh' - ? this.statRemoteTerminalPath(filePath, route.connectionId) - : stat(await resolveAuthorizedPath(filePath, this.host.requireStore()))) + stats = + route.kind === 'ssh' + ? await this.statRemoteTerminalPath(filePath, route.connectionId) + : await stat(await resolveAuthorizedPath(filePath, this.host.requireStore())) } catch (error) { if ( isENOENT(error) || @@ -200,6 +217,11 @@ export class RuntimeFileCommandsWithConstructor extends RuntimeFileCommandsWithA } throw error } + // Why: a directory named `notes.pdf` still exists, so an existence check would report + // opened:true and leave a PDF tab that cannot load it. Open is for a file. + if (stats.isDirectory()) { + throw new Error(`EISDIR: illegal operation on a directory, open '${filePath}'`) + } } async openMobileDiff( diff --git a/src/shared/files-pdf-desktop-open-capability.ts b/src/shared/files-pdf-desktop-open-capability.ts new file mode 100644 index 000000000000..b618f8e03578 --- /dev/null +++ b/src/shared/files-pdf-desktop-open-capability.ts @@ -0,0 +1,3 @@ +// Why: files.open used to answer opened:false for a PDF. An already-installed mobile client +// treats opened:true as "activate the synced file tab", then files.read rejects the PDF. +export const FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY = 'files.open.pdf-desktop.v1' as const diff --git a/src/shared/remote-runtime-client-capabilities.ts b/src/shared/remote-runtime-client-capabilities.ts index b156684c1773..ee77092db51c 100644 --- a/src/shared/remote-runtime-client-capabilities.ts +++ b/src/shared/remote-runtime-client-capabilities.ts @@ -12,6 +12,7 @@ import { type RuntimeCapability } from './protocol-version' import { AGENT_SESSION_BACKGROUND_TASK_CHILD_VIEWS_CAPABILITY } from './agent-session-background-task-child-views-capability' +import { FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY } from './files-pdf-desktop-open-capability' import { SKILL_INSTALL_RESULT_V2_CAPABILITY } from './skill-install-capability' // Keep every remote request transport on the same mixed-version capability contract. @@ -32,6 +33,9 @@ export function remoteRuntimeClientCapabilities( WORKTREE_VISIBILITY_DEFAULTS_RUNTIME_CAPABILITY, WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY, AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, + // CLI and mobile both call files.open. Without this, a new host still answers a PDF + // the way the previous release did: opened false, no desktop viewer. + FILES_PDF_DESKTOP_OPEN_RUNTIME_CAPABILITY, ...additionalCapabilities ]) ) diff --git a/src/shared/runtime-file-contracts.ts b/src/shared/runtime-file-contracts.ts index 17ffe6d2c3af..5bb98577273d 100644 --- a/src/shared/runtime-file-contracts.ts +++ b/src/shared/runtime-file-contracts.ts @@ -18,7 +18,7 @@ export type RuntimeFileListResult = { export type RuntimeFileOpenResult = { worktree: string relativePath: string - kind: 'markdown' | 'text' | 'binary' | 'image' + kind: 'markdown' | 'text' | 'binary' | 'image' | 'pdf' opened: boolean }