diff --git a/lib/tui/stream.mjs b/lib/tui/stream.mjs index 69eba45..797f6dc 100644 --- a/lib/tui/stream.mjs +++ b/lib/tui/stream.mjs @@ -173,6 +173,9 @@ export function parseDeltaChunk(text, consumed = 0) { // fails the turn loudly (SSE error frame, no cache, counted on /health). Fail-loud is // the correct posture — a proxy that silently serves text the transcript disagrees with // is the exact class of bug ALIGNMENT.md exists to prevent. +// Sentinel for "no message seen yet". Deliberately not null/undefined — see the constructor. +const NO_MESSAGE_YET = Symbol("no-message-yet"); + export class TuiDeltaAssembler { constructor({ holdbackChars = DEFAULT_HOLDBACK_CHARS, detectError = detectTuiUpstreamError } = {}) { this.holdbackChars = holdbackChars; @@ -180,7 +183,12 @@ export class TuiDeltaAssembler { this.emitted = ""; // bytes ALREADY written to the client — unretractable this.pending = ""; // held back, not yet written this.released = false; - this.messageId = null; + // NOT null: a payload may legitimately carry message_id === null, and if the sentinel were + // also null the FIRST such payload would compare equal to it, register no boundary, and + // leave `messages` at 0 — which used to disarm the restartedAfterEmit guard below entirely. + // A unique object is === to nothing a JSON payload can produce, so the first fire ALWAYS + // registers as message 1, whatever its message_id is (or isn't). + this.messageId = NO_MESSAGE_YET; this.deltas = 0; // hook fires seen this.messages = 0; // distinct message_ids seen this.restartedAfterEmit = false; @@ -198,7 +206,12 @@ export class TuiDeltaAssembler { this.messages++; if (this.emitted === "") { this.pending = ""; // safe: the transcript will drop this message too - } else if (this.messages > 1) { + } else { + // A boundary while bytes are ALREADY out is unrecoverable, full stop — the count of + // messages seen so far is irrelevant. The old `else if (this.messages > 1)` guard was + // the sole reason a null-message_id first payload could disarm F1: it left `messages` + // at 0, so the real boundary evaluated 1 > 1 === false and never armed. The invariant + // is "a boundary occurred while emitted !== ''", and that is exactly what this says. this.restartedAfterEmit = true; // unrecoverable — finalize() will refuse the turn } } diff --git a/test-features.mjs b/test-features.mjs index 8384166..17140ae 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -3598,6 +3598,26 @@ test("F1: an auth banner rendered as a LATER message is never forwarded to the c assert.equal(a.finalize(BANNER).ok, false, "and the turn is refused, not served"); }); +test("F1: a first payload with message_id:null cannot disarm the guard", () => { + // The residual bypass the reviewer found by probing. `this.messageId` used to be initialized + // to null, so a first payload carrying message_id:null compared EQUAL to it → no boundary + // registered → `messages` stayed 0 → when the REAL boundary arrived, `messages > 1` evaluated + // 1 > 1 === false → restartedAfterEmit never armed → the released branch forwarded the banner. + // The whole F1 guard was disarmed by a single null field. parseDeltaChunk does not validate + // message_id, so such a payload does reach push(). + const a = new TuiDeltaAssembler({ holdbackChars: 100 }); + const narration = "I'll check that file for you and then report back with what I find inside it."; + a.push({ hook_event_name: "MessageDisplay", message_id: null, delta: narration + narration }); + assert.notEqual(a.emitted, "", "precondition: the narration released to the client"); + assert.equal(a.messages, 1, "a null message_id is still a MESSAGE — it must register as one"); + + const BANNER = "Please run /login · API Error: 401 Invalid authentication credentials"; + const out = a.push({ hook_event_name: "MessageDisplay", message_id: "m2", delta: BANNER }); + assert.equal(out, null, "the banner must not be forwarded — the guard must arm regardless"); + assert.equal(a.restartedAfterEmit, true); + assert.ok(!a.emitted.includes("401")); +}); + test("F1: whitespace cannot buy a release — the holdback screens TRIMMED length", () => { // detectTuiUpstreamError() TRIMS before applying its <=100-char rule, so gating release on // the UNTRIMMED pending.length let 101 spaces trim to "" → the detector has nothing to