Skip to content

Commit 1089015

Browse files
fix: replace client role on attach
1 parent d5fabc3 commit 1089015

4 files changed

Lines changed: 173 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
synchronization, so a re-attach or writable-to-readonly mode switch cannot
2525
emit the previous mode's stale screen or queued output. Reconnects establish
2626
the same fresh `GEOMETRY``SCREEN``DATA`/`EXIT` baseline.
27+
- Valid `ATTACH` and `PEEK` messages now explicitly replace the socket's client
28+
role. In particular, attaching after a read-only peek restores input, resize,
29+
and shared-grid geometry participation; malformed attaches preserve the
30+
existing role and synchronization generation.
2731

2832
### Non-unique display names with unambiguous session resolution
2933

docs/client.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,13 @@ including writable-to-readonly mode changes, so stale screen or queued output
479479
from the previous mode is not emitted. A reconnect starts the same ordering
480480
contract again with a fresh `GEOMETRY` and `SCREEN`.
481481

482+
Each valid `ATTACH` or `PEEK` also replaces the socket's current role rather
483+
than accumulating state. `ATTACH` makes the socket writable, installs its
484+
requested geometry, and restores `DATA`/`RESIZE` handling and shared-grid
485+
participation. `PEEK` makes it read-only and removes its geometry constraint.
486+
A malformed `ATTACH` payload leaves the prior role and synchronization
487+
generation unchanged.
488+
482489
### `TERMINAL_SANITIZE: string`
483490

484491
ANSI sequence that resets all terminal modes (mouse tracking, cursor visibility, alternate screen, etc.). Useful after disconnecting from a session.

