From 9b7d5b8dff16ba04d2db0eca67428629c523b2a8 Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:20:25 +0200 Subject: [PATCH 1/2] fix: replace client role on attach --- CHANGELOG.md | 4 + docs/client.md | 7 ++ src/server.ts | 1 + tests/integration.test.ts | 161 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc7f5ea..05a627f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ synchronization, so a re-attach or writable-to-readonly mode switch cannot emit the previous mode's stale screen or queued output. Reconnects establish the same fresh `GEOMETRY` → `SCREEN` → `DATA`/`EXIT` baseline. +- Valid `ATTACH` and `PEEK` messages now explicitly replace the socket's client + role. In particular, attaching after a read-only peek restores input, resize, + and shared-grid geometry participation; malformed attaches preserve the + existing role and synchronization generation. ### Atomic exact-id metadata patching diff --git a/docs/client.md b/docs/client.md index 420da0c..3929442 100644 --- a/docs/client.md +++ b/docs/client.md @@ -517,6 +517,13 @@ including writable-to-readonly mode changes, so stale screen or queued output from the previous mode is not emitted. A reconnect starts the same ordering contract again with a fresh `GEOMETRY` and `SCREEN`. +Each valid `ATTACH` or `PEEK` also replaces the socket's current role rather +than accumulating state. `ATTACH` makes the socket writable, installs its +requested geometry, and restores `DATA`/`RESIZE` handling and shared-grid +participation. `PEEK` makes it read-only and removes its geometry constraint. +A malformed `ATTACH` payload leaves the prior role and synchronization +generation unchanged. + ### `TERMINAL_SANITIZE: string` ANSI sequence that resets all terminal modes (mouse tracking, cursor visibility, alternate screen, etc.). Useful after disconnecting from a session. diff --git a/src/server.ts b/src/server.ts index 6761300..aafc822 100644 --- a/src/server.ts +++ b/src/server.ts @@ -654,6 +654,7 @@ export class PtyServer { // to its own size, which would then look like it had matched. const sizeMatched = size.rows === this.terminal.rows && size.cols === this.terminal.cols; + client.readonly = false; client.rows = size.rows; client.cols = size.cols; client.attachSeq = ++this.attachCounter; diff --git a/tests/integration.test.ts b/tests/integration.test.ts index c1c142e..3886f95 100644 --- a/tests/integration.test.ts +++ b/tests/integration.test.ts @@ -13,6 +13,7 @@ import { encodeData, encodeDetach, encodeExit, + encodePacket, encodePeek, encodeResize, encodeStatus, @@ -850,6 +851,166 @@ describe("integration", () => { liveClient.destroy(); }); + it("replaces a same-socket PEEK role with ATTACH", async () => { + const name = uniqueName(); + await startServer(name, "cat"); + + const client = await connect(name); + const packets = recordPackets(client); + client.write(encodePeek()); + await packets.waitFor((received) => + received.some((packet) => packet.type === MessageType.SCREEN) + ); + + client.write(encodeAttach(20, 70)); + await packets.waitFor( + (received) => + received.filter((packet) => packet.type === MessageType.SCREEN).length === 2 + ); + client.write(encodeData("writable-again\n")); + await packets.waitFor((received) => + received.some( + (packet) => + packet.type === MessageType.DATA && + packet.payload.toString().includes("writable-again") + ) + ); + + const statsClient = await connect(name); + const statsReader = new PacketReader(); + statsClient.write(encodeStatus()); + const status = await waitForType(statsClient, statsReader, MessageType.STATUS); + const stats = JSON.parse(status.payload.toString()); + expect(stats.terminal).toMatchObject({ rows: 20, cols: 70 }); + expect(stats.clients).toMatchObject({ attached: 1, readOnly: 0 }); + + client.write(encodeResize(18, 60)); + await packets.waitFor((received) => + received.some( + (packet) => + packet.type === MessageType.GEOMETRY && + packet.payload.readUInt16BE(0) === 18 && + packet.payload.readUInt16BE(2) === 60 + ) + ); + + client.destroy(); + statsClient.destroy(); + }); + + it("replaces a same-socket ATTACH role with PEEK", async () => { + const name = uniqueName(); + await startServer(name, "cat"); + + const client = await connect(name); + const clientPackets = recordPackets(client); + client.write(encodeAttach(20, 70)); + await clientPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.SCREEN) + ); + client.write(encodePeek()); + await clientPackets.waitFor( + (received) => + received.filter((packet) => packet.type === MessageType.SCREEN).length === 2 + ); + + client.write( + Buffer.concat([ + encodeResize(18, 60), + encodeData("must-not-reach-cat\n"), + encodeStatus(), + ]) + ); + await clientPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.STATUS) + ); + const status = clientPackets.packets + .filter((packet) => packet.type === MessageType.STATUS) + .at(-1)!; + const stats = JSON.parse(status.payload.toString()); + expect(stats.clients).toMatchObject({ attached: 0, readOnly: 1 }); + expect(stats.terminal).toMatchObject({ rows: 20, cols: 70 }); + + const observer = await connect(name); + const observerPackets = recordPackets(observer); + observer.write(encodeAttach(20, 70)); + await observerPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.SCREEN) + ); + observer.write(encodeData("accepted-by-cat\n")); + await observerPackets.waitFor((received) => + received.some( + (packet) => + packet.type === MessageType.DATA && + packet.payload.toString().includes("accepted-by-cat") + ) + ); + const observedOutput = observerPackets.packets + .filter( + (packet) => + packet.type === MessageType.SCREEN || packet.type === MessageType.DATA + ) + .map((packet) => packet.payload.toString()) + .join(""); + expect(observedOutput).not.toContain("must-not-reach-cat"); + + client.destroy(); + observer.destroy(); + }); + + it("does not change either role for a malformed ATTACH payload", async () => { + const name = uniqueName(); + await startServer(name, "cat"); + + const peeker = await connect(name); + const peekPackets = recordPackets(peeker); + peeker.write(encodePeek()); + await peekPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.SCREEN) + ); + peeker.write( + Buffer.concat([ + encodePacket(MessageType.ATTACH, Buffer.alloc(2)), + encodeStatus(), + ]) + ); + await peekPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.STATUS) + ); + const peekStatus = peekPackets.packets + .filter((packet) => packet.type === MessageType.STATUS) + .at(-1)!; + expect(JSON.parse(peekStatus.payload.toString()).clients).toMatchObject({ + attached: 0, + readOnly: 1, + }); + + const attached = await connect(name); + const attachedPackets = recordPackets(attached); + attached.write(encodeAttach(20, 70)); + await attachedPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.SCREEN) + ); + attached.write( + Buffer.concat([ + encodePacket(MessageType.ATTACH, Buffer.alloc(2)), + encodeStatus(), + ]) + ); + await attachedPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.STATUS) + ); + const status = attachedPackets.packets + .filter((packet) => packet.type === MessageType.STATUS) + .at(-1)!; + const stats = JSON.parse(status.payload.toString()); + expect(stats.clients).toMatchObject({ attached: 1, readOnly: 1 }); + expect(stats.terminal).toMatchObject({ rows: 20, cols: 70 }); + + peeker.destroy(); + attached.destroy(); + }); + it("skips the redraw SIGWINCH nudge at the session's current size", async () => { const name = uniqueName(); const marker = path.join(testCwd, `${name}-winch`); From 2c865a56c25e94a7f2546bc1d1dc002db96f023f Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:25:02 +0200 Subject: [PATCH 2/2] test: preserve sync generation on malformed attach --- tests/integration.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/integration.test.ts b/tests/integration.test.ts index 3886f95..1c23432 100644 --- a/tests/integration.test.ts +++ b/tests/integration.test.ts @@ -960,14 +960,16 @@ describe("integration", () => { it("does not change either role for a malformed ATTACH payload", async () => { const name = uniqueName(); - await startServer(name, "cat"); + const server = await startServer(name, "cat"); + const terminalWrites = holdTerminalWrites(server); const peeker = await connect(name); const peekPackets = recordPackets(peeker); peeker.write(encodePeek()); await peekPackets.waitFor((received) => - received.some((packet) => packet.type === MessageType.SCREEN) + received.some((packet) => packet.type === MessageType.GEOMETRY) ); + expect(terminalWrites.pendingWrites).toHaveLength(1); peeker.write( Buffer.concat([ encodePacket(MessageType.ATTACH, Buffer.alloc(2)), @@ -984,6 +986,11 @@ describe("integration", () => { attached: 0, readOnly: 1, }); + await terminalWrites.releaseWrites(); + await peekPackets.waitFor((received) => + received.some((packet) => packet.type === MessageType.SCREEN) + ); + terminalWrites.restore(); const attached = await connect(name); const attachedPackets = recordPackets(attached);