From 5258d5d3959724489358d4ed3861ccda4f036437 Mon Sep 17 00:00:00 2001 From: dtzp555-max Date: Mon, 13 Jul 2026 15:16:30 +1000 Subject: [PATCH] feat(tui): pin spawn effort via OCP_TUI_EFFORT (default low) (#156) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(tui): pin spawn effort via OCP_TUI_EFFORT (default low) buildTuiCmd never passed --effort, so the pane's claude inherited a HOME-dependent effortLevel: real-home mode inherits the operator's ~/.claude/settings.json (high/xhigh on typical operator hosts), env-token scratch mode inherits claude's built-in default — proxied-turn latency silently depended on which HOME mode resolveTuiHome() picked and on an unrelated operator setting. Pass --effort explicitly, from new env var OCP_TUI_EFFORT (default "low"; allowlist low|medium|high|xhigh|max per `claude --help` 2.1.207; "inherit" restores the pre-flag argv byte-for-byte; an invalid value warns and falls back to "low" so a typo can never reach the pane argv). Not endpoint-touching: no server.mjs change, no wire-level change — the flag rides the existing interactive spawn (ADR 0007). Billing-pool safety verified per the docs/plans/2026-07-13-tui-latency banner protocol: startup banner stays "Claude Max" with "low effort". Measured through a test OCP instance (:3979, TUI mode, real-home, claude-sonnet-4-6, n=5+5, same ~1850-token prompt as floor.sh): before: median 11.30s range 9.05-12.38s (spread 3.32s) banner: high effort - Claude Max after: median 9.55s range 9.27-9.77s (spread 0.50s) banner: low effort - Claude Max OCP_TUI_EFFORT=inherit: banner back to "high effort" (pre-flag behavior restored) README: new row in the Environment Variables table (release_kit new_feature_doc_expectations: new env var -> README table). Tests: 4 new buildTuiCmd cases (default, explicit level, inherit, invalid fallback); suite 267 passed / 0 failed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR * docs(readme): review nit — 'pre-v3.22' → 'pre-flag' (next version not fixed yet) Reviewer nit from the Iron Rule 10 independent review of PR #156. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR --------- Co-authored-by: dtzp555 Co-authored-by: Claude Fable 5 --- README.md | 1 + lib/tui/session.mjs | 26 ++++++++++++++++++++ test-features.mjs | 59 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/README.md b/README.md index 204bf54..68f506f 100644 --- a/README.md +++ b/README.md @@ -958,6 +958,7 @@ See [Subscription-pool (TUI) mode](#subscription-pool-tui-mode) and ADR 0007 PR- | `OCP_TUI_CWD` | `$HOME/.ocp-tui/work` | (TUI-mode) Scratch working directory where interactive claude sessions run. Transcripts land under `/.claude/projects//`. Created automatically. | | `OCP_TUI_HOME` | *(auto)* | (TUI-mode) `HOME` claude runs under. **When unset, OCP picks it for you:** if `CLAUDE_CODE_OAUTH_TOKEN` is set → a **credential-isolated** scratch home `$HOME/.ocp-tui/home` (no `credentials.json`, env-token auth — **recommended**); if no env token → the operator's real home (legacy shared `credentials.json`). Setting this to an **explicit** path overrides the auto-default. The credential handling at that path still follows the env token: **with** the env token it is credential-free (env-token auth, no `credentials.json` written); **without** the env token (and the path ≠ real home) it uses the legacy symlinked-credentials scratch mode, which carries the credential-fork caveat — see ADR 0007. | | `OCP_TUI_ENTRYPOINT` | `cli` | (TUI-mode) Billing-classifier labeling: `cli` (default) pins `cc_entrypoint=cli` deterministically; `auto` lets claude self-classify via TTY detection; `off` leaves the inherited env untouched. Honest only when the spawn is a genuine interactive PTY — see ADR 0007. | +| `OCP_TUI_EFFORT` | `low` | (TUI-mode) Effort level passed to the interactive `claude` as an explicit `--effort` flag: `low` (default), `medium`, `high`, `xhigh`, `max`, or `inherit` to omit the flag (the pre-flag behaviour: the pane inherits a HOME-dependent effort — the operator's `~/.claude/settings.json` `effortLevel` in real-home mode, claude's built-in default in env-token scratch mode). Explicit `low` cuts measured TTFT p50 by ~40% and collapses run-to-run variance ~15× versus an inherited `xhigh` (see `docs/plans/2026-07-13-tui-latency/`); proxied requests rarely benefit from extended thinking. Banner-verified to stay on the subscription pool (`· Claude Max`). An invalid value logs a warning and falls back to `low`. | | `OCP_TUI_MAX_CONCURRENT` | `2` | (TUI-mode) Max concurrent interactive TUI turns. **Independent** of `CLAUDE_MAX_CONCURRENT` (which bounds the `-p`/stream-json path; TUI never uses it). A TUI turn is heavy (per-request cold-boot of tmux+claude + up to `CLAUDE_TUI_WALLCLOCK_MS` wallclock), so the default is low to keep small hosts (e.g. a Pi 4) alive under a burst. Excess turns **queue** (bounded); a full queue yields a 503. See ADR 0007 PR-B amendment. | | `OCP_SKIP_AUTH_TEST` | *(unset)* | When `=1`, skip the `claude -p` auth probe during `setup.mjs`. After 2026-06-15 this probe draws from the Agent SDK credit pool; set this to avoid burning a metered credit on re-installs or `ocp update` runs. Auth is validated at the first real request. | | `OCP_TUI_FULL_TOOLS` | *(unset)* | (TUI-mode, **single-user only**) When `=1`, grant the interactive session the **same tool surface as the `-p` path** — `--allowedTools` (+ optional `--mcp-config`, read from `CLAUDE_ALLOWED_TOOLS` / `CLAUDE_MCP_CONFIG`) — instead of the default MCP-walled, built-in-tools-only set. Lets a trusted single-operator TUI deployment run a **tool-using / MCP agent** (e.g. an OpenClaw assistant) on the subscription pool. Safe because TUI **refuses to boot under `AUTH_MODE=multi`** (hard exit) — no guest key can ever reach the TUI path, so this gate cannot expose tools to an untrusted caller. (Under `AUTH_MODE=shared` + `OCP_TUI_ALLOW_LAN=1`, anyone holding the single shared key reaches it — that is the existing TUI trust model, unchanged.) Note: `--dangerously-skip-permissions` / `CLAUDE_SKIP_PERMISSIONS` is **not** supported for TUI — claude v2.1.x shows an interactive bypass-acceptance screen in headless tmux that cannot be answered, bricking the pane. Use scratch-home `settings.json` `additionalDirectories` instead. See [Subscription-pool (TUI) mode](#subscription-pool-tui-mode) and ADR 0007. | diff --git a/lib/tui/session.mjs b/lib/tui/session.mjs index 1054618..1f6ade6 100644 --- a/lib/tui/session.mjs +++ b/lib/tui/session.mjs @@ -377,12 +377,38 @@ export function buildTuiCmd(claudeBin, model, sessionId, ehome, entrypointMode) } else { toolArgs = ["--strict-mcp-config", "--disallowedTools", shq("mcp__*")]; } + + // Effort: pass --effort EXPLICITLY. Without it, the pane's claude inherits a + // HOME-dependent effortLevel — real-home mode inherits the operator's + // ~/.claude/settings.json (whatever they set for their own interactive use), + // env-token scratch mode inherits claude's built-in default (prepareTuiHome never + // writes effortLevel) — so latency silently depends on which HOME mode + // resolveTuiHome() picked AND on an unrelated operator setting. Pinning it here + // removes both. Measured (docs/plans/2026-07-13-tui-latency): explicit low cuts + // direct-spawn TTFT p50 10.35s → 6.17s (−40%) and collapses the spread ~15×; + // banner-verified to stay on the subscription pool (`· Claude Max`). + // OCP_TUI_EFFORT=inherit restores the pre-flag argv byte-for-byte (no --effort). + // An unknown value falls back to the default rather than reaching claude's argv: + // a typo'd --effort value must not risk a spawn-time usage error in the pane. + const EFFORT_LEVELS = ["low", "medium", "high", "xhigh", "max"]; // claude 2.1.207 --help + const effortRaw = (process.env.OCP_TUI_EFFORT || "low").trim().toLowerCase(); + let effortArgs; + if (effortRaw === "inherit") { + effortArgs = []; + } else if (EFFORT_LEVELS.includes(effortRaw)) { + effortArgs = ["--effort", effortRaw]; + } else { + console.error(`[tui] invalid OCP_TUI_EFFORT=${JSON.stringify(process.env.OCP_TUI_EFFORT)}; using "low" (valid: ${EFFORT_LEVELS.join("|")}, or "inherit" to omit the flag)`); + effortArgs = ["--effort", "low"]; + } + return [ envPrefix, shq(claudeBin), "--model", shq(model), "--session-id", sessionId, ...toolArgs, + ...effortArgs, ].join(" "); } diff --git a/test-features.mjs b/test-features.mjs index 9fc09b0..dbec3cd 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -1795,6 +1795,65 @@ test("buildTuiCmd shq-escapes a token containing shell metacharacters (no inject } }); +// OCP_TUI_EFFORT (TUI latency, docs/plans/2026-07-13-tui-latency): the pane's claude +// must get an EXPLICIT --effort so its effort never depends on which HOME mode +// resolveTuiHome() picked (real-home inherits the operator's settings.json effortLevel; +// env-token scratch inherits claude's built-in default). +test("buildTuiCmd passes --effort low by default (OCP_TUI_EFFORT unset)", () => { + const save = process.env.OCP_TUI_EFFORT; + try { + delete process.env.OCP_TUI_EFFORT; + const cmd = buildTuiCmd("/usr/bin/claude", "m", "sid-eff1", "/home/u", "cli"); + assert.ok(cmd.includes("--effort low"), "default must pin --effort low"); + } finally { + if (save === undefined) delete process.env.OCP_TUI_EFFORT; + else process.env.OCP_TUI_EFFORT = save; + } +}); + +test("buildTuiCmd honors an explicit OCP_TUI_EFFORT level (case/space-normalized)", () => { + const save = process.env.OCP_TUI_EFFORT; + try { + process.env.OCP_TUI_EFFORT = " XHigh "; + const cmd = buildTuiCmd("/usr/bin/claude", "m", "sid-eff2", "/home/u", "cli"); + assert.ok(cmd.includes("--effort xhigh"), "explicit level must be passed, normalized"); + assert.ok(!cmd.includes("--effort low"), "default must not also appear"); + } finally { + if (save === undefined) delete process.env.OCP_TUI_EFFORT; + else process.env.OCP_TUI_EFFORT = save; + } +}); + +test("buildTuiCmd OCP_TUI_EFFORT=inherit omits --effort entirely (pre-flag argv)", () => { + const save = process.env.OCP_TUI_EFFORT; + try { + process.env.OCP_TUI_EFFORT = "inherit"; + const cmd = buildTuiCmd("/usr/bin/claude", "m", "sid-eff3", "/home/u", "cli"); + assert.ok(!/--effort/.test(cmd), "inherit must not add --effort"); + } finally { + if (save === undefined) delete process.env.OCP_TUI_EFFORT; + else process.env.OCP_TUI_EFFORT = save; + } +}); + +test("buildTuiCmd falls back to --effort low on an invalid OCP_TUI_EFFORT (never reaches argv)", () => { + const save = process.env.OCP_TUI_EFFORT; + const savedErr = console.error; + try { + process.env.OCP_TUI_EFFORT = "ludicrous'; rm -rf /;'"; + let warned = ""; + console.error = (...a) => { warned = a.join(" "); }; + const cmd = buildTuiCmd("/usr/bin/claude", "m", "sid-eff4", "/home/u", "cli"); + assert.ok(cmd.includes("--effort low"), "invalid value must fall back to low"); + assert.ok(!cmd.includes("ludicrous"), "invalid raw value must NOT reach the shell string"); + assert.ok(/invalid OCP_TUI_EFFORT/.test(warned), "must log a warning"); + } finally { + console.error = savedErr; + if (save === undefined) delete process.env.OCP_TUI_EFFORT; + else process.env.OCP_TUI_EFFORT = save; + } +}); + test("buildTuiCmd OCP_TUI_FULL_TOOLS=1 grants -p-equivalent tool surface (single-user opt-in)", () => { const save = { ...process.env }; const restore = () => {