diff --git a/src/main/providers/ssh-pty-provider.test.ts b/src/main/providers/ssh-pty-provider.test.ts index 40301906f52..64b133e08ee 100644 --- a/src/main/providers/ssh-pty-provider.test.ts +++ b/src/main/providers/ssh-pty-provider.test.ts @@ -584,7 +584,8 @@ describe('SshPtyProvider', () => { id: 'pty-old', cols: 80, rows: 24, - suppressReplayNotification: true + suppressReplayNotification: true, + requireReplay: true }, sourceActivationRequestOptions ) @@ -630,7 +631,8 @@ describe('SshPtyProvider', () => { id: 'pty-old', cols: 80, rows: 24, - suppressReplayNotification: true + suppressReplayNotification: true, + requireReplay: true }) expect(result).toEqual({ id: 'ssh:conn-1@@pty-old', @@ -655,6 +657,7 @@ describe('SshPtyProvider', () => { cols: 80, rows: 24, suppressReplayNotification: true, + requireReplay: true, expectedPaneKey: 'tab-a:leaf-a', expectedTabId: 'tab-a' }) @@ -674,7 +677,8 @@ describe('SshPtyProvider', () => { id: 'pty-old', cols: 80, rows: 24, - suppressReplayNotification: true + suppressReplayNotification: true, + requireReplay: true }, sourceActivationRequestOptions ) diff --git a/src/main/providers/ssh-pty-session-reattach.ts b/src/main/providers/ssh-pty-session-reattach.ts index 34974940caf..a3756681f9d 100644 --- a/src/main/providers/ssh-pty-session-reattach.ts +++ b/src/main/providers/ssh-pty-session-reattach.ts @@ -196,6 +196,12 @@ export async function reattachSshPtySession(args: { cols: args.options.cols, rows: args.options.rows, suppressReplayNotification: true, + // A reattach always paints into a NEW terminal: a reconnect bumps tab.generation, which is + // the pane's React key, so TerminalPane remounts and the old xterm is disposed with its + // buffer. Without this the relay sees a delivery still open under our unchanged client id, + // answers "you already have this", and the pane stays blank until new output arrives. + // Optional on the wire, so an older relay simply ignores it and behaves as it does today. + requireReplay: true, ...(expectedPaneKey ? { expectedPaneKey } : {}), ...(expectedTabId ? { expectedTabId } : {}) }, @@ -203,7 +209,8 @@ export async function reattachSshPtySession(args: { rememberPtyIncarnation: args.rememberPtyIncarnation }) console.warn( - `[ssh-pty] pty.attach succeeded for ${args.sessionId}, replay=${!!attachResult.replay}` + `[ssh-pty] pty.attach succeeded for ${args.sessionId}, replay=${!!attachResult.replay}` + + `, replayLen=${attachResult.replay?.length ?? 0}` ) return { id: toAppSshPtyId(args.connectionId, relaySessionId), diff --git a/src/relay/pty-handler-source-publication.test.ts b/src/relay/pty-handler-source-publication.test.ts index 04e45d2b384..f0041a57ce5 100644 --- a/src/relay/pty-handler-source-publication.test.ts +++ b/src/relay/pty-handler-source-publication.test.ts @@ -769,4 +769,47 @@ describe('PtyHandler negotiated source publication', () => { expect(publication.getDebugSnapshot()).toMatchObject({ sendCommitted: 1 }) expect(pausePty).not.toHaveBeenCalled() }) + + // The reported defect, in the shape measured against the live relay: the client attaches twice + // for the same PTY. The first registers a source delivery under the primary client's id; the + // second finds `current.clientId === context.clientId`, answers 'existing', and returns NO + // replay. Live, every pane on reconnect logged path=existing-delivery and painted nothing. + // + // A reattach always lands in a NEW terminal — a reconnect bumps tab.generation, which is the + // pane's React key, so TerminalPane remounts and the old xterm is disposed with its buffer. + describe('a second attach for the same client', () => { + async function attach(id: number, params: Record) { + writes = [] + dispatcher.feed(requestFrame(id, 'pty.attach', { id: 'pty-1', ...params })) + await vi.advanceTimersByTimeAsync(0) + return writes.map((buffer) => responseResult(buffer, id)).find(Boolean) + } + + beforeEach(async () => { + await spawn({}) + dataCallback!('scrollback-that-must-survive') + await vi.advanceTimersByTimeAsync(10) + await attach(10, { suppressReplayNotification: true }) + }) + + it('replays the scrollback when the client says it needs it', async () => { + const second = await attach(11, { + suppressReplayNotification: true, + requireReplay: true + }) + + expect(second, 'second attach returned no response').toBeTruthy() + expect(second!.replay).toContain('scrollback-that-must-survive') + }) + + // The early return is right for a duplicate attach from a client that IS still receiving the + // stream; only a client that has thrown its terminal away should ask. Pinned so the fix stays + // opt-in and cannot start double-rendering for callers that never asked. + it('still sends nothing when the client does not ask', async () => { + const second = await attach(12, { suppressReplayNotification: true }) + + expect(second, 'second attach returned no response').toBeTruthy() + expect(second!.replay).toBeUndefined() + }) + }) }) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index c077378ebda..31f3e81fea9 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -1732,7 +1732,23 @@ export class PtyHandler { ...(sourceActivation ? { sourceActivation } : {}) } } - if (activation === 'existing' && this.sourcePublication?.accepts(id)) { + // `existing` means a delivery is already open for this client, so it is already receiving live + // output and does not need its screen re-sent. That is true for a duplicate attach — and false + // for the case this skipped: an SSH reconnect. The client keeps its id there (the dispatcher + // refuses to detach the primary, and setWrite revives that same id), so the delivery outlives + // the dead transport, while the renderer has thrown its terminal away — a reconnect bumps + // tab.generation, which is the React key, so the pane remounts with a brand-new empty xterm and + // nothing captures the old buffer. Answering "you already have it" leaves the pane blank + // forever, until new output happens to arrive. + // + // So the client says which it is. Falling through rather than returning the replay inline is + // deliberate: the path below already drops the pending batched bytes that are also in the + // buffer, which is what stops the live delivery double-rendering them. + if ( + activation === 'existing' && + this.sourcePublication?.accepts(id) && + params.requireReplay !== true + ) { return { incarnationId: managed.incarnationId, ...(sourceActivation ? { sourceActivation } : {})