fix(shell): preserve final summaries beyond the output cap

docs/issue-247-validation.md

@@ -0,0 +1,39 @@
+# Issue #247: final shell output retention
+
+Reviewed against the original issue on 2026-10-01, based on main
+`892b27b405964b779b7a7d7215f3458278a3f038`.
+
+The original one-shot implementation retained a prefix up to 64 MiB and then
+discarded all subsequent chunks. The persistent shell retained only its first
+8,000 characters. Both could drop the real final summary.
+
+## Acceptance evidence
+
+| Original requirement | Implementation and regression evidence |
+| --- | --- |
+| Bounded head and rolling tail; continue draining both pipes | `BoundedOutput` owns two fixed buffers (7,920 payload bytes within an 8,000-byte body budget). Both execution paths continue reading and streaming stdout/stderr. Tests count every emitted byte across 66 MiB real child output. |
+| Actual final bytes survive beyond 64 MiB | `run_shell`, `run_tests`, direct user execution, and persistent user/model execution retain initial output plus distinct final stdout/stderr markers and authoritative exit 7. |
+| Single oversized chunk | A direct 65 MiB append retains `HEAD_247` and `FINAL SUMMARY: 1 failed`, with fixed retained capacity. |
+| Incremental UTF-8, interleaved pipes | Independent pipe decoders; unit tests cover two-, three-, and four-byte cuts, wrapping, and exact omission accounting. Child fixtures stagger individual UTF-8 bytes across both pipes; persistent callbacks never split surrogate pairs. |
+| Explicit omissions without invented summary | Rendered notices count omitted decoded UTF-8 bytes; final text is copied from actual output. Malformed input is decoded using Node's normal replacement semantics, so counts describe normalized UTF-8 rather than malformed raw bytes. |
+| Cancellation and timeout distinctions | Real post-cap termination preserves summaries and codes 130/124 for both execution paths. Persistent sessions visibly lose state, refuse execution until reset, and produce clean output afterward. |
+| Completion and isolation | Capture renders after pipe draining/decoder flush. Persistent protocol markers never appear in normal capture or callbacks; next commands have clean output. |
+
+Explicit `/shell-result` sharing also uses bounded head/tail capture so a long
+Unicode command cannot cause a second head-only truncation of the final summary.
+File/web/search `capHeadTail` behavior remains unchanged.
+
+## Validation
+
+Seven new integration regressions failed on the unmodified implementation and
+passed after the fix. Independent review included 5,000 randomized Unicode
+capture cases without a prefix/suffix, byte-count, boundary, or budget failure.
+
+Local environment: Linux, Node 24.19.0. The checked-in tests also run in the
+repository's Windows CI job; persistent Bash tests explicitly skip unsupported
+platforms.
+
+Final local suite and hosted-check results are recorded in the PR and issue
+closure comment. The initial full-suite run exposed two fixed-delay console
+routing fixtures; those scenarios passed independently, and their synchronization
+was hardened to observe completion rather than assume wall-clock timing.

src/commands/console_input.ts

@@ -1,6 +1,7 @@
 import { ShellSession, type ShellCommandEvent } from "../core/shell_session.js";
 import { randomUUID } from "node:crypto";
 import { ToolExecutor } from "../core/tool_executor.js";
+import { BoundedOutput } from "../core/bounded_output.js";
 import { sanitizeServerText } from "../core/transport.js";
 
 /** Classify before history, prompt rewriting, or the busy queue. */
@@ -58,7 +59,9 @@ export class ConsoleShell {
     } });
     if (fallback) this.event({ ...fallback, state: result.exitCode === 130 ? "cancelled" : "completed", exitCode: result.exitCode });
     const full = `!${input.command}\ncwd: ${this.session.cwd}\nexit: ${result.exitCode}\n${result.output}`;
-    this.result = Buffer.from(full).subarray(0, 8192).toString("utf8").replace(/\ufffd$/, "");
+    const shared = new BoundedOutput(8192);
+    shared.append(full);
+    this.result = shared.render();
     // Stream once; retain the bounded capture for explicit sharing. Refusal and
     // state-loss explanations still render even when some output was streamed.
     const visible = streamed ? result.output.split("\n", 1)[0]! : result.output;

