fix: command output streams live while it runs#15094

🟡 Medium Adapters/ClaudeTaskOutputTail.ts:113

When output ends with an incomplete UTF-8 sequence, tailClaudeTaskOutput silently drops the buffered bytes instead of emitting the documented U+FFFD replacement. The last branch returns without flushing decoder; call decoder.decode() once more and forward its result before returning.

No longer relevant as of ae8b5493b77616abdf5f03f0d8254d018199d9ba

🟡 Medium src/terminalOutput.ts:524

terminalOutputResumeText produces incorrect resync behavior: after ESC 7/CSI s, a later ESC 8/CSI u is ignored by the viewer, so abc\x1b7def followed by \x1b8X renders abcdefX instead of abcXef. The resume text also computes cursor movement from the full screen.lines even when state.text has been tail-truncated, positioning the cursor on the wrong displayed row after resync. Serialize the saved cursor and calculate the reconstructed cursor position relative to the retained output.

No longer relevant as of ae8b5493b77616abdf5f03f0d8254d018199d9ba

🟡 Medium Adapters/ClaudeAdapterV2.ts:3828

claudeCommandOutputText inserts an extra newline between nonempty stdout and stderr, so output such as { stdout: "ok\n", stderr: "warning\n" } is persisted as "ok\n\nwarning\n" instead of Claude's recorded "ok\nwarning\n". This makes the completed command row differ from the emitted output; concatenate the streams as ${stdout}${stderr} without adding a separator.

No longer relevant as of ae8b5493b77616abdf5f03f0d8254d018199d9ba

(agent reply) Partly agree, fixed differently in ae8b5493b7. Claude's Bash results trim trailing newlines from each stream (the recorded fixtures have e.g. "stdout":"~/apps/web$ tsgo --noEmit cache disabled" with no trailing \n), so plain ${stdout}${stderr} would glue the last stdout line to the first stderr line. It now adds the newline only when stdout doesn't already end in one, so neither case produces a blank or merged line.

— Claude Opus 5.5

🟠 High src/terminalOutput.ts:327

