fix(terminal): use real TTY exec and resize

2026-07-10 17:21:52 -04:00
parent 906f906ad3
commit c4d0f5219a
3 changed files with 124 additions and 70 deletions
@@ -1,6 +1,7 @@
 import { mkdir } from 'node:fs/promises';
 import path from 'node:path';
 import type { Readable } from 'node:stream';
+import Docker from 'dockerode';
 import { execa } from 'execa';

 import { env } from '../env';
@@ -10,6 +11,13 @@ type CommandResult = {
  output: string;
 };

+let dockerClient: Docker | undefined;
+
+export const getDockerClient = () => {
+  dockerClient ??= new Docker();
+  return dockerClient;
+};
+
 const environmentArgs = (environment: Record<string, string>) =>
  Object.entries(environment).flatMap(([name, value]) => [
    '-e',
@@ -1,11 +1,11 @@
-import { spawn } from 'node:child_process';
-import type { ChildProcessWithoutNullStreams } from 'node:child_process';
 import type { Server } from 'node:http';
+import type { Duplex } from 'node:stream';
 import type { WebSocket } from 'ws';
 import { WebSocketServer } from 'ws';

 import type { BoxHandle } from './user-container';
 import { env } from './env';
+import { getDockerClient } from './runtime/docker';
 import { verifyTerminalToken } from './terminal-token';
 import { acquireUserBox } from './user-container';
 import { getTerminalWorkspace } from './worker';
@@ -16,8 +16,26 @@ const clampDimension = (value: unknown) => {
  return Math.min(Math.max(n, 1), 1000);
 };

-// Single-quote a string for a POSIX shell.
-const shellQuote = (value: string) => `'${value.replaceAll("'", `'\\''`)}'`;
+export const parseResizeMessage = (
+  data: Buffer,
+  isBinary: boolean,
+): { cols: number; rows: number } | null => {
+  if (isBinary) return null;
+  try {
+    const message = JSON.parse(data.toString('utf8')) as {
+      type?: string;
+      cols?: number;
+      rows?: number;
+    };
+    if (message.type !== 'resize') return null;
+    const cols = clampDimension(message.cols);
+    const rows = clampDimension(message.rows);
+    if (!cols || !rows) return null;
+    return { cols, rows };
+  } catch {
+    return null;
+  }
+};

 const bridge = async (ws: WebSocket, jobId: string) => {
  const workspace = getTerminalWorkspace(jobId);
@@ -26,41 +44,27 @@ const bridge = async (ws: WebSocket, jobId: string) => {
    return;
  }

-  // bun can't load node-pty (native ABI mismatch) and dockerode can't attach to
-  // podman, so we drive the runtime CLI (`<runtime> exec -i`) and allocate the PTY
-  // *inside* the container with `script`, bridging the plain pipes to the socket.
-  //
  // Register the message handler immediately and buffer input/size until the exec
  // is ready (acquiring the box can take seconds on first connect), so the initial
  // resize and early keystrokes aren't dropped.
-  const procHolder: { current?: ChildProcessWithoutNullStreams } = {};
+  const execHolder: {
+    current?: { exec: import('dockerode').Exec; stream: Duplex };
+  } = {};
  const pendingInput: Buffer[] = [];
  let cols = 80;
  let rows = 24;

  ws.on('message', (data: Buffer, isBinary: boolean) => {
-    if (!isBinary) {
-      // Text frames are control messages (resize); anything else is raw input.
-      try {
-        const message = JSON.parse(data.toString('utf8')) as {
-          type?: string;
-          cols?: number;
-          rows?: number;
-        };
-        if (message.type === 'resize') {
-          const c = clampDimension(message.cols);
-          const r = clampDimension(message.rows);
-          if (c && r) {
-            cols = c;
-            rows = r;
-          }
-          return;
-        }
-      } catch {
-        // fall through: treat as raw input
-      }
+    const resize = parseResizeMessage(data, isBinary);
+    if (resize) {
+      cols = resize.cols;
+      rows = resize.rows;
+      void execHolder.current?.exec
+        .resize({ h: rows, w: cols })
+        .catch(() => undefined);
+      return;
    }
-    if (procHolder.current) procHolder.current.stdin.write(data);
+    if (execHolder.current) execHolder.current.stream.write(data);
    else pendingInput.push(data);
  });

@@ -72,7 +76,8 @@ const bridge = async (ws: WebSocket, jobId: string) => {
  const cleanup = () => {
    if (released) return;
    released = true;
-    procHolder.current?.kill();
+    execHolder.current?.stream.end();
+    execHolder.current?.stream.destroy();
    handleHolder.current?.release();
  };
  ws.on('close', cleanup);
@@ -103,54 +108,47 @@ const bridge = async (ws: WebSocket, jobId: string) => {
    return;
  }

-  // Reattach a persistent tmux session across reconnects when available, else a
-  // plain login shell. `stty` sizes the PTY to the client's viewport up front.
-  const launcher =
-    `stty rows ${rows} cols ${cols} 2>/dev/null; ` +
-    // Reattach a persistent tmux session when tmux is present; otherwise fall back
-    // to an interactive login shell (`-i` so it prints a prompt and line-edits).
-    // Check with `command -v` rather than `exec tmux || …`: a failed `exec` makes a
-    // non-interactive shell exit before the `||`, so the fallback never runs.
-    'if command -v tmux >/dev/null 2>&1; then exec tmux new-session -A -s spoon; ' +
-    'else exec bash -il; fi';
-  const envFlags = [
-    '-e',
-    'TERM=xterm-256color',
-    '-e',
-    `HOME=${workspace.containerHome}`,
-    ...workspace.secrets.flatMap((s) => ['-e', `${s.name}=${s.value}`]),
-  ];
-
-  const proc = spawn(
-    env.containerRuntime,
-    [
-      'exec',
-      '-i',
-      ...envFlags,
-      '-w',
-      workspace.containerRepo,
-      boxName,
+  const docker = getDockerClient();
+  const container = docker.getContainer(boxName);
+  const exec = await container.exec({
+    AttachStdin: true,
+    AttachStdout: true,
+    AttachStderr: true,
+    Tty: true,
+    Cmd: [
      '/bin/bash',
      '-lc',
-      `exec script -qfc ${shellQuote(launcher)} /dev/null`,
+      'if command -v tmux >/dev/null 2>&1; then exec tmux new-session -A -s spoon; else exec bash -il; fi',
    ],
-    { stdio: ['pipe', 'pipe', 'pipe'] },
-  );
-  procHolder.current = proc;
+    Env: [
+      'TERM=xterm-256color',
+      `HOME=${workspace.containerHome}`,
+      ...workspace.secrets.map((secret) => `${secret.name}=${secret.value}`),
+    ],
+    WorkingDir: workspace.containerRepo,
+  });
+  const stream = await exec.start({ hijack: true, stdin: true, Tty: true });
+  execHolder.current = { exec, stream };
+
+  if (isReleased()) {
+    stream.end();
+    stream.destroy();
+    return;
+  }
+
+  await exec.resize({ h: rows, w: cols }).catch(() => undefined);

  // Replay any keystrokes the client sent before the process was ready.
-  for (const buffered of pendingInput) proc.stdin.write(buffered);
+  for (const buffered of pendingInput) stream.write(buffered);
  pendingInput.length = 0;

-  const forward = (chunk: Buffer) => {
+  stream.on('data', (chunk: Buffer) => {
    if (ws.readyState === ws.OPEN) ws.send(chunk, { binary: true });
-  };
-  proc.stdout.on('data', forward);
-  proc.stderr.on('data', forward);
-  proc.on('exit', () => {
+  });
+  stream.on('end', () => {
    if (ws.readyState === ws.OPEN) ws.close();
  });
-  proc.on('error', () => {
+  stream.on('error', () => {
    if (ws.readyState === ws.OPEN) ws.close();
  });
 };
@@ -179,7 +177,9 @@ export const attachTerminalServer = (server: Server) => {
      return;
    }
    wss.handleUpgrade(request, socket, head, (ws) => {
-      void bridge(ws, jobId);
+      void bridge(ws, jobId).catch(() => {
+        if (ws.readyState === ws.OPEN) ws.close();
+      });
    });
  });