From 0e334f9cac6726af66080ca70225c6a495dadc79 Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Sat, 30 May 2026 18:50:44 +1000 Subject: [PATCH] docs(plan): A-path TUI-mode implementation plan + maintainer decisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing-plans output (PR-0..PR-3) grounded against real code; plan-review verdict ready-with-fixes (all code anchors verified accurate to the line). Plan covers: PR-0 TUI-only ISOLATION seed (default path byte-for-byte unchanged), PR-1 transcript reader (dual-signal completion, no quiescence in v1, string adapted to IR [{delta},{stop}] chunks in PR-3 branch — fits both getOrCompute array + getOrComputeStreaming consumers with zero server edits), PR-2 tmux session driver (T6 flags, T3 submit recipe, reaper), PR-3 CLAUDE_TUI_MODE wiring + ADR 0016 + README. Plus cross-cutting contracts, B-gate spike track (T2/T4/T5, parallel, non-blocking for A), PI231 test strategy (scratch only, prod :4567 untouched), jaekwon-park co-author. Maintainer decisions appended resolving plan-review P1-P5 + open questions: - P1 (PR-2 interface blocker): reuse isolationCtx.ephemeralRoot+reqId — no default-path call-site edits, preserves unchanged invariant - P2: reaper boot anchor = server.mjs:2417 isMain block (not :2334) - P3: A also uses --strict-mcp-config (defense-in-depth); preflight /mcp advisory-at-startup for A, hard-gate for B - P5: wall-clock cap = env CLAUDE_TUI_WALLCLOCK_MS default 120000 (config not constant, T5-tunable) - warm-pool + large-paste explicitly scoped OUT of initial A deliverable Plan ready to implement: PR-0 -> PR-1 -> PR-2 -> PR-3. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-30-tui-mode-A-path-implementation-plan.md | 435 ++++++++++++++++++ 1 file changed, 435 insertions(+) create mode 100644 docs/superpowers/plans/2026-05-30-tui-mode-A-path-implementation-plan.md diff --git a/docs/superpowers/plans/2026-05-30-tui-mode-A-path-implementation-plan.md b/docs/superpowers/plans/2026-05-30-tui-mode-A-path-implementation-plan.md new file mode 100644 index 0000000..f85d567 --- /dev/null +++ b/docs/superpowers/plans/2026-05-30-tui-mode-A-path-implementation-plan.md @@ -0,0 +1,435 @@ +# TUI-mode — Deployment-A Implementation Plan (PR-0 … PR-3) + +- **Date:** 2026-05-30 +- **Status:** Implementation plan (pre-code). Derived verbatim from the final design spec + `docs/superpowers/specs/2026-05-30-tui-mode-production-design.md` (3 review passes + spikes S1/S2/S3 + pre-code gates T1/T3/T6). **Decisions in the spec are NOT re-litigated here.** +- **Scope:** **Deployment A only** (single-user / OCP canary). Deployment B (multi-tenant) is DEFERRED behind spikes **T2** (body-capture `tools:[]`) + **T4** (concurrency). B's gating hooks (`--tools ""`, `--strict-mcp-config`, `--disallowedTools "mcp__*"`, per-spawn MCP-disable verification) are **wired in PR-2 but B is not enabled** — no per-key guest path ships in this plan. +- **Authority of record (to be created in PR-3):** ADR 0016 (or ADR 0009 Amendment 2) — see PR-3. +- **Iron Rules in force:** 10 (independent reviewer), 11 (minimum reviewable unit — one PR per layer), 12 (prior-art search done = the spikes). `ALIGNMENT.md` Rule 1 (cite authority) + Rule 2 (no inventing CLI behavior) + Rule 5 (release-kit). +- **Author credit (binding, §13):** every implementing commit carries `Co-Authored-By: jaekwon-park <…>` (pull the real email/handle from OCP PR #101 before committing — do NOT invent). ADR 0016 names PR #101 + jaekwon-park in its acknowledgment section. Add jaekwon-park to CONTRIBUTORS and notify on PR #101 at ship time. + +--- + +## 0. Ground-truth code anchors (verified against the real tree) + +Everything below cites the exact function/line the change hooks into. Re-verify line numbers at edit time (the files churn). + +| Surface | Location (verified) | Role in TUI-mode | +|---|---|---| +| `spawn(irRequest, authContext, isolationCtx)` (public contract) | `lib/providers/anthropic.mjs:1164` → delegates to `_spawnAndStream` | **PR-3** branches here on `CLAUDE_TUI_MODE`. Default falls through to `_spawnAndStream` (stream-json) UNCHANGED. | +| `_spawnAndStream(irRequest, authContext, spawnImpl, isolationCtx)` | `anthropic.mjs:872` | The default transport. **Not modified** by TUI-mode (PR-3 adds a sibling branch in the public `spawn`, it does not touch `_spawnAndStream`). | +| `buildCliArgs(model, systemPrompt)` | `anthropic.mjs:834` (returns `--model … --output-format stream-json --verbose --no-session-persistence --system-prompt …`) | TUI driver builds its **own** argv (no `-p`, no `--output-format`); it does NOT reuse `buildCliArgs`. Cited as the contrast surface. | +| `extractSystemPrompt(irRequest)` | `anthropic.mjs:123` (always prefixes `OLP_SYSTEM_PROMPT_WRAPPER` `:109`) | **REUSED unchanged** by the TUI driver to compute the `--system-prompt` value. | +| `irToAnthropic(irRequest)` | `anthropic.mjs:601` (serializes user/assistant/tool; skips `system`) | **REUSED unchanged** — produces the prompt body text the TUI driver writes to the prompt file (§6 recipe). | +| `ISOLATION` named export | `anthropic.mjs:1667` (`ephemeralEnvOverrides`→`{HOME}`, `credentialMounts`, `requiredHomePaths:['.claude']`, `hasInnerSandbox:false`) | **PR-0** EXTENDS with a TUI-only seed hook. | +| `prepareIsolatedEnvironment({provider,keyId,reqId})` | `lib/sandbox/manager.mjs:203` → returns `{ephemeralRoot, envOverrides, hardenedArgs, wrapForLayer3, cleanup}` | **PR-0** consumes the new seed step; **PR-2** driver calls it to get `ephemeralRoot`. Note the **test bypass at `:223`** (returns `_legacyShape()` under `test-features.mjs` unless `globalThis.__OLP_FORCE_ISOLATION_IN_TEST`). | +| Buffered spawn call site | `server.mjs:1347` (`prepareIsolatedEnvironment`) → `:1355` (`for await … hopProviderPlugin.spawn(...)`) inside `collectAllChunks()` (`:1299`); result cached via `cacheStore.getOrCompute(keyId, hopCacheKey, collectAllChunks)` at `:1445` | **computeFn returns an ARRAY of IR chunks.** TUI transport must yield `[{type:'delta',role:'assistant',content},{type:'stop',finish_reason:'stop'}]` so this path is unchanged. | +| Streaming spawn call site | `server.mjs:1564` (`prepareIsolatedEnvironment`) → `:1570` (`for await … streamPlugin.spawn(...)`) inside `sourceWithRelease()`; coordinated via `cacheStore.getOrComputeStreaming(keyId, streamCacheKey, sourceFactory, …)` at `:1587` | **sourceFactory returns an ASYNC GENERATOR of IR chunks.** TUI transport yields the same 2-chunk shape → SSE replay (`irChunkToOpenAISSE` at `server.mjs:1764`) is byte-identical to the stream-json path. This is the §3.1 single-buffered-then-replay mechanism. | +| `irChunkToOpenAISSE`, `SSE_DONE` | imported `server.mjs:38`; used `:1764`, `:1772` | **REUSED unchanged** for `stream:true` replay. | +| `max_tokens` parse | `lib/ir/openai-to-ir.mjs:182` (sets `ir.max_tokens`) | Accepted into IR, **dropped at CLI boundary** (§4.5). Same for `temperature` `:190`, `top_p` `:198`, `stop` `:206` (§4.6). | +| `validateKey` / `owner_tier` / `providers_enabled` | `lib/keys.mjs:414`; tiers `'owner'|'guest'|'anonymous'` (`:428`,`:463`) | **REUSED unchanged.** A's canary runs owner-tier. B's guest gating is wired but inert. | + +**Cache contract crux (load-bearing for PR-1).** `server.mjs` does NOT expect a string from the transport. It expects **IR chunks** — an array (buffered, `getOrCompute`) or an async generator (streaming, `getOrComputeStreaming`). The TUI transcript reader (PR-1) resolves a **single string**; the TUI driver/provider-branch (PR-2/PR-3) is responsible for the thin adapter `string → [delta, stop]` so both existing cache paths consume it with **zero modification**. This is the concrete meaning of spec §3.2 "returns a resolved response string adapted to the getOrCompute/singleflight cache contract." + +--- + +## Cross-cutting contracts (define these FIRST; every PR conforms) + +### C1. Transport interface (so node-pty can slot later — spec §8) + +A single interface in `lib/tui/session.mjs`; tmux is the only implementation in this plan; node-pty is a stubbed adapter behind the same interface. + +``` +interface TuiTransport { + // create the session bound to ephemeralRoot, spawn `claude` interactive, settle to input box + open({ bin, args, env, cwd, ephemeralRoot, reqId }): Promise + // submit one prompt body (T3 recipe: file → send-keys -- "$(cat f)" → separate Enter) + submit(handle, promptText): Promise + // teardown: kill session + nothing else (ephemeral root rm is the manager.cleanup's job, but + // the driver MUST also kill the session in a trap/finally — §8) + close(handle): Promise + // startup-time orphan reaper (kill restart-surviving sessions) — §5.5 + reapOrphans(): Promise<{ killed: string[] }> +} +``` + +`tmuxTransport` implements all four. `nodePtyTransport` is a stub that throws `NOT_IMPLEMENTED` (present so the interface boundary is real and reviewable). The transcript reader (C2) and IR mapping never import the transport — they only consume the deterministic transcript path, so swapping transports later touches nothing else. + +### C2. Transcript-reader interface (PR-1 owns it; transport-agnostic) + +``` +computeTranscriptPath({ ephemeralRoot, cwd, sessionId }): string // §4.1 formula, pure +readTurnResult({ transcriptPath, sinceUserContent, wallClockCapMs, pollMs }): + Promise<{ text: string, durationMs?: number, messageCount?: number }> // resolves the assistant text + // throws TuiCompletionError on guard-(B) terminal conditions (tool_use / wall-clock cap) — §4.4 +``` + +`readTurnResult` is the **dual-signal** completion engine. It never imports tmux/node-pty. It is unit-tested entirely against captured JSONL fixtures. + +### C3. `CLAUDE_TUI_MODE` flag semantics (binding) + +- **Unset / not `"1"`** → default path. **Byte-for-byte unchanged** from today: `_spawnAndStream` (stream-json), `ISOLATION` with NO seed, no `.claude.json` written, no new on-disk sensitive data. This is a **hard requirement** (spec §7.1) and is the regression invariant (C4). +- **`CLAUDE_TUI_MODE=1`** → TUI transport: ephemeral home seeded (PR-0), tmux interactive `claude` (PR-2), transcript-read completion (PR-1), provider branch (PR-3). +- The flag is read **once** in the provider `spawn()` branch (PR-3) — `process.env.CLAUDE_TUI_MODE === '1'`. It is the ONLY toggle. No config-file alternative in this plan. +- Sub-flags (A-only, all default-off, all gated under `CLAUDE_TUI_MODE=1`): `CLAUDE_TUI_WARM_POOL` (§7.2 — **out of scope for this plan; not implemented, only namespace-reserved**). + +### C4. Default-path-unchanged invariant + how to test it + +- **Invariant:** with `CLAUDE_TUI_MODE` unset, no code path added by PR-0..PR-3 executes. `ISOLATION` returns the same shape, `_spawnAndStream` is the only transport, no `.claude.json` is seeded. +- **Test (regression guard, runs in every PR):** the full existing `test-features.mjs` suite stays green. Additionally PR-0 adds an explicit assertion: `prepareIsolatedEnvironment` for the anthropic provider with `CLAUDE_TUI_MODE` unset produces an ephemeral root containing **no** `.claude.json` (only the symlinked `.credentials.json` + `.claude/` dir, as today). PR-3 adds: `spawn()` with the flag unset calls `_spawnAndStream` (assert via the existing `__setSpawnImpl` seam — the mock spawn is invoked, the TUI driver is NOT). + +--- + +## PR-0 — ISOLATION extend (TUI-only `.claude.json` seed) + +### 1. Goal +Seed a minimal `.claude.json` (onboarding/trust/bypass markers ONLY) into the ephemeral home **only when `CLAUDE_TUI_MODE` is active**, so a fresh-home interactive `claude` drops straight to the input box instead of hanging on first-run onboarding — while the default stream-json path's bootstrap stays byte-for-byte unchanged. + +### 2. Files touched +- `lib/providers/anthropic.mjs` — extend the `ISOLATION` block (`:1667`). +- `lib/sandbox/manager.mjs` — add the opt-in seed step to `prepareIsolatedEnvironment` (`:203`), gated so it is a no-op unless the caller requests it. +- `test-features.mjs` — new suite (seed-on / seed-off / permissions). +- *(no new file in PR-0)* + +### 3. Concrete changes + +**3a. `ISOLATION` gains a seed descriptor (NOT a function that reads the real home unconditionally).** Add to the anthropic `ISOLATION` object an OPTIONAL field describing the TUI seed, e.g.: + +``` +// anthropic.mjs ISOLATION (extend, after requiredHomePaths) +tuiSeed: { // consumed ONLY when prepareIsolatedEnvironment is called with { tui:true } + relPath: '.claude.json', // written under ephemeralRoot + mode: 0o600, // §5.5 — same care as the bearer + // builder is pure-ish: it reads the real ~/.claude.json ONCE to copy oauthAccount/userID, + // strips `projects`, and stamps onboarding/trust/bypass markers + a pre-trusted cwd. + build: ({ cwd }) => ({ /* hasCompletedOnboarding:true, oauthAccount, userID, + bypassPermissionsModeAccepted:true, + projects: { [cwd]: { hasTrustDialogAccepted:true, … } } */ }), +} +``` + +- **Authority/contract note:** ADR 0002 Amendment 9's `credentialMounts` is deliberately a static list (not a function) for auditability; the seed is a NEW optional field, so PR-0 must add a one-paragraph Amendment-9 note (in ADR 0002, co-merged or referenced) stating the seed reads the real `~/.claude.json` exactly once to copy `oauthAccount`/`userID`, writes mode-600, and carries **no MCP-disable weight** (T6 negative control, spec §5.2 / §7.1). The seed is onboarding/trust/bypass ONLY. +- **The seed does NOT disable managed MCP** (T6 negative control). PR-0 must NOT add `claudeAiMcpEverConnected` manipulation or any MCP field. A code comment cites spec §5.2 + T6. + +**3b. `prepareIsolatedEnvironment` gains a `tui` opt-in param.** Change the signature to `prepareIsolatedEnvironment({ provider, keyId, reqId, tui = false })` (`manager.mjs:203`). After the existing Layer-2 symlink loop (`:318`), add a guarded block: + +``` +if (tui && isolation?.tuiSeed) { + // chmod 700 the ephemeralRoot (§5.5), write isolation.tuiSeed.build({cwd}) JSON + // at join(ephemeralRoot, tuiSeed.relPath) with { mode: tuiSeed.mode }, never log contents. +} +``` + +- **Default path is untouched:** existing call sites at `server.mjs:1347` and `:1564` pass NO `tui` flag → `tui=false` → seed block is skipped → identity behavior. This satisfies C4. The TUI driver (PR-2) is the ONLY caller that passes `tui:true`. +- **`chmod 700` the ephemeral root** (§5.5) is applied **inside the `tui` block** so the default path's permission semantics are also unchanged. (The default path created the root via `mkdirSync` at `:250`; PR-0 does not alter that.) +- **Per-`keyId` isolation** is already structurally given by the `/tmp/olp-spawn///home` path (`manager.mjs:247`). PR-0 adds an assertion/comment that the parent `` dir is not world-traversable (chmod 700 on the chain) — §5.5. +- **Test bypass interaction (`manager.mjs:223`):** the existing test-runner bypass returns `_legacyShape()`. PR-0's seed tests MUST set `globalThis.__OLP_FORCE_ISOLATION_IN_TEST = true` to exercise the real path, then unset it in `finally` (this seam already exists). + +### 4. Unit tests + fixtures (`test-features.mjs`) +- [ ] **seed-off (default-path invariant, C4):** call `prepareIsolatedEnvironment({provider:anthropic, keyId, reqId})` (no `tui`) under `__OLP_FORCE_ISOLATION_IN_TEST` → assert ephemeralRoot has `.claude/.credentials.json` symlink + `.claude/` dir and **NO `.claude.json`**. +- [ ] **seed-on:** call with `{ tui:true }` → assert `.claude.json` exists, is mode `600`, parses as JSON, contains `hasCompletedOnboarding:true` + `bypassPermissionsModeAccepted:true` + a pre-trusted `projects[cwd]`, and contains **NO** `mcpServers`/`claudeAiMcpEverConnected` field (negative assertion — T6). +- [ ] **root permissions:** assert ephemeralRoot is mode `700` on the `tui:true` path. +- [ ] **no-real-home-mutation:** assert the real `~/.claude.json` is not written/modified (read-only copy). +- [ ] Fixture: a minimal fake `~/.claude.json` (via a temp HOME or an injected reader seam) carrying a dummy `oauthAccount`/`userID` so the test never touches the operator's real account file. +- [ ] Full existing suite stays green (regression). + +### 5. PI231 integration checkpoint (maintainer-supervised; /tmp scratch only) +Run on PI231 scratch (prod OLP on :4567 untouched): +- [ ] Drive `prepareIsolatedEnvironment({tui:true})` against a scratch keyId/reqId; `ls -la` the ephemeral root. +- [ ] **Pass criteria:** `.claude.json` present, mode `600`; root mode `700`; symlinked `.credentials.json` present; `cat` the seed shows onboarding/trust/bypass markers and **no MCP fields**; the real `~/.claude.json` mtime unchanged. +- [ ] Launch interactive `claude` by hand bound to that ephemeral HOME and confirm it **does not** hang on onboarding (drops to input box). (This is the load-bearing reason PR-0 exists.) + +### 6. Acceptance criteria (binding, testable) +- With `tui` unset, ephemeral home is byte-identical to today (no `.claude.json`). ✔ regression test + PI231. +- With `tui:true`, seed is written mode-600, root mode-700, onboarding/trust/bypass present, MCP fields absent. +- No change to default stream-json spawn behavior; full suite green. + +### 7. Reviewer (Iron Rule 10) +Fresh-context reviewer opens **spec §7.1 + §5.2 (T6 negative control) + ADR 0002 Amendment 9** and confirms: (a) the seed is gated on the opt-in `tui` param so the default path is unchanged; (b) the seed carries NO MCP-disable field (T6); (c) mode-600 seed + mode-700 root + per-keyId isolation per §5.5; (d) the Amendment-9 note documenting the new `tuiSeed` field is present. A review that does not name the §5.2 negative control is not a valid approval. + +### 8. Authority citation (commit + PR body) +`claude` CLI v2.1.158 § first-run onboarding (theme/login pickers) + `$HOME`-redirect behavior (ADR 0002 Amendment 9 anthropic ISOLATION pin); ADR 0002 Amendment 9 (ISOLATION contract); spec §7.1 + §5.2; PI231 ephemeral-home spike `docs/spikes/2026-05-29-ephemeral-home.md`. State explicitly: **the seed does NOT disable managed MCP — that is the spawn-argv flag in PR-2 (T6).** + +### Risk / rollback +Independently revertable (revert reinstates the pre-seed ISOLATION; default path was never touched). Default-off: nothing reaches users — the seed only fires when a caller passes `tui:true`, and no caller does until PR-2/PR-3. + +--- + +## PR-1 — Transcript reader (`lib/tui/transcript.mjs`) + +### 1. Goal +A transport-agnostic reader that computes the deterministic transcript path, polls for lazy file creation, detects turn completion via the **mandatory dual-signal guard** (turn_duration OR tool_use OR wall-clock cap; NO quiescence in v1), extracts the assistant text, and resolves a single response string. + +### 2. Files touched +- **NEW** `lib/tui/transcript.mjs`. +- `test-features.mjs` — new transcript-reader suite. +- Fixtures dir (NEW) `docs/spikes/fixtures/tui/` — captured real JSONL (see §4). + +### 3. Concrete changes (exports + signatures) + +- `export function computeTranscriptPath({ ephemeralRoot, cwd, sessionId })` — **pure.** Implements §4.1: `/.claude/projects//.jsonl` where `CWD_ENCODED` = `cwd` with **every** `/` → `-` **including the leading slash** (`/tmp/x` → `-tmp-x`). No filesystem access. (OLP generates `sessionId` and `cwd`, so the path is known before spawn.) +- `export async function readTurnResult({ transcriptPath, sinceUserContent, wallClockCapMs = 120_000, pollMs = 500, toolUseIsTerminal = true })`: + - **Lazy-create poll:** the file is created on first message, not at spawn (§4.1). Tolerate ENOENT; poll every `pollMs` until the file exists or `wallClockCapMs` elapses (then throw `TuiCompletionError('completion-marker timeout')`). + - **Dual-signal completion (§4.4, MANDATORY):** + - **(A) happy path:** a line `{"type":"system","subtype":"turn_duration"}` for this turn appears → done. Carries `durationMs` + `messageCount`. Do NOT rely on file-tail byte ordering (§4.3 trap): re-scan the file, find the matching `user` line for `sinceUserContent`, collect all subsequent `assistant`/`text` blocks. + - **(B) co-equal terminal guard (mandatory, never-hang):** if the last assistant message carries `stop_reason:"tool_use"` → throw `TuiCompletionError('tool-use turn unsupported in TUI-mode')` (maps to clean 502). If `wallClockCapMs` fires → throw `TuiCompletionError('completion-marker timeout')`. + - **NO quiescence cut in v1** (§4.4 ⚠️): do NOT abort on "file size-stable for N seconds" — a long Opus/extended-thinking turn legitimately produces no growth. Quiescence is added only after spike T5. (Comment cites §4.4 explicitly so a future contributor does not "helpfully" add it.) + - Do NOT key off `stop_reason:"end_turn"` alone (§4.3 trap — appears on both `thinking` and `text` blocks). + - **Assistant-text extraction (§4.2):** `JSON.parse` per line (native log → escaping-clean). Response = concatenation of `text`-type content blocks from `assistant` messages emitted **since the matching `user` line**. Return `{ text, durationMs, messageCount }`. + - **Trailing-newline normalization (§3.2 / §6 caveat):** the input box strips the source's single trailing newline. `sinceUserContent` matching MUST normalize the trailing newline before comparing source-prompt vs the transcript `user` line, or the "matching user line" lookup (and any cache-key reasoning) sees a spurious mismatch. +- **Cache-contract adapter note (does NOT live in PR-1, but PR-1's return shape is designed for it):** `readTurnResult` resolves a string; the PR-2/PR-3 layer wraps it as `[{type:'delta',role:'assistant',content:text},{type:'stop',finish_reason:'stop'}]`. PR-1's JSDoc states this adapter contract and points at `server.mjs:1299` (buffered array) + `server.mjs:1558` (streaming generator) so the reviewer sees the two consumers. **max_tokens/sampling graceful-drop boundary** (§4.5/§4.6): PR-1 documents that these IR fields never reach this layer (interactive `claude` has no flag); nothing to do — they are dropped at the CLI-args boundary in PR-2/PR-3. PR-1 adds a comment asserting `finish_reason` is always `'stop'` (no `length` mapping, since max_tokens is not enforced). + +### 4. Unit tests + fixtures +**Fixtures (capture REAL JSONL on PI231 — do not hand-fabricate the shapes):** +- [ ] `text-turn.jsonl` — a normal `end_turn` text answer ending in a `turn_duration` line. +- [ ] `refusal-turn.jsonl` — a refusal that still emits `turn_duration` (T1: `durationMs≈3221`). +- [ ] `tool-use-no-marker.jsonl` — **MANDATORY** (T1): a `tool_use` turn whose last assistant line is `stop_reason:"tool_use"` with **NO** `turn_duration` line. This is the hang case guard (B) must catch. +- [ ] `out-of-order.jsonl` — a text block flushed by byte-position AFTER `turn_duration` though `turn_duration` has the later timestamp (§4.3 trap) — proves the reader does not rely on file-tail ordering. +- [ ] `multiturn.jsonl` — two user lines so `sinceUserContent` selection is exercised (a `toolUseResult:true` user line within a turn must NOT be mistaken for a new submit — §6 step 5). + +**Tests:** +- [ ] `computeTranscriptPath` exact-string equality incl. leading-slash encoding. +- [ ] happy path returns concatenated text + `durationMs`/`messageCount`. +- [ ] refusal path returns refusal text (still completes). +- [ ] **tool-use fixture → throws `TuiCompletionError` (never hangs)** — assert with a short `wallClockCapMs` that the throw is the tool_use detection, not the timeout (distinguish the two error messages). +- [ ] wall-clock cap fires on a never-completing fixture (truncated file with no marker) → throws within cap. +- [ ] out-of-order fixture → correct text (no reliance on last byte). +- [ ] trailing-newline normalization: `sinceUserContent` with trailing `\n` still matches the transcript user line. +- [ ] **no-quiescence assertion:** a fixture that is size-stable for > pollMs but has not completed does NOT abort before the wall-clock cap (proves quiescence is excluded). +- [ ] Full existing suite stays green (PR-1 adds a new module + new tests only; touches no existing path). + +### 5. PI231 integration checkpoint (maintainer-supervised) +- [ ] Capture the 5 fixtures above from real `claude` v2.1.158 runs on PI231 scratch (this is also how the fixtures are sourced). Commit them under `docs/spikes/fixtures/tui/`. +- [ ] **Pass criteria:** `readTurnResult` against each freshly-captured fixture returns the same text a human reads in the transcript; the tool-use capture throws `TuiCompletionError` and never blocks; cap fires deterministically on a manually-truncated fixture. + +### 6. Acceptance criteria (binding) +- Deterministic path matches §4.1 exactly. +- Dual-signal completion: completes on `turn_duration`; **never hangs** on tool-use or a missing marker (guard B); **no quiescence cut**. +- Escaping-clean text extraction; trailing-newline normalized. +- Pure reader: zero tmux/node-pty import; fully fixture-testable. + +### 7. Reviewer (Iron Rule 10) +Fresh-context reviewer opens **spec §4.1–§4.4 (and §3.2 cache-contract / trailing-newline)** and confirms: (a) the path formula incl. leading-slash; (b) the dual-signal guard is present AND quiescence is explicitly excluded with a §4.4 citation; (c) the tool-use-no-marker fixture exists and the test proves a non-hanging terminal throw; (d) the resolved-string return is documented against the `getOrCompute`/`getOrComputeStreaming` consumers. A review missing the tool-use-no-marker check is not valid. + +### 8. Authority citation +`claude` CLI v2.1.158 § native session transcript JSONL (`turn_duration` is an undocumented internal-log behavior — pin to v2.1.158, re-verify per CLI/Ink bump); spec §4 (S2 PASS) + §4.4 (T1 PARTIAL); fixtures captured PI231 2026-05-30. No OpenAI-spec surface (reader is internal). ALIGNMENT Rule 2: the reader consumes a behavior `claude` actually emits — no invented format. + +### Risk / rollback +New file + new tests only; revert deletes the module and tests, default path untouched. Riskiest sub-step is the dual-signal guard's tool-use detection (the hang vector) — fully covered by the mandatory fixture. + +--- + +## PR-2 — Session driver (`lib/tui/session.mjs`) + +### 1. Goal +A tmux-backed interactive-`claude` driver behind the transport interface (C1): spawn with the T6 flag set, submit via the T3 recipe, auto-answer dialogs, run the per-spawn MCP-disable verification gate, guarantee teardown via trap/finally, and reap orphan sessions on startup — producing a single buffered response (via PR-1's reader) adapted to IR chunks for both cache paths. + +### 2. Files touched +- **NEW** `lib/tui/session.mjs` (tmux transport + node-pty stub + the driver `runTuiTurn`). +- `lib/sandbox/manager.mjs` — driver calls `prepareIsolatedEnvironment({…, tui:true})` (the param added in PR-0). +- `test-features.mjs` — driver suite (with a mock transport — no real tmux/claude in unit tests). +- *(server wiring is PR-3, NOT here)* + +### 3. Concrete changes + +**3a. Transport interface + tmux implementation (C1).** +- `export const tmuxTransport` implementing `open/submit/close/reapOrphans`. +- `export const nodePtyTransport` — stub throwing `NOT_IMPLEMENTED` (interface placeholder, §8 decision: tmux first). +- Session naming: `olp-tui--` so `reapOrphans` can pattern-match. + +**3b. Spawn argv (T6 flag set, §5.2) — the driver builds its OWN args (NOT `buildCliArgs`).** +``` +claude --model --session-id --system-prompt "" + --strict-mcp-config // T6 load-bearing: 0 managed-MCP (no --mcp-config supplied) + --disallowedTools "mcp__*" // deny MCP-namespaced tools + [--tools "" ] // B-only built-in lockdown — WIRED, gated off for A (see 3g) + // NO -p, NO --output-format → real TTY → cc_entrypoint=cli +env: CLAUDE_CODE_DISABLE_OFFICIAL_MARKETPLACE_AUTOINSTALL=1 // defense-in-depth + + carry-forward: CLAUDE_CODE_DISABLE_CLAUDE_MDS=1, unset ANTHROPIC_* (reuse buildSpawnEnv semantics) + + HOME= (from prepareIsolatedEnvironment envOverrides) +``` +- `--system-prompt` value comes from **`extractSystemPrompt(ir)` (`anthropic.mjs:123`) — REUSED.** Prompt body comes from **`irToAnthropic(ir)` (`anthropic.mjs:601`) — REUSED** (written to the prompt file, 3d). +- **`--bare` is forbidden** (§5.2 — breaks OAuth). Comment cites it. +- `--model` from `ir.model`; `--session-id` is the OLP-generated UUID also fed to `computeTranscriptPath`. + +**3c. Per-spawn MCP-disable verification gate (§5.2 (4) preflight semantics).** After `open()` settles, assert **0** dirs matching `$HOME/.cache/claude-cli-nodejs/*/mcp-logs-claude-ai-*` under the ephemeral root. **Do NOT run `/mcp` inside the serving session** (§5.2: it writes a transcript line, consumes a turn, corrupts the reader's matching-user-line semantics). For A's canary the cache-dir assertion is the in-band check; the `/mcp`-empty assertion belongs to a **separate preflight session at startup / CLI upgrade** (wire the preflight hook here but it is owner-tier advisory for A; it becomes a hard gate for B). On assertion failure: tear down + clean 502. + +**3d. Submit recipe (T3 PASS — binding for acceptance, §6).** +1. Write `irToAnthropic(ir)` to a file under the ephemeral root (NEVER interpolate into a shell line — backticks/`$()`/`&&`/quotes get mangled by the shell, §6 step 1). +2. `tmux send-keys -t -- "$(cat promptfile)"` — the leading `--` end-of-options guard is **required** (prompt starting with `-`). Embedded `\n` are soft line-breaks; do NOT submit. Do NOT use `send-keys -l` for the body (§6 step 2). +3. Settle ~1.5–2s for Ink render / paste-collapse (§6 step 3). (Production: poll the pane for input-box-ready / paste-collapse before Enter, or scale settle to payload size — §6 caveat.) +4. **Submit Enter as a SEPARATE tmux KEY TOKEN:** `tmux send-keys -t Enter` — never a literal `"\n"` appended to text (Ink #15553, §6 step 4). +5. **Verify via TRANSCRIPT** (not `capture-pane`): exactly one `user`-role line whose content equals source minus its single trailing newline (a second `user` line with `toolUseResult:true` is in-turn tool output, not a second submit — §6 step 5). Large pastes collapse to a `[Pasted text …]` placeholder so pane-scraping is impossible — transcript-read is mandatory. +6. **Retry** Enter (key token) up to ~4× as a defensive guard (§6 step 6). + +**3e. Dialog auto-answer (S3 footgun, §6).** With the PR-0 seed (trust + bypass pre-seeded) neither dialog should appear. Defensive handling if they do: trust-folder defaults to "1. Yes, I trust" → bare Enter confirms; the **bypass-permissions dialog defaults cursor to "1. No, exit"** — a naive Enter **kills the session** → must send **Down then Enter** to land on "2. Yes, I accept". Prefer the pre-seed; keep the Down+Enter recipe as fallback. + +**3f. Teardown (trap-guaranteed, §8) + orphan reaper (§5.5).** +- `close()` + ephemeral-root cleanup MUST run in a `finally` (NOT best-effort) — S3 noted best-effort `rm` left empty `home_*` dirs with stray cred symlinks. The driver wraps the whole turn in `try { … } finally { await transport.close(handle); await isolationCtx.cleanup(); }`. +- `tmuxTransport.reapOrphans()` runs at **server startup** (called from PR-3's boot path): list `olp-tui-*` tmux sessions surviving a restart, kill each + `rm -rf` its ephemeral root (these still hold the owner OAuth via the mounted ephemeral home — §5.5). This is the restart-time backstop complementing the steady-state finally. + +**3g. B-gate hooks wired but inert (scope discipline).** `--tools ""` (built-in lockdown) and the `/mcp`-empty hard gate are **present in the code path but only activated for `owner_tier === 'guest'`**, which no A/canary request is. A comment + the ADR state: **B does not launch until T2 (body-capture `tools:[]`) passes; serialized after T2; concurrent only after T4** (§5.2 gate semantics). PR-2 ships the flags; PR-3/B-enablement flips them on. No guest key is provisioned in this plan. + +**3h. Single buffered response + SSE replay (§3.1).** The driver's public entry, e.g. `export async function runTuiTurn({ ir, authContext, keyId, reqId, transport = tmuxTransport })`, returns the **resolved string** from PR-1's `readTurnResult`. The IR-chunk adapter `string → [{type:'delta',role:'assistant',content},{type:'stop',finish_reason:'stop'}]` is applied by PR-3's provider branch so both `getOrCompute` (buffered array) and `getOrComputeStreaming` (async generator) consume it unchanged — for `stream:true` the existing `irChunkToOpenAISSE` replay (`server.mjs:1764`) emits the completed text as one burst of delta(s) + `[DONE]` AFTER the turn finishes. **True token streaming is NOT possible** (§3.1) — `capture-pane` partial-text tapping is explicitly rejected (large pastes collapse to `[Pasted text …]`). + +### 4. Unit tests + fixtures +- [ ] **mock transport** (no real tmux/claude): assert the driver builds the exact T6 argv set (`--strict-mcp-config`, `--disallowedTools "mcp__*"`, no `-p`, no `--output-format`, no `--bare`, env `CLAUDE_CODE_DISABLE_OFFICIAL_MARKETPLACE_AUTOINSTALL=1`). +- [ ] submit recipe shape: prompt written to a file; `send-keys -- "$(cat …)"` issued; Enter is a SEPARATE token; on a simulated missed-Enter the retry fires ≤4×. +- [ ] dialog fallback: simulated bypass dialog → driver sends Down+Enter (not bare Enter). +- [ ] teardown: assert `close` + `cleanup` fire in `finally` on both happy and thrown paths (inject a throw mid-turn). +- [ ] reaper: seed fake `olp-tui-*` session records into the mock transport → `reapOrphans` kills them + rms roots. +- [ ] guest-gating: with `owner_tier:'guest'` the argv gains `--tools ""`; with `'owner'` it does not (B-hook wired-but-inert proof). +- [ ] string→IR-chunk adapter produces `[delta, stop]` with `finish_reason:'stop'`. +- [ ] T3 regression negative control (documented, runs on PI231 not in unit): a newline-as-text submit silently fails (Ink #15553) — guards against a future refactor reintroducing `-l`. +- [ ] Full existing suite green; default path (flag-unset) never reaches this module. + +### 5. PI231 integration checkpoint (maintainer-supervised; /tmp scratch + tmux only) +- [ ] Real multiline-code request (fenced code block + shell-special chars, ~50 lines per T3) through `runTuiTurn` against real `claude` v2.1.158 on PI231 scratch. +- [ ] **Pass criteria:** response text is correct and byte-for-byte intact; exactly ONE `user` submit in the transcript; **`cc_entrypoint=cli` verified** (transcript `turn_duration` line `entrypoint=cli` / `--debug` metadata); MCP-disable gate passes (0 `mcp-logs-claude-ai-*` dirs); the real `~/.claude` is **untouched** (mtime check on `~/.claude.json` + `~/.claude/projects`); session is killed + ephemeral root removed on completion (no stray `home_*`); reaper kills a deliberately-orphaned session on the next startup. +- [ ] Re-run the T3 negative control (newline-as-text fails to submit) to confirm the Ink #15553 control still holds on this CLI version. + +### 6. Acceptance criteria (binding) +- T6 flag set applied; MCP-disable gate asserts 0 managed-MCP (cache-dir evidence) per spawn. +- T3 submit: multiline/shell-special payload submits byte-for-byte, exactly one submit, transcript-verified. +- Trap-guaranteed teardown (no stray ephemeral roots / cred symlinks) + startup orphan reaper. +- Single buffered response; `stream:true` is SSE-replay (no token streaming). B hooks wired but inert. +- `cc_entrypoint=cli` confirmed; real `~/.claude` untouched. + +### 7. Reviewer (Iron Rule 10) +Fresh-context reviewer opens **spec §5.2 (T6) + §6 (T3) + §3.1 + §5.5 + §8** and confirms: (a) `--strict-mcp-config` with NO `--mcp-config` is the disable mechanism (not seed-editing); (b) `--bare` is NOT used; (c) the T3 recipe is file→`send-keys -- "$(cat)"`→separate Enter (not `-l`, not literal `\n`); (d) teardown is finally-based + a startup reaper exists; (e) B hooks (`--tools ""`, `/mcp` hard gate) are present but gated to guest and B is documented as blocked on T2/T4; (f) response is single-buffered with SSE replay, no token streaming. A review that does not open the live `claude --help` for `--strict-mcp-config`/`--disallowedTools` on v2.1.158 is not valid. + +### 8. Authority citation +`claude` CLI v2.1.158 § `--strict-mcp-config`, § `--disallowedTools`, § `--system-prompt`, § `--session-id`, § `--model` (live `--help` on PI231); env `CLAUDE_CODE_DISABLE_OFFICIAL_MARKETPLACE_AUTOINSTALL=1` (binary-confirmed); `tmux` 3.3a § `send-keys`/`send-keys -l`/key-tokens (Ink #15553 control); spec §5.2 (T6 PASS), §6 (T3 PASS), §3.1, §5.5, §8. ALIGNMENT Rule 2: every flag is one `claude` accepts — no invented flag. + +### Risk / rollback +Riskiest PR. Independently revertable (deletes the module + the `tui:true` caller; PR-0/PR-1 inert without it). Default-off: no server path invokes `runTuiTurn` until PR-3, and even then only under `CLAUDE_TUI_MODE=1`. + +--- + +## PR-3 — Provider wiring + ADR + README + +### 1. Goal +Add the `CLAUDE_TUI_MODE` branch in the anthropic provider `spawn()` so a flagged request routes to the TUI driver and yields IR chunks; default stays stream-json. Land ADR 0016 as authority of record and the README docs (quirks + non-honored params + grey-area framing). + +### 2. Files touched +- `lib/providers/anthropic.mjs` — branch in public `spawn()` (`:1164`); call `reapOrphans` from a boot hook (or export an init the server calls). +- `server.mjs` — call the orphan reaper at startup (near `bootstrapSandbox`, `:82`/boot path); pass `tui:true` to `prepareIsolatedEnvironment` ONLY on the TUI branch (the branch lives in the provider, so the simplest wiring is: the provider's TUI branch calls `prepareIsolatedEnvironment({…, tui:true})` itself; if the existing architecture composes isolation in `server.mjs` before `spawn`, PR-3 adds a flag-gated `tui` pass-through there — decide per the under-spec note below). +- `docs/adr/0016-tui-mode.md` — NEW (or ADR 0009 Amendment 2). +- `README.md` — env-var table, Troubleshooting, API/Configuration notes. +- `CHANGELOG.md` — Unreleased entry (no version bump mid-Phase per CLAUDE.md `phase_rolling_mode`). +- `CONTRIBUTORS` — add jaekwon-park. +- `test-features.mjs` — branch-selection tests. + +### 3. Concrete changes + +**3a. `spawn()` branch (`anthropic.mjs:1164`).** +``` +export async function* spawn(irRequest, authContext, isolationCtx) { + if (process.env.CLAUDE_TUI_MODE === '1') { + // import { runTuiTurn } from '../tui/session.mjs' + const text = await runTuiTurn({ ir: irRequest, authContext, keyId, reqId, … }); + yield { type: 'delta', role: 'assistant', content: text }; + yield { type: 'stop', finish_reason: 'stop' }; + return; + } + yield* _spawnAndStream(irRequest, authContext, _spawnImpl, isolationCtx); // UNCHANGED default +} +``` +- The default branch (`_spawnAndStream`) is **byte-for-byte unchanged**. C4 invariant holds. +- The 2-chunk yield is exactly what `collectAllChunks` (`server.mjs:1299`) buffers into an array for `getOrCompute`, and what `sourceWithRelease` (`server.mjs:1558`) yields for `getOrComputeStreaming` → SSE replay. No server change to the cache paths. +- **keyId/reqId access:** the provider `spawn()` currently receives `(irRequest, authContext, isolationCtx)` — it does NOT receive `keyId/reqId`. The TUI driver needs them (for ephemeral root + session name). **Under-spec — see §"Open implementation questions".** Options: (i) thread `keyId/reqId` into the TUI branch via `isolationCtx` (the manager already has `safeKeyId/safeReqId` and `ephemeralRoot`), so the driver reuses `isolationCtx.ephemeralRoot` rather than re-preparing; (ii) pass a `tui:true` to `prepareIsolatedEnvironment` at the server call site (flag-gated) and let the driver consume the returned `ephemeralRoot`. **Recommended: (i)** — the provider's TUI branch reads `isolationCtx.ephemeralRoot` + a reqId carried on `isolationCtx`, and PR-0's seed runs because the server passes `tui: (process.env.CLAUDE_TUI_MODE==='1')` to `prepareIsolatedEnvironment` at `server.mjs:1347` and `:1564`. This keeps the seed/ephemeral-root creation in the manager (one owner) and the tmux drive in the provider. Maintainer to confirm the threading before PR-2 finalizes its `runTuiTurn` signature. + +**3b. Orphan reaper at startup.** Call `tmuxTransport.reapOrphans()` from the server boot path (alongside `bootstrapSandbox`, `server.mjs:82` import region / router init at `:2334`+), gated on `CLAUDE_TUI_MODE==='1'` so default deployments incur zero tmux dependency. + +**3c. max_tokens / sampling graceful-drop (§4.5/§4.6) — already the behavior; just assert + document.** The TUI argv carries no `--max-tokens`/`--temperature`/etc. (interactive `claude` has none). `ir.max_tokens` (`openai-to-ir.mjs:182`), `temperature`, `top_p`, `stop` are accepted into IR and silently dropped at the argv boundary — same posture as the stream-json path. No error. Document in README (3e). + +**3d. ADR 0016 (authority of record).** New ADR: Context (2026-06-15 billing split + ADR 0009 Amd 1 premise), Decision (TTY-backed TUI transport behind `CLAUDE_TUI_MODE`, default stays stream-json), the spike record (S1/S2/S3 + T1/T3/T6 results; T2/T4/T5 open), the §5.2 security model + §5.5 credential coupling, the §3.1 no-token-streaming decision, the §4.5/§4.6 dropped-param decision, A-vs-B gate semantics (no B before T2; serialized after T2; concurrent after T4). **Acknowledgment section names OCP PR #101 + jaekwon-park** (adopted: interactive-TUI-for-subscription idea; redesigned: transcript-read not hook-file, no `--dangerously-skip-permissions`, structural tool-stripping for B). Supersede note on ADR 0009 Amendment 1's billing-pool lane (§Status of the spec). + +**3e. README.** Per CLAUDE.md `release_kit.new_feature_doc_expectations`: +- **Environment Variables table:** `CLAUDE_TUI_MODE` (default unset/off; opt-in TTY path; grey-area, billing-favorable, post-2026-06-15-inference), `CLAUDE_CODE_DISABLE_OFFICIAL_MARKETPLACE_AUTOINSTALL` (set by TUI driver). Reserve-note `CLAUDE_TUI_WARM_POOL` as A-only future. +- **Troubleshooting / TUI-mode §:** onboarding-hang quirk (fresh ephemeral home → seed required, PR-0); **OAuth-login requirement** (one `claude login` on the host; member keys hold OLP keys not OAuth); **NO true token-streaming** (§3.1 — `stream:true` is replay-after-completion, one burst); **`max_tokens`/sampling params not honored** (§4.5–§4.6); honest grey-area framing (§10.2 — opt-in, no anti-fingerprinting, drop-on-ban). +- Do NOT hand-edit the Supported Providers table (sourced from `models-registry.json`). + +### 4. Unit tests + fixtures +- [ ] `CLAUDE_TUI_MODE` unset → `spawn()` invokes `_spawnAndStream` (assert via `__setSpawnImpl` mock spawn is called; `runTuiTurn` is NOT). **C4 regression.** +- [ ] `CLAUDE_TUI_MODE='1'` → `spawn()` invokes `runTuiTurn` (inject a mock driver returning a fixed string) and yields `[delta, stop]` with `finish_reason:'stop'`. +- [ ] buffered path: a flagged request through the (mocked) provider produces a well-formed OpenAI JSON body (drive `getOrCompute`'s array consumer). +- [ ] streaming path: a flagged `stream:true` request replays as SSE delta(s) + `[DONE]` (drive `irChunkToOpenAISSE`). +- [ ] dropped-param: a request with `max_tokens`/`temperature` succeeds and ignores them (no error). +- [ ] reaper boot hook is a no-op when flag unset. +- [ ] Full existing suite green. + +### 5. PI231 integration checkpoint (maintainer-supervised) +- [ ] On PI231 scratch (prod :4567 untouched), run a real flagged request end-to-end through OLP scratch instance with `CLAUDE_TUI_MODE=1`: buffered `stream:false` returns correct JSON; `stream:true` returns valid SSE (one burst); flag-unset run is identical to today's stream-json. +- [ ] **Pass criteria:** flagged path returns correct text via tmux/transcript; `cc_entrypoint=cli`; default path unchanged (diff a flag-unset response against current prod behavior); reaper runs clean at startup; real `~/.claude` untouched. + +### 6. Acceptance criteria (binding) +- Flag unset → identical to current stream-json (C4). Flag set → TUI path, correct buffered + SSE-replay responses. +- ADR 0016 merged as authority of record, names PR #101 + jaekwon-park. +- README documents the env var, onboarding-hang, OAuth-login req, no-token-streaming, dropped params, grey-area framing. +- CHANGELOG Unreleased entry; CONTRIBUTORS updated; no mid-Phase version bump. + +### 7. Reviewer (Iron Rule 10) +Fresh-context reviewer opens **spec §3.1, §4.5–§4.6, §10.2, §12 (PR-3), §13 + ADR 0016** and confirms: (a) the default branch is unchanged and the flag is the sole toggle (C4); (b) the 2-chunk adapter slots into both cache paths without server cache-layer edits; (c) dropped params documented, no silent failure; (d) ADR 0016 acknowledges PR #101/jaekwon-park and the co-author trailer is on the commits; (e) README quirks present. A review that does not open ADR 0016 + confirm the author-credit obligation is not valid. + +### 8. Authority citation +OpenAI `/v1/chat/completions` spec (entry surface is unchanged; `stream`, `max_tokens`, `temperature`, `top_p`, `stop` fields — document non-honored set) — cite the OpenAI spec URL for the entry-surface PR portion; `claude` CLI v2.1.158 (provider branch); ADR 0016 (new authority of record) + ADR 0009 Amendment 1 (superseded billing lane) + ADR 0002 Amendment 9 (ISOLATION) + ADR 0014 (sandbox). spec §§3.1/4.5/4.6/10.2/12/13. Co-author trailer `jaekwon-park` on every commit (§13). + +### Risk / rollback +Independently revertable (revert removes the branch; provider returns to pure stream-json). **Default-off is the kill switch:** until an operator sets `CLAUDE_TUI_MODE=1`, nothing about TUI-mode executes. The OCP single-tenant canary (post-2026-06-15) is the first real enablement. + +--- + +## Parallel B-gate spike track (does NOT block A) + +These run independently of PR-0..PR-3 and gate Deployment B only. One paragraph each. + +- **T2 — body-capture `tools:[]` (security + credential-safety gate, §5.2(4)/§5.5).** Stand up a body-logging channel for the outbound `/v1/messages` from an interactive `claude` spawn (a local MITM proxy with a trusted cert in the ephemeral home, or a body-capturing forward proxy via `HTTPS_PROXY`). `--debug api` is insufficient (metadata only). Method: run a TUI turn under the full §5.2 flag set (`--strict-mcp-config` + `--disallowedTools "mcp__*"` + `--tools ""`), capture the wire request body, assert it carries `tools:[]` or no tools array. PASS is the hard gate that lets B launch (serialized). Per §5.5 this is a **credential-safety** gate, not mere MCP hygiene. +- **T4 — concurrency (§7.3).** Run K concurrent TUI turns sharing one owner OAuth, each with its own ephemeral `$HOME` + `--session-id` + cwd. Method: fire K parallel `runTuiTurn` calls; assert transcript isolation (no cross-session lines), billing entrypoint stays `cli` on all, no OAuth auth contention/refresh thrash, and that one credential tolerates K concurrent interactive sessions. Until PASS, B serializes (concurrency=1). This lifts B's concurrency limit only. +- **T5 — cold-start latency + inotify (§4.4 sizing / non-blocking).** Method: measure submit→transcript-available cold-start end-to-end (currently unmeasured); compare `inotifywait` vs 0.5s poll under load; measure Opus-class long-stream `turn_duration` ordering to size the §4.4 wall-clock cap (recommend ≥120s, tune here). Non-blocking for A; informs the cap constant and a possible future quiescence window (which §4.4 forbids in v1). + +--- + +## Test strategy on PI231 without breaking prod + +- **PI231 runs prod OLP on :4567.** It must stay untouched throughout. All TUI testing is **/tmp scratch + tmux**: a scratch OLP instance on a different port (or direct `node` invocation of the new modules), ephemeral homes under `/tmp/olp-spawn/*`, scratch tmux sessions `olp-tui-*`. +- **Never** point a TUI test at the prod `~/.claude` — the ephemeral-home seed + symlink keep the real home read-only; every PI231 checkpoint asserts `~/.claude.json` + `~/.claude/projects` mtime unchanged. +- **The canary is OCP single-tenant post-6/15** (spec §12.6): OCP is one user, no cross-tenant boundary, and is where PR #101 originated. Enable `CLAUDE_TUI_MODE=1` there first; watch billing entrypoint stays `cli`, cap behavior, completion reliability over real usage — before any OLP Deployment-B exposure. +- Mac mini is NEVER a test target (cc-mem rule). MacBook/PI231-scratch only. + +--- + +## Author credit (binding, §13) — checklist applied to every PR + +- [ ] Co-author trailer `Co-Authored-By: jaekwon-park <…>` on every implementing commit (pull real email/handle from OCP PR #101 first — do not invent). +- [ ] ADR 0016 names PR #101 + jaekwon-park (adopted idea vs redesigned implementation). +- [ ] Add jaekwon-park to CONTRIBUTORS. +- [ ] Notify on OCP PR #101 (comment linking the shipping PR) at ship time. + +--- + +## Open implementation questions (maintainer decides BEFORE code) + +1. **keyId/reqId into the TUI driver.** The provider `spawn(irRequest, authContext, isolationCtx)` does not receive `keyId/reqId` today. The driver needs them for the ephemeral root + tmux session name. Recommended: have the server pass `tui:(CLAUDE_TUI_MODE==='1')` to `prepareIsolatedEnvironment` at `server.mjs:1347`/`:1564` (so the seed + chmod fire in the manager), and thread `ephemeralRoot` (+ a reqId field) to the provider via `isolationCtx`; the TUI branch then reuses `isolationCtx.ephemeralRoot` rather than re-preparing. Confirm this threading before PR-2 fixes `runTuiTurn`'s signature. **(Spec §3.2 implies the reuse but does not specify the parameter plumbing.)** +2. **Warm pool (§7.2) is namespace-reserved, not built.** Confirm A's canary runs ephemeral-per-request (no warm pool) for this plan — the spec allows warm pool for A but it adds cross-request-context-leak risk and is out of the PR-0..PR-3 scope. +3. **Wall-clock cap constant.** Spec recommends ≥120s pending T5. Confirm the v1 value to bake into `readTurnResult` (PR-1) — or read it from config so T5 can tune it without a code change. +4. **Large-paste path (§6 caveat).** T3 validated ≤50 lines / 1.2 KB. Coding-proxy traffic carries multi-KB pastes. Decide whether PR-2 ships `send-keys` only (with the documented ≤50-line validation) or also wires the `paste-buffer`/`load-buffer` fallback for large bodies now (recommended as a fast-follow, non-blocking for A). +5. **Preflight MCP-disable session for A.** §5.2 makes the separate-preflight `/mcp`-empty assertion a hard gate for B. Confirm whether A's canary runs it as advisory-at-startup (recommended) or skips it (relying on the per-spawn cache-dir assertion alone). + +--- + +## Maintainer decisions — plan-review fixes + open questions RESOLVED (2026-05-30) + +Plan-review verdict was **ready-with-fixes**. All anchors verified accurate. Decisions below resolve P1–P5 + the open implementation questions; the plan is now ready to implement. + +| Ref | Decision | +|---|---| +| **P1 / OQ#1 — keyId/reqId plumbing** | **Reuse `isolationCtx.ephemeralRoot` + reqId.** The two existing spawn call sites (`server.mjs:1347`, `:1564`) already call `prepareIsolatedEnvironment` and pass `isolationCtx` into `spawn()`. PR-0 adds `ephemeralRoot` + `reqId` to the returned `isolationCtx`; the TUI branch reads them from there — **no new edits to the default-path call sites**, preserving the byte-for-byte-unchanged invariant. `runTuiTurn(isolationCtx, irRequest, opts)` takes `isolationCtx`, not raw keyId/reqId. | +| **P2 — reaper boot anchor** | Wire `reapOrphans()` into the real boot region: the `isMain` block at **`server.mjs:2417`** (NOT `:2334`, which is wrong; `:82` is the import). Co-locate with the existing `await bootstrapSandbox()` call. | +| **P3 / OQ#5 — A preflight `/mcp`** | **A also spawns with `--strict-mcp-config` + `--disallowedTools "mcp__*"`** (defense-in-depth — even the owner does not want a prompt-injected client reaching the owner's own Gmail/Drive). The separate preflight `/mcp`-empty session is **advisory-at-startup for A** (log a warning if managed MCP still attaches; do NOT block), and a **hard gate for B**. Decided line item for PR-2, no longer open. | +| **P4 — tier citation** | Cite accurately: manifest `owner_tier ∈ {'owner','guest'}` (`keys.mjs:143`); `'anonymous'` is a runtime fallback identity (`:439`), not a manifest tier. Cosmetic; correct the anchor table. | +| **P5 / OQ#3 — wall-clock cap** | **Config, not constant.** Read from env `CLAUDE_TUI_WALLCLOCK_MS` (default `120000`) so T5 can tune it without a code change. Baked into `readTurnResult` (PR-1). | +| **OQ#2 / OQ#5 — warm pool** | **Out of PR-0..PR-3 scope.** Initial A = per-request ephemeral session (cleanest, matches B). Warm pool is a later opt-in optimization (`CLAUDE_TUI_WARM_POOL`), process-reuse-not-context per spec §7.2, tracked separately. | +| **OQ#4 — large-paste (>50 lines)** | **Defer to fast-follow.** PR-2 ships the `send-keys -- "$(cat file)"` recipe with the documented ≤50-line / multi-KB validation from T3; the `paste-buffer`/`load-buffer` path for very large bodies is a non-blocking follow-up PR. Document the current bound in the README. | + +**Net:** P1 (the one true PR-2-interface blocker) is decided = reuse `isolationCtx`. P2/P4 are anchor corrections. P3/P5 are decided line items. Warm-pool + large-paste are explicitly scoped out of the initial A deliverable. Implementation may proceed PR-0 → PR-1 → PR-2 → PR-3.