Skip to content

Commit f4efe71

Browse files
serge-the-devclaude
andcommitted
fix(runner): a file reaches the Pilot — a tool result keeps its lines and is capped per tool, not collapsed to 240 chars (#700)
`toolResult()` in `headless.ts` did two things to every engine `tool_result` before the pane existed: collapsed `\s+` to a single space, and cut it to 240 characters. The consequence is not that results were short — it is that the SHAPE of a result no longer depended on the command that produced it. `cat`, `sed -n '1,50p'`, `cat -n | head -60` and "print every character" all arrived identically sized, on one line, indentation gone. So a Pilot that could not see a file had no move left. Live run `csess_6ce3627a`: twelve of sixteen instructions were one file being asked for twelve ways, each a full BYOK Claude decision round, and step 9292 concluded "the CLI is summarizing file contents rather than showing raw output" — the correct reading of the evidence in front of it, and the wrong diagnosis, because the evidence had been destroyed upstream. Measured across 20 sessions: 150 of 251 tool-result lines (59.8%) hit the cap, 43 of them `Read`. WHAT CHANGED, and what each costs - **Line structure survives.** `transcript-lines.ts` keeps newlines; every line after the first carries ` │ `. This is the half that matters most and costs 4 characters a line: `cat -n` output is unreadable space-joined at ANY length, so the collapse made the cap worse than its size alone implies. The prefix also means a result can no longer FORGE the framing — a file containing a line starting `⚙ ` would otherwise parse as a call the engine never made. - **The cap is per tool, deliberately not one raised number.** 1,500 characters over 60 lines for `Read`/`Bash`/`Grep`/`Glob`/`BashOutput`/`NotebookRead`; the historical 240 for everything else, because an `Edit`'s "has been updated successfully" was never near the cap and widening it would spend pane and buy nothing. An unknown tool takes the conservative budget. - **A cut states its own size** — `…[cut: 1,500 of 18,432 chars]` — so "ask for a slice that fits" is a strategy rather than a guess. WHAT IT COSTS THE PANE, stated rather than implied A `terminal` row stores 8,000 characters and one Pilot decision reads 6,000, so every character bought here is bought by evicting history. 1,500 is derived from that, not picked: a quarter of the Pilot's window in content, and `RESULT_RENDERED_MAX` (~1,840) once per-line framing and the disclosure are paid for — under a third of the window, so three consecutive content results coexist with the `⚙`/`↳` framing and the narrative instead of replacing them. Both figures are pinned as arithmetic in `transcript-lines.test.ts`; anyone raising the cap has to argue with them. TWO CONSEQUENCES ELSEWHERE, both fixed here because leaving either would make this a net loss - `parseEngineToolCalls` read the arrow line only, so a 34-line result would have been reported as its first line, and `outputCut` (which keyed on a trailing `…`) would have read a cut result as a complete one — the #503 shape. It now reads ` │ ` continuations, bounded by an `inResult` flag so a stranded tail whose arrow line fell outside the 8,000-char window is dropped rather than attributed to the wrong call. - `FEED_TOOLCALL_BYTES` 12,000 → 24,000. That knee was measured against 240-char results; at the new cap the same mean production row (12.05 calls) serialises to ~25,000 B, and a call dropped by that cap is gone for good — no cursor reaches it. 24,000 is one of the two numbers the original measurement showed behaviour-identical on that corpus. THE PROMPT NOW STATES A MECHANISM, NOT A NUMBER `coding-loop.ts` said "cut to ~240 characters on a single line" and "NO shell command can show you the contents of a file". True of every runner then published, false of an upgraded one — the same defect pointing the other way, and the cloud cannot tell which machine it is talking to. It now names the SIGNAL a Pilot can read on either: a cut result says it was cut and by how much; ask for a slice that fits ONCE; if the next result is cut at the same size and shape, the cap is on the result and re-asking is the same request. The untruncated reply channel is still named, because it works on every runner ever published. VERIFIED END TO END, not at the unit boundary `headless.test.ts` drives a REAL spawned stream-json engine returning the same ~4,000-character file two ways. The `Read` route now delivers 34 numbered lines as lines (`marker001`…`marker030` present, `marker092` absent, the cut figures stated) against ~5 lines space-joined before, with the `⚙ Read` and ` ↳✓` framing intact. That harness is the one this issue's earlier round used to disprove the pane-window attribution, and it is the level the claim has to hold at. SHIPPING: `packages/cli` 0.4.54 → 0.4.55. The runner half reaches a machine only through a published CLI, and 0.4.54 is already on npm so CI's publish step has been skipping. This one bump also carries #636 (a stopped application lands under Cancelled) and #641 (a handoff whose page was closed is lost, not solved), both of which have been sitting on `main` finished and unshipped for exactly that reason. Not run: `pnpm test:e2e`. This change touches no `store/`, console or `agents/coder/web` code, so nothing Playwright exercises is in the diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9c3d45e commit f4efe71

11 files changed

Lines changed: 691 additions & 117 deletions

File tree

‎packages/browser-runner/src/coding/headless.test.ts‎

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { tmpdir } from "node:os";
33
import { join } from "node:path";
44
import { afterAll, beforeAll, describe, expect, it } from "vitest";
55
import { buildClaudeArgs, defaultStatePath, HeadlessSession, parseCommand, mergeEnv } from "./headless.js";
6+
import { RESULT_RENDERED_MAX, toolResultBudget } from "./transcript-lines.js";
67

78
/**
89
* A stand-in for `claude -p --input-format stream-json --output-format stream-json`:
@@ -594,13 +595,14 @@ describe("HeadlessSession — a stream-json turn reports its own error (#545)",
594595
*
595596
* The run this pins spent twelve of sixteen Pilot decisions rephrasing one request — `cat`, then
596597
* `sed -n '1,100p'`, then `cat -n | head -60`, then "print every character" — because the file
597-
* never arrived and nothing told it why. Every one of those routes through a tool_result, and
598-
* {@link toolResult} caps it at 240 characters and collapses `\s+` to a single space BEFORE the
599-
* pane exists, so all four produce the same size and the same shape. The search space was empty.
598+
* never arrived and nothing told it why. Every one of those routes through a tool_result, which the
599+
* runner cut to 240 characters with `\s+` collapsed to a single space BEFORE the pane existed, so
600+
* all four produced the same size and the same shape. The search space was empty.
600601
*
601-
* These two tests measure both routes against the SAME ~4,000-character file, because the prompt
602-
* fix is only honest if the channel it points at is actually wider. The reply-text route is the
603-
* one the prompt now names; if this pair ever stops holding, that advice is a placebo.
602+
* These tests measure both routes against the SAME ~4,000-character file, through a REAL spawned
603+
* engine rather than by calling the renderer directly — the claim being pinned is end-to-end
604+
* ("a file reaches the Pilot"), and the earlier attribution of this failure to the pane window
605+
* rather than the runner is what a unit-level check would have missed again.
604606
*/
605607
describe("HeadlessSession — how much of a file reaches the pane (#700)", () => {
606608
let dir: string;
@@ -615,7 +617,7 @@ describe("HeadlessSession — how much of a file reaches the pane (#700)", () =>
615617
});
616618
afterAll(() => rmSync(dir, { recursive: true, force: true }));
617619

618-
it("a tool_result loses ~94% of it — 240 chars, on one line, whatever command produced it", async () => {
620+
it("a Read result arrives as LINES, ~1,500 characters of them, and says how much it lost", async () => {
619621
expect(file.length).toBeGreaterThan(4000);
620622
const s = new HeadlessSession({ id: "file-tool", workDir: dir, clientType: "claude", bin, statePath: defaultStatePath(dir) });
621623
s.start();
@@ -624,25 +626,46 @@ describe("HeadlessSession — how much of a file reaches the pane (#700)", () =>
624626
s.input("read web/e2e/helpers.ts"); // no "quote" — the engine uses its Read tool
625627
await until(() => s.snapshot().includes("↳"), 8000, "the tool result line");
626628
const pane = s.snapshot();
627-
628-
const resultLine = pane.split("\n").find((l) => l.startsWith(" ↳"));
629-
expect(resultLine).toBeDefined();
630-
// `" ↳✓ "` (6) + the 240-char cap + the ellipsis. 246 is the maximum observed in production
631-
// across 251 sampled result lines, and this is where that number comes from.
632-
expect((resultLine as string).length).toBeLessThanOrEqual(246);
633-
expect(resultLine as string).toMatch(/…$/);
634-
635-
// The head survives; everything past ~6 lines of a 92-line file does not.
636-
expect(resultLine as string).toContain("marker001");
637-
expect(resultLine as string).not.toContain("marker010");
629+
const lines = pane.split("\n");
630+
631+
// The framing `engine-tool-calls.ts` parses is intact: the call line, then the arrow line.
632+
expect(lines.some((l) => l.startsWith("⚙ Read "))).toBe(true);
633+
const arrow = lines.findIndex((l) => l.startsWith(" ↳"));
634+
expect(arrow).toBeGreaterThanOrEqual(0);
635+
expect(lines[arrow].startsWith(" ↳✓ ")).toBe(true);
636+
637+
// THE fix: the file's own lines are lines again. Before this, not one of them survived as
638+
// its own line at any length, which is why `cat -n` and `sed -n '1,50p'` were the same
639+
// request to the Pilot.
640+
const body = lines.slice(arrow + 1).filter((l) => l.startsWith(" │ "));
641+
expect(body.length).toBeGreaterThan(30);
642+
expect(body[0]).toContain("marker002");
643+
644+
// ~34 of 92 lines rather than ~5, and the cut is disclosed WITH its size — the number that
645+
// tells the Pilot how narrow a slice would arrive whole.
646+
expect(pane).toContain("marker010"); // the old 240-char cap stopped short of this
647+
expect(pane).toContain("marker030");
638648
expect(pane).not.toContain("marker092");
639-
640-
// And line structure is gone: not one line of the file survives as its own line, so
641-
// `cat -n`, `sed -n '1,50p'` and `head -60` are indistinguishable at this resolution.
642-
expect(pane.split("\n").some((l) => l.startsWith(" const marker"))).toBe(false);
649+
expect(pane).toMatch(/…\[cut: 1,500 of 4,0\d\d chars\]/);
650+
651+
// And it is still bounded: the whole block — content, per-line framing and the disclosure —
652+
// honours RESULT_RENDERED_MAX, which is under a third of the Pilot's 6,000-character window,
653+
// so two more results can follow it before anything is evicted.
654+
const block = [lines[arrow], ...body].join("\n");
655+
expect(block.length).toBeLessThanOrEqual(RESULT_RENDERED_MAX);
656+
expect(RESULT_RENDERED_MAX * 3).toBeLessThanOrEqual(6000);
643657
s.stop();
644658
}, 15_000);
645659

660+
it("a status-string tool keeps the old 240, so the widening is paid for only where it buys something", async () => {
661+
// Same pane, different tool: `FAKE_CLAUDE_TOOLS` answers a `Bash` and a `Read`. The point is
662+
// the negative one — a per-tool cap means an `Edit`/`TodoWrite` receipt cannot spend the
663+
// window. Measured here on the budget function's own boundary rather than by spawning a
664+
// fourth fake engine.
665+
expect(toolResultBudget("Edit")).toEqual({ chars: 240, lines: 6 });
666+
expect(toolResultBudget("Read").chars).toBeGreaterThan(240);
667+
});
668+
646669
it("the engine's own REPLY text arrives whole — the one channel a file can travel", async () => {
647670
const s = new HeadlessSession({ id: "file-reply", workDir: dir, clientType: "claude", bin, statePath: defaultStatePath(dir) });
648671
s.start();

‎packages/browser-runner/src/coding/headless.ts‎

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { type ChildProcess, spawn } from "node:child_process";
22
import { classifyCommand, commandFromToolInput, type EngineActRecord, fillTargetFromResult, toolCallOk, toolResultMark } from "./engine-acts.js";
33
import { type EngineUsageRecord, parseEngineUsage } from "./engine-usage.js";
44
import { type EngineTurnReport, turnReportFromExit, turnReportFromResult } from "./engine-turn.js";
5+
import { renderToolResult, shortInput, stripAnsi } from "./transcript-lines.js";
56

67
/**
78
* Merge the platform's resolved engine env over the machine's, where an EMPTY value means
@@ -191,6 +192,15 @@ export class HeadlessSession {
191192
* damages the record exactly as much as missing a real one.
192193
*/
193194
private awaitingResult = new Map<string, EngineActRecord[]>();
195+
/**
196+
* `tool_use_id` → tool name, for the ONE decision the result event cannot make on its own: how
197+
* much of it reaches the pane (#700, `toolResultBudget` in `transcript-lines.ts`).
198+
*
199+
* Separate from {@link awaitingResult}, which holds only the small minority of calls that
200+
* classify as consequential acts. Entries are deleted when their result arrives and the whole
201+
* map is cleared at the turn boundary, where a call that never got one is known never to.
202+
*/
203+
private toolNames = new Map<string, string>();
194204
/** Turn counter — only used to build a fallback id when the CLI's event has no `uuid`. */
195205
private usageSeq = 0;
196206
/**
@@ -669,15 +679,25 @@ export class HeadlessSession {
669679
if (block.type === "text" && typeof block.text === "string" && block.text.trim()) {
670680
this.push(`[${stamp()}] ${block.text.trim()}`); // timestamped agent reply
671681
} else if (block.type === "tool_use") {
672-
this.push(`⚙ ${String(block.name ?? "tool")} ${shortInput(block.input)}`); // ⚙
682+
const name = String(block.name ?? "tool");
683+
this.push(`⚙ ${name} ${shortInput(block.input)}`); // ⚙
684+
// The result arrives in a LATER event carrying only `tool_use_id`, and how much
685+
// of it reaches the pane depends on which tool it was (#700) — so the name is
686+
// remembered here and read back in `settleAct`'s sibling branch below.
687+
if (typeof block.id === "string" && block.id) this.toolNames.set(block.id, name);
673688
this.noteAct(block);
674689
}
675690
}
676691
break;
677692
case "user": // tool results come back as a synthetic user message
678693
for (const block of ev.message?.content ?? []) {
679694
if (block.type === "tool_result") {
680-
this.push(` ↳${toolResultMark(block)} ${toolResult(block.content)}`); // ↳✓ / ↳✗ (#597)
695+
const id = typeof block.tool_use_id === "string" ? block.tool_use_id : "";
696+
// `""` when the call was not seen (a pane that began mid-turn, a runner restart):
697+
// an unknown tool takes the conservative budget rather than the generous one.
698+
const tool = this.toolNames.get(id) ?? "";
699+
this.toolNames.delete(id);
700+
this.push(renderToolResult(toolResultMark(block), block.content, tool)); // ↳✓ / ↳✗ (#597)
681701
this.settleAct(block);
682702
}
683703
}
@@ -703,13 +723,16 @@ export class HeadlessSession {
703723
// never saw whether it worked" is a materially different claim from silence, and
704724
// silence is what a supervisor would read as "it did nothing".
705725
this.flushAwaitingActs();
726+
this.toolNames.clear(); // no further result is coming for anything still named here
706727
this.run = "idle"; // the turn is OVER — a fact, not a guess
707728
break;
708729
}
709730
default:
710731
break;
711732
}
712-
// Keep the in-memory transcript bounded.
733+
// Keep the in-memory transcript bounded. Counts ENTRIES, and an entry may now be a
734+
// multi-line block (a result, or a long assistant reply) rather than one line — the
735+
// character bound that matters is `MAX_PANE` in runtime.ts, applied on the way out.
713736
if (this.transcript.length > 4000) this.transcript = this.transcript.slice(-3000);
714737
}
715738

@@ -735,8 +758,8 @@ export class HeadlessSession {
735758
*
736759
* The result carries more than the outcome: `gh pr create --fill` states its PR number nowhere
737760
* but its own stdout, so an unnumbered `pr.open`/`pr.merge` takes it from here (#417). It is read
738-
* from the RAW `block.content`, never from `toolResult()`'s display line — that truncates to 240
739-
* characters for the transcript and would cut the URL off a verbose result.
761+
* from the RAW `block.content`, never from `renderToolResult()`'s display lines — those are cut to
762+
* the pane's budget (`transcript-lines.ts`) and would drop the URL off a verbose result.
740763
*
741764
* This path (and `noteAct`) is reachable ONLY from the structured stream-json handling above
742765
* (`assistant` → `tool_use`, `user` → `tool_result`). A Codex/Grok session is a raw spawn with no
@@ -889,36 +912,6 @@ export function buildClaudeArgs(userArgs: string[], resumeId: string | null): st
889912
return args;
890913
}
891914

892-
/** Strip ANSI/VT escape sequences so a raw CLI's coloured output reads as plain text. */
893-
function stripAnsi(s: string): string {
894-
// biome-ignore lint/suspicious/noControlCharactersInRegex: stripping terminal escape/control codes from terminal output.
895-
return s.replace(/\x1b\[[0-9;?]*[ -/]*[@-~]/g, "").replace(/\x1b[()][AB0-2]/g, "").replace(/[\x00-\x08\x0b\x0c\x0e-\x1f]/g, "");
896-
}
897-
898-
/** Compact a tool_use input object to a single readable line. */
899-
function shortInput(input: unknown): string {
900-
if (input == null) return "";
901-
try {
902-
const s = typeof input === "string" ? input : JSON.stringify(input);
903-
return s.length > 160 ? `${s.slice(0, 160)}…` : s;
904-
} catch {
905-
return "";
906-
}
907-
}
908-
909-
/** Render a tool_result's content (string, or array of {type:text,text}) to text. */
910-
function toolResult(content: unknown): string {
911-
let text = "";
912-
if (typeof content === "string") text = content;
913-
else if (Array.isArray(content)) {
914-
text = content
915-
.map((b) => (b && typeof b === "object" && "text" in b ? String((b as { text: unknown }).text) : ""))
916-
.join(" ");
917-
}
918-
text = text.replace(/\s+/g, " ").trim();
919-
return text.length > 240 ? `${text.slice(0, 240)}…` : text;
920-
}
921-
922915
// ── tiny on-disk store: our session id → Claude's session id (resume key) ──────
923916
// So a runner restart can `--resume` the conversation. One JSON file, best-effort.
924917

0 commit comments

Comments
 (0)