A large horizontal cursor move such as \x1b[999999999999C makes the next printable character throw RangeError: Invalid string length in writeLine, allowing arbitrary command output to crash the output path. moveTo accepts the unbounded column and line 126 passes it to " ".repeat; clamp cursor columns to a bounded tail size before writing.

No longer relevant as of ae8b5493b77616abdf5f03f0d8254d018199d9ba

🟡 Medium orchestration-v2/CommandOutputHub.ts:217

An eventSink.stream failure is swallowed as a successful, terminated fiber, so if the initial projection is still running, the subscription waits forever in Queue.take and never emits the declared OrchestrationV2CommandOutputError or closes. Propagate the stream failure to the subscription instead of converting it to a warning-only completion; finish does not wake subscribers.

No longer relevant as of ae8b5493b77616abdf5f03f0d8254d018199d9ba

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟠 High src/ws.ts:1998

subscribeCommandOutput creates an unbounded subscription for arbitrary threadId/itemId values. For a nonexistent or non-command item, CommandOutputHub.subscribe never settles, while the forked eventSink.stream remains subscribed to the global turn-item.updated PubSub before filtering; repeated RPC subscriptions therefore retain every future update once per subscriber and can exhaust server memory. Validate that the item is a command before subscribing and create the event stream with bounded: true.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove two unused exports that make the Knip CI check fail.

Two new constants are exported, but no module imports them. Knip reports both, and vp run knip:check fails.

• packages/shared/src/terminalOutput.ts#L70-L70: Remove export from LIVE_LINES.
• apps/server/src/orchestration-v2/Adapters/ClaudeTaskOutputTail.ts#L19-L19: Remove export from CLAUDE_TASK_OUTPUT_POLL_MS.

As per coding guidelines: "Did you run knip? A new export with no importer fails it."

Sources: Coding guidelines, Pipeline failures

✅ Addressed in commit ae8b549

@scratchyone Thanks for the fix. Making LIVE_LINES module-private addresses the finding in this file. Keep exports used by the client and tests; the second constant I flagged was CLAUDE_TASK_OUTPUT_POLL_MS in ClaudeTaskOutputTail.ts, which you also confirmed is fixed.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove the unused exports. They fail the Knip CI check.

The CI Lint job fails. Knip reports that COMMAND_OUTPUT_MAX_APPEND_CHARS and persistedCommandOutput are exported, but no other module imports them. Only this module uses them. Remove the export keyword from both declarations. The repository guideline states: "Knip fails CI on an unused export."

As per coding guidelines: "make stays private unless another module imports it. Knip fails CI on an unused export."

Also applies to: 91-91

Sources: Coding guidelines, Pipeline failures

✅ Addressed in commit ae8b549

(agent reply) Fixed in ae8b5493b7. COMMAND_OUTPUT_MAX_APPEND_CHARS and persistedCommandOutput are module-private now, as are CLAUDE_TASK_OUTPUT_POLL_MS and COMMAND_OUTPUT_STREAMING_COMMAND, which Knip also flagged.

— Claude Opus 5.5

@scratchyone Thanks for confirming the fix in ae8b5493b7. Making both declarations module-private addresses this finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix the TS2367 typecheck failure in spanStyle.

Line 23 casts the joined string to TextStyle["textDecorationLine"]. Line 30 then compares that value to "", which is not a member of the union type. The TypeScript check fails with TS2367. Keep the joined value as a plain string, and apply the cast only when you assign it.

Source: Pipeline failures

✅ Addressed in commit ae8b549

@scratchyone Thanks for fixing this in ae8b5493b7. Selecting the decoration directly from the two flags removes the invalid empty-string comparison and avoids the cast.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟡 Medium Adapters/ClaudeTaskOutputTail.ts:87

Fast-tail reads can decode the first byte with the incomplete UTF-8 sequence buffered from the discarded read, producing corrupted output. When skipped is true, flush the existing TextDecoder state and replace it with a fresh decoder before decoding the discontinuous tail.

No longer relevant as of 13a5482e779fb75238a5f16cdb9bb8d23b9387e1

(agent reply) Fixed in 13a5482e77. When a read skips ahead, the reader now starts a fresh TextDecoder, so bytes held from before the gap can't combine with the new tail.

— Claude Opus 5.5

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟡 Medium src/terminalOutput.ts:372

CSI 3 J currently clears the visible lines as well as scrollback, so output that erases saved lines loses the current terminal frame. Handle mode 3 by clearing only settled and preserving the visible lines.

No longer relevant as of 1fb6f4eaafde3d53631ef4d4dbfa0483b3cfbcfc

(agent reply) Fixed in 1fb6f4eaaf, as suggested. ESC[3J now clears only the lines that already scrolled out, keeps the window, and marks the output truncated. There's a test for it.

— Claude Opus 5.5

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟡 Medium src/terminalOutput.ts:376

CSI 0 J removes every row below the cursor, so a subsequent cursor-down is clamped to the current last row and later output overwrites the wrong progress row. Blank those rows while preserving lines.length so later cursor movement can still reach them.

Real but narrow, and leaving it as is. The model has no screen height, and common redraws (log-update, docker, clearScreenDown then \n) move down with newlines, which still work. The suggested fix (blank the rows, keep lines.length) regresses the common case: a log-update frame that shrinks from 3 lines to 1 ends as "done\n\n\n", and the existing "erases from the cursor to the end of the screen" test gets a trailing blank row. Getting both right needs hidden-row bookkeeping in the window and in the resume text, which isn't worth it for cursor-down after ESC[J.

🟡 Medium src/terminalOutput.ts:334

Colon-delimited true-color SGR such as ESC[38:2:0:255:0:0m is rendered green instead of red. Replacing : with ; removes the distinction between the color-space identifier (Pi) and RGB components, so extendedColor interprets 0,255,0 as the color; skip that identifier when parsing colon-form 38:2/48:2 parameters.

No longer relevant as of 1fb6f4eaafde3d53631ef4d4dbfa0483b3cfbcfc

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟠 High src/terminalOutput.ts:495

terminal.lines can retain up to LIVE_LINES * maxChars characters even though state.text is limited to maxChars, so 64 long lines consume about 4 MiB per command at the default. Across multiple tracked commands, this defeats the intended per-command memory bound; enforce an aggregate screen budget by evicting or compacting older live lines.

No longer relevant as of d6c0862a926ed04131635bf9361bfcabc66bd706

🟡 Medium orchestration-v2/CommandOutputHub.ts:101

Large final outputs can display ANSI escape parameters such as [31m as literal text. persistedCommandOutput slices the raw string at a UTF-16 offset before normalizeTerminalOutput, so a retained suffix beginning after \u001b is no longer recognized as an escape sequence; normalize first or truncate at a terminal-safe boundary.

No longer relevant as of e81d4e3979f060e0cbb22adaf327d2bb97cfa185

🟠 High src/terminalOutput.ts:328

Alternating SGR styles make writeLine copy the entire run array for every character, so processing one moderately sized colour-heavy chunk takes O(N²) time and can block live output ingestion for seconds or minutes. The tail limit is applied only after feed finishes, so it does not protect this path; avoid copying all existing runs on each append (for example, use an append-friendly representation or batch writes).

No longer relevant as of 095c07a9757dcd0674e93e720dfa44b48cb3fe92

(agent reply) Fixed in 497a4bfdeb. Lines created during a chunk are now extended in place instead of copied on every write. And once a line passes 2,048 style runs, its oldest runs fold into one plain run, so a colour change on every character stays linear. A test feeds 200K alternating-colour characters in one chunk. Locally, 1M such characters take about 0.3 s.

— Claude Opus 5.5

🟡 Medium src/terminalOutput.ts:517

appendTerminalOutput produces different tails for the same raw input depending on chunk boundaries: with maxChars = 3, separate chunks "abcdef" and "\rX" return "Xef", while one chunk "abcdef\rX" returns "def". Lines are truncated to their suffix at line 517, so the later carriage return operates on different columns; defer truncating cursor-rewritable lines or preserve their logical column offset/content across appends.

No longer relevant as of 095c07a9757dcd0674e93e720dfa44b48cb3fe92

🟠 High src/terminalOutput.ts:337

A large newline-delimited chunk temporarily retains an additional near-full copy of the entire scrolled log in this.settled, so a multi-megabyte delta exceeds the intended 64K bound and causes a synchronous allocation spike before terminalOutputTail runs. Trim this.settled incrementally while processing scrolled lines, and preserve the truncation flag.

No longer relevant as of 095c07a9757dcd0674e93e720dfa44b48cb3fe92

(agent reply) Fixed in 497a4bfdeb. Settled scrollback is trimmed as lines leave the window, once it passes twice the tail, and the truncation flag is set. A single multi-megabyte chunk no longer builds a near-full copy of the log. A test checks that scrollback stays within twice the tail across a 200K-line chunk.

— Claude Opus 5.5

🟡 Medium chat/V2ItemInspector.tsx:106

When skip is true, historical Claude command rows render the persisted raw {"stdout", "stderr", "interrupted", ...} result as serialized JSON instead of showing stdout/stderr. The !skip guard prevents useTurnItemDetail from fetching the item that turnItemOutputText/commandOutputText would normalize, while persistedCommandOutput does not perform that conversion; preserve the detail fetch or normalize this payload before rendering.

No longer relevant as of d76bf451d5e6a2d08910bee9b4c3d626f7d4ed3d

(agent reply) Fixed in f2c583844e. Good catch: the live panel replaces #15505's fetch, which was what unwrapped these old rows. The hub now decodes a persisted {"stdout", "stderr", …} Claude result (with Schema.fromJsonString) into its stdout and stderr before cleaning it, using the same newline-aware join as the adapter. There's a test for it.

— Claude Opus 5.5

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟡 Medium Adapters/ClaudeAdapterV2.ts:5920

Foreground local_bash tasks with an omitted message.is_backgrounded field never start tailClaudeTaskOutput, so their live command output is missing even though the task file and final result exist. Treat an omitted field as foreground here, while continuing to exclude explicitly backgrounded tasks.

No longer relevant as of d4436d8b8827849339f37a7983dc5a1b1ad7f227

(agent reply) Fixed in f2c583844e, as suggested. The check is now is_backgrounded !== true, so an omitted flag counts as foreground, and explicitly backgrounded tasks are still excluded. Tailing stays harmless if the guess is ever wrong: it stops when the tool result arrives.

— Claude Opus 5.5

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Correct the frame contract: text still contains terminal escapes.

The comment says that frame text "is already stripped of terminal escapes." The producer does not strip them:

• CommandOutputHub builds replace frames from terminalOutputResumeText(tail). This text keeps canonical SGR codes (ESC[0;…m). It also adds cursor moves (ESC[nA, ESC[nG), cursor saves (ESC7), and any pending escape fragment.
• append frames carry raw provider chunks without any normalization.

A client that trusts this contract can render the text directly and show escape bytes. The client must instead feed text through appendTerminalOutput, as the web and mobile rows do. This schema is the cross-surface contract, so the comment must match the real behavior.

✅ Confirmed as addressed by @scratchyone

(agent reply) Fixed in f2c583844e. The comment now says frames carry terminal text, not display text. replace holds the cleaned tail plus the cursor and style escapes needed to resume, append holds raw chunks, and clients must feed both through appendTerminalOutput.

— Claude Opus 5.5

@scratchyone Thanks for the fix. The updated comment in packages/contracts/src/orchestrationV2.ts now describes the terminal escapes and the required appendTerminalOutput handling.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

You are interacting with an AI system.

🟡 Medium orchestration-v2/CommandOutputHub.ts:162

Evicting a tail at MAX_LIVE_COMMANDS leaves existing subscribers believing their client state is still based on that tail, so the next chunk is emitted as an append against stale output (for example, abc plus \rX displays Xbc instead of X). Mark the evicted command's subscribers as needing a replacement and discard their pending append before deleting the tail.

No longer relevant as of d4436d8b8827849339f37a7983dc5a1b1ad7f227

🟡 Medium src/terminalOutput.ts:532

A long OSC/DCS sequence split across chunks leaks its payload into rendered output, even though the same sequence in one chunk is discarded. When the buffered fragment exceeds MAX_PENDING_ESCAPE, feed returns an empty pending, so later chunks are parsed as printable text until the terminator; preserve a discard-until-terminator state instead of clearing pending.

No longer relevant as of d4436d8b8827849339f37a7983dc5a1b1ad7f227

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Decode valid legacy results regardless of JSON key order.

If a stored Bash result starts with {"stderr" or { "stdout", this prefix check returns the raw JSON before the schema can decode it. The completed command row then shows JSON instead of command output. Remove the prefix check and let decodeLegacyClaudeBashResult decide whether to preserve the original string.

🟠 High Adapters/CodexAdapterV2.ts:3884

High-volume commands accumulate every command_output.delta chunk in the unbounded events queue, so the server can exhaust memory before startEventPump reaches its special command_output.delta branch. outputDelta should send these chunks directly to the hub or use bounded backpressure/coalescing instead of enqueueing them through emitProviderEvent.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟠 High orchestration-v2/CommandOutputHub.ts:120

When persisted output exceeds MAX_FINAL_SOURCE_CHARS and the bounded suffix ends at its first newline, persistedCommandOutput returns an empty string, so the final replace erases the output shown for the command. Preserve the bounded suffix when skipping the partial line would remove the entire tail.

✅ Resolved in e81d4e3979f060e0cbb22adaf327d2bb97cfa185

🟡 Medium orchestration-v2/CommandOutputHub.ts:283

During the handoff after finish removes the live tail but before the persisted settlement is observed, a throttled frame replaces the panel with snapshotOutput—usually empty for delta-streaming providers—so the displayed output disappears. The running-frame path reads only tails; fall back to finished so it can keep displaying the retained tail during this interval.