fix(tui): clamp stream holdback to safe floor (A1) + record cc_entrypoint before honesty gates (A3) (#164)

Two OCP-internal correctness fixes on the TUI streaming/observation path, surfaced
by an independent (Codex) re-review. Neither touches the cli.js wire.

A1 — OCP_TUI_STREAM_HOLDBACK now has an enforced floor (DEFAULT_HOLDBACK_CHARS=100).
The C-1 auth-banner gate's first-message guarantee rests on the holdback being at
least the default banner detector's 100-char reach. The env var's own doc said
"Only raise it", but the code trusted the operator: a sub-floor value (e.g. 50) or a
NaN typo ("unlimited") let the first chars of a real auth banner stream to the client
before the end-of-turn detector could classify the whole message and reject the turn.
resolveStreamHoldback() clamps UP to the floor and returns {value, clamped}; server.mjs
emits a boot WARNING when it had to clamp. Default (unset) is unchanged and unflagged.

A3 — recordTuiEntrypoint() now runs the moment runTuiTurn() returns, BEFORE the honesty
gates (wall-clock truncation / auth banner / stream divergence) that throw. The entrypoint
(cli vs sdk-cli) is which billing pool the turn consumed; a turn that then fails a gate
STILL spent that pool, and those failed turns are exactly the ones most likely to signal a
silent degrade to the metered Agent SDK pool. The old placement recorded only on the success
path, so /health's lastEntrypoint and entrypointMismatches were blind to every failed turn —
the billing-drift signal missed the cases it most needed to catch. recordModelSuccess stays
on the success path. The catch block does not record the entrypoint, so there is no double
count; a client-disconnect (TuiAbortError) throws from inside runTuiTurn before the destructure,
so no phantom entrypoint is recorded.

ALIGNMENT.md Rule 2 (No Invention): no cli.js citation applies. cli.js does not perform either
operation — both are proxy-internal. A1 hardens OCP's own SSE holdback (a safety mechanism on
the Class B.1 OpenAI-compat streaming surface; wire format authority is the OpenAI spec via
ADR 0006). A3 reorders when OCP records its own /health observability counters (Class B.2,
grandfathered under ADR 0006; the TUI spawn authority is ADR 0007). No endpoint, header,
request field, or response field is added or altered; the bytes to and from cli.js are
byte-identical. This is observation/safety-layer hardening, not extension.

Tests: +5 mutation-proven unit tests for resolveStreamHoldback (deleting the floor clamp
fails 3 of them). Full suite 325 passed / 0 failed (was 320). A3 is a server.mjs control-flow
reorder; server.mjs is not imported by the test suite, so A3 is verified by reviewer inspection
of the diff, stated honestly here rather than vouched for by a test.

Version bump + CHANGELOG deliberately omitted: this is a fix PR, consolidated into a later
chore(release) PR per the repo's #148/#149/#150 -> #151 (v3.21.1) convention.

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude <claude-opus-4-8> <noreply@anthropic.com>
This commit is contained in:
dtzp555-max
2026-07-15 21:40:13 +10:00
committed by GitHub
co-authored by taodeng Claude <claude-opus-4-8> <noreply@anthropic.com>
parent 1d65bc309e
commit 63c2de7128
3 changed files with 81 additions and 14 deletions
+37 -1
View File
@@ -3453,7 +3453,7 @@ async function runAsyncTests() {
// ── TUI real streaming: MessageDisplay hook sink (backlog #2) ───────────────
// Pure-logic coverage for lib/tui/stream.mjs: sink parsing, the concat===T assertion,
// prefix-stability, the auth-banner holdback, message scoping, and the error paths.
import { TuiDeltaAssembler, parseDeltaChunk, buildStreamSettings, streamFilePath, HOOK_SCRIPT, prepareStreamHook } from "./lib/tui/stream.mjs";
import { TuiDeltaAssembler, parseDeltaChunk, buildStreamSettings, streamFilePath, HOOK_SCRIPT, prepareStreamHook, resolveStreamHoldback, DEFAULT_HOLDBACK_CHARS } from "./lib/tui/stream.mjs";
test("stream: parseDeltaChunk consumes only COMPLETE lines (a torn write stays unread)", () => {
const p = (i, d, final = false) => JSON.stringify({ hook_event_name: "MessageDisplay", session_id: "s", message_id: "m", index: i, final, delta: d });
@@ -3521,6 +3521,42 @@ test("stream: holdback releases once past the detector's reach, and only then",
assert.equal(a.push(mdFire(2, "tail")), "tail", "subsequent deltas stream straight through");
});
// ── resolveStreamHoldback: the FLOOR under OCP_TUI_STREAM_HOLDBACK (A1 fix) ────────────
// The C-1 auth-banner guarantee holds only while the holdback >= the default detector's
// 100-char reach. These tests pin that the resolver CLAMPS UP to the floor. They are
// mutation-proof: delete the `parsed < floor` branch and the sub-floor cases below fail
// (a 50 would pass straight through, reopening the leak). The clamped flag drives the boot
// warning in server.mjs, so its truthiness is asserted alongside every value.
test("holdback: a sub-floor value is clamped UP to the floor and flagged", () => {
assert.deepEqual(resolveStreamHoldback("50"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("0"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("-5"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("99"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
});
test("holdback: garbage / NaN falls back to the floor and is flagged (not silently 0)", () => {
assert.deepEqual(resolveStreamHoldback("unlimited"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
assert.deepEqual(resolveStreamHoldback("5MB"), { value: DEFAULT_HOLDBACK_CHARS, clamped: true });
});
test("holdback: an above-floor value passes through unchanged and is NOT flagged", () => {
assert.deepEqual(resolveStreamHoldback("200"), { value: 200, clamped: false });
assert.deepEqual(resolveStreamHoldback("101"), { value: 101, clamped: false });
assert.deepEqual(resolveStreamHoldback(String(DEFAULT_HOLDBACK_CHARS)), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
});
test("holdback: an unset env var takes the floor WITHOUT flagging (no spurious boot warning)", () => {
assert.deepEqual(resolveStreamHoldback(undefined), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
assert.deepEqual(resolveStreamHoldback(null), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
assert.deepEqual(resolveStreamHoldback(""), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
assert.deepEqual(resolveStreamHoldback(" "), { value: DEFAULT_HOLDBACK_CHARS, clamped: false });
});
test("holdback: the floor is a parameter, so a deployment can raise (never lower) it", () => {
assert.deepEqual(resolveStreamHoldback("150", 200), { value: 200, clamped: true }, "custom floor still clamps up");
assert.deepEqual(resolveStreamHoldback("300", 200), { value: 300, clamped: false });
});
test("stream: a short answer never passes the holdback and is delivered whole at terminal", () => {
const T = "The capital of France is Paris.";
const a = new TuiDeltaAssembler();