diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 2e6726a64b4..c64a6ba75a3 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -9319,6 +9319,10 @@ describe('connectPanePty', () => { // replacement PTY exists, whatever route created it. it('drops stale callbacks but delivers fresh replacement output after an old replay clear', async () => { const { connectPanePty } = await import('./pty-connection') + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState const { getTerminalScrollIntentKind, markTerminalPinnedViewport } = await import('@/lib/pane-manager/terminal-scroll-intent') const transport = createMockTransport('tab-pty') @@ -9568,6 +9572,11 @@ describe('connectPanePty', () => { }) transportFactoryQueue.push(transport) setReattachPaneTitle('Cursor Agent') + // After setReattachPaneTitle: it rewrites mockStoreState and would drop this. + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState const pane = createPane(1) const textarea = {} as HTMLTextAreaElement @@ -9669,6 +9678,12 @@ describe('connectPanePty', () => { }) transportFactoryQueue.push(transport) setReattachPaneTitle('Cursor Agent') + // SSH repo: the disconnected card is SSH-only, and setReattachPaneTitle rewrites + // mockStoreState, so this must come after it. + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState const pane = createPane(1) const textarea = {} as HTMLTextAreaElement @@ -9767,6 +9782,12 @@ describe('connectPanePty', () => { it('does not respawn a pane whose restore was answered with a not-found', async () => { const { connectPanePty } = await import('./pty-connection') + // SSH repo: these are SSH reattach scenarios, and the disconnected card is SSH-only by design. + // Without this the fixture has no connectionId and the pane is not SSH at all. + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState const transport = createMockTransport('tab-pty') transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { if (sessionId) { @@ -9809,6 +9830,12 @@ describe('connectPanePty', () => { // agent process on one transcript. That is the very defect this pane exists to prevent. it('starts a fresh shell rather than resuming the agent when the user starts a new terminal', async () => { const { connectPanePty } = await import('./pty-connection') + // SSH repo: these are SSH reattach scenarios, and the disconnected card is SSH-only by design. + // Without this the fixture has no connectionId and the pane is not SSH at all. + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState const transport = createMockTransport('tab-pty') const spawnOptions: Record[] = [] transport.connect.mockImplementation(async (options: { sessionId?: string }) => { @@ -9867,6 +9894,11 @@ describe('connectPanePty', () => { // Nothing else retracts the card, so the action must clear it or it sits over a live shell. expect(onPtyRecoveryState).toHaveBeenCalledWith(pane.id, null) expect(deps.clearTabPtyId).toHaveBeenCalledWith('tab-1', 'tab-pty') + // The transport falls back to the startup it was CONSTRUCTED with whenever a per-call field is + // absent, so omitting them is not enough — a "fresh" shell would resume the saved session. This + // asserts the explicit suppression, which is the only thing that actually closes it. Asserting + // the absence of per-call fields cannot: on a mock it passes either way. + expect(spawnOptions[0]).toMatchObject({ suppressSavedStartup: true }) // The resume path claims the launch config before spawning; a fresh shell has none to claim. expect(mockStoreState.registerAgentLaunchConfig).not.toHaveBeenCalled() expect(spawnOptions).toHaveLength(1) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index dfea76a1cea..f99f212ba04 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -552,6 +552,10 @@ type PendingStartupCommand = { type FreshSpawnOptions = { forceBlankRestoredViewport?: boolean + /** Ignore the startup this transport was constructed with. Omitting the fields is not enough: + * connect falls back to its constructor values, so a "fresh" shell would resume the saved agent + * session — the duplicate transcript the disconnected pane exists to prevent. */ + suppressSavedStartup?: boolean } type ColdRestoreAgentResumeStartup = PendingStartupCommand & { @@ -5129,6 +5133,13 @@ export function connectPanePty( * because nothing here knows whether the shell died. */ const publishUnreachablePane = (sessionId: string): void => { + // Guards live HERE, not at the seven call sites. They are identical at every one, and the + // review found three separate sites that had each forgotten a different one — a local pane + // shown an SSH-specific card it can never clear, and a disposed pane republishing state onto + // a reused numeric pane id. Centralising them makes the next call site correct by default. + if (disposed || !connectionId) { + return + } // Both actions clear the banner before acting: nothing else retracts it. The app-SSH // transport does not publish recovery states, so a card left up would sit over a live shell // with armed buttons. @@ -5140,6 +5151,14 @@ export function connectPanePty( unreachablePane: { onRetry: () => { clearBanner() + // Put the card back if the remount never happens. A stale generation, a refused remount + // or a throw all return false without scheduling anything, and swallowing that leaves + // the pane with no shell AND no action surface — worse than the toast this replaced. + const republishIfDeclined = (scheduled: boolean): void => { + if (!scheduled) { + publishUnreachablePane(sessionId) + } + } // Why the generation and instance: a recovery declined by the cooldown only schedules // a deferred retry when it can identify the generation to remount. Without them a // second click inside the window is refused silently, which on a surface whose whole @@ -5150,18 +5169,29 @@ export function connectPanePty( reason: 'restore-blocked', terminalRecoveryGeneration, terminalRecoveryInstanceId: terminalRecoveryInstance.id - }) + }).then(republishIfDeclined, () => republishIfDeclined(false)) }, onStartNewTerminal: () => { clearBanner() - deps.clearExitedPanePtyLayoutBinding(pane.id, sessionId) - deps.clearTabPtyId(deps.tabId, sessionId) - // A NEW shell, not a resumed one. The old automatic respawn passed the cold-restore - // startup, which carries the agent's providerSession — correct then, because it only - // ran on proof the shell was gone. Here the shell is probably still alive, so resuming - // would put a second agent process on one transcript: the exact defect this pane - // exists to prevent. `null` starts fresh, which is also what the button says. - startFreshSpawn(null, { forceBlankRestoredViewport: true }) + // A NEW shell, not a resumed one. The automatic respawn this replaced passed the + // cold-restore startup, which carries the agent's providerSession — correct there, + // because it only ran on proof the shell was gone. Here the shell is probably still + // alive, so resuming would put a second agent process on one transcript. `null` alone + // does not achieve that: connect falls back to the startup this transport was built + // with, so the suppression has to be explicit. + void startFreshSpawn(null, { + forceBlankRestoredViewport: true, + suppressSavedStartup: true + }).then((freshPtyId) => { + if (!freshPtyId) { + // The replacement never started. Keep the old binding and put the card back, or the + // pane is left blank and unbound with no way back to a shell that may still be up. + publishUnreachablePane(sessionId) + return + } + deps.clearExitedPanePtyLayoutBinding(pane.id, sessionId) + deps.clearTabPtyId(deps.tabId, sessionId) + }) } } }) @@ -5336,6 +5366,7 @@ export function connectPanePty( transportConnectInFlightSince = Date.now() const outputCallbacks = captureTransportOutputCallbacks(reportError) const spawnedRaw = transport.connect({ + ...(options.suppressSavedStartup ? { suppressSavedStartup: true } : {}), url: '', cols, rows, diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index ea6426b2268..5811d4193d4 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -147,6 +147,10 @@ export type PtyTransport = { envToDelete?: string[] launchConfig?: SleepingAgentLaunchConfig resumeProviderSession?: AgentProviderSessionMetadata + /** Start a genuinely new shell: ignore the startup this transport was constructed with. + * Omitting the fields is not enough — connect falls back to its constructor values, so a + * "fresh" spawn would silently resume the saved agent session. */ + suppressSavedStartup?: boolean launchToken?: string launchAgent?: TuiAgent startupCommandDelivery?: StartupCommandDelivery diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 96199cb3aa2..4acdeb2890a 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -818,16 +818,19 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra ...((options.envToDelete ?? envToDelete) ? { envToDelete: options.envToDelete ?? envToDelete } : {}), - command: options.command ?? command, - ...((options.launchConfig ?? launchConfig) + command: options.suppressSavedStartup ? options.command : (options.command ?? command), + ...((options.suppressSavedStartup ? options.launchConfig : (options.launchConfig ?? launchConfig)) ? { launchConfig: options.launchConfig ?? launchConfig } : {}), - ...((options.resumeProviderSession ?? resumeProviderSession) + ...((options.suppressSavedStartup + ? options.resumeProviderSession + : (options.resumeProviderSession ?? resumeProviderSession)) ? { - resumeProviderSession: options.resumeProviderSession ?? resumeProviderSession + resumeProviderSession: + options.resumeProviderSession ?? resumeProviderSession } : {}), - ...((options.launchToken ?? launchToken) + ...((options.suppressSavedStartup ? options.launchToken : (options.launchToken ?? launchToken)) ? { launchToken: options.launchToken ?? launchToken } : {}), ...((options.launchAgent ?? launchAgent)