From 4efbbbe0721fe96feef88cfe9f4da0634cefb6c7 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 23 Jul 2026 18:10:33 -0700 Subject: [PATCH] fix(agent-dashboard): admit main renderer to terminal-preview IPC for in-window dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminalPreview:* handlers gated every channel to the pop-out renderer, so the in-window overlay's terminal dialog (running in the main renderer) got { snapshot: null } from connect and falsely showed "No live terminal — this agent's pane has closed." for live agents. Accept the trusted UI renderer too — it already has full PTY access through the regular terminal channels, so this adds no reach. --- .../ipc/terminal-preview-admission.test.ts | 3 ++ src/main/ipc/terminal-preview.test.ts | 49 ++++++++++++++----- src/main/ipc/terminal-preview.ts | 20 +++++--- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/main/ipc/terminal-preview-admission.test.ts b/src/main/ipc/terminal-preview-admission.test.ts index 50f3b750179..11c8141c10e 100644 --- a/src/main/ipc/terminal-preview-admission.test.ts +++ b/src/main/ipc/terminal-preview-admission.test.ts @@ -15,6 +15,9 @@ vi.mock('electron', () => ({ ipcMain: ipcMainMock })) vi.mock('../window/dashboard-popout-window', () => ({ isDashboardPopoutRenderer: () => true })) +vi.mock('./ui', () => ({ + isTrustedUIRenderer: () => false +})) import { registerTerminalPreviewHandlers, diff --git a/src/main/ipc/terminal-preview.test.ts b/src/main/ipc/terminal-preview.test.ts index 396b1d93f7a..a01fd7c70eb 100644 --- a/src/main/ipc/terminal-preview.test.ts +++ b/src/main/ipc/terminal-preview.test.ts @@ -1,21 +1,26 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { handlers, ipcMainMock, isDashboardPopoutRendererMock } = vi.hoisted(() => { - const map = new Map unknown>() - return { - handlers: map, - ipcMainMock: { - removeHandler: vi.fn(), - handle: (channel: string, fn: (...args: unknown[]) => unknown) => map.set(channel, fn) - }, - isDashboardPopoutRendererMock: vi.fn(() => true) - } -}) +const { handlers, ipcMainMock, isDashboardPopoutRendererMock, isTrustedUIRendererMock } = + vi.hoisted(() => { + const map = new Map unknown>() + return { + handlers: map, + ipcMainMock: { + removeHandler: vi.fn(), + handle: (channel: string, fn: (...args: unknown[]) => unknown) => map.set(channel, fn) + }, + isDashboardPopoutRendererMock: vi.fn(() => true), + isTrustedUIRendererMock: vi.fn(() => false) + } + }) vi.mock('electron', () => ({ ipcMain: ipcMainMock })) vi.mock('../window/dashboard-popout-window', () => ({ isDashboardPopoutRenderer: isDashboardPopoutRendererMock })) +vi.mock('./ui', () => ({ + isTrustedUIRenderer: isTrustedUIRendererMock +})) import { registerTerminalPreviewHandlers } from './terminal-preview' @@ -82,6 +87,7 @@ describe('registerTerminalPreviewHandlers', () => { beforeEach(() => { handlers.clear() isDashboardPopoutRendererMock.mockReturnValue(true) + isTrustedUIRendererMock.mockReturnValue(false) }) afterEach(() => { vi.clearAllMocks() @@ -281,6 +287,27 @@ describe('registerTerminalPreviewHandlers', () => { expect(runtime.writeTerminalPreviewInput).not.toHaveBeenCalled() }) + // The in-window dashboard overlay hosts the preview dialog from the main + // renderer, which is trusted but is not the popout window. + it('admits the trusted main renderer when it is not the popout', async () => { + const runtime = makeRuntime() + registerTerminalPreviewHandlers(runtime as never) + const sender = makeSender() + isDashboardPopoutRendererMock.mockReturnValue(false) + isTrustedUIRendererMock.mockReturnValue(true) + + await expect( + handlers.get('terminalPreview:connect')!(eventFor(sender), { ptyId: 'p1' }) + ).resolves.toEqual({ + snapshot: { data: 'screen', cols: 80, rows: 20, seq: 5 }, + replay: [] + }) + await expect( + handlers.get('terminalPreview:input')!(eventFor(sender), { ptyId: 'p1', data: 'x' }) + ).resolves.toBe(true) + expect(runtime.writeTerminalPreviewInput).toHaveBeenCalledWith('p1', 'x') + }) + it('pushes a resync only when the PTY grid dimensions change', async () => { const runtime = makeRuntime() registerTerminalPreviewHandlers(runtime as never) diff --git a/src/main/ipc/terminal-preview.ts b/src/main/ipc/terminal-preview.ts index 39e75390737..9b621cf25ef 100644 --- a/src/main/ipc/terminal-preview.ts +++ b/src/main/ipc/terminal-preview.ts @@ -5,6 +5,7 @@ import type { } from '../../shared/terminal-preview' import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { isDashboardPopoutRenderer } from '../window/dashboard-popout-window' +import { isTrustedUIRenderer } from './ui' import { TERMINAL_PREVIEW_OUTPUT_BATCH_MAX_BYTES, TerminalPreviewOutputStream @@ -18,6 +19,13 @@ function isValidPtyId(value: unknown): value is string { return typeof value === 'string' && value.length > 0 && value.length <= PREVIEW_ID_MAX_LENGTH } +// Why: the preview dialog has two hosts — the pop-out window and the main +// renderer's in-window overlay. The trusted UI renderer already has full PTY +// access through the regular terminal channels, so admitting it adds no reach. +function isTerminalPreviewRenderer(sender: WebContents): boolean { + return isDashboardPopoutRenderer(sender) || isTrustedUIRenderer(sender) +} + function canRetainPreviewEntry( registry: Map>, contentsId: number, @@ -48,7 +56,7 @@ export function registerTerminalPreviewHandlers(runtime: OrcaRuntimeService): vo const subscriptionsByContents = new Map>() // Why: the preview dialog claims the PTY grid through the remote-desktop // viewer registry so the main-window pane parks and later reclaims its own - // geometry. Claims are tracked per popout webContents so an explicit + // geometry. Claims are tracked per viewer webContents so an explicit // unsubscribe or a destroyed window always releases the size floor. const fitClaimsByContents = new Map>() @@ -104,7 +112,7 @@ export function registerTerminalPreviewHandlers(runtime: OrcaRuntimeService): vo event, args: { ptyId?: unknown; opts?: { scrollbackRows?: unknown } } ): Promise => { - if (!isDashboardPopoutRenderer(event.sender) || !isValidPtyId(args?.ptyId)) { + if (!isTerminalPreviewRenderer(event.sender) || !isValidPtyId(args?.ptyId)) { return { snapshot: null, replay: [] } } const ptyId = args.ptyId @@ -183,7 +191,7 @@ export function registerTerminalPreviewHandlers(runtime: OrcaRuntimeService): vo 'terminalPreview:input', (event, args: { ptyId?: unknown; data?: unknown }): Promise => { if ( - !isDashboardPopoutRenderer(event.sender) || + !isTerminalPreviewRenderer(event.sender) || !isValidPtyId(args?.ptyId) || typeof args.data !== 'string' ) { @@ -197,7 +205,7 @@ export function registerTerminalPreviewHandlers(runtime: OrcaRuntimeService): vo 'terminalPreview:ack', (event, args: { ptyId?: unknown; bytes?: unknown }): void => { if ( - !isDashboardPopoutRenderer(event.sender) || + !isTerminalPreviewRenderer(event.sender) || !isValidPtyId(args?.ptyId) || typeof args.bytes !== 'number' || !Number.isFinite(args.bytes) || @@ -221,7 +229,7 @@ export function registerTerminalPreviewHandlers(runtime: OrcaRuntimeService): vo args: { ptyId?: unknown; cols?: unknown; rows?: unknown } ): Promise<{ cols: number; rows: number } | null> => { if ( - !isDashboardPopoutRenderer(event.sender) || + !isTerminalPreviewRenderer(event.sender) || !isValidPtyId(args?.ptyId) || typeof args.cols !== 'number' || typeof args.rows !== 'number' || @@ -271,7 +279,7 @@ export function registerTerminalPreviewHandlers(runtime: OrcaRuntimeService): vo ) ipcMain.handle('terminalPreview:unsubscribe', (event, args: { ptyId?: unknown }): void => { - if (!isDashboardPopoutRenderer(event.sender) || !isValidPtyId(args?.ptyId)) { + if (!isTerminalPreviewRenderer(event.sender) || !isValidPtyId(args?.ptyId)) { return } subscriptionsByContents.get(event.sender.id)?.get(args.ptyId)?.dispose()