fix(sse): reverse split disguise tokens in streaming responses (raw-string variant of #56) - #59
Open
hunandy14 wants to merge 2 commits into
Open
Conversation
reverseMap() only rewrites COMPLETE patterns. The streaming SSE response
transformer applied it per content_block_delta event, so a sanitized
identifier split across two delta events (".ocpla" then "tform") was
never reverse-mapped and the sanitized form leaked to the client. An
earlier slice-offset attempt instead emitted incomplete prefixes raw and
produced mangled output (".ocplalaw").
Add createSseEventTransformer() with a proper streaming replace:
- streamReverse() holds back trailing raw bytes that could still grow into
a pattern, emits only the safe prefix, and pulls the cut back so no
complete occurrence straddles it;
- the held tail is flushed as synthetic content_block_delta events on
content_block_stop, and flushAll() covers streams that end with no stop;
- thinking / redacted_thinking blocks remain byte-identical pass-throughs.
Export loadConfig/reverseMap/applySseReverseMapChunks and guard startServer
behind require.main so the transformer is unit-testable.
Adds test/sse-reversemap.test.js: asserts exact reconstruction at every
split offset (two-way and single-character) for text_delta and
input_json_delta, plus TCP-chunk splitting, flushAll, and thinking
byte-invariance.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zacdcook#56 review) Builds on zacdcook#56 (@rafaelreis-r, fixes zacdcook#55): streamReverse is kept verbatim; this only swaps the per-event JSON.parse/JSON.stringify for four hand-rolled raw-string codecs (findSseStringField, extractSseIntField, jsonStringDecode, jsonStringEncode) so the SSE reverse path never parses/re-serializes the body, preserving thinking-block byte-equality and prompt caching per the project's raw-string principle and the maintainer's zacdcook#56 review request. Only the single reversed string field is decoded/reversed/re-encoded in place; every other event byte passes through untouched; thinking/redacted_thinking remain byte-identical. jsonStringEncode also escapes lone surrogates so a value cut between a surrogate pair survives transport. Adds test/sse-reversemap.boundary.test.js; zacdcook#56's existing test/sse-reversemap.test.js passes unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on @rafaelreis-r's #56 (fixes #55). This is the raw-string variant requested in #56's review —
streamReverseis kept verbatim; the only change from #56 is swappingJSON.parse/JSON.stringifyfor hand-rolled raw-string codecs, so the SSE reverse path never parses the response body.Problem
On the SSE streaming response path,
reverseMapwas applied per event. A disguise token that the model streams token-by-token gets split across consecutive*_deltaevents, so no single event contains the complete string and the reverse mapping never matches. The sanitized form then leaks to the client. This affects:text_delta) — e.g.oc+platformreassembles toocplatform;input_json_delta) — including renamed property keys (thread_idsplit as{"thr+ead_id"...is never restored tosession_id) and renamed tool-name values, which then reach OpenClaw's tool runtime as the wrong key/value and break silently.Forward (request) and non-streaming responses operate on the whole buffer and are unaffected.
Root cause
transformEventranreverseMap(event)independently per SSE event.reverseMaponly rewrites complete patterns, and the in-code comment claiming patterns "can't span event boundaries" is wrong for token-by-token streaming (it only held for TCP fragmentation).Fix (raw-string, keeps
streamReverse)streamReversecarry-buffer algorithm is kept verbatim. It holds back trailing bytes that could still grow into a pattern, emits only the safe prefix, pulls the cut back so no complete occurrence straddles it, flushes the held tail as syntheticcontent_block_deltaevents oncontent_block_stop, andflushAll()covers streams that end without a stop. Per-(block index, field)buffering keeps interleaved blocks isolated.JSON.parse/JSON.stringifywith four hand-rolled raw-string codecs. Per the project's raw-string principle (byte-fidelity for thinking blocks + prompt caching), the body is never round-tripped through a JSON parser — a full re-encode would normalize\uXXXX,\/, numbers, whitespace, and key order across the whole event. Instead we locate just the one string field we must reverse, decode it,streamReverseit, and re-encode it in place:findSseStringField(s, field)— escape-aware scan returning the value's char offsets{start, end};extractSseIntField(s, field)— reads"index":Nwithout parsing;jsonStringDecode(s)— decodes a complete JSON string body to characters;jsonStringEncode(s)— re-encodes matchingJSON.stringify, and additionally escapes lone surrogates so a value cut between a surrogate pair survives transport and reassembles correctly.\"and cannot match.Key bit of the in-place rewrite:
Why this addresses the #56 review
The sole blocker on #56 was its use of
JSON.parse/JSON.stringify, which violates the "raw-string only, never parse the body" principle (byte-fidelity for thinking blocks and prompt caching). This variant does exactly what the review asked: keepsstreamReverseverbatim, extracts/encodes only the single reversed field value with hand-rolled helpers, and adds codec edge tests. No event is ever parsed or re-serialized as a whole.Test evidence
Both suites pass locally (
node --test):test/sse-reversemap.test.js— fix: reconstruct reverse-mapped tokens split across SSE delta events #56's own tests, unchanged, green against the new code. The behavior fix: reconstruct reverse-mapped tokens split across SSE delta events #56 specified is preserved exactly while the encoding mechanism changed.test/sse-reversemap.boundary.test.js— new boundary suite: every-offset 2-way/3-way splits, char-by-char, and byte-level TCP fragmentation, assertingreconstruct(stream) === reverseMap(whole)for real-quote and escaped-quote tool inputs (property keys, renamed tool-name values, paths), brand/dict targets in text, multi-byte/surrogate boundaries (including a cut inside a surrogate pair), escaped chars; thinking + redacted_thinking byte-equality; twotool_useblocks streaming concurrently stay isolated; flush withoutcontent_block_stop; escape-aware envelope anchoring; and the codec helpers. Every emitted event is asserted to be valid JSON.Credit & related PRs
streamReverseis theirs, kept verbatim; this PR is built on the same commit and only swaps the encoding for the raw-string variant the review requested.tool_use.inputmasking and buffersinput_json_delta(tool args only) on the response side, also viaJSON.parse. It conflicts with fix: reconstruct reverse-mapped tokens split across SSE delta events #56/this PR intransformEvent. They are complementary — fix: preserve tool_use input fields through forward transform pipeline #57's request-side mask plus this response-sidestreamReverseis the ideal combined state — but they collide in the same function and need reconciling by whoever lands second. This PR is response-side only.Scope
This PR is the streaming-reverse fix only (split disguise tokens / property keys / tool-name values in streaming responses). Out of scope and left for separate work:
reverseMap(external,usage quota,routing layer, etc. — unconditional substitution; needs anchoring/context limits, unrelated to streaming).tools:[]trailing comma, injectionindexOfcollision, prefill deletion, thinking-mask placeholder collision).