From b92e8dffade1c44e7b8194b5f55d5bec7b62ba0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 30 Jul 2026 04:59:06 +0000 Subject: [PATCH 1/3] keep terminals alive when officer restarts under them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the pty sidecar registered `term.onData` with the socket that happened to be live when the session was created. officer is a pm2 peer that restarts constantly, and every restart hands this process a brand new socket, so every pre-existing session went on writing to a closed one — where sendJson's readyState check dropped it silently. the shell survived and still accepted input, because input arrives on the new socket, but nothing ever came back. you typed and the terminal sat there. the only way out was to close the panel, which orphaned the shell. sendJson now reads the module-level socket at send time instead of taking one as an argument, so there is no socket to capture and go stale. that is the whole fix. the scrollback replay on re-attach becomes its own event, pty:replay -> 'replay' on the browser socket. it used to arrive as ordinary output, which was fine for a page load (fresh xterm) but not for a restart: the browser keeps its terminal, so replaying blind printed a second copy of everything still on screen. marked as history, Terminal.tsx resets and rebuilds from the sidecar's 50KB buffer instead. it also stays out of the `output` branch so it cannot re-trigger the command / initial-input logic that scrapes output for a sentinel. added an integration test, because this is a reconnect bug and nothing short of an actual reconnect proves it: it stands up a fake registration socket, runs the real sidecar against it, echoes into a real shell, kills the socket, rebinds the same port the way pm2 does, and asserts output still flows. verified it fails against the old sendJson (times out after 15s waiting for the post-restart echo) and passes in ~400ms with the fix. it never touches the running officer — the sidecar dials API_URL, overridden per spawn. Co-Authored-By: Claude Opus 4.8 --- src/servers/api/terminal/pty-sidecar.mjs | 37 +++-- src/servers/api/terminal/pty-sidecar.test.ts | 137 ++++++++++++++++++ src/servers/api/terminal/websocket.ts | 37 ++--- src/servers/sidecar/protocol.ts | 3 + .../officerdev/src/apps/Terminal/Terminal.tsx | 8 + 5 files changed, 186 insertions(+), 36 deletions(-) create mode 100644 src/servers/api/terminal/pty-sidecar.test.ts diff --git a/src/servers/api/terminal/pty-sidecar.mjs b/src/servers/api/terminal/pty-sidecar.mjs index c09d2999..f2b4f5a1 100644 --- a/src/servers/api/terminal/pty-sidecar.mjs +++ b/src/servers/api/terminal/pty-sidecar.mjs @@ -41,9 +41,17 @@ 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 { @@ -86,7 +94,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 +104,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,7 +125,7 @@ async function handleCommand(ws, msg) { } } - sendJson(ws, { type: 'pty:ready', id: msg.id, sessionId }); + sendJson({ type: 'pty:ready', id: msg.id, sessionId }); return; } @@ -163,8 +174,8 @@ async function handleCommand(ws, msg) { }); } 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; } @@ -179,16 +190,16 @@ async function handleCommand(ws, msg) { 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 +262,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 +274,7 @@ function connect() { return; } - handleCommand(ws, msg); + handleCommand(msg); } catch { // skip malformed messages } diff --git a/src/servers/api/terminal/pty-sidecar.test.ts b/src/servers/api/terminal/pty-sidecar.test.ts new file mode 100644 index 00000000..ba774e01 --- /dev/null +++ b/src/servers/api/terminal/pty-sidecar.test.ts @@ -0,0 +1,137 @@ +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/api/terminal/pty-sidecar.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}` }, + stdout: 'ignore', + stderr: 'ignore', + }); + + await officer.await((f) => f.type === 'register' && f.capabilities?.includes('terminal')); + + // A plain non-interactive shell: no rc files, no prompt, so the assertions below are about output + // the test itself caused. + officer.send({ + type: 'pty:init', + id: 'i1', + sessionId, + config: { + sessionId, + host: true, + shell: { command: '/bin/sh', args: [] }, + cwd: '/tmp', + homeDir: '/tmp', + cols: 80, + rows: 24, + }, + }); + await officer.await((f) => f.type === 'pty:ready' && f.sessionId === sessionId); + + 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, host: true, 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/servers/api/terminal/websocket.ts b/src/servers/api/terminal/websocket.ts index 682059b3..045fe9e0 100644 --- a/src/servers/api/terminal/websocket.ts +++ b/src/servers/api/terminal/websocket.ts @@ -16,8 +16,7 @@ type WSData = { type BridgeSession = { client: ServerWebSocket; sessionId: string; - unsubOutput: (() => void) | null; - unsubExit: (() => void) | null; + unsubs: Array<() => void>; }; const sessions = new Map, BridgeSession>(); @@ -67,28 +66,22 @@ export const terminalWebsocket = { rows: ws.data.rows, }; - // Subscribe to events for this session - const unsubOutput = on('pty:output', (msg) => { - if (msg.type === 'pty:output' && msg.sessionId === sessionId) { + // 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: 'output', data: msg.data })); + ws.send(JSON.stringify({ type: clientType, data: 'data' in msg ? msg.data : undefined })); } 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 }; + const session: BridgeSession = { + client: ws, + sessionId, + unsubs: [relay('pty:output', 'output'), relay('pty:replay', 'replay'), relay('pty:exit', 'exit')], + }; sessions.set(ws, session); // Send init command to PTY sidecar @@ -97,8 +90,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); } }, @@ -145,8 +137,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/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 2386e42a..522df93d 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -153,4 +153,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/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'); From 1824f53c89f709ce73c35d9388ca39e4fe1a5460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 30 Jul 2026 05:02:10 +0000 Subject: [PATCH 2/3] let the pty sidecar decide what shell it runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit officer built the whole PtyInitConfig: it read the owner's SHELL (defaulting to /bin/zsh), added `-i`, read their HOME, expanded `~` against it, and hardcoded `host: true`. none of that is a proxy's business — the sidecar is the process that calls pty.spawn, so it is the one that should know what to spawn and where. the config now carries only what the bridge actually knows: sessionId, the folder the panel was opened on, and the client's cols/rows. shell, args, home and cwd resolution moved into the sidecar. home comes from HOME_DIR ?? HOME, mirroring data-path.ts:getOwnerHomeDir — terminal was the one host-executing surface reading process.env.HOME directly, which is identical here and divergent anywhere HOME_DIR is set to something else. deleted the bwrap sandbox branch rather than moving it. it was selected by `config.host`, which officer hardcoded to true, so it never ran — and it expected `shell` to contain a fully-built bwrap command that nothing on either side ever built. it could not have worked. a terminal here is the owner's own shell on the owner's own machine by design (platform/CLAUDE.md), so there is no jail to preserve. its ensureUserFiles half duplicated api/users/provision.ts:seedShellConfigs, which is the live seeder of those same templates and stays. also deleted the 'cwd' handler that turned a message into `cd \r` typed at the shell. no frontend has ever sent that message — the browser composes its own cd — so it was unreachable, and synthesizing keystrokes is not something a relay should do. the integration test pins SHELL and HOME_DIR now that the sidecar reads them, and asserts the shell starts in the resolved `~` rather than officer having resolved it. Co-Authored-By: Claude Opus 4.8 --- src/servers/api/terminal/pty-sidecar.mjs | 113 +++++-------------- src/servers/api/terminal/pty-sidecar.test.ts | 33 +++--- src/servers/api/terminal/websocket.ts | 36 ++---- src/servers/sidecar/protocol.ts | 9 +- 4 files changed, 57 insertions(+), 134 deletions(-) diff --git a/src/servers/api/terminal/pty-sidecar.mjs b/src/servers/api/terminal/pty-sidecar.mjs index f2b4f5a1..72cc72d6 100644 --- a/src/servers/api/terminal/pty-sidecar.mjs +++ b/src/servers/api/terminal/pty-sidecar.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,7 +22,31 @@ 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 ── @@ -59,32 +69,6 @@ const sendJson = (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) { @@ -130,47 +114,18 @@ async function handleCommand(msg) { } // 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'; @@ -179,13 +134,7 @@ async function handleCommand(msg) { 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) => { diff --git a/src/servers/api/terminal/pty-sidecar.test.ts b/src/servers/api/terminal/pty-sidecar.test.ts index ba774e01..000de990 100644 --- a/src/servers/api/terminal/pty-sidecar.test.ts +++ b/src/servers/api/terminal/pty-sidecar.test.ts @@ -81,31 +81,28 @@ describe('pty sidecar', () => { const port = officer.port; child = Bun.spawn(['node', SIDECAR], { - env: { ...process.env, API_URL: `ws://127.0.0.1:${port}` }, + 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')); - // A plain non-interactive shell: no rc files, no prompt, so the assertions below are about output - // the test itself caused. - officer.send({ - type: 'pty:init', - id: 'i1', - sessionId, - config: { - sessionId, - host: true, - shell: { command: '/bin/sh', args: [] }, - cwd: '/tmp', - homeDir: '/tmp', - cols: 80, - rows: 24, - }, - }); + // `~` 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')); @@ -121,7 +118,7 @@ describe('pty sidecar', () => { // 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, host: true, cols: 80, rows: 24 } }); + 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'); diff --git a/src/servers/api/terminal/websocket.ts b/src/servers/api/terminal/websocket.ts index 045fe9e0..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'; @@ -34,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; @@ -54,17 +46,10 @@ 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 = { - 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, - }; + // 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) => @@ -118,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 diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 522df93d..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) From ab261cf52dd8a9e95b240c758fee656de69913a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 30 Jul 2026 05:03:03 +0000 Subject: [PATCH 3/3] move the pty sidecar under src/servers/sidecar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit it was the only sidecar living outside src/servers/sidecar/ — it sat in api/terminal/ next to the bridge that talks to it, which is the one place a reader looking for "the sidecars" would not check. now src/servers/sidecar/pty/index.mjs, matching every peer, with a note on the pm2 entry about why this one is node and .mjs (node-pty is a native addon) rather than bun and typescript like the rest. the templates/ directory went to api/users/, next to provision.ts:seedShellConfigs, which is now its only consumer — the sidecar's duplicate seeder went with the sandbox branch in the previous commit. api/terminal/ is left holding exactly one thing: the websocket bridge. no behaviour change. the pm2 entry's script path changed, so `pm2 restart officer-pty` is not enough — pm2 remembers the old path until the entry is deleted and started again. commands are in SIDECAR_WORK_LOG.md. Co-Authored-By: Claude Opus 4.8 --- ecosystem.config.cjs | 5 ++++- src/servers/api/users/provision.ts | 2 +- src/servers/api/{terminal => users}/templates/.tmux.conf | 0 src/servers/api/{terminal => users}/templates/.zshenv | 0 src/servers/api/{terminal => users}/templates/.zshrc | 0 .../api/{terminal => users}/templates/starship-officer.toml | 0 .../{api/terminal/pty-sidecar.mjs => sidecar/pty/index.mjs} | 0 .../pty-sidecar.test.ts => sidecar/pty/index.test.ts} | 2 +- 8 files changed, 6 insertions(+), 3 deletions(-) rename src/servers/api/{terminal => users}/templates/.tmux.conf (100%) rename src/servers/api/{terminal => users}/templates/.zshenv (100%) rename src/servers/api/{terminal => users}/templates/.zshrc (100%) rename src/servers/api/{terminal => users}/templates/starship-officer.toml (100%) rename src/servers/{api/terminal/pty-sidecar.mjs => sidecar/pty/index.mjs} (100%) rename src/servers/{api/terminal/pty-sidecar.test.ts => sidecar/pty/index.test.ts} (98%) 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/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/api/terminal/pty-sidecar.mjs b/src/servers/sidecar/pty/index.mjs similarity index 100% rename from src/servers/api/terminal/pty-sidecar.mjs rename to src/servers/sidecar/pty/index.mjs diff --git a/src/servers/api/terminal/pty-sidecar.test.ts b/src/servers/sidecar/pty/index.test.ts similarity index 98% rename from src/servers/api/terminal/pty-sidecar.test.ts rename to src/servers/sidecar/pty/index.test.ts index 000de990..8ed7af62 100644 --- a/src/servers/api/terminal/pty-sidecar.test.ts +++ b/src/servers/sidecar/pty/index.test.ts @@ -8,7 +8,7 @@ import type { ServerWebSocket, Subprocess } from 'bun'; // // Nothing here touches the running officer — the sidecar dials API_URL, which is overridden per spawn. -const SIDECAR = 'src/servers/api/terminal/pty-sidecar.mjs'; +const SIDECAR = 'src/servers/sidecar/pty/index.mjs'; type Frame = Record;