fix(agents): exec output shows escape codes when a sequence spans a chunk#111364
Conversation
The local exec runtime sanitized each stdout/stderr chunk with the stateless `sanitizeBinaryOutput`, so an escape sequence straddling a read boundary was escaped into visible text instead of being consumed. The remote bash path already uses a per-stream parser via `createStreamingBinaryOutputSanitizer` (openclaw#103706); this applies the same treatment to the local path, with separate parsers for stdout and stderr so neither stream can consume the other's pending sequence.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 6:55 AM ET / 10:55 UTC. Summary PR surface: Source +4, Tests +126. Total +130 across 2 files. Reproducibility: no. direct current-main execution was performed in this read-only review, but the stateless per-callback sanitizer path and the supplied split-chunk harness provide a high-confidence source reproduction path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Merge the local exec wiring after the current-head CI/maintainer review confirms the focused parser-state fix remains cleanly applicable. Do we have a high-confidence way to reproduce the issue? No direct current-main execution was performed in this read-only review, but the stateless per-callback sanitizer path and the supplied split-chunk harness provide a high-confidence source reproduction path. Is this the best way to solve the issue? Yes. Reusing the already-merged per-stream streaming-sanitizer pattern is the narrowest maintainable fix; separate stdout and stderr instances preserve stream isolation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against bbcfec9e969d. Label changesLabel justifications:
Evidence reviewedPR surface: Source +4, Tests +126. Total +130 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
|
Merged via squash.
|
…nclaw#111364) The local exec runtime sanitized each stdout/stderr chunk with the stateless `sanitizeBinaryOutput`, so an escape sequence straddling a read boundary was escaped into visible text instead of being consumed. The remote bash path already uses a per-stream parser via `createStreamingBinaryOutputSanitizer` (openclaw#103706); this applies the same treatment to the local path, with separate parsers for stdout and stderr so neither stream can consume the other's pending sequence.
2 participants
What Problem This Solves
Fixes an issue where users running the
exectool would see escape codes asliteral garbage in command output. Running anything with coloured or progress
output —
npm install, a coloured test run,docker build— can split an escapesequence across a stream read, and when that happens the tool prints the sequence
as visible text (
\x1b[31mREDinstead ofRED). The corrupted text also lands inthe transcript the model reads, so the agent can misread the command's output.
Chunk boundaries are arbitrary, so this shows up intermittently on exactly the
long, noisy commands where output clarity matters most.
Why This Change Was Made
The remote bash path already solved this. #103706 replaced the stateless
sanitizeBinaryOutputwith a per-stream parser insrc/agents/sessions/bash-executor.ts:51, and the helper it added carries theintent in its own doc comment:
The local
execruntime was not converted and still called the stateless helperon every chunk. That helper is behaving as documented — it deliberately escapes a
pending CSI so a split final byte "cannot leak from the following chunk"
(
shell-utils.ts:359) — but that contract only makes sense when no parser stateis carried between calls. This change gives the local path the same per-stream
parser the remote path uses.
stdout and stderr get separate parsers. They are independent streams, and a
shared parser would let one stream consume the other's pending sequence.
Scope is limited to wiring the existing helper into the two handlers; no new
sanitization behavior is introduced and the
detectCursorKeyModeprobe still runson the raw chunk before stripping, as before.
User Impact
execoutput no longer shows stray escape codes when a sequence spans a chunkboundary. Users see the text the command actually produced, and the model reads
the same clean text. Commands whose escape sequences never straddle a read
boundary behave exactly as before.
Evidence
Real-behavior proof — real child process, real pipe, real OS-level split.
A real
nodechild writesstart:, thenESC [, then sleeps 250 ms, then writes31mRED ESC [0m:end. The sleep guarantees the pipe delivers the sequence in twoseparate chunks. The real
runExecProcessconsumes it through the realProcessSupervisor— nothing mocked.Before (this branch with the source change reverted):
After:
Regression tests. Three cases in a new focused suite: ANSI and OSC sequences
split across stdout chunks (mirroring the sibling suite added by #103706), a
sequence split across stderr chunks, and a case where both streams leave a
sequence dangling simultaneously — which fails if the two streams share one
parser.
The tests are load-bearing. Reverting only the source change (tests untouched)
turns all three red, showing the exact leaked text:
Local checks. New suite 3 passed;
bash-tools.exec-runtime.pty-fallback.test.tspassed;
src/scripts/test-projects.test.ts85 passed.oxlintclean,oxfmt --checkclean,check-max-lines-ratchetclean.Two failures in neighbouring suites on this Windows machine
(
bash-executor.test.ts"stores truncated full output in an owner-only temp file",and
shell-utils.test.tsdetectRuntimeShellreporting an empty platform-gatedsuite) are pre-existing: they reproduce identically with this branch's source
change reverted.
AI-assisted.