Files
ocp/lib/tui/semaphore.mjs
1b324968f4 feat(tui): real SSE streaming via claude's MessageDisplay hook (OCP_TUI_STREAM, default off) (#159)
* 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>
2026-07-14 08:19:24 +10:00

193 lines
11 KiB
JavaScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
// 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,
};
}