keep terminals alive when officer restarts under them
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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<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}` },
|
||||
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);
|
||||
});
|
||||
@@ -16,8 +16,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>();
|
||||
@@ -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<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);
|
||||
}
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user