src/core/bounded_output.ts

@@ -0,0 +1,73 @@
+/** Bounded UTF-8 capture of decoded output, in observed pipe-event order.
+ * Callers decode each pipe independently before appending. The fixed buffers
+ * own their bytes: even a huge input cannot leave a retained backing buffer.
+ */
+export class BoundedOutput {
+  private readonly head: Buffer;
+  private readonly tail: Buffer;
+  private headLength = 0;
+  private tailLength = 0;
+  private tailNext = 0;
+  private totalBytes = 0;
+
+  constructor(maxBytes = 8000) {
+    if (!Number.isSafeInteger(maxBytes) || maxBytes < 128) throw new RangeError("output budget must be at least 128 bytes");
+    // Reserve space for the omission notice, including a safe-integer count.
+    const capacity = maxBytes - 80;
+    this.head = Buffer.alloc(Math.floor(capacity / 3));
+    this.tail = Buffer.alloc(capacity - this.head.length);
+  }
+
+  /** Fixed allocation, independent of the number/size of incoming chunks. */
+  get capacityBytes(): number { return this.head.length + this.tail.length; }
+  get retainedBytes(): number { return this.headLength + this.tailLength; }
+
+  append(text: string): void {
+    const bytes = Buffer.from(text, "utf8");
+    this.totalBytes += bytes.length;
+    let offset = 0;
+    if (this.headLength < this.head.length) {
+      const count = Math.min(bytes.length, this.head.length - this.headLength);
+      bytes.copy(this.head, this.headLength, 0, count);
+      this.headLength += count;
+      offset = count;
+    }
+    const remaining = bytes.length - offset;
+    if (remaining >= this.tail.length) {
+      bytes.copy(this.tail, 0, bytes.length - this.tail.length);
+      this.tailLength = this.tail.length;
+      this.tailNext = 0;
+    } else if (remaining > 0) {
+      const first = Math.min(remaining, this.tail.length - this.tailNext);
+      bytes.copy(this.tail, this.tailNext, offset, offset + first);
+      bytes.copy(this.tail, 0, offset + first);
+      this.tailNext = (this.tailNext + remaining) % this.tail.length;
+      this.tailLength = Math.min(this.tail.length, this.tailLength + remaining);
+    }
+  }
+
+  render(): string {
+    const tail = this.tailLength < this.tail.length
+      ? this.tail.subarray(0, this.tailLength)
+      : Buffer.concat([this.tail.subarray(this.tailNext), this.tail.subarray(0, this.tailNext)]);
+    const head = this.head.subarray(0, this.headLength);
+    if (this.totalBytes <= this.capacityBytes) return Buffer.concat([head, tail]).toString("utf8");
+
+    // Never manufacture replacement characters at either elision boundary.
+    // Appended text contains complete code points; only our cuts can split one.
+    let headEnd = head.length;
+    if (headEnd > 0) {
+      let start = headEnd - 1;
+      while (start > 0 && (head[start]! & 0xc0) === 0x80) start--;
+      const lead = head[start]!;
+      const width = lead < 0x80 ? 1 : lead < 0xe0 ? 2 : lead < 0xf0 ? 3 : 4;
+      if (headEnd - start < width) headEnd = start;
+    }
+    let tailStart = 0;
+    while (tailStart < tail.length && (tail[tailStart]! & 0xc0) === 0x80) tailStart++;
+    const omitted = this.totalBytes - headEnd - (tail.length - tailStart);
+    return head.subarray(0, headEnd).toString("utf8")
+      + `\n…[${omitted} UTF-8 bytes elided]…\n`
+      + tail.subarray(tailStart).toString("utf8");
+  }
+}

src/core/shell_session.ts

@@ -4,6 +4,7 @@ import { realpathSync, statSync } from "node:fs";
 import { resolve, sep } from "node:path";
 import type { Readable } from "node:stream";
 import { StringDecoder } from "node:string_decoder";
