diff --git a/ecosystem.config.cjs b/ecosystem.config.cjs index 15c25a28..7a44b4e9 100644 --- a/ecosystem.config.cjs +++ b/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, }, { diff --git a/src/servers/api/terminal/websocket.ts b/src/servers/api/terminal/websocket.ts index 682059b3..97dcb95e 100644 --- a/src/servers/api/terminal/websocket.ts +++ b/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; sessionId: string; - unsubOutput: (() => void) | null; - unsubExit: (() => void) | null; + unsubs: Array<() => void>; }; const sessions = new Map, BridgeSession>(); @@ -35,13 +33,6 @@ const sendOutput = (ws: ServerWebSocket, 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) { 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 \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) { 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); } diff --git a/src/servers/api/users/provision.ts b/src/servers/api/users/provision.ts index 69b70b4e..9cdcebcf 100644 --- a/src/servers/api/users/provision.ts +++ b/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; diff --git a/src/servers/api/terminal/templates/.tmux.conf b/src/servers/api/users/templates/.tmux.conf similarity index 100% rename from src/servers/api/terminal/templates/.tmux.conf rename to src/servers/api/users/templates/.tmux.conf diff --git a/src/servers/api/terminal/templates/.zshenv b/src/servers/api/users/templates/.zshenv similarity index 100% rename from src/servers/api/terminal/templates/.zshenv rename to src/servers/api/users/templates/.zshenv diff --git a/src/servers/api/terminal/templates/.zshrc b/src/servers/api/users/templates/.zshrc similarity index 100% rename from src/servers/api/terminal/templates/.zshrc rename to src/servers/api/users/templates/.zshrc diff --git a/src/servers/api/terminal/templates/starship-officer.toml b/src/servers/api/users/templates/starship-officer.toml similarity index 100% rename from src/servers/api/terminal/templates/starship-officer.toml rename to src/servers/api/users/templates/starship-officer.toml diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 2386e42a..f9177f72 100644 --- a/src/servers/sidecar/protocol.ts +++ b/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; }; // 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 }; diff --git a/src/servers/api/terminal/pty-sidecar.mjs b/src/servers/sidecar/pty/index.mjs similarity index 58% rename from src/servers/api/terminal/pty-sidecar.mjs rename to src/servers/sidecar/pty/index.mjs index c09d2999..72cc72d6 100644 --- a/src/servers/api/terminal/pty-sidecar.mjs +++ b/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} */ +// ── 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} */ 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 } diff --git a/src/servers/sidecar/pty/index.test.ts b/src/servers/sidecar/pty/index.test.ts new file mode 100644 index 00000000..8ed7af62 --- /dev/null +++ b/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; + +// One fake officer. `stop()` drops the socket; a new instance on the same port is the restart. +function fakeOfficer(port?: number) { + let socket: ServerWebSocket | 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((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); +}); diff --git a/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx b/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx index 97af268c..8e4963f8 100644 --- a/src/workspaces/officerdev/src/apps/Terminal/Terminal.tsx +++ b/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');