fix: frame intentional machine detach outcomes

Author: schickling-assistant

CHANGELOG.md

@@ -39,7 +39,10 @@
 
 - `pty attach --attach-stream-fd-v1 <fd> <ref>` keeps stdin/stdout as the
   controlling terminal while writing ordered, existing-protocol `GEOMETRY`,
-  `SCREEN`, `DATA`, and `EXIT` packets to a dedicated inherited descriptor.
+  `SCREEN`, and `DATA` packets plus a terminal `EXIT` or `DETACH` outcome to a
+  dedicated inherited descriptor. Intentional local detach is framed and
+  flushed even when it occurs before the initial daemon baseline, so consumers
+  can distinguish it from a truncated stream.
   The descriptor remains caller-owned and must be closed by the caller for
   consumers to observe EOF. Invalid descriptors, write failures, and daemons
   that do not provide the v1 geometry-first contract fail clearly on stderr.

docs/client.md

@@ -336,14 +336,19 @@ Interactive attach with bidirectional I/O. Takes over stdin/stdout. Ctrl+\ to de
 Set `attachStreamFdV1` to a writable inherited descriptor (3 or greater) for
 machine mode. stdin and stdout remain the controlling terminal for input and
 resize events, but terminal output is written only to that descriptor using the
-existing protocol framing. Version 1 emits ordered `GEOMETRY`, `SCREEN`, `DATA`,
-and `EXIT` packets. Each initial attach or reconnect starts with `GEOMETRY`; a
+existing protocol framing. Version 1 emits ordered `GEOMETRY`, `SCREEN`, and
+`DATA` packets followed by one terminal outcome: `EXIT` when the session process
+ends or `DETACH` when the local user intentionally detaches. `DETACH` may be the
+first packet when the user detaches before the daemon supplies its initial
+baseline. Each initial attach or reconnect otherwise starts with `GEOMETRY`; a
 daemon that sends terminal data first is rejected as unsupported.
 
 The descriptor remains caller-owned. `attach()` flushes its writer but does not
 close the descriptor, so a consumer sees EOF only when the caller closes its
-copy (or the process exits). Descriptor errors fail the attach and are reported
-on stderr; stderr text is never written into the framed stream.
+copy (or the process exits). A clean EOF follows a framed `EXIT` or `DETACH`;
+EOF without either outcome is a truncated stream. Descriptor errors fail the
+attach and are reported on stderr; stderr text is never written into the framed
+stream.
 
 ### `peek(options: PeekOptions): void`
 

src/cli.ts

@@ -120,8 +120,8 @@ Flags:
                        the session's name/id ON THE REMOTE
   --attach-stream-fd-v1 <fd>
                        Machine mode for a running session. Write ordered framed
-                       GEOMETRY, SCREEN, DATA, and EXIT events to inherited fd
-                       (>= 3); keep stdin/stdout as the controlling terminal
+                       GEOMETRY, SCREEN, DATA, and terminal EXIT or DETACH outcome
+                       to inherited fd (>= 3); keep stdin/stdout controlling TTY
 
 Examples:
   pty attach myserver

src/client.ts

@@ -500,6 +500,27 @@ export function attach(options: AttachOptions): void {
       completeExit(code);
     }
   }
