From 1d87f181b4afedb8fbc51a7add45ce95b303eca4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 26 Jul 2026 15:05:39 -0700 Subject: [PATCH] fix(daemon): respawn on PTY write dropped to a dead daemon socket (STA-2373) (#10065) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(daemon): respawn on PTY write dropped to a dead daemon socket (STA-2373) DaemonPtyAdapter.write() sends keystrokes via fire-and-forget client.notify(). When the daemon dies (retirement, crash, kill), the socket disconnects and the notify is silently dropped — no rejection reaches withDaemonRetry, so the dead-endpoint respawn never fires and the attached pane freezes. Only a request/reply RPC (e.g. createOrAttach from opening a new terminal) detected the death and forked a replacement. DaemonClient.notify() now reports delivery; a dropped write to a still-active session drives the shared respawn coalescer directly (reconnecting the permanent client before releasing the temporary adoption lease, mirroring withDaemonRetry's ordering), so the pane self-heals like the createOrAttach path. Cross-platform + SSH-safe: no platform assumptions, pure adapter logic. Complements (does not duplicate) #8426, which fixes the adjacent in-daemon bug where a thrown node-pty write no longer marks the handle dead. That is daemon-side; this is the app-side dropped-notify that never triggered respawn. * fix(daemon): restore adapter state after dropped-write respawn * fix(daemon): recover writes after endpoint respawn * fix(terminal): remount panes after daemon death * fix(daemon): recover sibling panes after daemon death, not just the written one When a daemon dies, its dropped-write respawn only remounted the pane whose write detected the dead endpoint. Sibling panes (alive at death but not typed into) were left frozen: stale prompt pixels, silently-dropped input, no live child, and no recovery even on later keystrokes — the exact STA-2373 frozen-typing symptom on non-triggering panes. DaemonPtyAdapter now fans a write-unavailable signal out to every active session when it recovers from a dead endpoint, emitted while the sessions are still in activeSessionIds so the renderer's liveness gate still reads them live. pty.ts forwards each to the existing pty:writeUnavailable channel, so all panes remount + re-attach through the same path the written pane already used. Adds a revert-sensitive regression test: with two sessions and only one written after the daemon dies, the sibling must also be signaled to recover. * revert(format): drop repo-wide oxfmt churn unrelated to STA-2373 A review pass ran `oxfmt --write .` across the tree, pulling seven files with no bearing on the dead-daemon respawn fix into the PR diff. Restored to origin/main byte-for-byte so the diff carries only the respawn change. * fix(daemon): snapshot active sessions before the write-unavailable fan-out A listener that kills a pane mutates activeSessionIds mid-iteration, which can skip the very sibling the fan-out exists to reach. Matches the snapshot fanoutSyntheticExits already takes. * fix(daemon): re-arm dead-endpoint recovery on every daemon death The respawn-storm latch was only released once every awaiting session rebound. Background sessions have no mounted pane, so nothing ever calls createOrAttach for them and they hold the awaiting set non-empty forever — latching the fan-out off after the first death and silently making the whole fix one-shot. Re-arm on the disconnect event instead, which fires once per established connection, so the storm guard still holds within a single incident. * fix(daemon): route the write-unavailable fan-out through the pty router Main subscribes on the routed provider, and DaemonPtyRouter is the live localProvider whenever a legacy daemon socket exists — the common case when an in-place update bumps PROTOCOL_VERSION with terminals running. It forwarded write but not onWriteUnavailable, so the fan-out reached no listener and only the written pane recovered: STA-2373 unfixed, silently. Also stop rejecting writes on adapters that cannot respawn. Legacy adapters have no respawn, so the remount reattaches to nothing and rebuilds the pane empty, losing scrollback the user could still read — worse than the pre-existing silent drop. And guard the renderer's write-unavailable handler on ptyId like its sibling data/replay handlers, so a transport that rebinds without detaching cannot remount a healthy pane. * fix(daemon): route the write-unavailable fan-out through the degraded provider DegradedDaemonPtyProvider is the live localProvider in degraded launch mode and main subscribes on it, but it forwarded onData/onExit/onReplay/ onBackgroundStreamEvent and not onWriteUnavailable — so the fan-out reached no listener and siblings stayed frozen. Same defect as the router, one provider over. The file sat at its max-lines ceiling, so make room by reusing one combineUnsubscribes helper across the three places that already repeated that loop rather than bumping the limit. Forward to the daemon adapters only: the local fallback has no dead-socket problem. * refactor(daemon): share the listener-fanout unsubscribe combination Adding onWriteUnavailable to both provider wrappers left each file at exactly 300/300 lines, so the next line anyone added would have broken max-lines with no sanctioned escape hatch. Both already repeated the same combine-unsubscribes loop, so lift it into one module: duplication drops and each file gets its headroom back. * fix(test): stop the fake emitter colliding with the private adapter emitter DaemonPtyAdapter.emitWriteUnavailable is private, so declaring a public member of the same name on a mock intersected with DaemonPtyAdapter collapsed the whole type to never — one collision produced 54 typecheck errors, taking out pre-existing assertions in both files too. Rename the fake to triggerWriteUnavailable and declare onWriteUnavailable on ProviderMock, which IPtyProvider does not carry on this branch. vitest does not typecheck, which is why a red build sat behind a green suite. --- src/main/daemon/client.test.ts | 9 +- src/main/daemon/client.ts | 6 +- src/main/daemon/combine-unsubscribes.ts | 9 + src/main/daemon/daemon-pty-adapter.test.ts | 258 ++++++++++++++++++ src/main/daemon/daemon-pty-adapter.ts | 114 +++++++- src/main/daemon/daemon-pty-router.test.ts | 38 +++ src/main/daemon/daemon-pty-router.ts | 18 +- .../degraded-daemon-pty-provider.test.ts | 37 +++ .../daemon/degraded-daemon-pty-provider.ts | 27 +- src/main/ipc/pty.test.ts | 31 ++- src/main/ipc/pty.ts | 48 +++- .../providers/pty-write-unavailable-error.ts | 10 + src/preload/api-types.ts | 1 + src/preload/index.ts | 6 + .../terminal-pane/pty-connection.test.ts | 25 ++ .../terminal-pane/pty-connection.ts | 9 +- .../terminal-pane/pty-dispatcher.ts | 7 + .../terminal-pane/pty-transport-types.ts | 1 + .../terminal-pane/pty-transport.test.ts | 18 ++ .../components/terminal-pane/pty-transport.ts | 25 +- 20 files changed, 661 insertions(+), 36 deletions(-) create mode 100644 src/main/daemon/combine-unsubscribes.ts create mode 100644 src/main/providers/pty-write-unavailable-error.ts diff --git a/src/main/daemon/client.test.ts b/src/main/daemon/client.test.ts index a3d6d9f61..60a22232d 100644 --- a/src/main/daemon/client.test.ts +++ b/src/main/daemon/client.test.ts @@ -548,12 +548,19 @@ describe('DaemonClient', () => { client = new DaemonClient({ socketPath, tokenPath }) await client.ensureConnected() - client.notify('write', { sessionId: 'session-1', data: 'hello' }) + const delivered = client.notify('write', { sessionId: 'session-1', data: 'hello' }) + expect(delivered).toBe(true) await waitFor(() => received.length > 0) const msg = received[0] as { id: string; type: string } expect(msg.id).toMatch(/^notify_/) expect(msg.type).toBe('write') }) + + it('reports a dropped delivery when not connected', () => { + // Why: STA-2373 relies on this false to detect a write silently swallowed by a dead socket. + client = new DaemonClient({ socketPath, tokenPath }) + expect(client.notify('write', { sessionId: 'session-1', data: 'hello' })).toBe(false) + }) }) }) diff --git a/src/main/daemon/client.ts b/src/main/daemon/client.ts index d0dd24534..277b7278f 100644 --- a/src/main/daemon/client.ts +++ b/src/main/daemon/client.ts @@ -216,14 +216,16 @@ export class DaemonClient { }) } - notify(type: string, payload: unknown): void { + // Why: fire-and-forget writes need a local delivery signal to trigger dead-endpoint recovery. + notify(type: string, payload: unknown): boolean { if (!this.connected || !this.controlSocket) { - return + return false } const id = `${NOTIFY_PREFIX}${++this.requestCounter}` const msg = { id, type, ...(payload !== undefined ? { payload } : {}) } this.controlSocket.write(encodeNdjson(msg)) + return true } onEvent(listener: (event: unknown) => void): () => void { diff --git a/src/main/daemon/combine-unsubscribes.ts b/src/main/daemon/combine-unsubscribes.ts new file mode 100644 index 000000000..d30e4d559 --- /dev/null +++ b/src/main/daemon/combine-unsubscribes.ts @@ -0,0 +1,9 @@ +// Both daemon provider wrappers fan a listener out to every routed adapter and hand +// back one unsubscribe; this is that combination step, shared so neither file repeats it. +export function combineUnsubscribes(unsubscribes: (() => void)[]): () => void { + return () => { + for (const unsubscribe of unsubscribes) { + unsubscribe() + } + } +} diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 37614abe0..cec5bd880 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -19,6 +19,7 @@ import type { SubprocessHandle } from './session' import type { DaemonFileLog } from './daemon-file-log' import type * as DaemonHealthModule from './daemon-health' import { getDaemonSocketPath } from './daemon-spawner' +import { PtyWriteUnavailableError } from '../providers/pty-write-unavailable-error' const { getMacDaemonSystemResolverHealthMock } = vi.hoisted(() => ({ getMacDaemonSystemResolverHealthMock: vi.fn(async () => 'unknown') @@ -39,6 +40,7 @@ function createTestDir(): string { } function createMockSubprocess(dataOnSubscribe?: string): SubprocessHandle & { + write: ReturnType void>> pause: ReturnType void>> resume: ReturnType void>> _simulateData: (data: string) => void @@ -507,6 +509,262 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { }) }) + describe('dead-endpoint write respawn (STA-2373)', () => { + function restartServerOnRespawn(): void { + server = new DaemonServer({ + socketPath, + tokenPath, + log: daemonLog, + spawnSubprocess: (opts) => { + lastSpawnOpts = opts + lastSubprocess = createMockSubprocess() + return lastSubprocess + } + }) + } + + it('rejects stale input until createOrAttach remounts the pane onto the new daemon', async () => { + let respawnServer: DaemonServer | undefined + let respawnSubprocess: ReturnType | undefined + const respawn = vi.fn(async () => { + respawnServer = new DaemonServer({ + socketPath, + tokenPath, + spawnSubprocess: () => { + respawnSubprocess = createMockSubprocess() + return respawnSubprocess + } + }) + await respawnServer.start() + }) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + try { + const { id } = await healingAdapter.spawn({ cols: 80, rows: 24 }) + const internals = healingAdapter as unknown as { + sessionsAwaitingDaemonRecovery: Set + } + + await server.shutdown() + await waitFor(() => internals.sessionsAwaitingDaemonRecovery.has(id)) + + expect(() => healingAdapter.write(id, 'first')).toThrow(PtyWriteUnavailableError) + expect(() => healingAdapter.write(id, 'second')).toThrow(PtyWriteUnavailableError) + await waitFor(() => respawn.mock.calls.length === 1) + + expect(respawnSubprocess).toBeUndefined() + expect(() => healingAdapter.write(id, 'still-stale')).toThrow(PtyWriteUnavailableError) + + await healingAdapter.spawn({ sessionId: id, cols: 80, rows: 24 }) + expect(() => healingAdapter.write(id, 'rebound')).not.toThrow() + await waitFor( + () => + respawnSubprocess !== undefined && + vi.mocked(respawnSubprocess.write).mock.calls.length === 1 + ) + expect(respawnSubprocess?.write).toHaveBeenCalledWith('rebound') + expect(respawn).toHaveBeenCalledTimes(1) + } finally { + healingAdapter.dispose() + await respawnServer?.shutdown() + } + }) + + it('requires createOrAttach before writing to a session that survives a socket disconnect', async () => { + const respawn = vi.fn(async () => {}) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + try { + const { id } = await healingAdapter.spawn({ cols: 80, rows: 24 }) + const client = (healingAdapter as unknown as { client: DaemonClient }).client + + client.disconnect() + expect(() => healingAdapter.write(id, 'stale')).toThrow(PtyWriteUnavailableError) + await waitFor(() => client.isConnected()) + expect(() => healingAdapter.write(id, 'still-stale')).toThrow(PtyWriteUnavailableError) + + await healingAdapter.spawn({ sessionId: id, cols: 80, rows: 24 }) + healingAdapter.write(id, 'rebound') + await waitFor(() => lastSubprocess.write.mock.calls.length > 0) + expect(lastSubprocess.write.mock.calls).toEqual([['rebound']]) + expect(healingAdapter.hasPty(id)).toBe(true) + expect(respawn).not.toHaveBeenCalled() + } finally { + healingAdapter.dispose() + } + }) + + it('does not spawn a daemon per keystroke after respawn fails', async () => { + const respawn = vi.fn(async () => { + throw new Error('daemon unavailable') + }) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const { id } = await healingAdapter.spawn({ cols: 80, rows: 24 }) + const client = (healingAdapter as unknown as { client: DaemonClient }).client + await server.shutdown() + await waitFor(() => !client.isConnected()) + + expect(() => healingAdapter.write(id, 'a')).toThrow(PtyWriteUnavailableError) + await waitFor(() => respawn.mock.calls.length === 1) + for (let i = 0; i < 100; i += 1) { + expect(() => healingAdapter.write(id, 'b')).toThrow(PtyWriteUnavailableError) + } + + expect(respawn).toHaveBeenCalledTimes(1) + } finally { + warn.mockRestore() + healingAdapter.dispose() + } + }) + + it('joins a request-path respawn instead of forking a second daemon', async () => { + let releaseRespawn!: () => void + const respawn = vi.fn(async () => { + await new Promise((resolve) => { + releaseRespawn = resolve + }) + restartServerOnRespawn() + await server.start() + }) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + try { + const { id } = await healingAdapter.spawn({ cols: 80, rows: 24 }) + const client = (healingAdapter as unknown as { client: DaemonClient }).client + await server.shutdown() + await waitFor(() => !client.isConnected()) + + const newSpawn = healingAdapter.spawn({ + sessionId: 'request-path-session', + cols: 80, + rows: 24 + }) + await waitFor(() => releaseRespawn !== undefined) + expect(() => healingAdapter.write(id, 'queued')).toThrow(PtyWriteUnavailableError) + releaseRespawn() + + await expect(newSpawn).resolves.toMatchObject({ id: 'request-path-session' }) + expect(respawn).toHaveBeenCalledTimes(1) + } finally { + healingAdapter.dispose() + } + }) + + it('does not respawn when a dropped write targets no active session', async () => { + const respawn = vi.fn(async () => { + restartServerOnRespawn() + await server.start() + }) + const idleAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + try { + const client = (idleAdapter as unknown as { client: DaemonClient }).client + await idleAdapter.listProcesses() + + await server.shutdown() + await waitFor(() => !client.isConnected()) + + idleAdapter.write('never-attached-session', 'ls\n') + + await new Promise((r) => setTimeout(r, 50)) + expect(respawn).not.toHaveBeenCalled() + } finally { + idleAdapter.dispose() + } + }) + + it('signals every active pane to recover when one pane hits the dead endpoint', async () => { + const respawn = vi.fn(async () => {}) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + const recovered: string[] = [] + healingAdapter.onWriteUnavailable(({ id }) => recovered.push(id)) + try { + const { id: a } = await healingAdapter.spawn({ sessionId: 'pane-a', cols: 80, rows: 24 }) + const { id: b } = await healingAdapter.spawn({ sessionId: 'pane-b', cols: 80, rows: 24 }) + const client = (healingAdapter as unknown as { client: DaemonClient }).client + + await server.shutdown() + await waitFor(() => !client.isConnected()) + + // Only pane A is written; pane B is a passive sibling the user never typed into. + expect(() => healingAdapter.write(a, 'typed-into-a')).toThrow(PtyWriteUnavailableError) + + // Why revert-sensitive: a dead endpoint takes down EVERY session on the + // daemon, so both panes must be told to remount + re-attach. Without the + // fan-out, only the written pane (a) recovers and sibling b stays frozen + // with silently dropped input (STA-2373 sibling-freeze regression). + expect(recovered).toContain(a) + expect(recovered).toContain(b) + } finally { + healingAdapter.dispose() + } + }) + + it('keeps dropping writes silently on an adapter that cannot respawn', async () => { + // Why revert-sensitive: legacy adapters are built with no respawn, so a remount + // reattaches to nothing and rebuilds the pane EMPTY, losing scrollback the user + // could still read. Rejecting the write is only an improvement where the endpoint + // can actually come back, so an unrecoverable one must keep the old silent drop. + const legacyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath }) + const recovered: string[] = [] + legacyAdapter.onWriteUnavailable(({ id }) => recovered.push(id)) + try { + const { id } = await legacyAdapter.spawn({ sessionId: 'legacy-pane', cols: 80, rows: 24 }) + const client = (legacyAdapter as unknown as { client: DaemonClient }).client + + await server.shutdown() + await waitFor(() => !client.isConnected()) + + expect(() => legacyAdapter.write(id, 'typed')).not.toThrow() + expect(recovered).toEqual([]) + } finally { + legacyAdapter.dispose() + } + }) + + it('re-arms recovery for a second daemon death when a background session never rebinds', async () => { + const respawn = vi.fn(async () => { + restartServerOnRespawn() + await server.start() + }) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + const recovered: string[] = [] + healingAdapter.onWriteUnavailable(({ id }) => recovered.push(id)) + try { + await healingAdapter.spawn({ sessionId: 'pane-a', cols: 80, rows: 24 }) + // A backgrounded session: no pane is mounted for it, so nothing in the + // renderer ever calls createOrAttach to rebind it after a daemon death. + await healingAdapter.spawn({ sessionId: 'background-b', cols: 80, rows: 24 }) + const client = (healingAdapter as unknown as { client: DaemonClient }).client + + await server.shutdown() + await waitFor(() => !client.isConnected()) + expect(() => healingAdapter.write('pane-a', 'first-death')).toThrow( + PtyWriteUnavailableError + ) + await waitFor(() => respawn.mock.calls.length === 1) + await waitFor(() => client.isConnected()) + + // Only the mounted pane rebinds; background-b keeps the awaiting set non-empty. + await healingAdapter.spawn({ sessionId: 'pane-a', cols: 80, rows: 24 }) + recovered.length = 0 + + await server.shutdown() + await waitFor(() => !client.isConnected()) + + // Why revert-sensitive: the storm latch is otherwise only released when the + // awaiting set empties, which a never-rebinding background session prevents + // forever. That silently downgrades the fix to one-shot — every daemon death + // after the first would respawn nothing and leave siblings frozen again. + expect(() => healingAdapter.write('pane-a', 'second-death')).toThrow( + PtyWriteUnavailableError + ) + expect(recovered).toContain('pane-a') + await waitFor(() => respawn.mock.calls.length === 2) + } finally { + healingAdapter.dispose() + } + }) + }) + describe('background stream thinning compatibility', () => { it('reports authoritative snapshot support only for protocol v20 and newer', () => { const legacy = new DaemonPtyAdapter({ socketPath, tokenPath, protocolVersion: 19 }) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 10efd2ec4..02c2c9aa7 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -47,6 +47,7 @@ import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import type { PtyIncarnationId } from '../../shared/pty-incarnation' import { resolveSafePtyDefaultCwd } from '../providers/pty-default-cwd' +import { PtyWriteUnavailableError } from '../providers/pty-write-unavailable-error' import { ColdRestorePayloadCache, type ColdRestorePayload } from './cold-restore-payload-cache' import { PtyProcessListAdmission } from '../providers/pty-process-list-admission' @@ -132,6 +133,8 @@ export class DaemonPtyAdapter implements IPtyProvider { private respawnAdoptionClosed = false // Why: concurrent spawn() calls hitting a dead daemon would each fork their own; this promise coalesces respawns so only the first forks and the rest await it. private respawnPromise: Promise | null = null + private writeRecoveryPromise: Promise | null = null + private writeRecoveryAttempted = false private dataListeners: ((payload: { id: string data: string @@ -145,6 +148,8 @@ export class DaemonPtyAdapter implements IPtyProvider { incarnationId?: PtyIncarnationId }) => void)[] = [] private backgroundStreamListeners: ((payload: PtyBackgroundStreamEvent) => void)[] = [] + // Why: lets main fan a dead-endpoint signal to every affected pane, not just the written one (STA-2373 sibling-freeze). + private writeUnavailableListeners: ((payload: { id: string }) => void)[] = [] private removeEventListener: (() => void) | null = null private initialCwds = new Map() private wslDistrosBySessionId = new Map() @@ -156,6 +161,8 @@ export class DaemonPtyAdapter implements IPtyProvider { this.sleepRestoreSessionIds.delete(sessionId) }) private activeSessionIds = new Set() + // A replacement daemon has none of the old PTYs; only createOrAttach can make their bindings writable again. + private sessionsAwaitingDaemonRecovery = new Set() private sessionIncarnations = new Map() private pendingSpawnOperationsBySessionId = new Map>() private pendingClaimSpawnOperations = new Set() @@ -211,6 +218,16 @@ export class DaemonPtyAdapter implements IPtyProvider { this.supportsAuthoritativeBufferSnapshots = this.protocolVersion >= 20 this.supportsStartupIngress = supportsPtyStartupIngress(this.protocolVersion) this.client.onDisconnected(() => { + if (!this.respawnAdoptionClosed) { + // Why re-arm here: the latch is otherwise only cleared when every awaiting + // session rebinds, and background sessions (no mounted pane, so nothing ever + // calls createOrAttach for them) never do — which would leave the fan-out + // permanently latched off after the first death. Fires once per connection. + this.writeRecoveryAttempted = false + for (const id of this.activeSessionIds) { + this.sessionsAwaitingDaemonRecovery.add(id) + } + } for (const id of this.pausedProducerSessionIds) { this.producerResumesOwedOnReconnect.add(id) } @@ -280,7 +297,8 @@ export class DaemonPtyAdapter implements IPtyProvider { ) { throw new Error('agent_session_claim_unavailable') } - let sessionId = opts.sessionId! + const requestedSessionId = opts.sessionId! + let sessionId = requestedSessionId let wslDistro = resolveWslSessionContext({ cwd: opts.cwd, sessionId, @@ -436,6 +454,9 @@ export class DaemonPtyAdapter implements IPtyProvider { } let result = await createOrAttach(scrollback) await adoptSpawnResultSession(result) + // Both ids: adoptSpawnResultSession may have rewritten sessionId to the claim owner. + this.clearSessionAwaitingDaemonRecovery(requestedSessionId) + this.clearSessionAwaitingDaemonRecovery(sessionId) const exitedResult = this.resultForExitBeforeSpawnReply(sessionId, result, operation) if (exitedResult) { return exitedResult @@ -705,6 +726,7 @@ export class DaemonPtyAdapter implements IPtyProvider { cols: 80, rows: 24 }) + this.clearSessionAwaitingDaemonRecovery(id) } hasPty(id: string): boolean { @@ -713,7 +735,26 @@ export class DaemonPtyAdapter implements IPtyProvider { write(id: string, data: string): void { this.markSessionDirty(id) - this.client.notify('write', { sessionId: id, data }) + // Why recoverable and not just active: rejecting a write asks the pane to remount, + // which only helps if this endpoint can come back. A legacy adapter has no respawn, + // so its reattach fails and the pane rebuilds empty — losing scrollback the user + // could still read. Keep the pre-existing silent drop for those. + const recoverable = + this.activeSessionIds.has(id) && !this.respawnAdoptionClosed && Boolean(this.respawnFn) + if ( + recoverable && + (this.sessionsAwaitingDaemonRecovery.has(id) || !this.client.isConnected()) + ) { + this.sessionsAwaitingDaemonRecovery.add(id) + this.reconnectAfterWriteFailure() + throw new PtyWriteUnavailableError(`Daemon PTY "${id}" is awaiting recovery`) + } + const delivered = this.client.notify('write', { sessionId: id, data }) + if (!delivered && recoverable) { + this.sessionsAwaitingDaemonRecovery.add(id) + this.reconnectAfterWriteFailure() + throw new PtyWriteUnavailableError(`Daemon PTY "${id}" is awaiting recovery`) + } } resize(id: string, cols: number, rows: number): void { @@ -823,6 +864,7 @@ export class DaemonPtyAdapter implements IPtyProvider { remainingRequestTimeoutMs(opts.deadlineMs) ) this.activeSessionIds.delete(id) + this.clearSessionAwaitingDaemonRecovery(id) this.dirtySessionVersions.delete(id) if (!opts.keepHistory) { this.coldRestoreCache.delete(id) @@ -1197,6 +1239,8 @@ export class DaemonPtyAdapter implements IPtyProvider { fanoutSyntheticExits(code: number): void { const ids = [...this.activeSessionIds] this.activeSessionIds.clear() + this.sessionsAwaitingDaemonRecovery.clear() + this.writeRecoveryAttempted = false this.dirtySessionVersions.clear() this.lastFullCheckpointAt.clear() this.sessionsNeedingFullCheckpoint.clear() @@ -1282,8 +1326,27 @@ export class DaemonPtyAdapter implements IPtyProvider { } } + onWriteUnavailable(callback: (payload: { id: string }) => void): () => void { + this.writeUnavailableListeners.push(callback) + return () => { + const idx = this.writeUnavailableListeners.indexOf(callback) + if (idx !== -1) { + this.writeUnavailableListeners.splice(idx, 1) + } + } + } + + private emitWriteUnavailable(id: string): void { + // oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may unsubscribe during iteration + for (const listener of [...this.writeUnavailableListeners]) { + listener({ id }) + } + } + dispose(): void { this.respawnAdoptionClosed = true + this.sessionsAwaitingDaemonRecovery.clear() + this.writeRecoveryAttempted = false this.releasePendingRespawnAdoptionLease() this.stopCheckpointTimer() this.dirtySessionVersions.clear() @@ -1317,6 +1380,8 @@ export class DaemonPtyAdapter implements IPtyProvider { async disconnectOnly(): Promise { if (!this.disconnectOnlyPromise) { this.respawnAdoptionClosed = true + this.sessionsAwaitingDaemonRecovery.clear() + this.writeRecoveryAttempted = false this.releasePendingRespawnAdoptionLease() this.disconnectOnlyPromise = this.finishDisconnectOnly([...this.keepHistoryShutdowns]) } @@ -1666,6 +1731,50 @@ export class DaemonPtyAdapter implements IPtyProvider { } } + private reconnectAfterWriteFailure(): void { + if ( + this.writeRecoveryPromise || + this.writeRecoveryAttempted || + this.respawnAdoptionClosed || + !this.respawnFn + ) { + return + } + this.writeRecoveryAttempted = true + // Why: the dead endpoint took down every session on this daemon. Signal all + // active panes now — while they are still in activeSessionIds, so the + // renderer's liveness gate still reads them live — so background panes + // remount + re-attach alongside the one that was written, instead of being + // left frozen with silently dropped input until each is typed into. + this.notifyActiveSessionsWriteUnavailable() + const recovery = this.withDaemonRetry(() => this.ensureConnected()) + .catch((error) => console.warn('[daemon] Failed to recover after rejected PTY input:', error)) + .finally(() => { + this.releasePendingRespawnAdoptionLease() + if (this.writeRecoveryPromise === recovery) { + this.writeRecoveryPromise = null + } + }) + this.writeRecoveryPromise = recovery + } + + private notifyActiveSessionsWriteUnavailable(): void { + // Snapshot first: a listener that kills a pane would mutate activeSessionIds + // mid-iteration and silently skip the sibling this fan-out exists to reach. + const ids = [...this.activeSessionIds] + for (const id of ids) { + this.sessionsAwaitingDaemonRecovery.add(id) + this.emitWriteUnavailable(id) + } + } + + private clearSessionAwaitingDaemonRecovery(sessionId: string): void { + this.sessionsAwaitingDaemonRecovery.delete(sessionId) + if (this.sessionsAwaitingDaemonRecovery.size === 0) { + this.writeRecoveryAttempted = false + } + } + private async withHistorySpawnLock( sessionId: string, operation: () => Promise @@ -1846,6 +1955,7 @@ export class DaemonPtyAdapter implements IPtyProvider { return } this.activeSessionIds.delete(event.sessionId) + this.clearSessionAwaitingDaemonRecovery(event.sessionId) this.dirtySessionVersions.delete(event.sessionId) // Why: a reused sessionId must not inherit the dead session's owed resume (stray resumePty) or backgrounded/thinned state. this.pausedProducerSessionIds.delete(event.sessionId) diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index 0861df788..107928001 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -12,6 +12,7 @@ type AdapterMock = DaemonPtyAdapter & { emitData: (id: string, data: string, sequenceChars?: number) => void emitBackground: (event: PtyBackgroundStreamEvent) => void emitExit: (id: string, code: number, incarnationId?: string) => void + triggerWriteUnavailable: (id: string) => void } const LARGE_RECONCILE_SESSION_COUNT = 150_000 @@ -34,6 +35,7 @@ function createAdapter( const dataListeners: ((payload: { id: string; data: string; sequenceChars?: number }) => void)[] = [] const backgroundListeners: ((payload: PtyBackgroundStreamEvent) => void)[] = [] + const writeUnavailableListeners: ((payload: { id: string }) => void)[] = [] const exitListeners: ((payload: { id: string; code: number; incarnationId?: string }) => void)[] = [] return { @@ -106,6 +108,15 @@ function createAdapter( } } }), + onWriteUnavailable: vi.fn((callback: (payload: { id: string }) => void) => { + writeUnavailableListeners.push(callback) + return () => { + const idx = writeUnavailableListeners.indexOf(callback) + if (idx !== -1) { + writeUnavailableListeners.splice(idx, 1) + } + } + }), onExit: vi.fn( (callback: (payload: { id: string; code: number; incarnationId?: string }) => void) => { exitListeners.push(callback) @@ -137,10 +148,37 @@ function createAdapter( listener({ id, code, ...(incarnationId ? { incarnationId } : {}) }) } }, + triggerWriteUnavailable: (id: string) => { + for (const listener of writeUnavailableListeners) { + listener({ id }) + } + }, _writes: writes } as unknown as AdapterMock } +it('forwards dead-endpoint write-unavailable signals from every routed adapter', () => { + // Why revert-sensitive: main subscribes on the ROUTED provider, so if the router + // does not forward this the STA-2373 fan-out never reaches the renderer and only + // the written pane recovers — siblings stay frozen. The router is the live + // localProvider whenever a legacy daemon socket exists (protocol bump mid-session). + const current = createAdapter('current') + const legacy = createAdapter('legacy') + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + const recovered: string[] = [] + + const unsubscribe = router.onWriteUnavailable(({ id }) => recovered.push(id)) + current.triggerWriteUnavailable('current-pane') + legacy.triggerWriteUnavailable('legacy-pane') + + expect(recovered).toEqual(['current-pane', 'legacy-pane']) + + unsubscribe() + current.triggerWriteUnavailable('after-unsubscribe') + legacy.triggerWriteUnavailable('after-unsubscribe') + expect(recovered).toEqual(['current-pane', 'legacy-pane']) +}) + it('rejects completion inspection when no daemon owns the session', async () => { const router = new DaemonPtyRouter({ current: createAdapter('current'), diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 8f2a8b410..c81da8341 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -1,4 +1,5 @@ import type { DaemonPtyAdapter } from './daemon-pty-adapter' +import { combineUnsubscribes } from './combine-unsubscribes' import type { IPtyProvider, PtyBackgroundStreamEvent, @@ -242,14 +243,17 @@ export class DaemonPtyRouter implements IPtyProvider { } onBackgroundStreamEvent(callback: (payload: PtyBackgroundStreamEvent) => void): () => void { - const unsubscribes = this.allAdapters().map((adapter) => - adapter.onBackgroundStreamEvent(callback) + return combineUnsubscribes( + this.allAdapters().map((adapter) => adapter.onBackgroundStreamEvent(callback)) + ) + } + + // Why: main subscribes on the routed provider, so without this the dead-endpoint + // fan-out never reaches the renderer and only the written pane recovers (STA-2373). + onWriteUnavailable(callback: (payload: { id: string }) => void): () => void { + return combineUnsubscribes( + this.allAdapters().map((adapter) => adapter.onWriteUnavailable(callback)) ) - return () => { - for (const unsubscribe of unsubscribes) { - unsubscribe() - } - } } onReplay(_callback: (payload: { id: string; data: string }) => void): () => void { diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index cccd6e0cb..e597b852b 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -9,6 +9,8 @@ type ProviderMock = IPtyProvider & { emitData: (id: string, data: string, sequenceChars?: number) => void emitReplay: (id: string, data: string) => void emitExit: (id: string, code: number) => void + triggerWriteUnavailable: (id: string) => void + onWriteUnavailable: (callback: (payload: { id: string }) => void) => () => void } function createProvider( @@ -20,6 +22,7 @@ function createProvider( [] const replayListeners: ((payload: { id: string; data: string }) => void)[] = [] const exitListeners: ((payload: { id: string; code: number }) => void)[] = [] + const writeUnavailableListeners: ((payload: { id: string }) => void)[] = [] return { spawn: vi.fn(async (opts: PtySpawnOptions): Promise => { const id = opts.sessionId ?? `${label}-new` @@ -94,6 +97,20 @@ function createProvider( for (const listener of exitListeners) { listener({ id, code }) } + }, + onWriteUnavailable: vi.fn((callback: (payload: { id: string }) => void) => { + writeUnavailableListeners.push(callback) + return () => { + const idx = writeUnavailableListeners.indexOf(callback) + if (idx !== -1) { + writeUnavailableListeners.splice(idx, 1) + } + } + }), + triggerWriteUnavailable: (id: string) => { + for (const listener of writeUnavailableListeners) { + listener({ id }) + } } } } @@ -116,6 +133,26 @@ function createDaemonAdapter( } as unknown as DaemonPtyAdapter & ProviderMock } +it('forwards dead-endpoint write-unavailable signals from the daemon adapters', () => { + // Why revert-sensitive: this provider is the live localProvider in degraded launch + // mode and main subscribes on it, so without forwarding the STA-2373 fan-out reaches + // no listener and sibling panes stay frozen. + const current = createDaemonAdapter('daemon') + const legacy = createDaemonAdapter('legacy') + const fallback = createProvider('fallback') + const provider = new DegradedDaemonPtyProvider({ current, legacy: [legacy], fallback }) + const recovered: string[] = [] + + const unsubscribe = provider.onWriteUnavailable(({ id }) => recovered.push(id)) + current.triggerWriteUnavailable('daemon-pane') + legacy.triggerWriteUnavailable('legacy-pane') + expect(recovered).toEqual(['daemon-pane', 'legacy-pane']) + + unsubscribe() + current.triggerWriteUnavailable('after-unsubscribe') + expect(recovered).toEqual(['daemon-pane', 'legacy-pane']) +}) + it('rejects completion inspection instead of borrowing the fallback provider', async () => { const provider = new DegradedDaemonPtyProvider({ current: createDaemonAdapter('daemon'), diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index e3bd38af6..8ca0f39bd 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -1,4 +1,5 @@ import type { DaemonPtyAdapter } from './daemon-pty-adapter' +import { combineUnsubscribes } from './combine-unsubscribes' import { shutdownDegradedFallbackSessions } from './degraded-daemon-fallback-shutdown' import { inspectPtyProviderProcess } from '../providers/pty-process-inspection' import type { IPtyProvider, PtyBackgroundStreamEvent } from '../providers/types' @@ -196,14 +197,18 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { } onBackgroundStreamEvent(callback: (payload: PtyBackgroundStreamEvent) => void): () => void { - const unsubscribes = this.allProviders().flatMap( - (provider) => provider.onBackgroundStreamEvent?.(callback) ?? [] + return combineUnsubscribes( + this.allProviders().flatMap((provider) => provider.onBackgroundStreamEvent?.(callback) ?? []) + ) + } + + // Why: main subscribes on the routed provider, so without this the dead-endpoint + // fan-out reaches no listener and only the written pane recovers (STA-2373). Daemon + // adapters only — the local fallback has no dead-socket problem. + onWriteUnavailable(callback: (payload: { id: string }) => void): () => void { + return combineUnsubscribes( + this.allDaemonAdapters().map((adapter) => adapter.onWriteUnavailable(callback)) ) - return () => { - for (const unsubscribe of unsubscribes) { - unsubscribe() - } - } } onReplay(callback: (payload: { id: string; data: string }) => void): () => void { @@ -218,9 +223,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { if (idx !== -1) { this.unsubscribers.splice(idx, 1) } - for (const unsubscribe of unsubscribes) { - unsubscribe() - } + combineUnsubscribes(unsubscribes)() } this.unsubscribers.push(trackedUnsubscribe) return trackedUnsubscribe @@ -272,9 +275,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { } disposeProviderOnly(): void { - for (const unsubscribe of this.unsubscribers.splice(0)) { - unsubscribe() - } + combineUnsubscribes(this.unsubscribers.splice(0))() } async shutdownFallbackSessions(): Promise { diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 79f2cf765..f10355161 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -11,6 +11,7 @@ import { redactPtyIdForDiagnostics } from '../../shared/pty-delivery-diagnostics import { FLOATING_TERMINAL_WORKTREE_ID } from '../../shared/constants' import type { TuiAgent } from '../../shared/types' import type { AgentSessionOwnerBinding } from '../../shared/agent-session-host-authority' +import { PtyWriteUnavailableError } from '../providers/pty-write-unavailable-error' const isWindowsHost = process.platform === 'win32' const posixOnlyIt = isWindowsHost ? it.skip : it @@ -542,11 +543,11 @@ describe('registerPtyHandlers', () => { return writeCall[1] as (event: unknown, args: { id: string; data: string }) => void } - function installDaemonTestProvider() { + function installDaemonTestProvider(overrides: Record = {}) { const spawn = vi.fn(async (options: { sessionId?: string }) => ({ id: options.sessionId ?? 'daemon-pty' })) - setLocalPtyProvider({ + const provider = { spawn, write: vi.fn(), resize: vi.fn(), @@ -568,8 +569,10 @@ describe('registerPtyHandlers', () => { listProcesses: vi.fn(async () => []), attach: vi.fn(), getDefaultShell: vi.fn(), - getProfiles: vi.fn() - } as never) + getProfiles: vi.fn(), + ...overrides + } + setLocalPtyProvider(provider as never) return spawn } @@ -13353,6 +13356,26 @@ describe('registerPtyHandlers', () => { expect(mockProc.proc.write).toHaveBeenCalledTimes(1) }) + it('asks the renderer to remount when the provider rejects a stale daemon write', async () => { + const write = vi.fn(() => { + throw new PtyWriteUnavailableError('daemon generation lost') + }) + installDaemonTestProvider({ write }) + registerPtyHandlers(mainWindow as never) + const result = (await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24 + })) as { id: string } + mainWindow.webContents.send.mockClear() + + getPtyWriteListener()(mainWindowIpcEvent, { id: result.id, data: 'x' }) + + expect(write).toHaveBeenCalledWith(result.id, 'x') + expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:writeUnavailable', { + id: result.id + }) + }) + it('rejects malformed and cross-window pty write IPC before provider writes', async () => { const mockProc = createMockProc() spawnMock.mockReturnValue(mockProc.proc) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index a1dd1ab84..ff0894325 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -58,6 +58,7 @@ import { detectPiAgentKindFromCommand, type PiAgentKind } from '../../shared/pi- import { isPwshAvailable } from '../pwsh' import { LocalPtyProvider } from '../providers/local-pty-provider' import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers/types' +import { isPtyWriteUnavailableError } from '../providers/pty-write-unavailable-error' import { inspectPtyProviderProcess } from '../providers/pty-process-inspection' import { PtyProcessListAdmission, @@ -1375,6 +1376,7 @@ export function restorePtyIncarnation(id: string, incarnationId: string): void { let localDataUnsub: (() => void) | null = null let localExitUnsub: (() => void) | null = null let localBackgroundStreamUnsub: (() => void) | null = null +let localWriteUnavailableUnsub: (() => void) | null = null let didFinishLoadHandler: (() => void) | null = null let didFinishLoadWebContents: WebContents | null = null let rendererLifecycleResetWebContents: WebContents | null = null @@ -1579,9 +1581,11 @@ export function unbindLocalProviderListeners(): void { localDataUnsub?.() localExitUnsub?.() localBackgroundStreamUnsub?.() + localWriteUnavailableUnsub?.() localDataUnsub = null localExitUnsub = null localBackgroundStreamUnsub = null + localWriteUnavailableUnsub = null } // ─── IPC Registration ─────────────────────────────────────────────── @@ -2754,6 +2758,27 @@ export function registerPtyHandlers( localDataUnsub?.() localExitUnsub?.() localBackgroundStreamUnsub?.() + localWriteUnavailableUnsub?.() + + // Why: a daemon death takes down every session at once. The provider signals + // each affected pane here so background panes remount + re-attach too, not + // just the pane whose write happened to detect the dead endpoint (STA-2373). + // Typed at the call site (not on the capped IPtyProvider): only respawnable + // endpoints like the daemon adapter implement it. + const writeUnavailableSource = localProvider as { + onWriteUnavailable?: (callback: (payload: { id: string }) => void) => () => void + } + localWriteUnavailableUnsub = + writeUnavailableSource.onWriteUnavailable?.((payload) => { + if ( + mainWindow.isDestroyed() || + (typeof mainWindow.webContents.isDestroyed === 'function' && + mainWindow.webContents.isDestroyed()) + ) { + return + } + mainWindow.webContents.send('pty:writeUnavailable', { id: payload.id }) + }) ?? null // Daemon keep-tail thinning facts, in byte order with onData: markers flip transient-fact scan authority; a gap forces renderer restore from the snapshot. localBackgroundStreamUnsub = @@ -4950,6 +4975,18 @@ export function registerPtyHandlers( } ) + const reportUnavailablePtyWrite = (id: string, error: unknown): void => { + if ( + !isPtyWriteUnavailableError(error) || + mainWindow.isDestroyed() || + (typeof mainWindow.webContents.isDestroyed === 'function' && + mainWindow.webContents.isDestroyed()) + ) { + return + } + mainWindow.webContents.send('pty:writeUnavailable', { id }) + } + const writePtyProviderInputWithinLimit = ( provider: IPtyProvider, id: string, @@ -4981,8 +5018,12 @@ export function registerPtyHandlers( } return tooLarge .then((result) => (result ? false : writePtyProviderInputWithinLimit(provider, id, data))) - .catch(() => false) - } catch { + .catch((error) => { + reportUnavailablePtyWrite(id, error) + return false + }) + } catch (error) { + reportUnavailablePtyWrite(id, error) return false } } @@ -5006,7 +5047,8 @@ export function registerPtyHandlers( nextChunk = chunks.next() } return true - } catch { + } catch (error) { + reportUnavailablePtyWrite(id, error) return false } } diff --git a/src/main/providers/pty-write-unavailable-error.ts b/src/main/providers/pty-write-unavailable-error.ts new file mode 100644 index 000000000..13e471518 --- /dev/null +++ b/src/main/providers/pty-write-unavailable-error.ts @@ -0,0 +1,10 @@ +export class PtyWriteUnavailableError extends Error { + constructor(message: string) { + super(message) + this.name = 'PtyWriteUnavailableError' + } +} + +export function isPtyWriteUnavailableError(error: unknown): error is PtyWriteUnavailableError { + return error instanceof PtyWriteUnavailableError +} diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 0b72440f5..2592e08e9 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1338,6 +1338,7 @@ export type PreloadApi = { }> write: (id: string, data: string) => void writeAccepted: (id: string, data: string) => Promise + onWriteUnavailable?: (callback: (payload: { id: string }) => void) => () => void resize: (id: string, cols: number, rows: number) => void claimViewport: (id: string, cols: number, rows: number) => void reportGeometry: (id: string, cols: number, rows: number) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index dee8542ee..21014fc1a 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -844,6 +844,12 @@ const api = { }, writeAccepted: (id: string, data: string): Promise => ipcRenderer.invoke('pty:writeAccepted', { id, data }), + onWriteUnavailable: (callback: (payload: { id: string }) => void): (() => void) => { + const handler = (_event: Electron.IpcRendererEvent, payload: { id: string }): void => + callback(payload) + ipcRenderer.on('pty:writeUnavailable', handler) + return () => ipcRenderer.removeListener('pty:writeUnavailable', handler) + }, resize: (id: string, cols: number, rows: number): void => { ipcRenderer.send('pty:resize', { id, cols, rows }) 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 422c25307..ca3b9cb77 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -236,6 +236,7 @@ type ConnectCallbacks = { ) => void onReplayData?: (data: string, meta?: { clearBeforeReplay?: boolean }) => void onError?: (msg: string) => void + onWriteUnavailable?: () => void } type MockTransport = { @@ -4778,6 +4779,30 @@ describe('connectPanePty', () => { expect(window.api.agentStatus.inferInterrupt).not.toHaveBeenCalled() }) + it('remounts a connected pane when main reports its daemon write unavailable', async () => { + const { connectPanePty } = await import('./pty-connection') + const { _resetTerminalPaneRecoveryForTests } = await import('./terminal-pane-recovery') + _resetTerminalPaneRecoveryForTests() + const remountTerminalTabForRecovery = vi.fn<(tabId: string) => boolean>(() => true) + mockStoreState = { ...mockStoreState, remountTerminalTabForRecovery } as StoreState + const transport = createMockTransport('daemon-pty') + let writeUnavailable: (() => void) | undefined + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + writeUnavailable = callbacks.onWriteUnavailable + return { id: 'daemon-pty' } + }) + transportFactoryQueue.push(transport) + + connectPanePty(createPane(1) as never, createManager(1) as never, createDeps() as never) + await flushAsyncTicks(6) + writeUnavailable?.() + await flushAsyncTicks(6) + + expect(window.api.pty.hasPty).toHaveBeenCalledWith('daemon-pty') + expect(remountTerminalTabForRecovery).toHaveBeenCalledWith('tab-1') + _resetTerminalPaneRecoveryForTests() + }) + it('recovers a wedged write pipeline after accepted input without renderer output', async () => { vi.useFakeTimers() const { connectPanePty } = await import('./pty-connection') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index df1e39e6e..af4675970 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -3607,8 +3607,8 @@ export function connectPanePty( // pre-existing session when a late reattach resolves, so a remount racing // a slow-but-alive connect costs a wasted view rebuild, not a shell. const TRANSPORT_CONNECT_SETTLE_GRACE_MS = 60_000 - const requestRecoveryForUndeliverableInput = (): void => { - if (transport.isConnected?.() && transport.getPtyId() !== null) { + const requestRecoveryForUndeliverableInput = (providerRejected = false): void => { + if (!providerRejected && transport.isConnected?.() && transport.getPtyId() !== null) { return } // Why: input rejected while a connect/reattach is still settling is "not @@ -5327,6 +5327,11 @@ export function connectPanePty( onError(message) } }, + onWriteUnavailable: (): void => { + if (isCurrent()) { + requestRecoveryForUndeliverableInput(true) + } + }, onRecoveryStateChange: (state: PtyTransportRecoveryState): void => { if (isCurrent()) { // Why: cached pixels remain visible while detached; expose transport truth for diagnostics and recovery UI. diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 0a5ddcd3b..73f756c08 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -64,6 +64,7 @@ const ptyExitSidecars = new Map< string, Set<(code: number, context: { hadPrimary: boolean }) => void> >() +export const ptyWriteUnavailableHandlers = new Map void>() let ptyDispatcherAttached = false let pushListenerUnsubscribes: (() => void)[] = [] @@ -167,6 +168,12 @@ function handleDispatchedPtyData(payload: { } function attachPtySecondaryPushListeners(unsubscribes: (() => void)[]): void { + const unsubscribeWriteUnavailable = window.api.pty.onWriteUnavailable?.((payload) => { + ptyWriteUnavailableHandlers.get(payload.id)?.() + }) + if (unsubscribeWriteUnavailable) { + unsubscribes.push(unsubscribeWriteUnavailable) + } unsubscribes.push( window.api.pty.onReplay((payload) => { if (bufferPtyShutdownReplayData(payload.id, payload.data)) { 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 2b835045c..81cfd5575 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -80,6 +80,7 @@ type PtyCallbacks = { onStatus?: (shell: string) => void onError?: (message: string, errors?: string[]) => void onExit?: (code: number) => void + onWriteUnavailable?: () => void onRecoveryStateChange?: (state: PtyTransportRecoveryState) => void } diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 3df5d04c0..80bf875cc 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -22,6 +22,7 @@ describe('createIpcPtyTransport', () => { let onExit: | ((payload: { id: string; code: number; preserveRendererBinding?: boolean }) => void) | null = null + let onWriteUnavailable: ((payload: { id: string }) => void) | null = null function flushPtySideEffects(): Promise { return new Promise((resolve) => setTimeout(resolve, 0)) @@ -32,6 +33,7 @@ describe('createIpcPtyTransport', () => { onData = null onReplay = null onExit = null + onWriteUnavailable = null ;(globalThis as { window: typeof window }).window = { ...originalWindow, @@ -42,6 +44,10 @@ describe('createIpcPtyTransport', () => { spawn: vi.fn().mockResolvedValue({ id: 'pty-1' }), write: vi.fn(), writeAccepted: vi.fn().mockResolvedValue(true), + onWriteUnavailable: vi.fn((callback: (payload: { id: string }) => void) => { + onWriteUnavailable = callback + return () => {} + }), resize: vi.fn(), kill: vi.fn(), onData: vi.fn((callback: (payload: { id: string; data: string }) => void) => { @@ -89,6 +95,18 @@ describe('createIpcPtyTransport', () => { transport.disconnect() }) + it('routes a rejected daemon write to the owning transport recovery callback', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const recovery = vi.fn() + const transport = createIpcPtyTransport({}) + await transport.connect({ url: '', callbacks: { onWriteUnavailable: recovery } }) + + onWriteUnavailable?.({ id: 'pty-1' }) + + expect(recovery).toHaveBeenCalledOnce() + transport.disconnect() + }) + it('does not create a second kill authority when a mounted pane detaches', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const kill = window.api.pty.kill as unknown as ReturnType diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index d3bdd6082..2fded96d6 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -18,6 +18,7 @@ import { ptyExitHandlers, ptyTeardownHandlers, ptyShutdownLifecycleHandlers, + ptyWriteUnavailableHandlers, ensurePtyDispatcher, getEagerPtyBufferHandle, isPtyDataHandlerShutdownPending @@ -525,7 +526,11 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra // Why: a new pane can attach to the same ptyId before the old instance's detach() runs; track owned handlers so unregister never deletes the live one. const ownedDataAndReplayHandlers = new Map< string, - { data: (data: string, meta?: PtyDataMeta) => void; replay: (data: string) => void } + { + data: (data: string, meta?: PtyDataMeta) => void + replay: (data: string) => void + writeUnavailable: () => void + } >() const ownedExitHandlers = new Map void>() @@ -553,6 +558,9 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra if (ptyReplayHandlers.get(id) === owned.replay) { ptyReplayHandlers.delete(id) } + if (ptyWriteUnavailableHandlers.get(id) === owned.writeUnavailable) { + ptyWriteUnavailableHandlers.delete(id) + } } ownedDataAndReplayHandlers.delete(id) } @@ -584,7 +592,20 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra ) } ptyDataHandlers.set(id, dataHandler) - ownedDataAndReplayHandlers.set(id, { data: dataHandler, replay: replayHandler }) + // Guard like the data/replay handlers: a transport that rebinds to a new id without + // detaching leaves this entry behind, and a fan-out for the stale id would otherwise + // remount a healthy pane. + const writeUnavailable = (): void => { + if (ptyId === id) { + storedCallbacks.onWriteUnavailable?.() + } + } + ptyWriteUnavailableHandlers.set(id, writeUnavailable) + ownedDataAndReplayHandlers.set(id, { + data: dataHandler, + replay: replayHandler, + writeUnavailable + }) if (!isPtyDataHandlerShutdownPending(id)) { drainPreHandlerPtyData(id, dataHandler) drainRolledBackPtyShutdownData(id)