mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-19 09:44:07 +00:00
* feat(tui): real SSE streaming via claude's MessageDisplay hook (OCP_TUI_STREAM, default off) Backlog #2. TUI-mode `stream:true` turns can now emit real SSE `delta.content` chunks as `claude` generates them, instead of buffering the turn and replaying it with streamStringAsSSE. Opt-in: with OCP_TUI_STREAM unset/0 the spawn argv, the SSE bytes and the cache behaviour are byte-for-byte unchanged (asserted by test). This PR does NOT mirror any cli.js function, so no `cli.js:NNNN` citation applies, and per CLAUDE.md's hard requirement #1 that is stated explicitly here rather than left implicit: - We consume claude's OWN `MessageDisplay` hook surface AS EMITTED — forwarding, not inventing. No new endpoint, no fabricated protocol, no new field. - The TUI spawn is OCP-owned surface: ADR 0007 owns it, not cli.js. - The SSE wire shapes are the OpenAI chat/completions streaming spec, adopted by ADR 0006. Every frame emitted here (role chunk, content-delta chunk, stop chunk, `[DONE]`, and the post-header {error:{message,type}} frame) is COPIED from callClaudeStreaming, the -p path. /health gains additive fields only (streamEnabled + 4 counters) — same grandfathered B.2 rationale as the existing tui block (ADR 0006). Existing keys are untouched. `claude` fires MessageDisplay per rendered block, handing the hook the RAW MARKDOWN SOURCE of an incremental delta on stdin. The hook is registered with `--settings` on the ordinary interactive spawn (no -p, no --bare) — verified to leave the billing pool alone. Sink: a static sh hook script appends each payload to `<streamDir>/<session_id>.jsonl`; OCP polls that file and forwards deltas as SSE. The per-session-id keying is MANDATORY, not an optimization — OCP_TUI_MAX_CONCURRENT defaults to 2, so two claude panes already run at once and a shared sink would splice one client's deltas into another's stream. Warm-pool compatible (a separate in-flight PR depends on this): the hook script AND the settings file are static — nothing request-specific is baked in at spawn time. The sink path reaches the pane through its own env (OCP_TUI_STREAM_FILE) and derives from the session-id, which a pre-booted pane fixes at boot. The hook is SYNCHRONOUS (forceSyncExecution: claude blocks on it), so the script writes and exits: one `cat` append, nothing else. Measured p50 7.2ms / p90 14.7ms per fire, ~50ms across a whole turn — noise against a 6-10s turn. It remains the terminal-turn signal, the source of the returned/cached text T, and the input to the honesty gates. The delta stream is a low-latency MIRROR, never a replacement: - the truncation gate (C-2) and auth-banner gate (C-1, issue #133) run BEFORE anything is committed or flushed, unchanged; - at end of turn the streamed bytes are asserted against T. Equal -> serve. A strict PREFIX of T -> top up from the transcript so the client still receives exactly T (counted). NOT a prefix -> REFUSE the turn: SSE error frame, no cache, no success, streamDivergences++. Serving text the transcript disagrees with is the failure class ALIGNMENT.md exists to prevent, so this fails loud rather than degrading quietly; - only T is ever cached — never the concatenated deltas. The auth banner needs prevention, not just detection (SSE deltas cannot be un-sent), so the first OCP_TUI_STREAM_HOLDBACK (100) chars are withheld: the default banner detector cannot match a message longer than 100 chars, so releasing past that provably cannot leak a banner. A custom CLAUDE_TUI_ERROR_PATTERNS has no such bound — OCP warns at boot. - BANNER, before/after the spawn change: `Sonnet 4.6 with low effort · Claude Max` both, including on the pane the server itself spawns. Never `API Usage Billing`. Transcript entrypoint stays "cli". --settings is not a --bare-class flag. - --settings MERGES with <HOME>/.claude/settings.json rather than clobbering it (the user-level settings' `env` block still reached the hook), so the isolated-HOME settings story (permissions / additionalDirectories) survives. - EXACTNESS: 8/8 varied prompts (short, long, markdown, code fence, multilingual, JSON, table, unicode) byte-exact vs transcript T, streamed AND buffered. 0 top-ups, 0 divergences over 15 streamed turns. - TTFT: buffered delivers NOTHING until the turn ends (TTFB == total, 7.5-15.8s). Streamed sends headers at ~25ms (heartbeat covers the pre-first-delta silence) and first content mid-generation, e.g. markdown 7.9s first chunk / 12.8s total; long 9.7s / 17.4s. - CONCURRENCY DEMUX: two concurrent streamed turns (ALPHA/BRAVO), tui.inflight peaked at 2, each read its own session-keyed transcript, ZERO cross-contamination. - AUTH-BANNER GATE under streaming, both layers: a short banner-like turn reached the client as 0 content chunks + an SSE error frame (never emitted); a long one was streamed but still ended on an error frame, not finish_reason:"stop", and was not cached. - DISCONNECT mid-turn: pane torn down and semaphore slot released within 1s (info-logged, not booked as a model error). - THINKING: not leaked. Opus 4.8 + xhigh turns carry a thinking block with a signature but `thinking:""` (the reasoning text is not persisted in interactive mode), both MessageDisplay text-extraction sites in the 2.1.207 bundle filter type==="text", and no reasoning prose appeared in any delta; concat===T held exactly on the single-message turn. - npm test: 282 passed, 0 failed (was 267 on main; +15). The transcript keeps only the model's LAST assistant message. A turn where the model narrates before calling a tool therefore has two messages, and T is only the second. If the narration exceeds the holdback it has already been streamed and cannot be retracted -> the turn is REFUSED. Reproduced live: Opus narrated 475 chars before a Bash call. The assembler discards a prior message's text when nothing has been emitted yet (so short narration is handled correctly and stays exact), and raising OCP_TUI_STREAM_HOLDBACK above the narration length rescues the turn — verified on that exact transcript: holdback>=500 -> served, exact=true. Documented in README and ADR 0007; this is why streaming is opt-in and off by default. ADR 0007 line 59 ("no real token streaming — deliberate") is amended, not silently contradicted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tui): re-integrate streaming onto the warm-pane pool (#158) — install the hook at BOOT Rebasing backlog #2 (streaming) onto #158 (warm pane pool) is not a textual merge: #158 split the monolithic runTuiTurn into bootTuiPane + runTuiTurn, and streaming had patched the monolith. Re-integrating it in the OLD shape would have compiled, passed every existing test, and been WRONG. The bug that shape would have shipped: the sink was derived at TURN time from a streamDir argument. But a POOLED pane is pre-booted long before any request exists — so on a pool HIT runTuiTurn never cold-boots, no hook was ever registered on that pane, and the turn would silently serve BUFFERED. Every miss streams, every hit does not; no error, no failing test. The operator sees "streaming does nothing in production" and has nothing to grep for. Fix — install the hook where the pane is born: - bootTuiPane({ streamDir }) registers the MessageDisplay hook at spawn and returns the pane's own sink (pane.streamFile), keyed by the pane's own --session-id. The hook script and settings file are STATIC (one pair per streamDir); the only per-turn thing is the sink path, and it is fixed at boot. So nothing request-specific is baked into a spawn. - runTuiTurn reads pane.streamFile — never recomputes it — so a warm pane and a cold pane stream through byte-for-byte the same path. - server.mjs threads the same streamDir into the pool's bootPane closure, so pre-booted panes carry the hook too. TUI_STREAM/TUI_STREAM_DIR now declare before the pool needs them. Three regression guards added (test-features.mjs), and the third was MUTATION-TESTED: with the fix reverted to the turn-time shape it fails ("the pooled pane's deltas must reach the client"), with the fix in place it passes. A guard nobody has watched fail is not a guard. /health: pool + stream* fields are now a union — the shape assertion asserts CONTAINMENT of the seven grandfathered keys plus an exact added-set, so a future field that silently REPLACED an original key cannot pass. Class B (ADR 0007, OCP-owned TUI spawn) — cli.js does NOT perform this operation. npm test: 313 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR * fix(tui): close the streaming auth-banner leak + 6 further review findings (PR #159) Independent review (Iron Rule 10) found a HIGH bug by EXECUTING the code, not reading it. All seven findings fixed. F1 and F3 were merge-blocking. F1 (HIGH) — the auth-banner holdback was bypassed after the first release. TuiDeltaAssembler.released was set once and never reset at a message_id boundary, so the holdback + detectError predicate guarded only the FIRST message of a turn. In production's own configuration (OCP_TUI_FULL_TOOLS=1, where multi-message tool-using turns are the norm): the model narrates past the holdback before a tool call -> released; credentials expire mid-turn -> claude renders the 401 as ordinary assistant TEXT as a NEW message -> push() took the `if (this.released)` branch and handed the banner verbatim to the client. That is precisely the silent-error case the C-1 gate exists to prevent. Detection survived (the turn was still refused at finalize) but PREVENTION did not. Fix: once a message boundary follows an emit, the turn is already unrecoverable — finalize() will refuse it — so push() now emits NOTHING further for the rest of the turn. Second hole in the same predicate: detectTuiUpstreamError() trims before applying its <=100-char rule, so 101 whitespace chars trimmed to "" -> detector had nothing to classify -> returned null -> release fired having screened nothing. Release now gates on the TRIMMED length, so both sides of the check talk about the same string. F2 — the "provably safe" claim in stream.mjs, ADR 0007 and README was unsound as written. Restated with both required halves: (i) nothing is emitted until the trimmed accumulation exceeds the detector's max banner length, AND (ii) no emission at all once a message boundary follows an emit. Half (i) alone only ever covered a turn's first message. F3 (blocker) — prepareStreamHook was write-if-missing, so md-hook.sh could never be updated OR repaired: a host that booted once under an older version was stuck on that HOOK_SCRIPT forever, and a non-atomic write interrupted mid-flight left a TRUNCATED script that existsSync() called fine — on a hook claude BLOCKS on synchronously. Now written unconditionally via tmp+renameSync (the pattern already used by ensureTuiCwdTrusted). F4 — the two spawn paths differed for non-streaming requests: the pool installed the hook whenever OCP_TUI_STREAM was on (correct — a pre-booted pane cannot know what request it will serve), but the cold path gated it on this turn's onDelta. So one stream:false request got --settings on a pool HIT and not on a MISS: two spawn argvs for the identical request, on this project's billing-classification surface. Both paths now gate on TUI_STREAM alone; whether the sink is POLLED remains correctly gated on onDelta. F5 — pool._drop() killed the pane but orphaned its sink file; the reap tick drains the whole pool, so sinks accumulated with no GC path. Now removed best-effort on every drop path. F6 — /health counters did not measure what they documented: streamTurns was incremented only AFTER the honesty gates, hiding exactly the turns an operator most wants to see (and making streamDivergences/streamTurns a meaningless ratio); streamDeltas counted every fire while claiming to count forwarded ones. Counters and docs now agree. F7 — total hook failure was silent: zero fires per turn still yields ok:true/exact:false and a normal, fully-buffered answer. Only streamTopUps moved, which the code itself calls benign. Added streamZeroDeltaTurns (+ a tui_stream_zero_deltas warning) to separate "the hook is dead" from "one fire was dropped". Tests: 316 passed, 0 failed (was 313). Every new guard MUTATION-TESTED — with each fix reverted the guard named for it fails, and passes with the fix restored: - drop the restartedAfterEmit guard -> 2 failed (incl. the strengthened old test) - revert trim() in the release gate -> 1 failed - revert F3 to write-if-missing -> 1 failed The pre-existing test "new message_id AFTER an emit" asserted finalize().ok === false but never checked what push() RETURNED — so it passed while F1 was live, documenting the leak instead of catching it. Strengthened to assert the emission, not just the verdict. Class B (ADR 0007, OCP-owned TUI spawn) — cli.js does NOT perform this operation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VqgWJcjxrjjL9L9SkpZyXR --------- Co-authored-by: dtzp555 <dtzp555@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
193 lines
11 KiB
JavaScript
193 lines
11 KiB
JavaScript
// TUI-path concurrency limiter (audit finding C-4).
|
||
//
|
||
// WHY THIS EXISTS, SEPARATE FROM server.mjs's MAX_CONCURRENT:
|
||
// The global MAX_CONCURRENT gate lives in spawnClaudeProcess() (the -p / stream-json
|
||
// path). callClaudeTui() NEVER calls spawnClaudeProcess — it calls runTuiTurn(), which
|
||
// boots a full interactive `claude` inside a fresh tmux session. So nothing bounded the
|
||
// TUI path: N concurrent TUI requests spawned N simultaneous cold-boot tmux+claude
|
||
// processes. On a small host (a Pi 4 serving a family) a burst of ~5 is an OOM risk, and
|
||
// it also multiplies subscription rate-limit pressure. This is an INDEPENDENT limiter for
|
||
// the TUI path that mirrors MAX_CONCURRENT's intent without coupling to it (the two pools
|
||
// are different shapes: a stream-json spawn is cheap and fast; a TUI turn is a heavy
|
||
// cold-boot + up to 120s wallclock).
|
||
//
|
||
// QUEUE vs REJECT: we QUEUE (await a slot), mirroring the spirit of MAX_CONCURRENT's
|
||
// intent not to drop requests, rather than rejecting immediately. To avoid unbounded
|
||
// memory growth from a runaway client, the wait queue itself is bounded by maxQueue
|
||
// (default: a generous multiple of the concurrency limit). When the queue is full, run()
|
||
// rejects with a tui_queue_full error (the caller surfaces it as a 503) — a deterministic
|
||
// backpressure signal rather than silent OOM.
|
||
//
|
||
// Pure + importable so test-features.mjs can assert the bound directly (no server boot).
|
||
|
||
// Thrown by acquire() when the caller-supplied AbortSignal fires before a slot was granted
|
||
// (audit finding F2 — a client that disconnects while queued must never receive a slot; the
|
||
// queue entry is spliced out, not just flagged, so `queued` accounting stays exact). Distinct
|
||
// `name` lets callers (server.mjs acquireClaudeSlot) tell "client went away" apart from
|
||
// "queue is full" without string-matching the message.
|
||
export class SemaphoreAbortError extends Error {
|
||
constructor(message) { super(message); this.name = "SemaphoreAbortError"; }
|
||
}
|
||
|
||
export class TuiSemaphore {
|
||
// limit: max concurrent slots. maxQueue: max waiters before run() rejects with backpressure.
|
||
constructor(limit, { maxQueue } = {}) {
|
||
this.limit = Math.max(1, parseInt(limit, 10) || 1);
|
||
// Default queue cap: 32× the limit. Large enough that real family-burst traffic never
|
||
// hits it, small enough that a pathological flood can't grow the queue without bound.
|
||
this.maxQueue = Number.isFinite(maxQueue) ? maxQueue : this.limit * 32;
|
||
this._inflight = 0;
|
||
this._waiters = []; // FIFO queue of resolve callbacks waiting for a slot
|
||
}
|
||
|
||
get inflight() { return this._inflight; }
|
||
get queued() { return this._waiters.length; }
|
||
|
||
// Runtime-adjust the concurrency limit (audit finding F1 — a PATCH /settings maxConcurrent
|
||
// change must actually take effect, not just be ignored until every currently-inflight task
|
||
// happens to finish). Lowering the limit is handled lazily by release() (see below) — it
|
||
// simply stops re-granting until inflight drains under the new, lower limit. Raising the
|
||
// limit has immediate headroom, so we wake as many queued waiters as now fit.
|
||
setLimit(limit) {
|
||
this.limit = Math.max(1, parseInt(limit, 10) || 1);
|
||
while (this._inflight < this.limit && this._waiters.length > 0) {
|
||
const next = this._waiters.shift();
|
||
this._inflight++;
|
||
next();
|
||
}
|
||
}
|
||
|
||
// Acquire a slot. Resolves once a slot is free (immediately if under the limit, otherwise
|
||
// when an in-flight task releases). Rejects synchronously-ish if the wait queue is full.
|
||
// `signal` (optional AbortSignal, F2) lets the caller cancel a QUEUED wait — e.g. wired to
|
||
// a client's socket "close" event so a request that disconnects before a slot is granted
|
||
// is removed from the queue instead of eventually being handed a slot for a dead socket.
|
||
// If `signal` is already aborted, reject immediately without ever touching the queue.
|
||
acquire(signal) {
|
||
if (signal?.aborted) {
|
||
return Promise.reject(new SemaphoreAbortError("acquire aborted before requesting a slot"));
|
||
}
|
||
if (this._inflight < this.limit) {
|
||
this._inflight++;
|
||
return Promise.resolve();
|
||
}
|
||
if (this._waiters.length >= this.maxQueue) {
|
||
return Promise.reject(new Error(
|
||
`tui_queue_full: TUI concurrency limit (${this.limit}) reached and wait queue ` +
|
||
`(${this.maxQueue}) is full`));
|
||
}
|
||
return new Promise((resolve, reject) => {
|
||
let waiter; // the FIFO entry — captured so onAbort can find + splice exactly this one
|
||
const onAbort = () => {
|
||
const idx = this._waiters.indexOf(waiter);
|
||
if (idx === -1) return; // already granted a slot (shifted out by release()/setLimit) — too late to cancel
|
||
this._waiters.splice(idx, 1); // remove, not just flag — keeps `queued` accounting exact
|
||
reject(new SemaphoreAbortError("acquire aborted while queued"));
|
||
};
|
||
waiter = () => {
|
||
signal?.removeEventListener("abort", onAbort);
|
||
resolve();
|
||
};
|
||
signal?.addEventListener("abort", onAbort, { once: true });
|
||
this._waiters.push(waiter);
|
||
});
|
||
}
|
||
|
||
// Release a slot. Always frees the caller's own slot first, then re-grants it to the next
|
||
// waiter ONLY if the (post-decrement) inflight count is still under the current limit (F1
|
||
// fix). This is what makes a runtime-lowered limit actually bite: if the limit was lowered
|
||
// while over-subscribed, releases stop re-granting and inflight drains toward the new limit
|
||
// instead of a freed slot being handed straight back out at the old, higher occupancy.
|
||
release() {
|
||
if (this._inflight > 0) this._inflight--;
|
||
if (this._inflight < this.limit) {
|
||
const next = this._waiters.shift();
|
||
if (next) {
|
||
this._inflight++;
|
||
next();
|
||
}
|
||
}
|
||
}
|
||
|
||
// Run fn() under one slot. Releases in a finally so a throw (PR-A's honesty gates,
|
||
// wallclock truncation, paste-not-landed, tmux spawn failure) NEVER leaks a slot.
|
||
// `signal` (optional, F2) is forwarded to acquire() so a queued run() can be cancelled.
|
||
async run(fn, signal) {
|
||
await this.acquire(signal);
|
||
try {
|
||
return await fn();
|
||
} finally {
|
||
this.release();
|
||
}
|
||
}
|
||
}
|
||
|
||
// ── TUI drift observability (audit C-5) — pure helpers, importable for testing ──
|
||
|
||
// Record an observed cc_entrypoint into the (mutable) tuiStats counter. Sets lastEntrypoint
|
||
// unconditionally and increments entrypointMismatches when the spawn was supposed to be
|
||
// subscription-pool ("cli") but the transcript reported something else (a silent drift to
|
||
// the metered Agent SDK pool — the audit's top risk after the 6/15 billing flip).
|
||
// Returns true iff this observation was a mismatch (so the caller can also emit a log).
|
||
export function recordTuiEntrypoint(tuiStats, observed, expectedMode = "cli") {
|
||
tuiStats.lastEntrypoint = observed ?? null;
|
||
const mismatch = expectedMode === "cli" && observed !== "cli";
|
||
if (mismatch) tuiStats.entrypointMismatches++;
|
||
return mismatch;
|
||
}
|
||
|
||
// Build the additive /health `tui` block (ADR 0007 PR-B amendment). Pure: given the
|
||
// config + live counters, returns the exact object embedded in /health. New fields only —
|
||
// behaviour-preserving for existing /health consumers (grandfathered B.2 under ADR 0006).
|
||
//
|
||
// `pool` (optional, warm pane pool — lib/tui/pool.mjs): a TuiPanePool, or null/undefined
|
||
// when the pool is off (the default). Reported as `pool: null` when off so the block's
|
||
// shape stays stable, and as the pool's stats (size / warm / hits / misses / …) when on —
|
||
// the operator's window onto both the hit rate and the standing idle-process cost.
|
||
//
|
||
// Streaming fields (backlog #2, OCP_TUI_STREAM) are ADDITIVE too:
|
||
// streamEnabled — is real (MessageDisplay-hook) SSE streaming on for TUI turns?
|
||
// streamTurns — streamed turns ATTEMPTED, counted before the truncation/auth-banner
|
||
// gates run (F6) — so a turn REFUSED by those gates still shows up
|
||
// here, which is exactly the turn an operator most wants visible.
|
||
// Counting only turns that survived the gates would silently exclude
|
||
// a turn's worst-case outcome from its own denominator.
|
||
// streamDeltas — MessageDisplay hook fires OBSERVED, including held-back ones (F6) —
|
||
// NOT only the ones forwarded to a client. This is what makes
|
||
// streamZeroDeltaTurns meaningful: a turn can have streamDeltas
|
||
// incrementing while still emitting nothing to the client (fully held
|
||
// back, e.g. a short answer), which is healthy, vs. a hook that fired
|
||
// zero times at all, which is not (see streamZeroDeltaTurns).
|
||
// streamTopUps — turns where the delta stream was a safe PREFIX of the transcript but
|
||
// not equal to it; OCP topped up from the transcript and served T.
|
||
// Benign but worth watching — a persistent rate means the hook is
|
||
// losing fires.
|
||
// streamDivergences — turns REFUSED because emitted bytes were not a prefix of the
|
||
// transcript. THE field to alert on for CORRECTNESS: it means the hook
|
||
// and the transcript disagreed and OCP chose to fail rather than serve
|
||
// unverifiable text.
|
||
// streamZeroDeltaTurns — streamed turns where the hook fired ZERO times (F7). THE field to
|
||
// alert on for AVAILABILITY: streamTopUps climbing is one fire dropped
|
||
// here and there (benign); this climbing means the hook is not firing
|
||
// AT ALL — e.g. `--settings` silently stopped registering it (a claude
|
||
// version bump), or F3's truncated-script failure mode — and every
|
||
// streamed turn is quietly degrading to fully-buffered with no error.
|
||
export function buildTuiHealthBlock({ enabled, entrypointMode, maxConcurrent, streamEnabled = false }, tuiStats, semaphore, pool = null) {
|
||
return {
|
||
enabled,
|
||
entrypointMode, // cli | auto | off
|
||
lastEntrypoint: tuiStats.lastEntrypoint, // last observed cc_entrypoint, or null
|
||
entrypointMismatches: tuiStats.entrypointMismatches,
|
||
inflight: semaphore.inflight, // current concurrent TUI turns
|
||
queued: semaphore.queued, // turns waiting for a slot
|
||
maxConcurrent,
|
||
pool: pool ? pool.stats() : null, // warm pane pool, or null when disabled
|
||
streamEnabled,
|
||
streamTurns: tuiStats.streamTurns ?? 0,
|
||
streamDeltas: tuiStats.streamDeltas ?? 0,
|
||
streamTopUps: tuiStats.streamTopUps ?? 0,
|
||
streamDivergences: tuiStats.streamDivergences ?? 0,
|
||
streamZeroDeltaTurns: tuiStats.streamZeroDeltaTurns ?? 0,
|
||
};
|
||
}
|