diff --git a/apps/p2/docs/p2-notes.md b/apps/p2/docs/p2-notes.md index 4f3c58810..718145525 100644 --- a/apps/p2/docs/p2-notes.md +++ b/apps/p2/docs/p2-notes.md @@ -878,7 +878,8 @@ translated phone drawer and intercept chat taps. session switch, showing that session's terminals without starting one. Only explicit terminal-button/tab taps request auto-start; attachment updates replace the route, never push or reopen a later destination. Outdated hosts show Restart (ends all terminals), but idle outdated hosts are not automatically replaced yet; the - agent's calls to an outdated host fail with that advice (`OUTDATED_HOST`), and archiving cannot close its terminals. + agent's calls to an outdated host fail with that advice (`OUTDATED_HOST`); archiving closes its terminals with the + viewer protocol every host speaks (list, then attach and force-close each). Each terminal has a number in its session (`index`: the lowest free one from 1, kept while it lives), an optional name and an `agent` mark; tabs show the name, else the title, else "Terminal N", agent terminals with a "Pi" badge. Every connection that listed a session hears its list change (`terminals`: opened, closed, exited, renamed, by any @@ -918,6 +919,7 @@ translated phone drawer and intercept chat taps. the tool end to end (scripts, shared with the owner both ways, errors, guest turns, stale workers and foreign environments refused, archiving, `tool_search` without code mode); `terminal-tool-unit.test.ts`, `terminal-commands.test.ts`, `terminal-plain.test.ts` and `terminal-keys.test.ts` the pure parts; + `terminal-service.test.ts` closing a session (no host started; an outdated host's terminals closed as the panel does); `ghostty.test.ts` and `terminal-input.test.ts` cover the copied browser primitives. PTY tests skip when no daemon build exists (`npm run build:daemon` in `packages/env` builds a debug one). - **Search** (`plugins/search`): `SearchIndex` (server local, `search.sqlite`: root/subagent user/assistant text in FTS5 and diff --git a/apps/p2/plugins/terminals/service.ts b/apps/p2/plugins/terminals/service.ts index df4481110..2a5e7bb17 100644 --- a/apps/p2/plugins/terminals/service.ts +++ b/apps/p2/plugins/terminals/service.ts @@ -1,5 +1,6 @@ import { randomBytes } from "node:crypto"; import type { IncomingMessage } from "node:http"; +import type { Socket } from "node:net"; import type { Duplex } from "node:stream"; import { forbidden } from "@p2/app/v1"; import { WebSocket, WebSocketServer } from "ws"; @@ -10,9 +11,11 @@ import { terminalHostVersion, } from "./detached.ts"; import { + encodeFrame, type HostCall, type HostResult, MAX_BACKLOG, + readFrames, type TerminalEvent, type TerminalInfo, type TerminalRequest, @@ -295,11 +298,23 @@ export class TerminalService { return (await this.#calls).call(call, signal); } - /** Closes the session's terminals; without a running host there are none, and none is started. */ + /** + * Closes the session's terminals; without a running host there are none, and none is started. A host running older + * code does not know `closeSession`: it closes them as the panel does, attaching to each and closing it. + */ async closeSession(sessionId: string): Promise { - const probe = await connectRunningTerminalHost(this.directory); - if (!probe) return; - probe.destroy(); + const socket = await connectRunningTerminalHost(this.directory); + if (!socket) return; + try { + const events = hostEvents(socket); + const hello = await events.next( + (event): event is Extract => event.type === "hello", + ); + if (hello.version !== (await terminalHostVersion(this.app))) + return await closeAsViewer(socket, events, sessionId); + } finally { + socket.destroy(); + } await this.call({ op: "closeSession", sessionId }); } @@ -347,3 +362,64 @@ export class TerminalService { await new Promise((resolve) => this.#sockets.close(() => resolve())); } } + +/** How long closing a session's terminals on a host running older code may take. */ +const VIEWER_CLOSE_MS = 10_000; + +/** A host connection's events, in order: `next` waits for the first one after the last taken that `wanted` accepts. */ +type HostEvents = { next(wanted: (event: TerminalEvent) => event is E): Promise }; + +function hostEvents(socket: Socket): HostEvents { + const queue: TerminalEvent[] = []; + let wake = () => {}; + let ended = false; + readFrames(socket, (frame) => { + if (frame instanceof Uint8Array) return; + queue.push(frame as TerminalEvent); + wake(); + }); + socket.once("close", () => { + ended = true; + wake(); + }); + return { + async next(wanted) { + const deadline = Date.now() + VIEWER_CLOSE_MS; + for (;;) { + for (let event = queue.shift(); event; event = queue.shift()) if (wanted(event)) return event; + if (ended) throw new Error("Terminal host disconnected"); + const left = deadline - Date.now(); + if (left <= 0) throw new Error("The terminal host did not answer"); + await new Promise((resolve) => { + const timer = setTimeout(resolve, left); + wake = () => { + clearTimeout(timer); + resolve(); + }; + }); + } + }, + }; +} + +/** + * Closes the session's terminals with the viewer protocol every host version speaks: list them, then attach to each + * and force its close; resolves once the host reported each closed. + */ +async function closeAsViewer(socket: Socket, events: HostEvents, sessionId: string): Promise { + socket.write(encodeFrame({ type: "list", sessionId } satisfies TerminalRequest)); + const listed = await events.next( + (event): event is Extract => event.type === "list", + ); + const open = new Set(listed.terminals.map((terminal) => terminal.id)); + for (const terminalId of open) { + socket.write(encodeFrame({ type: "attach", terminalId } satisfies TerminalRequest)); + socket.write(encodeFrame({ type: "close", force: true } satisfies TerminalRequest)); + } + while (open.size > 0) { + const closed = await events.next( + (event): event is Extract => event.type === "closed", + ); + open.delete(closed.terminalId); + } +} diff --git a/apps/p2/test/p2/terminal-service.test.ts b/apps/p2/test/p2/terminal-service.test.ts new file mode 100644 index 000000000..73baeca0d --- /dev/null +++ b/apps/p2/test/p2/terminal-service.test.ts @@ -0,0 +1,83 @@ +// Closing an archived session's terminals (`TerminalService.closeSession`): no host is started when none runs, and a +// host running older code, which does not know `closeSession`, has them closed with the viewer protocol (list, then +// attach and force-close each). The old host here is a fake speaking that protocol. +import assert from "node:assert/strict"; +import { mkdtemp, rm } from "node:fs/promises"; +import { createServer, type Socket } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import { connectRunningTerminalHost, terminalHostAddress } from "../../plugins/terminals/detached.ts"; +import { encodeFrame, readFrames, type TerminalInfo, type TerminalRequest } from "../../plugins/terminals/protocol.ts"; +import { TerminalService } from "../../plugins/terminals/service.ts"; +import { APP } from "./helpers.ts"; + +const terminal = (id: string, sessionId: string): TerminalInfo => ({ + id, + sessionId, + index: 0, + cwd: "/", + env: null, + title: "", + createdAt: 0, + cols: 80, + rows: 24, +}); + +test("closing a session starts no host, and closes an outdated host's terminals as the panel does", async (t) => { + const directory = await mkdtemp(join(tmpdir(), "p2-terminal-service-")); + t.after(() => rm(directory, { recursive: true, force: true })); + const service = new TerminalService(directory, APP); + t.after(() => service.close()); + + // No host runs: nothing to close, and none is started. + await service.closeSession("s1"); + assert.equal(await connectRunningTerminalHost(directory), undefined); + + // A host running older code: it only knows the viewer requests. + const terminals = [terminal("a", "s1"), terminal("b", "s1"), terminal("c", "s2")]; + const requests: TerminalRequest[] = []; + const host = createServer((socket: Socket) => { + let attached: string | undefined; + socket.write(encodeFrame({ type: "hello", version: "older", running: terminals.length, pid: 1 })); + readFrames(socket, (frame) => { + const request = frame as TerminalRequest; + requests.push(request); + if (request.type === "list") + socket.write( + encodeFrame({ + type: "list", + terminals: terminals.filter((each) => each.sessionId === request.sessionId), + }), + ); + else if (request.type === "attach") attached = request.terminalId; + else if (request.type === "close" && request.force && attached) { + terminals.splice( + terminals.findIndex((each) => each.id === attached), + 1, + ); + socket.write(encodeFrame({ type: "closed", terminalId: attached })); + attached = undefined; + } else socket.write(encodeFrame({ type: "error", message: "Unknown request" })); + }); + }); + const address = await terminalHostAddress(directory); + await new Promise((resolve) => host.listen(address, resolve)); + t.after(() => new Promise((resolve) => host.close(() => resolve()))); + await service.closeSession("s1"); + assert.deepEqual(requests, [ + { type: "list", sessionId: "s1" }, + { type: "attach", terminalId: "a" }, + { type: "close", force: true }, + { type: "attach", terminalId: "b" }, + { type: "close", force: true }, + ]); + assert.deepEqual( + terminals.map((each) => each.id), + ["c"], + ); + // A session without terminals: one listing, nothing closed. + requests.length = 0; + await service.closeSession("s3"); + assert.deepEqual(requests, [{ type: "list", sessionId: "s3" }]); +});