+import { BoundedOutput } from "./bounded_output.js";
 import { childEnv } from "./child_env.js";
 import type { RunOptions, ToolResult } from "./tool_executor.js";
 
@@ -157,19 +158,17 @@ export class ShellSession {
     emit("running");
     return new Promise<ToolResult>((settle) => {
       const marker = `\x1e${commandId}\x1f`;
-      let output = "";
+      const output = new BoundedOutput();
       let completed = false;
       let control = "";
       let code: number | null = null;
       let cwd: string | null = null;
       let outDone = false;
       let errDone = false;
       const retain = (text: string): void => {
-        if (output.length < 8000) {
-          const kept = text.slice(0, 8000 - output.length);
-          output += kept;
-          options.onOutput?.(kept);
-        }
+        if (!text) return;
+        output.append(text);
+        options.onOutput?.(text);
       };
       const streamReader = (stream: Readable, end: () => void): (() => void) => {
         let pending = "";
@@ -183,7 +182,9 @@ export class ShellSession {
             end();
           } else {
             // A marker may span chunks. Drain everything except its suffix.
-            const safe = Math.max(0, pending.length - marker.length + 1);
+            let safe = Math.max(0, pending.length - marker.length + 1);
+            // The marker lookbehind must not divide an astral code point.
+            if (safe > 0 && /[\uD800-\uDBFF]/.test(pending[safe - 1]!)) safe--;
             retain(pending.slice(0, safe));
             pending = pending.slice(safe);
           }
@@ -197,7 +198,7 @@ export class ShellSession {
         clearTimeout(timer);
         options.signal?.removeEventListener("abort", abort);
         cleanupOut(); cleanupErr();
-        if (state !== "completed") result.output += output;
+        result.output += output.render();
         fd.off("data", onControl);
         this.failActive = null;
         emit(state, result.exitCode);
@@ -213,7 +214,7 @@ export class ShellSession {
           return;
         }
         this.cwd = physical;
-        finish({ output: `[exit ${code}]\n${output}`, exitCode: code }, "completed");
+        finish({ output: `[exit ${code}]\n`, exitCode: code }, "completed");
       };
       const cleanupOut = streamReader(child.stdout!, () => { outDone = true; check(); });
       const cleanupErr = streamReader(child.stderr!, () => { errDone = true; check(); });

src/core/tool_executor.ts

@@ -9,6 +9,8 @@
 import { spawn, spawnSync } from "node:child_process";
 import { closeSync, constants as fsConstants, existsSync, mkdirSync, openSync, readFileSync, readdirSync, realpathSync, statSync, writeFileSync } from "node:fs";
 import { dirname, relative, resolve, sep } from "node:path";
+import { StringDecoder } from "node:string_decoder";
+import { BoundedOutput } from "./bounded_output.js";
 import type { ToolName } from "./brain_protocol.js";
 import { validateToolCall } from "./tool_registry.js";
 import { GitCommitGuard, SpawnGitRunner } from "./git_commit_guard.js";
@@ -257,18 +259,19 @@ export class ToolExecutor {
         stdio: ["ignore", "pipe", "pipe"],
       });
 
-      let out = "";
-      let bytes = 0;
-      const CAP = 64 * 1024 * 1024;
-      const absorb = (chunk: Buffer): void => {
-        options.onOutput?.(chunk.toString("utf8"));
-        bytes += chunk.length;
-        // Keep draining past the cap so the pipe never blocks the child, but
-        // stop retaining; capHeadTail trims the ends at the boundary anyway.
-        if (bytes <= CAP) out += chunk.toString("utf8");
+      const output = new BoundedOutput(MAX_OUTPUT);
+      const absorb = (text: string): void => {
+        if (!text) return;
+        output.append(text);
+        options.onOutput?.(text);
       };
-      child.stdout?.on("data", absorb);
-      child.stderr?.on("data", absorb);
+      // stdout and stderr may interleave mid-codepoint. Each owns a decoder,
+      // while capture and live output receive every complete decoded chunk.
+      for (const pipe of [child.stdout, child.stderr]) {
+        const decoder = new StringDecoder("utf8");
+        pipe?.on("data", (chunk: Buffer) => absorb(decoder.write(chunk)));
+        pipe?.on("end", () => absorb(decoder.end()));
+      }
 
       let settled = false;
       let verdict: "timeout" | "aborted" | null = null;
@@ -324,7 +327,7 @@ export class ToolExecutor {
       // 'close' rather than 'exit': it fires once the pipes are drained, so a
       // test summary arriving with the exit is not lost.
       child.on("close", (code, sig) => {
-        const body = capHeadTail(out, MAX_OUTPUT);
+        const body = output.render();
         if (verdict === "timeout") {
           finish({ output: `[timeout after ${Math.round(timeoutMs / 1000)}s]\n${body}`, exitCode: 124 });
           return;

test/bounded_output.test.ts

@@ -0,0 +1,69 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { BoundedOutput } from "../src/core/bounded_output.js";
+
+function verify(capture: BoundedOutput, source: string, budget: number): void {
+  const rendered = capture.render();
+  assert.ok(Buffer.byteLength(rendered) <= budget);
+  assert.ok(capture.retainedBytes <= capture.capacityBytes);
+  const match = /\n…\[(\d+) UTF-8 bytes elided\]…\n/.exec(rendered);
+  if (!match) { assert.equal(rendered, source); return; }
+  const head = rendered.slice(0, match.index);
+  const tail = rendered.slice(match.index + match[0].length);
+  assert.ok(source.startsWith(head));
+  assert.ok(source.endsWith(tail));
+  assert.equal(Number(match[1]), Buffer.byteLength(source) - Buffer.byteLength(head + tail));
+  assert.doesNotMatch(head + tail, /\ufffd|[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/u);
+}
+
+test("bounded output preserves short text and code points split at the internal head boundary", () => {
+  const capture = new BoundedOutput(128);
+  const source = "a".repeat(15) + "😀€界";
+  capture.append(source);
+  verify(capture, source, 128);
+  assert.equal(capture.render(), source);
+});
+
+test("one chunk beyond 64 MiB retains the genuine head and tail in fixed owned buffers", () => {
+  const capture = new BoundedOutput();
+  const source = "HEAD_247" + "x".repeat(65 * 1024 * 1024) + "FINAL SUMMARY: 1 failed";
+  capture.append(source);
+  verify(capture, source, 8000);
+  assert.match(capture.render(), /^HEAD_247/);
+  assert.ok(capture.render().endsWith("FINAL SUMMARY: 1 failed"));
+  assert.equal(capture.capacityBytes, 7920);
+  assert.equal(capture.retainedBytes, 7920);
+});
+
+test("rolling tail wraps correctly across many chunks and repeated renders", () => {
+  const capture = new BoundedOutput(256);
+  let source = "";
+  for (let index = 0; index < 1500; index++) {
+    const chunk = `${index}:` + "€😀界z".repeat(index % 11);
+    capture.append(chunk);
+    source += chunk;
+    verify(capture, source, 256);
+  }
+});
+
+test("UTF-8 head and tail cuts omit complete partial boundary bytes accurately", () => {
+  for (const point of ["é", "€", "😀"]) {
+    for (let offset = 0; offset < 8; offset++) {
+      const capture = new BoundedOutput(128);
+      const source = "x".repeat(offset) + point.repeat(100) + "FINAL";
+      for (const character of source) capture.append(character);
+      verify(capture, source, 128);
+      assert.ok(capture.render().endsWith("FINAL"));
+    }
+  }
+});
+
+test("byte count describes normalized decoded UTF-8, including actual replacement characters", () => {
+  const capture = new BoundedOutput(128);
+  const source = "\ufffd".repeat(100);
+  capture.append(source);
+  const rendered = capture.render();
+  const match = /\[(\d+) UTF-8 bytes elided\]/.exec(rendered)!;
+  const retained = rendered.replace(/\n…\[\d+ UTF-8 bytes elided\]…\n/, "");
+  assert.equal(Number(match[1]), Buffer.byteLength(source) - Buffer.byteLength(retained));
+});