diff --git a/README.md b/README.md index ad03631cd9..523f84bd0d 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,7 @@ bun run build bun test ``` -The suite runs against the live herdr server; there are no mocks. It's read-only apart from the `herdr-web-ui-test` workspaces it creates and deletes. It covers the generator freshness and determinism gate, the HTTP contract, the WS attach stream, roles (an observe connection cannot resize or type, enforced server-side), the status collector's pushed `pane-status` and `pane-exited` for unattached panes, token auth, the bind address, the herdr client, the PTY sidecar's env pass-through (`server/pty/session.test.ts`) and the pure client modules (key bar, draft, notifications, snapshot merge). At the time of writing that's 63 tests across 9 files. +The suite runs against the live herdr server; there are no mocks. It's read-only apart from the `herdr-web-ui-test` workspaces it creates and deletes. It covers the generator freshness and determinism gate, the HTTP contract, the WS attach stream, roles (an observe connection cannot resize or type, enforced server-side), concurrent attaches to one pane sharing a single pty, the status collector's pushed `pane-status` and `pane-exited` for unattached panes, token auth, the bind address, the herdr client, the PTY sidecar's env pass-through (`server/pty/session.test.ts`) and the pure client modules (key bar, draft, notifications, snapshot merge). At the time of writing that's 63 tests across 9 files. ## API diff --git a/server/api.contract.test.ts b/server/api.contract.test.ts index 5b55e8daf3..0a1f350650 100644 --- a/server/api.contract.test.ts +++ b/server/api.contract.test.ts @@ -370,6 +370,47 @@ describe("WebSocket roles and status push", () => { }, 40_000); }); +describe("WebSocket concurrent attach", () => { + /** A pane nobody holds yet: the race is in CREATING the attachment. */ + let raceWorkspaceId: string | null = null; + let racePaneId: string | null = null; + + beforeAll(async () => { + const created = await herdrRpc<{ workspace: { workspace_id: string }; root_pane: { pane_id: string } }>( + "workspace.create", + { label: "herdr-web-ui-test-race", cwd: "/tmp", focus: false }, + ); + raceWorkspaceId = created.workspace.workspace_id; + racePaneId = created.root_pane.pane_id; + }); + + afterAll(async () => { + if (raceWorkspaceId) await herdrRpc("workspace.close", { workspace_id: raceWorkspaceId }).catch(() => undefined); + }); + + it("shares one pty between clients whose attaches to the same pane race", async () => { + const paneId = racePaneId!; + const first = await RecordingSocket.connect(`ws://localhost:${server.port}/ws`); + const second = await RecordingSocket.connect(`ws://localhost:${server.port}/ws`); + try { + // same tick: both attaches are in flight before either resolves the terminal + first.send({ type: "attach", pane_id: paneId, cols: 100, rows: 30 }); + second.send({ type: "attach", pane_id: paneId, cols: 120, rows: 40 }); + await first.waitFor((m) => m.type === "pty-data" && m.pane_id === paneId, "first pty-data", 15_000); + await second.waitFor((m) => m.type === "pty-data" && m.pane_id === paneId, "second pty-data", 15_000); + + // one attachment means one client set: a resize reaches both. Two attachments + // would leave one client on an orphaned record that never hears it. + first.send({ type: "resize", pane_id: paneId, cols: 90, rows: 25 }); + await first.waitFor((m) => m.type === "pane-geometry" && m.cols === 90 && m.rows === 25, "first hears resize", 5000); + await second.waitFor((m) => m.type === "pane-geometry" && m.cols === 90 && m.rows === 25, "second hears resize", 5000); + } finally { + first.close(); + second.close(); + } + }, 40_000); +}); + /** * Bun's WebSocket client sends request headers, but this project compiles with lib.dom, * whose WebSocket type only knows subprotocols (bun-types steps aside via diff --git a/server/index.ts b/server/index.ts index 1177e9bb89..f02f0038d0 100644 --- a/server/index.ts +++ b/server/index.ts @@ -52,6 +52,8 @@ export function createServer( options: { port?: number; hostname?: string; token?: string } = {}, ): { port: number; hostname: string; stop: () => void } { const attachments = new Map(); + /** attachments still resolving their terminal, so concurrent attaches share one pty */ + const pendingAttachments = new Map>(); const clients = new Set(); const hostname = options.hostname ?? process.env["HOST"] ?? "0.0.0.0"; /** Empty token = gate disabled; every route then behaves exactly as it did before auth existed. */ @@ -104,10 +106,21 @@ export function createServer( broadcast(paneId, { type: "pane-geometry", pane_id: paneId, cols, rows }); } - async function ensureAttachment(paneId: string, cols: number, rows: number, forObserver: boolean): Promise { + function ensureAttachment(paneId: string, cols: number, rows: number, forObserver: boolean): Promise { const existing = attachments.get(paneId); - if (existing) return existing; + if (existing) return Promise.resolve(existing); + // a second attach arriving while the first is still resolving the terminal joins + // that creation: two creations would spawn two ptys, and the orphaned one keeps + // streaming into the surviving attachment and kills it when it exits + const pending = pendingAttachments.get(paneId); + if (pending) return pending; + + const created = spawnAttachment(paneId, cols, rows, forObserver).finally(() => pendingAttachments.delete(paneId)); + pendingAttachments.set(paneId, created); + return created; + } + async function spawnAttachment(paneId: string, cols: number, rows: number, forObserver: boolean): Promise { const { terminalId, rect } = await terminalInfoFor(paneId); // an observer-first attachment spawns at the pane's own grid (fallback 80x24 when // the layout has no rect for it): the attach must not seed the shared pty with a