diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 893d0eca2..e6f557211 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ ### Removed - Removed the `tsx` dependency from the shipped runtime. The Intercom broker's Node path now runs on Atomic's bundled `jiti` loader, which is dependency-free pure JavaScript, instead of resolving `tsx` first. `tsx` was the only thing pulling `esbuild`, so this removes `esbuild`, its 26 platform packages, and `fsevents` from `npm-shrinkwrap.json` and from every release archive — about 11.5 MB across 72 files. The `npx --no-install tsx` config pair remains a recognized compatibility sentinel, and explicit custom broker commands are unaffected ([#2208](https://github.com/bastani-inc/atomic/issues/2208)). - Behavior change worth noting for extension authors: TypeScript reached through the Node broker path is now transpiled by jiti (Babel-based) rather than esbuild. Both erase types without type-checking, but they are not identical — jiti defaults JSX support and `tsconfig` path aliases to off, and enables legacy decorators. A third-party extension that resolved `tsx` from the bundled `node_modules` was relying on an undocumented dependency and will no longer find it ([#2208](https://github.com/bastani-inc/atomic/issues/2208)). +- Reduced redundant repaints of a Ctrl+O-expanded live subagent widget, which were scrolling the chat window to the bottom and clearing terminal scrollback during a run. The widget was republished on every child session event, including assistant streaming deltas, so it repainted continuously instead of when its contents changed; each repaint of a row above the terminal fold costs a pi-tui full redraw that clears scrollback. Progress is now published only at milestones that change what the widget shows. A genuine above-fold milestone change can still require that redraw when the editor/footer region plus the live widget exceed the terminal height — that path is upstream (earendil-works/pi#4785, #7194) and is not fixed here ([#2213](https://github.com/bastani-inc/atomic/pull/2213), regression from [#2205](https://github.com/bastani-inc/atomic/pull/2205)). ## [0.9.13-alpha.1] - 2026-08-05 diff --git a/packages/subagents/CHANGELOG.md b/packages/subagents/CHANGELOG.md index 131dbe8da..bb9225a82 100644 --- a/packages/subagents/CHANGELOG.md +++ b/packages/subagents/CHANGELOG.md @@ -8,6 +8,7 @@ - Fixed the in-process depth guard and nested workflow-stage children. Admission-issued child depth now travels in the typed policy into every single, parallel, chain, async, and resume path, so the executor can reject delegation at the configured limit while Rust admission keeps the hard five-level ceiling; the orphaned process-environment depth bridge and its self-fulfilling tests are gone. In-process children now load the bundled package resources needed to register `subagent`, so a nested child no longer starts with only the base built-in tools and no way to delegate, while workflow-stage children suppress only the workflow extension lifecycle ([#2220](https://github.com/bastani-inc/atomic/pull/2220), regression from [#2205](https://github.com/bastani-inc/atomic/pull/2205)). - Fixed an agent's `maxSubagentDepth` being dropped at the in-process admission door. A child admitted from an agent whose definition tightened the limit received a policy carrying no maximum, so it could keep delegating as if only the global five-level ceiling applied. The effective limit — the stricter of the parent's limit and the child agent's own — now travels on the admitted child spec and policy, is reissued unchanged by a cold reload, and is applied by the executor's depth check alongside the local configuration. Admission also derives the limit from the agent definition when a caller supplies a child spec without one, so the door no longer issues an unbounded policy for an agent that declared a limit ([#2220](https://github.com/bastani-inc/atomic/pull/2220), regression from [#2205](https://github.com/bastani-inc/atomic/pull/2205)). - Fixed a resumed foreground child losing the delegation limit its agent definition had narrowed. Retained resume re-derived the limit from the current stage or process configuration, so a child that ran under an agent maximum of 1 resumed with the configured maximum instead. The effective limit is now recorded per retained child — parallel and chain branches can each carry a different one — and reused on resume, so editing an agent definition between a run and its resume cannot widen that child's budget ([#2220](https://github.com/bastani-inc/atomic/pull/2220), regression from [#2205](https://github.com/bastani-inc/atomic/pull/2205)). +- Reduced redundant live subagent repaints by publishing progress only for widget-visible milestones. The in-process runner published `AgentProgress` from a catch-all over every child session event, including the high-frequency `message_update` streaming deltas and `tool_execution_update`, and each publish rewrites the elapsed fields the widget renders. With a subagent expanded via Ctrl+O, that made the live widget repaint for the whole run rather than when its contents changed, and each repaint of a row above the terminal fold costs a destructive pi-tui full redraw that clears terminal scrollback. Progress is now published only on events that change what the widget shows — `agent_start`, `tool_execution_start`, `tool_execution_end`, and (throttled) `message_end` — which is the emission profile foreground subagents had before the in-process runner. Measured against a 20-second replay in the tight geometry, scrollback clears drop from 23 to 3; a live tmux run against the built CLI dropped from 8 clears (7 of them repaints that changed nothing visible) to 3 (none of them). **Genuine above-fold milestone changes can still require a pi-tui redraw**, so a short terminal whose editor/footer region plus the live widget exceed the terminal height will still lose scrollback at tool boundaries; that path is upstream (earendil-works/pi#4785, #7194) and outside this package. The 400 ms throttle, the forced-emit milestones, the depth guard, and the typed status contract are unchanged ([#2213](https://github.com/bastani-inc/atomic/pull/2213), regression from [#2205](https://github.com/bastani-inc/atomic/pull/2205)). ## [0.9.13-alpha.1] - 2026-08-05 diff --git a/packages/subagents/src/runs/inprocess/runner.ts b/packages/subagents/src/runs/inprocess/runner.ts index 969442192..07e553e77 100644 --- a/packages/subagents/src/runs/inprocess/runner.ts +++ b/packages/subagents/src/runs/inprocess/runner.ts @@ -478,6 +478,40 @@ function safeArgsPreview(args: unknown): string { } } +/** + * How a child session event should publish {@link AgentProgress} to the host UI. + * + * - `force` — the event changed progress the live widget shows; bypass the throttle. + * - `throttled` — worth publishing, but subject to the 400 ms throttle. + * - `none` — carries nothing the widget shows; do not publish. + * + * `none` is the important case. Foreground subagent results render into chat + * scrollback, which can sit above pi-tui's viewport fold. Every publish rewrites + * `durationMs`/`lastActivityAt` and repaints the widget, and a repaint of a row + * above the fold makes `TUI.doRender()` take its `firstChanged < viewportTop` + * branch, which issues a full redraw that writes `\x1b[2J\x1b[H\x1b[3J` — clearing + * the user's scrollback and snapping the terminal to the bottom. A catch-all + * publish therefore destroyed the scrollback ~2.5x/s for the whole run, because + * `AgentSessionEvent` includes high-frequency traffic (`message_update` streaming + * deltas, `tool_execution_update`, `entry_appended`) that the widget never shows. + * Keep this table narrow: add an event only when the widget renders something the + * event changed. + */ +export type ProgressEmission = "force" | "throttled" | "none"; + +export function progressEmissionFor(eventType: AgentSessionEvent["type"]): ProgressEmission { + switch (eventType) { + case "agent_start": + case "tool_execution_start": + case "tool_execution_end": + return "force"; + case "message_end": + return "throttled"; + default: + return "none"; + } +} + function writeEvent(pathValue: string | undefined, event: AgentSessionEvent): void { if (!pathValue) return; mkdirSync(dirname(pathValue), { recursive: true }); @@ -774,15 +808,14 @@ export class SubagentControlRuntime { }; unsubscribe = session.subscribe((event) => { writeEvent(admitted.spec.artifactJsonlPath, event); + const emission = progressEmissionFor(event.type); if (event.type === "agent_start") { this.native.publishChildStatus(admitted.identity.path, nativeStatus("running")); - emitProgress(true); } else if (event.type === "tool_execution_start") { progressState.toolCount += 1; progressState.currentTool = event.toolName; progressState.currentToolArgs = safeArgsPreview(event.args); progressState.currentToolStartedAt = Date.now(); - emitProgress(true); } else if (event.type === "tool_execution_end") { if (progressState.currentTool !== undefined) { progressState.recentTools.push({ @@ -795,7 +828,6 @@ export class SubagentControlRuntime { progressState.currentTool = undefined; progressState.currentToolArgs = undefined; progressState.currentToolStartedAt = undefined; - emitProgress(true); } else if (event.type === "message_end") { const message = (event as { message?: { role?: string; usage?: { input?: number; output?: number } } }) .message; @@ -804,10 +836,8 @@ export class SubagentControlRuntime { const usage = message.usage; if (usage) progressState.tokens += (usage.input ?? 0) + (usage.output ?? 0); } - emitProgress(false); - } else { - emitProgress(false); } + if (emission !== "none") emitProgress(emission === "force"); if (event.type === "model_fallback_start") { if (!attemptedModels.includes(event.to)) attemptedModels.push(event.to); effectiveModelId = event.to; diff --git a/test/unit/subagents-inprocess-progress-emission.test.ts b/test/unit/subagents-inprocess-progress-emission.test.ts new file mode 100644 index 000000000..a4342ee21 --- /dev/null +++ b/test/unit/subagents-inprocess-progress-emission.test.ts @@ -0,0 +1,102 @@ +import assert from "node:assert/strict"; +import type { AgentSessionEvent } from "@bastani/atomic"; +import { describe, test } from "vitest"; +import { progressEmissionFor } from "../../packages/subagents/src/runs/inprocess/runner.ts"; + +/** + * The in-process runner publishes AgentProgress into chat scrollback, which can + * sit above pi-tui's viewport fold. A publish for an event the widget does not + * render still repaints an above-fold row, which makes pi-tui take its + * `firstChanged < viewportTop` branch and issue a scrollback-clearing full + * redraw. These tests pin the narrow emission table that keeps that from firing + * on every streaming delta. + */ + +type EventType = AgentSessionEvent["type"]; + +const FORCED: readonly EventType[] = ["agent_start", "tool_execution_start", "tool_execution_end"]; +const THROTTLED: readonly EventType[] = ["message_end"]; + +/** + * High-frequency traffic the live widget never renders. `message_update` is the + * assistant streaming delta and fires many times per second; publishing there is + * what destroyed the user's scrollback on every Ctrl+O-expanded subagent run. + */ +const SILENT: readonly EventType[] = [ + "message_update", + "message_start", + "tool_execution_update", + "turn_start", + "turn_end", + "entry_appended", + "queue_update", + "agent_settled", + "agent_end", + "bash_execution_update", + "session_info_changed", + "model_changed", + "thinking_level_changed", + "compaction_start", + "compaction_end", + "auto_retry_start", + "auto_retry_end", + "model_fallback_start", + "model_fallback_end", +]; + +describe("in-process runner progress emission profile", () => { + test("events that change rendered progress publish immediately", () => { + for (const eventType of FORCED) { + assert.equal(progressEmissionFor(eventType), "force", `${eventType} must bypass the throttle`); + } + }); + + test("message_end publishes under the 400 ms throttle", () => { + for (const eventType of THROTTLED) { + assert.equal(progressEmissionFor(eventType), "throttled", `${eventType} must stay throttled`); + } + }); + + test("high-frequency events the widget does not render publish nothing", () => { + for (const eventType of SILENT) { + assert.equal( + progressEmissionFor(eventType), + "none", + `${eventType} must not repaint the live subagent widget; a catch-all publish here clears the ` + + "user's terminal scrollback on every streaming delta", + ); + } + }); + + test("there is no catch-all: unknown events default to none", () => { + assert.equal(progressEmissionFor("some_future_event" as EventType), "none"); + }); + + test("streaming a realistic turn publishes once per milestone, not once per delta", () => { + // One assistant turn: 40 streaming deltas around a single tool call. + const stream: EventType[] = [ + "agent_start", + "turn_start", + "message_start", + ...Array.from({ length: 20 }, (): EventType => "message_update"), + "message_end", + "tool_execution_start", + ...Array.from({ length: 8 }, (): EventType => "tool_execution_update"), + "tool_execution_end", + "message_start", + ...Array.from({ length: 20 }, (): EventType => "message_update"), + "message_end", + "turn_end", + "agent_settled", + ]; + const published = stream.filter((eventType) => progressEmissionFor(eventType) !== "none"); + assert.deepEqual(published, [ + "agent_start", + "message_end", + "tool_execution_start", + "tool_execution_end", + "message_end", + ]); + assert.equal(published.length, 5, `55 session events must publish 5 progress updates, not ${stream.length}`); + }); +}); diff --git a/test/unit/subagents-live-widget-scrollback.test.ts b/test/unit/subagents-live-widget-scrollback.test.ts new file mode 100644 index 000000000..4009ad9ec --- /dev/null +++ b/test/unit/subagents-live-widget-scrollback.test.ts @@ -0,0 +1,298 @@ +import assert from "node:assert/strict"; +import type { AgentSessionEvent } from "@bastani/atomic"; +import type { AgentToolResult } from "@earendil-works/pi-agent-core"; +import { type Component, Container, type Terminal, Text, TUI } from "@earendil-works/pi-tui"; +import { describe, test } from "vitest"; +import { progressEmissionFor } from "../../packages/subagents/src/runs/inprocess/runner.ts"; +import type { AgentProgress, Details } from "../../packages/subagents/src/shared/types.js"; +import { renderSubagentResult } from "../../packages/subagents/src/tui/render.js"; +import { theme } from "./subagents-render-stability-helpers.js"; + +/** + * Scrollback accounting for the Ctrl+O-expanded live subagent widget. + * + * A foreground subagent result renders into chat scrollback. pi-tui's + * `TUI.doRender()` compares the whole line array and, when the earliest changed + * row is above `previousViewportTop`, gives up on a differential update and calls + * `fullRender(true)`, which writes `\x1b[2J\x1b[H\x1b[3J` — clear screen, home, + * **clear scrollback**. Each of those erases the user's terminal history and + * snaps the view to the bottom. + * + * These tests count that write per published progress update, not in aggregate, + * because an aggregate count stays small while individual tool boundaries still + * wipe the screen. + * + * Two things are pinned: + * + * 1. Non-milestone session events must produce no repaint at all. This is the + * regression #2205 introduced and what this change fixes. + * 2. When the live widget fits inside the viewport alongside the rows below it, + * a tool boundary must cost zero scrollback clears. + * + * The geometric limit in (2) is deliberately measured rather than assumed: once + * `rowsBelowWidget + widgetRows > terminalRows`, the widget's own top row sits + * above the fold and pi-tui clears for any genuine change there. That is a pi-tui + * behavior (upstream earendil-works/pi#4785, #7194) with no seam in this + * repository, so `documents the geometric limit` records it as a measured fact + * instead of letting it hide inside a loose aggregate assertion. + */ + +const NOW = 1_700_000_000_000; +const COLS = 110; +const CLEAR_SCROLLBACK = "\x1b[3J"; +const HISTORY_ROWS = 60; + +class RecordingTerminal implements Terminal { + writes: string[] = []; + readonly columns = COLS; + constructor(readonly rows: number) {} + start(): void {} + stop(): void {} + async drainInput(): Promise {} + write(data: string): void { + this.writes.push(data); + } + get kittyProtocolActive(): boolean { + return false; + } + moveBy(): void {} + hideCursor(): void {} + showCursor(): void {} + clearLine(): void {} + clearFromCursor(): void {} + clearScreen(): void {} + setTitle(): void {} + setProgress(): void {} +} + +/** The exact live shape `inprocess-run-sync.ts` publishes while a child runs. */ +function liveResult(overrides: Partial): AgentToolResult
{ + const progress = { + agent: "codebase-locator", + index: 0, + status: "running", + task: "Search packages/coding-agent/src for files mentioning scrollback and summarize each.", + durationMs: 12_000, + toolCount: 4, + tokens: 525, + recentTools: [{ tool: "grep", args: '{"pattern":"scrollback"}', endMs: NOW - 3_000 }], + recentOutput: ["found 5 files referencing scrollback"], + lastActivityAt: NOW - 200, + ...overrides, + } as AgentProgress; + return { + content: [{ type: "text", text: "running" }], + details: { + mode: "single", + results: [ + { + agent: "codebase-locator", + task: progress.task, + status: "continued", + messages: [], + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, cost: 0, turns: 0 }, + progress, + }, + ], + }, + }; +} + +class LiveSubagentWidget implements Component { + result = liveResult({}); + render(width: number): string[] { + return renderSubagentResult(this.result, { expanded: true, now: NOW, pulseFrame: 0 }, theme).render(width); + } + invalidate(): void {} +} + +interface Harness { + terminal: RecordingTerminal; + tui: TUI; + widget: LiveSubagentWidget; + widgetRows: number; + rowsBelowWidget: number; + totalRows: number; +} + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +/** + * Mirrors how the interactive chat mounts things: history, then the running + * subagent tool component as the last chat child, then the editor/footer region. + */ +async function mountChat(terminalRows: number, rowsBelowWidget: number): Promise { + const terminal = new RecordingTerminal(terminalRows); + const tui = new TUI(terminal, false, "/tmp"); + const chat = new Container(); + for (let i = 0; i < HISTORY_ROWS; i += 1) chat.addChild(new Text(`history ${i}`, 0, 0)); + const widget = new LiveSubagentWidget(); + chat.addChild(widget); + tui.addChild(chat); + const below = new Container(); + for (let i = 0; i < rowsBelowWidget; i += 1) below.addChild(new Text(`footer ${i}`, 0, 0)); + tui.addChild(below); + tui.requestRender(); + await sleep(40); + const widgetRows = widget.render(COLS).length; + return { + terminal, + tui, + widget, + widgetRows, + rowsBelowWidget, + totalRows: chat.render(COLS).length + rowsBelowWidget, + }; +} + +/** Publish one update and report the writes it alone produced. */ +async function publish(harness: Harness, next: AgentToolResult
): Promise<{ clears: number; writes: number }> { + harness.widget.result = next; + harness.terminal.writes = []; + harness.tui.requestRender(); + await sleep(40); + return { + clears: harness.terminal.writes.filter((data) => data.includes(CLEAR_SCROLLBACK)).length, + writes: harness.terminal.writes.length, + }; +} + +const toolStart = () => + liveResult({ + toolCount: 5, + currentTool: "read", + currentToolArgs: '{"path":"src/modes/interactive/interactive-render-chat.ts"}', + currentToolStartedAt: NOW - 900, + }); +const toolEnd = () => liveResult({ toolCount: 5 }); + +/** One assistant turn with a tool call, as the runner observes it. */ +function realisticEventStream(): AgentSessionEvent["type"][] { + return [ + "agent_start", + "turn_start", + "message_start", + ...Array.from({ length: 10 }, (): AgentSessionEvent["type"] => "message_update"), + "message_end", + "tool_execution_start", + ...Array.from({ length: 6 }, (): AgentSessionEvent["type"] => "tool_execution_update"), + "tool_execution_end", + "turn_end", + "agent_settled", + ]; +} + +describe("expanded live subagent widget in chat scrollback", () => { + test("a tool boundary costs zero scrollback clears when the widget fits the viewport", async () => { + // 26-row terminal, widget last in chat, 8 rows of editor/footer below it. + const harness = await mountChat(26, 8); + assert.ok( + harness.rowsBelowWidget + harness.widgetRows <= harness.terminal.rows, + `precondition: widget (${harness.widgetRows}) + rows below (${harness.rowsBelowWidget}) must fit in ` + + `${harness.terminal.rows} terminal rows`, + ); + + const start = await publish(harness, toolStart()); + assert.equal(start.clears, 0, `tool_execution_start cleared scrollback ${start.clears} time(s)`); + + const end = await publish(harness, toolEnd()); + assert.equal(end.clears, 0, `tool_execution_end cleared scrollback ${end.clears} time(s)`); + + // Repeat: a per-boundary guarantee has to hold every cycle, not on average. + for (let cycle = 0; cycle < 3; cycle += 1) { + const s = await publish(harness, toolStart()); + assert.equal(s.clears, 0, `cycle ${cycle} tool_execution_start cleared scrollback`); + const e = await publish(harness, toolEnd()); + assert.equal(e.clears, 0, `cycle ${cycle} tool_execution_end cleared scrollback`); + } + }); + + test("replaying a turn publishes at milestones only, and costs far fewer clears than a catch-all", async () => { + const stream = realisticEventStream(); + const silent = stream.filter((eventType) => progressEmissionFor(eventType) === "none"); + assert.ok(silent.length >= 16, `precondition: the stream must carry high-frequency traffic (${silent.length})`); + + // Replay the same stream twice against the same geometry: once under the + // shipped emission table, once under the catch-all #2205 shipped + // (`} else { emitProgress(false); }`), which published for every event. + const replay = async (publishesEverything: boolean) => { + // Deliberately the tight geometry, so every avoidable publish is a + // visible scrollback wipe rather than a silent differential update. + const harness = await mountChat(26, 20); + const perBoundary: Record = {}; + let publishes = 0; + let clears = 0; + let toolCount = 4; + let currentTool: string | undefined; + for (const [index, eventType] of stream.entries()) { + const emission = progressEmissionFor(eventType); + if (!publishesEverything && emission === "none") continue; + if (eventType === "tool_execution_start") { + toolCount += 1; + currentTool = "read"; + } + if (eventType === "tool_execution_end") currentTool = undefined; + publishes += 1; + // Mirror emitProgress: every publish rewrites the elapsed fields. + // Spacing the stream across a realistic ~20 s run is what exposes + // the catch-all's true cost -- it refreshed durationMs on every + // event, so the widget's elapsed readout ticked (and wiped + // scrollback) about once a second for the whole run. + const result = await publish( + harness, + liveResult({ + toolCount, + currentTool, + currentToolArgs: currentTool ? '{"path":"a.ts"}' : undefined, + currentToolStartedAt: currentTool ? NOW - 900 : undefined, + durationMs: 12_000 + index * 1_000, + lastActivityAt: NOW - 200, + }), + ); + clears += result.clears; + if (eventType === "tool_execution_start" || eventType === "tool_execution_end") { + perBoundary[eventType] = result.clears; + } + } + return { publishes, clears, perBoundary }; + }; + + const shipped = await replay(false); + const catchAll = await replay(true); + + assert.equal( + shipped.publishes, + 4, + "agent_start, message_end, tool_execution_start, tool_execution_end — nothing else", + ); + assert.equal(catchAll.publishes, stream.length, "the catch-all published for every session event"); + assert.ok( + shipped.clears * 3 < catchAll.clears, + `shipped emission cleared scrollback ${shipped.clears} times over the run against the catch-all's ` + + `${catchAll.clears}; the whole point of the fix is that this gap is large`, + ); + // The boundaries themselves are unchanged by the fix — they were always + // genuine progress changes. They are pinned in the two tests above. + assert.deepEqual(shipped.perBoundary, catchAll.perBoundary, "boundary cost is a geometry property, not a rate"); + }); + + test("documents the geometric limit pi-tui imposes on above-fold repaints", async () => { + // Same widget, same publishes, but the rows below it no longer leave room: + // 20 + widget height exceeds the 26-row terminal, so the widget's own top + // row is above `previousViewportTop` and pi-tui must full-redraw. + const harness = await mountChat(26, 20); + assert.ok( + harness.rowsBelowWidget + harness.widgetRows > harness.terminal.rows, + "precondition: widget must not fit alongside the rows below it", + ); + + const start = await publish(harness, toolStart()); + assert.equal( + start.clears, + 1, + "a genuine above-fold change still costs one pi-tui full redraw; see earendil-works/pi#4785 and #7194. " + + "If this ever reads 0, pi-tui gained a non-destructive above-fold path and the limitation note in " + + "evidence/README.md and both CHANGELOGs should be removed.", + ); + }); +});