Files
web-terminal/docs/plans/w2-pty-inject-queue.md

138 lines
15 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Server-side PTY-inject + idle-queued follow-up prompt
**Feature id:** `w2-pty-inject-queue` · **Branch base:** `develop`
A thin, Origin/CSRF-guarded HTTP route writes text into a live session's PTY, plus a **bounded per-session queue** whose head entry fires **once** when Claude next goes idle (Stop/SessionEnd), after a short **settle delay**. This is the unlocking primitive for templated launches, auto-continue, and issue-intake.
Grounding facts from the code I read:
- `writeInput(session, data)``src/session/session.ts:201` — no-op after PTY exit (L4); today called only from the WS input handler at `src/server.ts:876`.
- Idle is observable in the hook side-channel: the Stop/SessionEnd branch is `src/server.ts:414` (`if (ev.eventClass === 'Stop' || ev.eventClass === 'SessionEnd')`), immediately after `manager.handleHookEvent(...)` at `:412`. `handleHookEvent` (`src/session/manager.ts:223`) sets `session.claudeStatus` and broadcasts.
- `broadcast(session, msg)``src/session/session.ts:52` — fan-out to all `session.clients`.
- CSRF helper `requireAllowedOrigin(req, res)``src/server.ts:352`; per-IP `createRateLimiter(max, windowMs)``src/server.ts:109`; `isLoopback``src/server.ts:151`; `SESSION_ID_RE` (UUID v4, M7) — `src/protocol.ts:22`.
- Timer/hold precedent: `pendingApprovals` map + `setTimeout` live in **server.ts** (`:232`, `:459`), not the manager — the manager owns session state; the server owns wiring/timers. The queue follows the same split.
- Session shape `src/types.ts:201`; `LiveSessionInfo` `:246`; `ServerMessage` union `:109`; `SessionManager` iface `:314`. `timeline`/`stuckNotified`/`telemetry` are the precedent for "mutable runtime handle on immutable meta, replaced wholesale".
---
## Contract
### New HTTP routes (all in `src/server.ts`, registered near the `/live-sessions/:id/preview` block ~`:334`)
| Method / path | Guard | Body | Success | Errors |
|---|---|---|---|---|
| `POST /live-sessions/:id/queue` | `requireAllowedOrigin` (CSRF) + per-IP rate limit + `SESSION_ID_RE` check | `{ text: string, appendEnter?: boolean }` (`express.json({limit:'16kb'})`) | `200 { length }` | `400` bad id / empty text / non-string; `413` text > `QUEUE_ITEM_MAX_BYTES`; `404` unknown or exited session; `409` queue at `QUEUE_MAX_ITEMS`; `429` rate; `503` when `QUEUE_ENABLED=false` |
| `GET /live-sessions/:id/queue` | none (read-only, same threat model as `/live-sessions`) | — | `200 { length, items: string[] }` | `404` unknown |
| `DELETE /live-sessions/:id/queue` | `requireAllowedOrigin` | — | `200 { length: 0 }` (escape hatch: cancel all pending) | `404` unknown |
Not loopback-gated (unlike `/hook`): these are LAN-device actions, so Origin-guarded like `/open-in-editor` (`:384`).
### `src/types.ts` (coordination edit — the frozen shared-contract source)
- **`ServerMessage`** — add a variant so all attached devices see pending count live:
`| { type: 'queue'; length: number }`
- **`Session`** — add a mutable runtime handle (precedent: `timeline`):
`queue: readonly string[]` (verbatim byte strings, head fires first; replaced wholesale, never mutated in place).
- **`LiveSessionInfo`** — add optional `readonly queueLength?: number;` (additive/optional like `lastOutputAt` at `:261`) so `/live-sessions` and the manage grid show depth.
- **`SessionManager`** — add three methods:
- `enqueueFollowup(id: string, text: string): EnqueueResult`
- `drainOne(id: string): string | null` *(pops head, writes to PTY, broadcasts; null if none/exited)*
- `clearQueue(id: string): boolean`
- New result type:
`export type EnqueueResult = { ok: true; length: number } | { ok: false; reason: 'unknown' | 'full' | 'exited' };`
### `src/config.ts` env vars (add to `Config` in types.ts `:21` block **and** `loadConfig`)
| Env | Field | Default | Parser (existing helper) |
|---|---|---|---|
| `QUEUE_ENABLED` | `queueEnabled: boolean` | `true` | `parseBool` (`config.ts:89`) |
| `QUEUE_MAX_ITEMS` | `queueMaxItems: number` | `10` | `parseNonNegativeInt` (`:73`) |
| `QUEUE_ITEM_MAX_BYTES` | `queueItemMaxBytes: number` | `4096` | `parseNonNegativeInt` |
| `QUEUE_SETTLE_MS` | `queueSettleMs: number` | `1500` | `parseNonNegativeInt` |
### Client→server protocol
No new `ClientMessage`. Enqueue is HTTP POST (Origin-guarded), deliberately **not** a WS frame — it must survive "walked away, zero tabs open" and be usable from the manage page for any session, not just the WS-bound one. (Contrast: quick-reply chips send *immediately* over the active WS via `sendToActive`; the queue is *deferred* + cross-session, so HTTP is the right seam.)
---
## Files to change
| Path | Concrete change |
|---|---|
| `src/types.ts` | **Coordination edit.** Add `queue` to `Session`; `queueLength?` to `LiveSessionInfo`; `{type:'queue';length}` to `ServerMessage`; 4 `Config` fields; 3 `SessionManager` methods; `EnqueueResult` type. |
| `src/config.ts` | Parse the 4 new env vars in `loadConfig` (helpers already exist); include in returned `Config`. |
| `src/session/session.ts` | Init `queue: Object.freeze([])` in the `createSession` session literal (~`:137`, beside `timeline`). No other logic here — reuse existing `writeInput` (`:201`) and `broadcast` (`:52`). |
| `src/session/manager.ts` | Implement `enqueueFollowup` / `drainOne` / `clearQueue`; add `queueLength: s.queue.length` to `list()` map (`:184`); import `writeInput` from `./session.js` (add to the existing import at `:43`). Add the three names to the returned object (`:337`). |
| `src/server.ts` | Register the 3 routes; add a `QUEUE_RATE_MAX` const + a `createRateLimiter` instance (beside `:218`); in the Stop/SessionEnd branch (`:414`) call a new local `scheduleDrain(sessionId)` that debounces a `setTimeout(cfg.queueSettleMs)` (map `drainTimers: Map<string, Timeout>`, `.unref()`); on fire, re-check idle + stable `lastOutputAt`, then `manager.drainOne(id)`. Clear all `drainTimers` in `doShutdown` (`:930`). |
| `public/queue.ts` **(new)** | Tiny FE module: `enqueueFollowup(sessionId, text, appendEnter): Promise<Result>` — POSTs `/live-sessions/:id/queue` (same-origin), never throws; `clearQueue(sessionId)`; parse `{type:'queue'}` frames to update a badge. |
| `public/tabs.ts` | Handle the incoming `{type:'queue'}` frame (near the existing status/telemetry frame handling) to show a "N queued" badge on the tab; add an "Queue…" affordance (long-press on the quick-reply `+`, or a small button) that calls `enqueueFollowup(this.activeSessionId(), text)` instead of `sendToActive` (`:875`). |
| `public/manage.*` (grid) | Optional: render `queueLength` per card and a cancel (DELETE) button. |
---
## TDD steps (ordered — RED → GREEN, matching repo style)
1. **`src/types.ts`** — make the coordination edit first so everything compiles. Update **every** test `CFG` fixture that lists all Config fields (`test/manager.test.ts:47`, and the same literal in `test/session.test.ts`, plus any others — `grep -rl "worktreeTimeoutMs" test/`) to add the 4 new fields. (Compile gate, no assertion.)
2. **`test/config.test.ts`** (node) — RED: assert defaults (`queueEnabled=true`, `queueMaxItems=10`, `queueItemMaxBytes=4096`, `queueSettleMs=1500`), env overrides parse, and invalid (`QUEUE_MAX_ITEMS=-1`) throws (fail-fast, like existing `parseNonNegativeInt` tests). → GREEN in `src/config.ts`.
3. **`test/manager.test.ts`** (node, node-pty mocked via `createMockPty`) — RED then GREEN in `src/session/manager.ts`:
- `enqueueFollowup` appends → returns `{ok:true,length:1}` and **broadcasts** `{type:'queue',length:1}` to a stub `WebSocketLike` (assert `ws.send` payload via `serialize`). Second call → `length:2`.
- Cap: with `queueMaxItems:2`, third enqueue → `{ok:false,reason:'full'}`, no broadcast, queue unchanged (immutability).
- Unknown id → `{ok:false,reason:'unknown'}`.
- `drainOne` on a 2-item queue → returns head string, asserts **`mockPty.write` called with that exact string**, queue now length 1, broadcasts `{type:'queue',length:1}`.
- `drainOne` empty queue → `null`, no write. Exited session (`session.exitedAt` set) → `null` (double-guards L4).
- `clearQueue` → empties + broadcasts `length:0`, returns true.
- `list()` includes `queueLength`.
*(Use the existing mock-pty `write` spy pattern from `test/session.test.ts:364`.)*
4. **`test/integration/queue.test.ts`** (new, node, real `startServer` + `fetch`, PTY-gated with the `itPty` helper at `server.test.ts:51`) — RED then GREEN for the routes in `src/server.ts`:
- `POST /live-sessions/:id/queue``403` foreign Origin; `403`/missing Origin default-deny (mirror `server.test.ts:770`).
- Allowed Origin, malformed id → `400`; empty `text``400`; `text` of `queueItemMaxBytes+1``413`; unknown session → `404`; over rate → `429`.
- Happy path on a **real attached** session (open WS, attach, capture `sessionId`): `200 {length:1}`, then `GET /live-sessions` shows `queueLength:1`; `DELETE …/queue``queueLength:0`.
- **Idle-drain wiring** (`itPty` + `vi.useFakeTimers`): attach real session, enqueue `"echo QUEUED_MARKER\r"`, POST `/hook` (loopback) with `{hook_event_name:'Stop', ...}` and header `x-webterm-session`, advance timers past `queueSettleMs`, assert the client WS receives an `output` frame containing `QUEUED_MARKER` (the shell echoes it). Also assert a *second* enqueue does **not** fire until the next Stop (one-per-idle pacing).
- Settle guard: enqueue, POST Stop, then before `queueSettleMs` push a `/hook` event that produces output (changes `lastOutputAt`) → advance timers → assert **no** drain (Claude still active). *(This targets the `scheduleDrain` cursor check.)*
5. **`test/queue.test.ts`** (new, **jsdom**, mocked `fetch`) — RED then GREEN in `public/queue.ts`:
- `enqueueFollowup` POSTs to the right URL/body, returns parsed result; on non-2xx returns `{ok:false}` and **never throws**; on network reject returns `{ok:false}` (mirrors quick-reply's never-throw discipline, `quick-reply.ts:99`).
- A `{type:'queue',length:3}` frame → badge helper returns/sets 3.
6. **`test/tabs.test.ts`** — extend: a `{type:'queue',length:N}` server frame updates the active tab's badge; the enqueue affordance calls `enqueueFollowup` with `activeSessionId()` (not `sendToActive`).
**Coverage:** manager + config + routes are node-testable deterministically (queue mutation, caps, broadcasts, drain, rate/Origin/validation all hit without a real PTY). The FE module is small and fully jsdom-mockable. Only the real echo-through-PTY assertion is `itPty`-gated (auto-skips in sandbox, runs in CI) — keeps the 80% gate.
---
## Edge cases & failure modes
- **Idle flapping / repeated Stop:** `scheduleDrain` **debounces** — clears any existing `drainTimers` entry and restarts the settle timer on each Stop; only fires once the window elapses.
- **New output during settle window:** capture `outputCursor = session.lastOutputAt` at schedule time; on timer fire, drain **only if** `session.lastOutputAt === outputCursor` **and** `claudeStatus === 'idle'` **and** `exitedAt === null`. Otherwise skip (a later genuine Stop reschedules). Prevents injecting mid-render.
- **One-per-idle pacing (intended):** `drainOne` fires exactly one entry; the injected prompt makes Claude work again → its next Stop drains the next entry. If Claude errors and never emits Stop, remaining items **wait** (no spamming).
- **Session exits with items queued:** `drainOne` guards on `exitedAt` (returns null); on session removal (`onSessionExit` L2 / `killById`) the queue dies with the session. Server clears its `drainTimers` entry in `doShutdown`; stale timers are harmless (drain returns null) and `.unref()`ed.
- **Queue full → `409`** (never silently drop). **Oversized text → `413`.** Both actionable to the caller.
- **Concurrent enqueue from two devices:** single-threaded, immutable array replace → both land; `{type:'queue'}` broadcast keeps every device's badge consistent.
- **Enqueue to exited/unknown session → `404`** (checked before append).
- **`QUEUE_ENABLED=false`** → routes `503` (graceful disable, like `/push/vapid-key:478`); Stop branch skips scheduling.
- **Verbatim bytes / Enter:** queue stores the exact string (byte-shuttle invariant). FE decides `appendEnter` (append `\r`) at enqueue time, mirroring quick-reply's `appendEnter` (`quick-reply.ts:24`). No server-side text parsing.
---
## Security
- **CSRF:** `POST`/`DELETE …/queue` are state-changing and cause **shell input**, so they carry `requireAllowedOrigin` (`:352`) — the same guard as the DELETE-session routes. Without it a foreign page could inject commands into a running Claude. `GET` is read-only (queue length + prompt text the user themselves queued) → no guard, consistent with `/live-sessions`.
- **Not loopback-gated:** intentionally Origin-gated (LAN device), not `isLoopback``/hook` is loopback (host-only) but enqueue must work from the phone.
- **Path/ID containment:** validate `:id` against `SESSION_ID_RE` (`protocol.ts:22`) before any Map lookup or PTY write; reject non-UUID with `400`. The id is only a Map key — never touches argv/fs.
- **Input validation at the boundary:** `text` must be a non-empty `string`; byte length (`Buffer.byteLength`) ≤ `queueItemMaxBytes` → else `413`. `appendEnter` coerced to boolean. Body capped by `express.json({limit:'16kb'})`. Bytes are passed **verbatim** to the PTY (raw keyboard bytes — do not filter content, per the protocol rule), but bounded in size and count.
- **Rate limit:** dedicated per-IP `createRateLimiter(QUEUE_RATE_MAX, RATE_LIMIT_WINDOW_MS)` (e.g. 20/min) → `429`, matching the `DECISION_RATE_MAX`/`SUBSCRIBE_RATE_MAX` pattern (`:75`). Bounds injection-flood risk.
- **DoS bounds:** `queueMaxItems` caps depth; `queueItemMaxBytes` caps size; drain writes one entry per idle → no unbounded PTY write burst.
- **No capability tokens needed** — this reuses the app's existing LAN/Origin trust boundary (same as every other control route); it does **not** widen it. No secrets logged; sanitize any queued text before logging via existing `sanitizeForLog` (`:162`).
- **Loopback drain source:** the drain trigger is the Stop hook, which is already loopback-gated at `/hook` (`:399`) — so the *timing* signal can't be forged remotely; only the *content* (Origin-gated) and it fires against the caller's own session.
---
## Effort & dependencies
- **Rough effort:** ~23 dev-days. Backend (types + config + manager methods + 3 routes + settle-timer wiring) ~1.5 d incl. tests; FE (`public/queue.ts` + tabs badge/affordance) ~0.51 d.
- **Depends on:** nothing new — builds entirely on shipped primitives (`writeInput`, `broadcast`, `handleHookEvent` idle branch, `requireAllowedOrigin`, `createRateLimiter`, `LiveSessionInfo`). No schema migrations, no new deps.
- **Coordination:** the `src/types.ts` edit is the only cross-cutting change — freeze it first (it touches `Config`, so every all-fields test `CFG` fixture must be updated in the same commit).
- **Unlocks (roadmap):** templated launches / auto-continue (queue a follow-up prompt on kickoff), issue-intake (external POST that enqueues), and any "when Claude finishes, do X" automation. The manage-page `queueLength` surface also feeds the multi-session workbench view.