src/server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,7 @@ export class PtyServer {
643643
// to its own size, which would then look like it had matched.
644644
const sizeMatched =
645645
size.rows === this.terminal.rows && size.cols === this.terminal.cols;
646+
client.readonly = false;
646647
client.rows = size.rows;
647648
client.cols = size.cols;
648649
client.attachSeq = ++this.attachCounter;

tests/integration.test.ts

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
encodeData,
1414
encodeDetach,
1515
encodeExit,
16+
encodePacket,
1617
encodePeek,
1718
encodeResize,
1819
encodeStatus,
@@ -850,6 +851,166 @@ describe("integration", () => {
850851
liveClient.destroy();
851852
});
852853

854+
it("replaces a same-socket PEEK role with ATTACH", async () => {
855+
const name = uniqueName();
856+
await startServer(name, "cat");
857+
858+
const client = await connect(name);
859+
const packets = recordPackets(client);
860+
client.write(encodePeek());
861+
await packets.waitFor((received) =>
862+
received.some((packet) => packet.type === MessageType.SCREEN)
863+
);
864+
865+
client.write(encodeAttach(20, 70));
866+
await packets.waitFor(
867+
(received) =>
868+
received.filter((packet) => packet.type === MessageType.SCREEN).length === 2
869+
);
870+
client.write(encodeData("writable-again\n"));
871+
await packets.waitFor((received) =>
872+
received.some(
873+
(packet) =>
874+
packet.type === MessageType.DATA &&
875+
packet.payload.toString().includes("writable-again")
876+
)
877+
);
878+
879+
const statsClient = await connect(name);
880+
const statsReader = new PacketReader();
881+
statsClient.write(encodeStatus());
882+
const status = await waitForType(statsClient, statsReader, MessageType.STATUS);
883+
const stats = JSON.parse(status.payload.toString());
884+
expect(stats.terminal).toMatchObject({ rows: 20, cols: 70 });
885+
expect(stats.clients).toMatchObject({ attached: 1, readOnly: 0 });
886+
887+
client.write(encodeResize(18, 60));
888+
await packets.waitFor((received) =>
889+
received.some(
890+
(packet) =>
891+
packet.type === MessageType.GEOMETRY &&
892+
packet.payload.readUInt16BE(0) === 18 &&
893+
packet.payload.readUInt16BE(2) === 60
894+
)
895+
);
896+
897+
client.destroy();
898+
statsClient.destroy();
899+
});
900+
901+
it("replaces a same-socket ATTACH role with PEEK", async () => {
902+
const name = uniqueName();
903+
await startServer(name, "cat");
904+
905+
const client = await connect(name);
906+
const clientPackets = recordPackets(client);
907+
client.write(encodeAttach(20, 70));
908+
await clientPackets.waitFor((received) =>
909+
received.some((packet) => packet.type === MessageType.SCREEN)
910+
);
911+
client.write(encodePeek());
912+
await clientPackets.waitFor(
913+
(received) =>
914+
received.filter((packet) => packet.type === MessageType.SCREEN).length === 2
915+
);
916+
917+
client.write(
918+
Buffer.concat([
919+
encodeResize(18, 60),
920+
encodeData("must-not-reach-cat\n"),
921+
encodeStatus(),
922+
])
923+
);
924+
await clientPackets.waitFor((received) =>
925+
received.some((packet) => packet.type === MessageType.STATUS)
926+
);
927+
const status = clientPackets.packets
928+
.filter((packet) => packet.type === MessageType.STATUS)
929+
.at(-1)!;
930+
const stats = JSON.parse(status.payload.toString());
931+
expect(stats.clients).toMatchObject({ attached: 0, readOnly: 1 });
932+
expect(stats.terminal).toMatchObject({ rows: 20, cols: 70 });
933+
934+
const observer = await connect(name);
935+
const observerPackets = recordPackets(observer);
936+
observer.write(encodeAttach(20, 70));
937+
await observerPackets.waitFor((received) =>
938+
received.some((packet) => packet.type === MessageType.SCREEN)
939+
);
940+
observer.write(encodeData("accepted-by-cat\n"));
941+
await observerPackets.waitFor((received) =>
942+
received.some(
943+
(packet) =>
944+
packet.type === MessageType.DATA &&
945+
packet.payload.toString().includes("accepted-by-cat")
946+
)
947+
);
948+
const observedOutput = observerPackets.packets
949+
.filter(
950+
(packet) =>
951+
packet.type === MessageType.SCREEN || packet.type === MessageType.DATA
952+
)
953+
.map((packet) => packet.payload.toString())
954+
.join("");
955+
expect(observedOutput).not.toContain("must-not-reach-cat");
956+
957+
client.destroy();
958+
observer.destroy();
959+
});
960+
961+
it("does not change either role for a malformed ATTACH payload", async () => {
962+
const name = uniqueName();
963+
await startServer(name, "cat");
964+
965+
const peeker = await connect(name);
966+
const peekPackets = recordPackets(peeker);
967+
peeker.write(encodePeek());
968+
await peekPackets.waitFor((received) =>
969+
received.some((packet) => packet.type === MessageType.SCREEN)
970+
);
971+
peeker.write(
972+
Buffer.concat([
973+
encodePacket(MessageType.ATTACH, Buffer.alloc(2)),
974+
encodeStatus(),
975+
])
976+
);
977+
await peekPackets.waitFor((received) =>
978+
received.some((packet) => packet.type === MessageType.STATUS)
979+
);
980+
const peekStatus = peekPackets.packets
981+
.filter((packet) => packet.type === MessageType.STATUS)
982+
.at(-1)!;
983+
expect(JSON.parse(peekStatus.payload.toString()).clients).toMatchObject({
984+
attached: 0,
985+
readOnly: 1,
986+
});
987+
988+
const attached = await connect(name);
989+
const attachedPackets = recordPackets(attached);
990+
attached.write(encodeAttach(20, 70));
991+
await attachedPackets.waitFor((received) =>
992+
received.some((packet) => packet.type === MessageType.SCREEN)
993+
);
994+
attached.write(
995+
Buffer.concat([
996+
encodePacket(MessageType.ATTACH, Buffer.alloc(2)),
997+
encodeStatus(),
998+
])
999+
);
1000+
await attachedPackets.waitFor((received) =>
1001+
received.some((packet) => packet.type === MessageType.STATUS)
1002+
);
1003+
const status = attachedPackets.packets
1004+
.filter((packet) => packet.type === MessageType.STATUS)
1005+
.at(-1)!;
1006+
const stats = JSON.parse(status.payload.toString());
1007+
expect(stats.clients).toMatchObject({ attached: 1, readOnly: 1 });
1008+
expect(stats.terminal).toMatchObject({ rows: 20, cols: 70 });
1009+
1010+
peeker.destroy();
1011+
attached.destroy();
1012+
});
1013+
8531014
it("skips the redraw SIGWINCH nudge at the session's current size", async () => {
8541015
const name = uniqueName();
8551016
const marker = path.join(testCwd, `${name}-winch`);

0 commit comments

Comments
 (0)