138 lines
15 KiB
Markdown
138 lines
15 KiB
Markdown
# 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:** ~2–3 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.5–1 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. |