From aaa6db6eac4c0974c2d2dd47ced0159b5ce60d31 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:33:55 -0800 Subject: [PATCH] test(terminal): prove refused starts recover through their own Retry Start the replaced-while-listed Sleep's successor only after the listed process's exit has settled its files, so its cold read is not refused mid-write. Prove a fresh pane refused before it started arms Retry and starts fresh on it, and that an agent session refused for unavailable host history is re-ensured with its original request and attached on Retry. Co-Authored-By: Claude Opus 5.5 (1M context) --- ...mon-pty-adapter-host-history-sleep.test.ts | 3 + .../pty-connection-owner-error-retry.spec.ts | 55 ++++++++- ...-pty-transport-history-unavailable.test.ts | 110 ++++++++++++++---- 3 files changed, 140 insertions(+), 28 deletions(-) diff --git a/src/main/daemon/providers/adapter/testing/daemon-pty-adapter-host-history-sleep.test.ts b/src/main/daemon/providers/adapter/testing/daemon-pty-adapter-host-history-sleep.test.ts index 3d8a3edaf20..b0b479e9471 100644 --- a/src/main/daemon/providers/adapter/testing/daemon-pty-adapter-host-history-sleep.test.ts +++ b/src/main/daemon/providers/adapter/testing/daemon-pty-adapter-host-history-sleep.test.ts @@ -277,6 +277,9 @@ describe('host Sleep through disk-only Wake', () => { const stale = relaunched.shutdown(sessionId, { keepHistory: true }) await held listed._simulateExit(0) + // The successor's cold read starts once the listed process's exit has settled its files. + await waitFor(() => harness!.server.host.listSessions().length === 0, 10_000) + await harness!.server.host.historyJournals!.settleAll() await relaunched.spawn(request(sessionId)) const successor = child() release.resolve() diff --git a/src/renderer/src/components/terminal-pane/pty-connection/reattach/pty-connection-owner-error-retry.spec.ts b/src/renderer/src/components/terminal-pane/pty-connection/reattach/pty-connection-owner-error-retry.spec.ts index fa3acb14a75..7940019556a 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/reattach/pty-connection-owner-error-retry.spec.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/reattach/pty-connection-owner-error-retry.spec.ts @@ -1,6 +1,7 @@ import { bindPtyConnectionTestMocks } from '../test-fixtures/pty-connection-mocks.spec' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { flushAsyncTicks } from '../test-fixtures/pty-connection-test-async' +import { sendTerminalInputThroughPane } from '../test-fixtures/pty-connection-test-dom' import { LEAF_2, createMockTransport, @@ -32,20 +33,24 @@ export function registerPtyConnectionOwnerErrorRetryTests(): void { return buildPaneConnectionDeps(() => mockStoreState, overrides) } - function seedUnverifiableRestoredPane() { + function seedPaneLayout(ptyId: string | null) { mockStoreState = { ...mockStoreState, - tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: 'unverifiable-pty' }] }, - ptyIdsByTabId: { 'tab-1': ['unverifiable-pty'] }, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId }] }, + ptyIdsByTabId: { 'tab-1': ptyId ? [ptyId] : [] }, terminalLayoutsByTabId: { 'tab-1': { root: { type: 'leaf', leafId: LEAF_2 }, activeLeafId: LEAF_2, expandedLeafId: null, - ptyIdsByLeafId: { [LEAF_2]: 'unverifiable-pty' } + ptyIdsByLeafId: ptyId ? { [LEAF_2]: ptyId } : {} } } } as StoreState + } + + function seedUnverifiableRestoredPane() { + seedPaneLayout('unverifiable-pty') return createDeps({ restoredLeafId: LEAF_2, restoredPtyIdByLeafId: { [LEAF_2]: 'unverifiable-pty' } @@ -175,6 +180,48 @@ export function registerPtyConnectionOwnerErrorRetryTests(): void { }) }) + // R1: a fresh start refused before it began offers Retry, which starts the same pane fresh. + it('starts a refused fresh pane on Retry and delivers its input', async () => { + const { connectPanePty } = await import('../connect-pane-pty') + const agentTitleMocks = await vi.importMock( + '@/lib/agent-status/aggregation/worktree-agent-status' + ) + const transport = createMockTransport() + transport.connect.mockImplementation(async (opts: { callbacks?: ConnectCallbacks }) => { + opts.callbacks?.onError?.('terminal_history_unavailable') + return undefined + }) + transportFactoryQueue.push(transport) + seedPaneLayout(null) + const deps = createDeps() + const pane = createPane(2) + + connectPanePty( + ...mockPanePtyArgs(pane, createManager(2), bindPaneAgentTitleMocks(deps, agentTitleMocks)) + ) + await flushAsyncTicks() + + expect(deps.onPtyErrorRef.current).toHaveBeenCalledWith( + 2, + 'terminal_history_unavailable_before_start' + ) + expect(deps.syncPanePtyLayoutBinding).not.toHaveBeenCalled() + transport.connect.mockImplementation(async ({ callbacks }) => { + callbacks?.onConnect?.() + return { id: 'fresh-pty' } + }) + // The refused start settles through several turns before it arms Retry. + await vi.waitFor(() => expect(transport.retryRecovery?.()).toBe(true)) + await flushAsyncTicks() + + expect(transport.connect).toHaveBeenCalledTimes(2) + expect(transport.connect.mock.calls[1]?.[0].sessionId).toBeUndefined() + expect(createdTransportOptions).toHaveLength(1) + expect(deps.onPtyErrorRef.current).toHaveBeenCalledTimes(1) + sendTerminalInputThroughPane(pane, 'typed') + expect(transport.sendInput.mock.lastCall?.[0]).toBe('typed') + }) + it.each([false, true])( 'preserves an exhausted pane until explicit Retry and clears only the current owner error (fresh: %s)', async (fresh) => { diff --git a/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-history-unavailable.test.ts b/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-history-unavailable.test.ts index 70b5205d3c2..248bb8d6c7c 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-history-unavailable.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport/remote-runtime/remote-runtime-pty-transport-history-unavailable.test.ts @@ -75,7 +75,13 @@ describe('remote terminal whose host history is briefly unavailable', () => { }) }) -type CreateParams = { clientMutationId?: string; reconcileExisting?: boolean } +type CreateParams = { + clientMutationId?: string + reconcileExisting?: boolean + providerSession?: { id: string } +} + +const RESUME = { key: 'session_id', id: 'provider-session-1' } as const // GH1346: the host may refuse a create for unavailable history after spawning its terminal. describe('remote terminal create refused for unavailable host history', () => { @@ -86,45 +92,81 @@ describe('remote terminal create refused for unavailable host history', () => { runtimeCall.mockImplementation( async (request: { method: string; selector: string; params?: CreateParams }) => { if (request.method === 'status.get') { - return { ok: true, result: { capabilities } } + return { + ok: true, + result: { + runtimeProtocolVersion: 3, + minCompatibleRuntimeClientVersion: 2, + capabilities + } + } } - if (request.method !== 'terminal.create') { + const ensure = request.method === 'terminal.ensureAgentSession' + if (request.method !== 'terminal.create' && !ensure) { return healthyCall?.(request) } - const mutation = request.params?.clientMutationId - if (!request.params?.reconcileExisting || !spawned.includes(mutation)) { - spawned.push(mutation) + // An ensure adopts its provider session's terminal; a create adopts only when reconciling. + const key = ensure ? request.params?.providerSession?.id : request.params?.clientMutationId + const existing = spawned.includes(key) + if (!existing || !(ensure || request.params?.reconcileExisting)) { + spawned.push(key) } return historyReady - ? { ok: true, result: { terminal: { handle: 'terminal-1' } } } + ? { + ok: true, + result: { + terminal: { handle: 'terminal-1' }, + ...(ensure ? { disposition: existing ? 'adopted' : 'created' } : {}) + } + } : { ok: false, error: { code: 'runtime_error', message: 'terminal_history_unavailable' } } } ) - const creates = (): CreateParams[] => + const calls = (method: string): CreateParams[] => runtimeCall.mock.calls - .filter(([request]) => request.method === 'terminal.create') + .filter(([request]) => request.method === method) .map(([request]) => request.params) return { spawned, - creates, + creates: () => calls('terminal.create'), + ensures: () => calls('terminal.ensureAgentSession'), restoreHistory: () => { historyReady = true } } } - async function mountRefusedPane() { + async function mountRefusedPane(agent?: { + launchAgent: 'claude' + resumeProviderSession: typeof RESUME + }) { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const transport = createRemoteRuntimePtyTransport('env-1', { worktreeId: 'wt-1', tabId: 'tab-1', - leafId: 'pane:1' + leafId: 'pane:1', + ...agent }) void transport.connect({ url: '', cols: 80, rows: 24, callbacks: {} }) await vi.advanceTimersByTimeAsync(REMOTE_RUNTIME_AUTO_RECOVERY_TIMEOUT_MS + 1_000) return transport } + async function attachAndType(transport: Awaited>) { + expect(transport.retryRecovery?.()).toBe(true) + await vi.waitFor(() => expect(subscribeFrameCount()).toBe(1)) + const { streamId } = latestSubscribePayload() + callbacks?.onResponse({ ok: true, result: { type: 'subscribed', streamId } }) + emitSnapshot(streamId, 'prompt$ ') + await vi.advanceTimersByTimeAsync(0) + expect(transport.sendInput('typed\r', 'driving')).toBe(true) + await vi.waitFor(() => expect(inputFrameTexts().join('')).toBe('typed\r')) + expect(subscribedTerminalHandles()).toEqual(['terminal-1']) + expect(new Set(runtimeCall.mock.calls.map(([request]) => request.selector))).toEqual( + new Set(['env-1']) + ) + } + it('reconciles that one create, then Retry attaches it and delivers input', async () => { vi.useFakeTimers() try { @@ -137,15 +179,8 @@ describe('remote terminal create refused for unavailable host history', () => { expect(reconciles.length).toBeGreaterThan(0) host.restoreHistory() - expect(transport.retryRecovery?.()).toBe(true) - await vi.waitFor(() => expect(subscribeFrameCount()).toBe(1)) - const { streamId } = latestSubscribePayload() - callbacks?.onResponse({ ok: true, result: { type: 'subscribed', streamId } }) - emitSnapshot(streamId, 'prompt$ ') - await vi.advanceTimersByTimeAsync(0) - expect(transport.sendInput('typed\r', 'driving')).toBe(true) + await attachAndType(transport) - await vi.waitFor(() => expect(inputFrameTexts().join('')).toBe('typed\r')) expect(host.spawned).toEqual([first.clientMutationId]) expect(first.clientMutationId).toBeTruthy() for (const reconcile of host.creates().slice(1)) { @@ -154,10 +189,37 @@ describe('remote terminal create refused for unavailable host history', () => { reconcileExisting: true }) } - expect(subscribedTerminalHandles()).toEqual(['terminal-1']) - expect(new Set(runtimeCall.mock.calls.map(([request]) => request.selector))).toEqual( - new Set(['env-1']) - ) + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) + + it('re-ensures an agent session with its original request, then Retry attaches it', async () => { + vi.useFakeTimers() + try { + const host = installHost(['agent-session.host-authority.v1']) + const transport = await mountRefusedPane({ + launchAgent: 'claude', + resumeProviderSession: RESUME + }) + + expect(transport.getRecoveryState?.().phase).toBe('disconnected') + const [first, ...reensures] = host.ensures() + expect(first).toMatchObject({ + providerSession: RESUME, + placement: { tabId: 'tab-1', leafId: 'pane:1' } + }) + expect(reensures.length).toBeGreaterThan(0) + + host.restoreHistory() + await attachAndType(transport) + + expect(host.spawned).toEqual([RESUME.id]) + for (const reensure of host.ensures().slice(1)) { + expect(reensure).toEqual(first) + } + expect(host.creates()).toEqual([]) transport.destroy?.() } finally { vi.useRealTimers()