+  function finishDetach(): void {
+    if (exitHandled) return;
+    exitHandled = true;
+    detaching = true;
+    const completeDetach = () => {
+      if (exitCompleted) return;
+      exitCompleted = true;
+      options.onDetach?.();
+    };
+    if (attachStream && !attachStream.destroyed && !attachStream.writableEnded) {
+      attachStream.write(encodeDetach());
+      try { socket.write(encodeDetach()); } catch {}
+      cleanExit();
+      attachStream.end(completeDetach);
+    } else {
+      try { socket.write(encodeDetach()); } catch {}
+      cleanExit();
+      stdout.write(TERMINAL_SANITIZE + CURSOR_TO_BOTTOM + "\r\n[detached]\r\n");
+      completeDetach();
+    }
+  }
 
   attachStream?.on("error", (error) => {
     console.error(`pty attach: machine stream descriptor ${attachStreamFd} failed: ${error.message}`);
@@ -536,15 +557,7 @@ export function attach(options: AttachOptions): void {
             lastDetachKeyTime = now;
             setTimeout(() => {
               if (lastDetachKeyTime === now) {
-                detaching = true;
-                try { socket.write(encodeDetach()); } catch {}
-                cleanExit();
-                if (attachStream) {
-                  attachStream.end(() => options.onDetach?.());
-                } else {
-                  stdout.write(TERMINAL_SANITIZE + CURSOR_TO_BOTTOM + "\r\n[detached]\r\n");
-                  options.onDetach?.();
-                }
+                finishDetach();
               }
             }, DOUBLE_TAP_MS);
           }

tests/attach-stream.test.ts

@@ -180,6 +180,150 @@ describe("pty attach --attach-stream-fd-v1", () => {
     expect(packets.at(-1)?.type).toBe(MessageType.EXIT);
   }, 15_000);
 
+  it("frames an intentional local detach before the shipped bin launcher closes fd 3", async () => {
+    const root = fs.mkdtempSync(path.join(testRoot, "launcher-detach-"));
+    const name = `launcher-detach-${process.pid}`;
+    const launcherEnv: NodeJS.ProcessEnv = {
+      ...process.env,
+      PTY_ROOT: root,
+      PTY_ROOT_LEGACY_SILENT: "1",
+    };
+    delete launcherEnv.PTY_SESSION;
+    delete launcherEnv.PTY_SERVER_CONFIG;
+    const created = spawnSync(
+      nodeBin,
+      [cliPath, "run", "-d", "--id", name, "--", "sh", "-c", "printf DETACH_READY; sleep 300"],
+      { env: launcherEnv, encoding: "utf8" },
+    );
+    expect(created.status, created.stderr).toBe(0);
+
+    const child = spawn(
+      nodeBin,
+      [binPath, "attach", "--attach-stream-fd-v1", "3", name],
+      { env: launcherEnv, stdio: ["pipe", "pipe", "pipe", "pipe"] },
+    );
+    const stdout = collect(child.stdout);
+    const stderr = collect(child.stderr);
+    const streamChunks: Buffer[] = [];
+    const reader = new PacketReader();
+    let requestedDetach = false;
+    (child.stdio[3] as NodeJS.ReadableStream).on("data", (chunk) => {
+      const data = Buffer.from(chunk);
+      streamChunks.push(data);
+      for (const packet of reader.feed(data)) {
+        if (packet.type === MessageType.SCREEN && !requestedDetach) {
+          requestedDetach = true;
+          child.stdin.write(Buffer.from([0x1c]));
+        }
+      }
+    });
+
+    const status = await new Promise<number | null>((resolve, reject) => {
+      const timer = setTimeout(() => reject(new Error("bin launcher detach timed out")), 10_000);
+      child.once("exit", (code) => {
+        clearTimeout(timer);
+        resolve(code);
+      });
+    });
+    spawnSync(nodeBin, [cliPath, "kill", name], { env: launcherEnv, encoding: "utf8" });
+
+    expect(status).toBe(0);
+    expect(await stdout).toEqual(Buffer.alloc(0));
+    expect(await stderr).toEqual(Buffer.alloc(0));
+    const packets = new PacketReader().feed(Buffer.concat(streamChunks));
+    expect(packets[0].type).toBe(MessageType.GEOMETRY);
+    expect(packets[1].type).toBe(MessageType.SCREEN);
+    expect(packets[1].payload.toString()).toContain("DETACH_READY");
+    expect(packets.at(-1)?.type).toBe(MessageType.DETACH);
+    expect(packets.some((packet) => packet.type === MessageType.EXIT)).toBe(false);
+  }, 15_000);
+
+  it("frames an intentional local detach before the initial daemon baseline", async () => {
+    const { server, port } = await listen();
+    const attached = new Promise<void>((resolve) => {
+      server.once("connection", (socket) => socket.once("data", () => resolve()));
+    });
+    const child = spawn(nodeBin, ["--input-type=module", "-e", attachScript(port)], {
+      stdio: ["pipe", "pipe", "pipe", "pipe"],
+    });
+    const stdout = collect(child.stdout);
+    const stderr = collect(child.stderr);
+    const stream = collect(child.stdio[3] as NodeJS.ReadableStream);
+    await attached;
+    child.stdin.write(Buffer.from([0x1c]));
+    const status = await new Promise<number | null>((resolve, reject) => {
+      const timer = setTimeout(() => reject(new Error("pre-baseline detach timed out")), 5_000);
+      child.once("exit", (code) => {
+        clearTimeout(timer);
+        resolve(code);
+      });
+    });
+    server.close();
+
+    expect(status).toBe(0);
+    expect(await stdout).toEqual(Buffer.alloc(0));
+    expect(await stderr).toEqual(Buffer.alloc(0));
+    expect(new PacketReader().feed(await stream).map((packet) => packet.type)).toEqual([
+      MessageType.DETACH,
+    ]);
+  });
+
+  it("does not frame DETACH when EXIT wins the pending detach-key window", async () => {
+    const { server, port } = await listen();
+    let daemonSocket: net.Socket | undefined;
+    server.once("connection", (socket) => {
+      daemonSocket = socket;
+      socket.once("data", () => socket.write(Buffer.concat([
+        encodeGeometry(24, 80),
+        encodeScreen("exit wins"),
+      ])));
+    });
+    const script = `
+      import net from "node:net";
+      import { attach } from ${JSON.stringify(clientUrl)};
+      const socket = net.createConnection({ host: "127.0.0.1", port: ${port} });
+      socket.once("connect", () => attach({
+        name: "fixture",
+        socket,
+        attachStreamFdV1: 3,
+        onExit: (code) => setTimeout(() => process.exit(code), 600),
+        onDetach: () => process.exit(42),
+      }));
+    `;
+    const child = spawn(nodeBin, ["--input-type=module", "-e", script], {
+      stdio: ["pipe", "pipe", "pipe", "pipe"],
+    });
+    const stdout = collect(child.stdout);
+    const stderr = collect(child.stderr);
+    const chunks: Buffer[] = [];
+    const reader = new PacketReader();
+    (child.stdio[3] as NodeJS.ReadableStream).on("data", (chunk) => {
+      const data = Buffer.from(chunk);
+      chunks.push(data);
+      if (reader.feed(data).some((packet) => packet.type === MessageType.SCREEN)) {
+        child.stdin.write(Buffer.from([0x1c]));
+        setTimeout(() => daemonSocket?.end(encodeExit(0)), 50);
+      }
+    });
+    const status = await new Promise<number | null>((resolve, reject) => {
+      const timer = setTimeout(() => reject(new Error("exit/detach race timed out")), 5_000);
+      child.once("exit", (code) => {
+        clearTimeout(timer);
+        resolve(code);
+      });
+    });
+    server.close();
+
+    expect(status).toBe(0);
+    expect(await stdout).toEqual(Buffer.alloc(0));
+    expect(await stderr).toEqual(Buffer.alloc(0));
+    expect(new PacketReader().feed(Buffer.concat(chunks)).map((packet) => packet.type)).toEqual([
+      MessageType.GEOMETRY,
+      MessageType.SCREEN,
+      MessageType.EXIT,
+    ]);
+  });
+
   it("reframes fragmented and coalesced daemon packets in order without stdout output", async () => {
     const geometry = encodeGeometry(31, 97);
     const screen = encodeScreen("\x1b[31mred\x1b[0m");