Merge branch 'sidecars-pty' into sidecars

ecosystem.config.cjs

@@ -24,10 +24,13 @@ module.exports = {
       args: 'run src/servers/sidecar/email/index.ts',
       watch: false,
     },
+    // The only sidecar run by `node` rather than `bun`, and the only one that is not TypeScript: node-pty
+    // is a native addon. It also does not use sidecar/connect.ts, and carries its own copy of the
+    // reconnect loop.
     {
       name: 'officer-pty',
       script: 'node',
-      args: 'src/servers/api/terminal/pty-sidecar.mjs',
+      args: 'src/servers/sidecar/pty/index.mjs',
       watch: false,
     },
     {

src/servers/api/terminal/websocket.ts

@@ -1,5 +1,4 @@
 import type { ServerWebSocket } from 'bun';
-import { join } from 'node:path';
 import { sendPtyCommand, sendPtyCommandAsync, on, isTerminalConnected } from '@@/sidecar-registry';
 import type { PtyInitConfig } from '../../sidecar/protocol';
 
@@ -16,8 +15,7 @@ type WSData = {
 type BridgeSession = {
   client: ServerWebSocket<WSData>;
   sessionId: string;
-  unsubOutput: (() => void) | null;
-  unsubExit: (() => void) | null;
+  unsubs: Array<() => void>;
 };
 
 const sessions = new Map<ServerWebSocket<WSData>, BridgeSession>();
@@ -35,13 +33,6 @@ const sendOutput = (ws: ServerWebSocket<WSData>, data: string) => {
   }
 };
 
-const resolveCwd = (home: string, cwd?: string) => {
-  if (!cwd || cwd === '~') return home;
-  if (cwd.startsWith('~/')) return join(home, cwd.slice(2));
-  if (cwd.startsWith('/')) return cwd;
-  return home;
-};
-
 export const terminalWebsocket = {
   async open(ws: ServerWebSocket<WSData>) {
     const { email, username } = ws.data;
@@ -55,40 +46,27 @@ export const terminalWebsocket = {
 
     const sessionId = ws.data.sessionId ?? `host-${ws.data.userId}`;
 
-    // The server owner is the only account, so the terminal is always a plain host shell.
-    const config: PtyInitConfig = {
+    // Everything this bridge knows: which session, which folder the panel was opened on, and how big the
+    // client's viewport is. The shell, its arguments and the home directory are the sidecar's — it is the
+    // process that spawns them, and officer has no business reading the owner's SHELL and HOME to guess.
+    const config: PtyInitConfig = { sessionId, cwd: ws.data.cwd, cols: ws.data.cols, rows: ws.data.rows };
+
+    // The sidecar emits one global stream, so each frame is filtered down to this session and relabelled.
+    const relay = (event: 'pty:output' | 'pty:replay' | 'pty:exit', clientType: string) =>
+      on(event, (msg) => {
+        if (msg.type !== event || msg.sessionId !== sessionId) return;
+        try {
+          ws.send(JSON.stringify({ type: clientType, data: 'data' in msg ? msg.data : undefined }));
+        } catch {
+          // ws already closed
+        }
+      });
+
+    const session: BridgeSession = {
+      client: ws,
       sessionId,
-      host: true,
-      shell: { command: process.env.SHELL ?? '/bin/zsh', args: ['-i'] },
-      cwd: resolveCwd(process.env.HOME!, ws.data.cwd),
-      homeDir: process.env.HOME!,
-      userLabel: email,
-      cols: ws.data.cols,
-      rows: ws.data.rows,
+      unsubs: [relay('pty:output', 'output'), relay('pty:replay', 'replay'), relay('pty:exit', 'exit')],
     };
-
-    // Subscribe to events for this session
-    const unsubOutput = on('pty:output', (msg) => {
-      if (msg.type === 'pty:output' && msg.sessionId === sessionId) {
-        try {
-          ws.send(JSON.stringify({ type: 'output', data: msg.data }));
-        } catch {
-          // ws already closed
-        }
-      }
-    });
-
-    const unsubExit = on('pty:exit', (msg) => {
-      if (msg.type === 'pty:exit' && msg.sessionId === sessionId) {
-        try {
-          ws.send(JSON.stringify({ type: 'exit' }));
-        } catch {
-          // ws already closed
-        }
-      }
-    });
-
-    const session: BridgeSession = { client: ws, sessionId, unsubOutput, unsubExit };
     sessions.set(ws, session);
 
     // Send init command to PTY sidecar
@@ -97,8 +75,7 @@ export const terminalWebsocket = {
     } catch (err) {
       const message = err instanceof Error ? err.message : 'Failed to initialize terminal';
       sendOutput(ws, `\r\n[Terminal error] ${message}\r\n`);
-      unsubOutput();
-      unsubExit();
+      for (const unsub of session.unsubs) unsub();
       sessions.delete(ws);
     }
   },
@@ -126,16 +103,9 @@ export const terminalWebsocket = {
             });
           }
           break;
-        case 'cwd':
-          if (msg.path) {
-            sendPtyCommand({
-              type: 'pty:input',
-              id: nextId(),
-              sessionId: session.sessionId,
-              data: `cd ${JSON.stringify(msg.path)}\r`,
-            });
-          }
-          break;
+        // There was a 'cwd' case here that typed `cd <path>\r` into the user's shell. No frontend sends
+        // that message — the browser composes its own `cd` (Terminal.tsx / CommandTerminalWrapper.tsx) —
+        // so it was unreachable, and synthesizing keystrokes is not a thing a proxy should do.
       }
     } catch {
       // ignore malformed messages
@@ -145,8 +115,7 @@ export const terminalWebsocket = {
   close(ws: ServerWebSocket<WSData>) {
     const session = sessions.get(ws);
     if (session) {
-      session.unsubOutput?.();
-      session.unsubExit?.();
+      for (const unsub of session.unsubs) unsub();
       // Don't kill PTY — it can be reattached
       sessions.delete(ws);
     }

src/servers/api/users/provision.ts

@@ -3,7 +3,7 @@ import { join } from 'node:path';
 import { DATA_PATH, getHomeDir, toShellUsername } from '@@/data-path';
 import { generateContainerContext, generateClaudeSettings } from '@@/generate-container-context';
 
-const TEMPLATE_DIR = join(import.meta.dir, '../terminal/templates');
+const TEMPLATE_DIR = join(import.meta.dir, 'templates');
 
 const copyTemplate = async (src: string, dest: string) => {
   if (existsSync(dest)) return;

src/servers/sidecar/protocol.ts

@@ -130,16 +130,15 @@ export type VncSessionInfo = {
 
 // ── PTY types ──
 
+// What officer knows about a terminal, and nothing more. The shell, its arguments, the home directory and
+// whether the shell is sandboxed are the sidecar's own decisions — they used to travel in here, which is
+// how officer ended up reading the owner's SHELL and HOME and hardcoding `host: true`.
 export type PtyInitConfig = {
   sessionId: string;
-  shell?: { command: string; args?: string[] };
+  /** The folder the panel was opened on. `~`, `~/x` and absolute paths only; resolved by the sidecar. */
   cwd?: string;
-  homeDir?: string;
-  userLabel?: string;
-  host?: boolean;
   cols?: number;
   rows?: number;
-  env?: Record<string, string>;
 };
 
 // PTY commands (API → PTY sidecar)
@@ -153,4 +152,7 @@ export type PtyCommand =
 export type PtyEvent =
   | { type: 'pty:ready'; id: string; sessionId: string }
   | { type: 'pty:output'; sessionId: string; data: string }
+  // Scrollback sent on re-attach, which the client may already be showing in part — distinct from
+  // `pty:output` so it can rebuild the screen rather than append a second copy of it.
+  | { type: 'pty:replay'; sessionId: string; data: string }
   | { type: 'pty:exit'; sessionId: string; exitCode: number; signal?: number };

src/servers/sidecar/pty/index.mjs

@@ -10,24 +10,10 @@ process.on('SIGINT', () => {
 });
 process.on('SIGTERM', () => process.emit('SIGINT'));
 
-import { existsSync } from 'node:fs';
-import { cp, mkdir } from 'node:fs/promises';
-import { join, dirname } from 'node:path';
-import { fileURLToPath } from 'node:url';
-import { execFile } from 'node:child_process';
+import { join } from 'node:path';
 import WebSocket from 'ws';
 import * as pty from 'node-pty';
 
-const run = (cmd, args, opts = {}) =>
-  new Promise((resolve) => {
-    const proc = execFile(cmd, args, { stdio: 'ignore', ...opts }, () => resolve());
-    proc.on('error', () => resolve());
-  });
-
-const __dirname = dirname(fileURLToPath(import.meta.url));
-
-const templateDir = join(__dirname, 'templates');
-
 import 'dotenv/config';
 
 const API_URL = process.env.API_URL ?? `ws://127.0.0.1:${process.env.PORT ?? '5000'}`;
@@ -36,14 +22,46 @@ const REGISTER_URL = `${API_URL}/api/sidecar/register`;
 const BUFFER_MAX = 50 * 1024;
 const RECONNECT_DELAYS = [200, 500, 1000, 2000, 4000, 8000, 15000];
 
-/** @type {Map<string, { term: import('node-pty').IPty, buffer: string, cols: number, rows: number, initConfig: object }>} */
+// ── What kind of shell this process runs ──
+//
+// These used to arrive inside every pty:init, which meant officer chose the owner's shell and read the
+// owner's HOME to do it. They are this process's business: it is the one that spawns the thing.
+//
+// HOME_DIR mirrors `data-path.ts:getOwnerHomeDir` — on a host where the owner's real login home differs
+// from this process's HOME, the shell should open in the former, like every other host-executing surface.
+const HOME_DIR = process.env.HOME_DIR ?? process.env.HOME ?? process.cwd();
+const SHELL = { command: process.env.SHELL ?? '/bin/zsh', args: ['-i'] };
+// A terminal is always a plain host shell: the owner is the only account and it is their own machine
+// (`platform/CLAUDE.md` — do not add a jail without being asked). There used to be a second branch here
+// for a bwrap sandbox, selected by `config.host`, which officer hardcoded to true. Nothing ever built the
+// bwrap command it expected, on either side, so it could not have run — it is in git history if the
+// decision is ever revisited, and the `ensureUserFiles` half of it duplicated
+// `api/users/provision.ts:seedShellConfigs`, which is the live seeder of those templates.
+
+const resolveCwd = (cwd) => {
+  if (!cwd || cwd === '~') return HOME_DIR;
+  if (cwd.startsWith('~/')) return join(HOME_DIR, cwd.slice(2));
+  if (cwd.startsWith('/')) return cwd;
+  // Relative paths have no meaning here — this process's cwd is the repo, not the user's folder.
+  return HOME_DIR;
+};
+
+/** @type {Map<string, { term: import('node-pty').IPty, buffer: string, cols: number, rows: number }>} */
 const sessions = new Map();
 
 // ── Helpers ──
 
-const sendJson = (ws, msg) => {
+// Always resolve the CURRENT registration socket, never one captured in a closure.
+//
+// `term.onData` used to close over the socket that was live when the session was created. Officer is a
+// PM2 peer that restarts often, and each restart gives this process a brand new socket — so every
+// pre-existing session went on writing to a closed one, where the readyState check below dropped it
+// silently. The shell stayed alive and kept accepting input (that arrives on the new socket), but its
+// output never came back: the terminal looked frozen until you closed the panel. Reading the module
+// variable at send time is the whole fix.
+const sendJson = (msg) => {
   try {
-    if (ws.readyState === WebSocket.OPEN) {
+    if (ws && ws.readyState === WebSocket.OPEN) {
       ws.send(JSON.stringify(msg));
     }
   } catch {
@@ -51,32 +69,6 @@ const sendJson = (ws, msg) => {
   }
 };
 
-const ensureUserFiles = async (homeDir) => {
-  await mkdir(homeDir, { recursive: true });
-  await mkdir(join(homeDir, '.config'), { recursive: true });
-  await mkdir(join(homeDir, '.local', 'bin'), { recursive: true });
-
-  const zshrcPath = join(homeDir, '.zshrc');
-  if (!existsSync(zshrcPath)) {
-    await cp(join(templateDir, '.zshrc'), zshrcPath);
-  }
-
-  const tmuxconfPath = join(homeDir, '.tmux.conf');
-  if (!existsSync(tmuxconfPath)) {
-    await cp(join(templateDir, '.tmux.conf'), tmuxconfPath);
-  }
-
-  const starshipPath = join(homeDir, '.config', 'starship-officer.toml');
-  if (!existsSync(starshipPath)) {
-    await cp(join(templateDir, 'starship-officer.toml'), starshipPath);
-  }
-
-  const ohMyZshPath = join(homeDir, '.oh-my-zsh');
-  if (!existsSync(ohMyZshPath)) {
-    await run('git', ['clone', '--depth=1', 'https://github.com/ohmyzsh/ohmyzsh.git', ohMyZshPath]);
-  }
-};
-
 const appendBuffer = (session, data) => {
   session.buffer += data;
   if (session.buffer.length > BUFFER_MAX) {
@@ -86,7 +78,7 @@ const appendBuffer = (session, data) => {
 
 // ── Command handler ──
 
-async function handleCommand(ws, msg) {
+async function handleCommand(msg) {
   switch (msg.type) {
     case 'pty:init': {
       const { sessionId, config } = msg;
@@ -96,9 +88,12 @@ async function handleCommand(ws, msg) {
       console.log(`[pty-sidecar] init sessionId=${sessionId} existing=${!!existing} total=${sessions.size}`);
 
       if (existing) {
-        // Replay buffer
+        // Re-attach. The scrollback goes out as `pty:replay`, not as ordinary output, because the client
+        // may already be showing some of it: after an officer restart the browser keeps its terminal and
+        // reconnects, so replaying blind appended a second copy of everything on screen. Marked as
+        // history, the client can reset and rebuild from it instead.
         if (existing.buffer.length > 0) {
-          sendJson(ws, { type: 'pty:output', sessionId, data: existing.buffer });
+          sendJson({ type: 'pty:replay', sessionId, data: existing.buffer });
         }
 
         // Resize PTY to new client dimensions
@@ -114,81 +109,46 @@ async function handleCommand(ws, msg) {
           }
         }
 
-        sendJson(ws, { type: 'pty:ready', id: msg.id, sessionId });
+        sendJson({ type: 'pty:ready', id: msg.id, sessionId });
         return;
       }
 
       // New session — spawn PTY
-      const shell = config.shell ?? { command: '/bin/bash', args: ['-i'] };
-      const cwd = config.cwd ?? process.cwd();
-      const homeDir = config.homeDir ?? process.cwd();
-      const userLabel = config.userLabel ?? 'officer';
+      const cwd = resolveCwd(config.cwd);
       const cols = config.cols ?? 80;
       const rows = config.rows ?? 24;
-      const isHost = !!config.host;
-
-      let spawnCommand;
-      let spawnArgs;
-      let ptyEnv;
-
-      if (isHost) {
-        spawnCommand = shell.command;
-        spawnArgs = shell.args ?? [];
-        ptyEnv = { ...process.env, TERM: 'xterm-256color', ...(config.env ?? {}) };
-      } else {
-        // Sandboxed mode: shell config contains the full bwrap command
-        spawnCommand = shell.command;
-        spawnArgs = shell.args ?? [];
-
-        try {
-          await ensureUserFiles(homeDir);
-        } catch (err) {
-          console.error('[pty-sidecar] ensureUserFiles failed:', err);
-        }
-
-        // bwrap sets env vars internally via --setenv, so use minimal host env
-        ptyEnv = { TERM: 'xterm-256color' };
-      }
-
-      const ptyCwd = isHost ? cwd : undefined;
 
       let term;
       try {
-        term = pty.spawn(spawnCommand, spawnArgs, {
+        term = pty.spawn(SHELL.command, SHELL.args, {
           name: 'xterm-256color',
           cols,
           rows,
-          cwd: ptyCwd,
-          env: ptyEnv,
+          cwd,
+          env: { ...process.env, TERM: 'xterm-256color' },
         });
       } catch (err) {
         const message = err instanceof Error ? err.message : 'Failed to start terminal';
-        sendJson(ws, { type: 'pty:output', sessionId, data: `\r\n[Terminal error] ${message}\r\n` });
-        sendJson(ws, { type: 'pty:exit', sessionId, exitCode: 1 });
+        sendJson({ type: 'pty:output', sessionId, data: `\r\n[Terminal error] ${message}\r\n` });
+        sendJson({ type: 'pty:exit', sessionId, exitCode: 1 });
         return;
       }
 
-      const session = {
-        term,
-        buffer: '',
-        cols,
-        rows,
-        initConfig: { shell, cwd, homeDir, userLabel },
-      };
+      const session = { term, buffer: '', cols, rows };
       sessions.set(sessionId, session);
 
       term.onData((output) => {
         appendBuffer(session, output);
-        sendJson(ws, { type: 'pty:output', sessionId, data: output });
+        sendJson({ type: 'pty:output', sessionId, data: output });
       });
 
       term.onExit(({ exitCode, signal }) => {
         console.log(`[pty-sidecar] session ${sessionId} exited code=${exitCode} signal=${signal}`);
-        sendJson(ws, { type: 'pty:exit', sessionId, exitCode, signal });
+        sendJson({ type: 'pty:exit', sessionId, exitCode, signal });
         sessions.delete(sessionId);
       });
 
-      sendJson(ws, { type: 'pty:ready', id: msg.id, sessionId });
+      sendJson({ type: 'pty:ready', id: msg.id, sessionId });
       return;
     }
 
@@ -251,7 +211,7 @@ function connect() {
   ws.on('open', () => {
     reconnectAttempt = 0;
     console.log('[pty-sidecar] connected, sending registration...');
-    sendJson(ws, { type: 'register', name: 'pty', capabilities: ['terminal'] });
+    sendJson({ type: 'register', name: 'pty', capabilities: ['terminal'] });
   });
 
   ws.on('message', (data) => {
@@ -263,7 +223,7 @@ function connect() {
         return;
       }
 
-      handleCommand(ws, msg);
+      handleCommand(msg);
     } catch {
       // skip malformed messages
     }

src/servers/sidecar/pty/index.test.ts

@@ -0,0 +1,134 @@
+import { describe, test, expect, afterAll } from 'bun:test';
+import type { ServerWebSocket, Subprocess } from 'bun';
+
+// Integration test for the one thing about the pty sidecar that cannot be reasoned about from the code
+// alone: what happens to a live shell when officer goes away and comes back. It stands up a fake
+// registration socket, runs the real sidecar against it, then kills the socket and rebinds the same port
+// the way `pm2 restart officer` does.
+//
+// Nothing here touches the running officer — the sidecar dials API_URL, which is overridden per spawn.
+
+const SIDECAR = 'src/servers/sidecar/pty/index.mjs';
+
+type Frame = Record<string, any>;
+
+// One fake officer. `stop()` drops the socket; a new instance on the same port is the restart.
+function fakeOfficer(port?: number) {
+  let socket: ServerWebSocket<unknown> | null = null;
+  const frames: Frame[] = [];
+  const waiters: Array<{ match: (f: Frame) => boolean; resolve: (f: Frame) => void }> = [];
+
+  const server = Bun.serve({
+    port: port ?? 0,
+    fetch(req, srv) {
+      if (new URL(req.url).pathname === '/api/sidecar/register' && srv.upgrade(req)) return undefined;
+      return new Response('no', { status: 404 });
+    },
+    websocket: {
+      open(ws) {
+        socket = ws;
+      },
+      message(_ws, raw) {
+        const frame = JSON.parse(String(raw)) as Frame;
+        frames.push(frame);
+        for (let i = waiters.length - 1; i >= 0; i--) {
+          if (waiters[i]!.match(frame)) waiters.splice(i, 1)[0]!.resolve(frame);
+        }
+      },
+      close() {
+        socket = null;
+      },
+    },
+  });
+
+  return {
+    port: server.port,
+    send: (msg: Frame) => socket?.send(JSON.stringify(msg)),
+    /** Resolve on the first frame matching `match`, including ones already received. */
+    await: (match: (f: Frame) => boolean, timeoutMs = 15_000) =>
+      new Promise<Frame>((resolve, reject) => {
+        const seen = frames.find(match);
+        if (seen) return resolve(seen);
+        const timer = setTimeout(() => reject(new Error(`timed out waiting for a frame`)), timeoutMs);
+        waiters.push({
+          match,
+          resolve: (f) => {
+            clearTimeout(timer);
+            resolve(f);
+          },
+        });
+      }),
+    /** Output frames for one session, concatenated — the terminal's visible text. */
+    outputFor: (sessionId: string) =>
+      frames
+        .filter((f) => f.type === 'pty:output' && f.sessionId === sessionId)
+        .map((f) => f.data as string)
+        .join(''),
+    stop: () => server.stop(true),
+  };
+}
+
+const isOutput = (sessionId: string, needle: string) => (f: Frame) =>
+  f.type === 'pty:output' && f.sessionId === sessionId && String(f.data).includes(needle);
+
+let child: Subprocess | null = null;
+afterAll(() => child?.kill());
+
+describe('pty sidecar', () => {
+  test('a shell keeps streaming output after officer restarts under it', async () => {
+    const sessionId = 'test-restart';
+    let officer = fakeOfficer();
+    const port = officer.port;
+
+    child = Bun.spawn(['node', SIDECAR], {
+      env: {
+        ...process.env,
+        API_URL: `ws://127.0.0.1:${port}`,
+        // The sidecar now chooses the shell and the home itself, so the test pins both rather than
+        // spawning the owner's interactive zsh (which would read their rc files and their history).
+        SHELL: '/bin/sh',
+        HOME_DIR: '/tmp',
+        ENV: '/dev/null',
+      },
+      stdout: 'ignore',
+      stderr: 'ignore',
+    });
+
+    await officer.await((f) => f.type === 'register' && f.capabilities?.includes('terminal'));
+
+    // `~` is resolved by the sidecar against its own HOME_DIR, not by officer.
+    officer.send({ type: 'pty:init', id: 'i1', sessionId, config: { sessionId, cwd: '~', cols: 80, rows: 24 } });
+    await officer.await((f) => f.type === 'pty:ready' && f.sessionId === sessionId);
+
+    officer.send({ type: 'pty:input', id: 'in0', sessionId, data: 'pwd\n' });
+    await officer.await(isOutput(sessionId, '/tmp'));
+
+    officer.send({ type: 'pty:input', id: 'in1', sessionId, data: 'echo before-restart\n' });
+    await officer.await(isOutput(sessionId, 'before-restart'));
+
+    // ── the restart ──
+    officer.stop();
+    officer = fakeOfficer(port);
+    await officer.await((f) => f.type === 'register');
+
+    // The shell is the same process; only officer changed. Before the sendJson fix this input was
+    // accepted and executed, but its output went to the socket captured at init time and vanished.
+    officer.send({ type: 'pty:input', id: 'in2', sessionId, data: 'echo after-restart\n' });
+    await officer.await(isOutput(sessionId, 'after-restart'));
+
+    // Re-attaching replays the scrollback, marked as history so the client can rebuild rather than
+    // append — and it contains what happened on both sides of the restart.
+    officer.send({ type: 'pty:init', id: 'i2', sessionId, config: { sessionId, cols: 80, rows: 24 } });
+    const replay = await officer.await((f) => f.type === 'pty:replay' && f.sessionId === sessionId);
+    expect(String(replay.data)).toContain('before-restart');
+    expect(String(replay.data)).toContain('after-restart');
+
+    // Re-attach must not spawn a second shell, and must not replay as ordinary output.
+    await officer.await((f) => f.type === 'pty:ready' && f.id === 'i2');
+    expect(officer.outputFor(sessionId)).not.toContain('before-restart');
+
+    officer.send({ type: 'pty:close', id: 'c1', sessionId });
+    await officer.await((f) => f.type === 'pty:exit' && f.sessionId === sessionId);
+    officer.stop();
+  }, 30_000);
+});

src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx

@@ -218,6 +218,14 @@ export const TerminalView = ({
                 onCommandDoneRef.current(exitCode, output);
               }
             }
+          } else if (msg.type === 'replay') {
+            // Scrollback for a session we are re-attaching to. On a page load this terminal is empty and
+            // the reset is a no-op; after an officer restart it still holds what it had before the socket
+            // dropped, and the replay overlaps it — so rebuild from the sidecar's copy rather than append
+            // a second one. Deliberately outside the `output` branch: replay must not re-trigger the
+            // command/initial-input logic above.
+            term.reset();
+            term.write(msg.data);
           } else if (msg.type === 'exit') {
             processExited = true;
             term.write('\r\n[Process exited]\r\n');