From c5227f7570455032539b1304f129f5c7fdd82298 Mon Sep 17 00:00:00 2001 From: deseltrus Date: Mon, 6 Apr 2026 09:52:08 +0200 Subject: [PATCH 1/4] perf(tui): render-skip, frame isolation, Text cache guard, dispose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Container.render() now returns a stable array reference when output is unchanged — TUI.doRender() skips ALL post-processing (isImageLine scans, applyLineResets, differential diffs) when the reference matches. Loader decouples spinner frame rotation from Text content updates. Previously every 80ms tick called setText() which invalidated Text's wrapTextWithAnsi/visibleWidth caches. Now the frame is prepended in render() while Text caches the message separately. Text.setText() returns early when text is unchanged, avoiding cache invalidation on redundant updates. ToolExecutionComponent.dispose() clears heavy references (image maps, diff previews, result data) so GC can reclaim memory when components are removed from the chat history. --- .../interactive/components/tool-execution.ts | 9 +++++ packages/pi-tui/src/components/loader.ts | 37 ++++++++++++++----- packages/pi-tui/src/components/text.ts | 1 + packages/pi-tui/src/tui.ts | 32 ++++++++++++++++ 4 files changed, 69 insertions(+), 10 deletions(-) diff --git a/packages/pi-coding-agent/src/modes/interactive/components/tool-execution.ts b/packages/pi-coding-agent/src/modes/interactive/components/tool-execution.ts index 10bd5f02c..a5f80da87 100644 --- a/packages/pi-coding-agent/src/modes/interactive/components/tool-execution.ts +++ b/packages/pi-coding-agent/src/modes/interactive/components/tool-execution.ts @@ -137,6 +137,15 @@ export class ToolExecutionComponent extends Container { return isBuiltInName && !hasCustomRenderers; } + dispose(): void { + this.convertedImages.clear(); + this.imageComponents = []; + this.imageSpacers = []; + this.editDiffPreview = undefined; + this.writeHighlightCache = undefined; + this.result = undefined; + } + updateArgs(args: any): void { this.args = args; if (this.toolName === "write" && this.isPartial) { diff --git a/packages/pi-tui/src/components/loader.ts b/packages/pi-tui/src/components/loader.ts index a55a2570c..5115f8337 100644 --- a/packages/pi-tui/src/components/loader.ts +++ b/packages/pi-tui/src/components/loader.ts @@ -2,13 +2,16 @@ import type { TUI } from "../tui.js"; import { Text } from "./text.js"; /** - * Loader component that updates every 80ms with spinning animation + * Loader component that updates every 80ms with spinning animation. + * Frame rotation is isolated from message text to avoid invalidating + * Text's render cache (wrapTextWithAnsi, visibleWidth) on every tick. */ export class Loader extends Text { private frames = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; private currentFrame = 0; private intervalId: NodeJS.Timeout | null = null; private ui: TUI | null = null; + private _lastMessage: string = ""; constructor( ui: TUI, @@ -22,18 +25,38 @@ export class Loader extends Text { } render(width: number): string[] { - return ["", ...super.render(width)]; + // Only update Text content when message actually changes — + // frame rotation is prepended below without touching the cache + if (this.message !== this._lastMessage) { + this.setText(this.messageColorFn(this.message)); + this._lastMessage = this.message; + } + const messageLines = super.render(width); + // Shallow copy so we don't mutate cachedLines from Text + const result = ["", ...messageLines]; + // Prepend spinner frame to first content line + if (result.length > 1) { + const frame = this.frames[this.currentFrame]; + result[1] = this.spinnerColorFn(frame) + " " + result[1]; + } + return result; } start() { if (this.intervalId) { clearInterval(this.intervalId); } - this.updateDisplay(); + this.currentFrame = 0; this.intervalId = setInterval(() => { this.currentFrame = (this.currentFrame + 1) % this.frames.length; - this.updateDisplay(); + if (this.ui) { + this.ui.requestRender(); + } }, 80); + // Trigger initial render + if (this.ui) { + this.ui.requestRender(); + } } stop() { @@ -50,12 +73,6 @@ export class Loader extends Text { setMessage(message: string) { this.message = message; - this.updateDisplay(); - } - - private updateDisplay() { - const frame = this.frames[this.currentFrame]; - this.setText(`${this.spinnerColorFn(frame)} ${this.messageColorFn(this.message)}`); if (this.ui) { this.ui.requestRender(); } diff --git a/packages/pi-tui/src/components/text.ts b/packages/pi-tui/src/components/text.ts index efcf25b45..a9519bfdf 100644 --- a/packages/pi-tui/src/components/text.ts +++ b/packages/pi-tui/src/components/text.ts @@ -23,6 +23,7 @@ export class Text implements Component { } setText(text: string): void { + if (this.text === text) return; this.text = text; this.cachedText = undefined; this.cachedWidth = undefined; diff --git a/packages/pi-tui/src/tui.ts b/packages/pi-tui/src/tui.ts index 8e3db6f05..493c1c727 100644 --- a/packages/pi-tui/src/tui.ts +++ b/packages/pi-tui/src/tui.ts @@ -166,20 +166,33 @@ export interface OverlayHandle { */ export class Container implements Component { children: Component[] = []; + private _prevRender: string[] | null = null; addChild(component: Component): void { this.children.push(component); + this._prevRender = null; } removeChild(component: Component): void { const index = this.children.indexOf(component); if (index !== -1) { + const child = this.children[index]; this.children.splice(index, 1); + if ('dispose' in child && typeof (child as any).dispose === 'function') { + (child as any).dispose(); + } + this._prevRender = null; } } clear(): void { + for (const child of this.children) { + if ('dispose' in child && typeof (child as any).dispose === 'function') { + (child as any).dispose(); + } + } this.children = []; + this._prevRender = null; } invalidate(): void { @@ -194,6 +207,17 @@ export class Container implements Component { const rendered = child.render(width); for (let i = 0; i < rendered.length; i++) lines.push(rendered[i]); } + // Return stable reference if output unchanged — allows doRender() + // to skip ALL post-processing (isImageLine, applyLineResets, diffs) + const prev = this._prevRender; + if (prev && prev.length === lines.length) { + let same = true; + for (let i = 0; i < lines.length; i++) { + if (lines[i] !== prev[i]) { same = false; break; } + } + if (same) return prev; + } + this._prevRender = lines; return lines; } } @@ -222,6 +246,7 @@ export class TUI extends Container { private previousViewportTop = 0; // Track previous viewport top for resize-aware cursor moves private fullRedrawCount = 0; private stopped = false; + private _lastRenderedComponents: string[] | null = null; // Overlay stack for modal components rendered on top of base content private focusOrderCounter = 0; @@ -599,6 +624,13 @@ export class TUI extends Container { // Render all components to get new lines let newLines = this.render(width); + // Skip ALL post-processing if component output is unchanged. + // Container.render() returns the same array reference when stable. + if (newLines === this._lastRenderedComponents && this.overlayStack.length === 0) { + return; + } + this._lastRenderedComponents = newLines; + // Composite overlays into the rendered lines (before differential compare) if (this.overlayStack.length > 0) { newLines = compositeOverlays(newLines, this.overlayStack, width, height, this.maxLinesRendered); From 0b40d39b0efe031500c43e9d9e07c88c0809d5f4 Mon Sep 17 00:00:00 2001 From: deseltrus Date: Mon, 6 Apr 2026 09:52:20 +0200 Subject: [PATCH 2/4] perf(interactive): cap rendered chat components + kill orphan descendants Chat component cap: After 100 rendered components, oldest are removed from the container (session transcript persists on disk via SessionManager). Prevents unbounded memory growth in long sessions where thousands of tool calls accumulate DOM-like component trees. Orphan process prevention: On shutdown, listDescendants(process.pid) finds ALL child processes (including those spawned by the Bash tool that bg-shell doesn't track) and kills them with SIGTERM + 500ms grace + SIGKILL. Prevents orphaned dev servers, build processes, etc. from persisting after session exit. --- .../src/modes/interactive/interactive-mode.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts b/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts index d3bd71f27..ca98db678 100644 --- a/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts @@ -7,6 +7,7 @@ import * as crypto from "node:crypto"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import { listDescendants } from "@gsd/native"; import type { AgentMessage } from "@gsd/pi-agent-core"; import type { AssistantMessage, ImageContent, Message, Model, OAuthProviderId } from "@gsd/pi-ai"; import type { @@ -157,6 +158,10 @@ export interface InteractiveModeOptions { } export class InteractiveMode { + // Cap rendered chat components to prevent unbounded memory/CPU growth. + // Only render-components are removed — session transcript stays on disk. + private static readonly MAX_CHAT_COMPONENTS = 100; + private session: AgentSession; private ui: TUI; private chatContainer: Container; @@ -2138,6 +2143,18 @@ export class InteractiveMode { const _exhaustive: never = message; } } + this.trimChatHistory(); + } + + /** + * Remove oldest components when chat exceeds MAX_CHAT_COMPONENTS. + * Only render-components are removed — session data stays in SessionManager. + */ + private trimChatHistory(): void { + while (this.chatContainer.children.length > InteractiveMode.MAX_CHAT_COMPONENTS) { + const oldest = this.chatContainer.children[0]; + this.chatContainer.removeChild(oldest); + } } /** @@ -2232,6 +2249,7 @@ export class InteractiveMode { } this.pendingTools.clear(); + this.trimChatHistory(); this.ui.requestRender(); } @@ -2325,6 +2343,21 @@ export class InteractiveMode { if (shutdownBehavior === "stop_ui") { return; } + + // Kill ALL descendant processes to prevent orphans (next-server, pnpm dev, etc.) + try { + const descendants = listDescendants(process.pid); + for (const childPid of descendants) { + try { process.kill(childPid, "SIGTERM"); } catch {} + } + if (descendants.length > 0) { + await new Promise(resolve => setTimeout(resolve, 500)); + for (const childPid of descendants) { + try { process.kill(childPid, "SIGKILL"); } catch {} + } + } + } catch {} + process.exit(0); } From 886c5837ffdcee91b16496ad281c60ac85bb277d Mon Sep 17 00:00:00 2001 From: deseltrus Date: Mon, 6 Apr 2026 09:52:36 +0200 Subject: [PATCH 3/4] fix(bg-shell): prevent signal handler accumulation + cap alert queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signal handlers (SIGTERM, SIGINT, beforeExit) were registered on every session_start but never removed. Over multiple sessions within the same process, handlers accumulated — each adding another cleanupAll() call and descendant kill sweep on exit. Fix: session_shutdown now calls process.off() for each handler before cleanupAll(), preventing accumulation. Also: signalCleanup now kills ALL descendant processes (not just those tracked by bg-shell) to catch bash-tool spawned children. Alert queue: pendingAlerts is capped at 50 entries to prevent unbounded growth when background processes generate rapid alerts faster than the agent consumes them. pushAlert signature updated to accept null bg parameter for system-level alerts that don't originate from a tracked process. --- .../extensions/bg-shell/bg-shell-lifecycle.ts | 26 ++++++++++++++----- .../extensions/bg-shell/process-manager.ts | 10 +++++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/resources/extensions/bg-shell/bg-shell-lifecycle.ts b/src/resources/extensions/bg-shell/bg-shell-lifecycle.ts index 688db06c4..32ee56455 100644 --- a/src/resources/extensions/bg-shell/bg-shell-lifecycle.ts +++ b/src/resources/extensions/bg-shell/bg-shell-lifecycle.ts @@ -16,6 +16,7 @@ import { import { processes, pendingAlerts, + pushAlert, cleanupAll, cleanupSessionProcesses, persistManifest, @@ -37,19 +38,30 @@ export function registerBgShellLifecycle(pi: ExtensionAPI, state: BgShellSharedS } } - // Clean up on session shutdown - pi.on("session_shutdown", async () => { - cleanupAll(); - }); - // Register signal handlers to clean up bg processes on unexpected exit (fixes #428) const signalCleanup = () => { cleanupAll(); + // Also kill bash-tool spawned children that bg-shell doesn't track + try { + const { listDescendants } = require("@gsd/native") as typeof import("@gsd/native"); + const descendants = listDescendants(process.pid); + for (const childPid of descendants) { + try { process.kill(childPid, "SIGKILL"); } catch {} + } + } catch {} }; process.on("SIGTERM", signalCleanup); process.on("SIGINT", signalCleanup); process.on("beforeExit", signalCleanup); + // Clean up on session shutdown — remove signal handlers to prevent accumulation + pi.on("session_shutdown", async () => { + process.off("SIGTERM", signalCleanup); + process.off("SIGINT", signalCleanup); + process.off("beforeExit", signalCleanup); + cleanupAll(); + }); + // ── Compaction Awareness: Survive Context Resets ─────────────── /** Build a compact state summary of all alive processes for context re-injection */ @@ -65,7 +77,7 @@ export function registerBgShellLifecycle(pi: ExtensionAPI, state: BgShellSharedS return ` - id:${p.id} "${p.label}" [${p.processType}] status:${p.status} uptime:${formatUptime(Date.now() - p.startedAt)}${portInfo}${urlInfo}${errInfo}${groupInfo}`; }).join("\n"); - pendingAlerts.push( + pushAlert(null, `${reason} ${alive.length} background process(es) are still running:\n${processSummaries}\nUse bg_shell digest/output/kill with these IDs.` ); } @@ -150,7 +162,7 @@ export function registerBgShellLifecycle(pi: ExtensionAPI, state: BgShellSharedS ` - ${s.id}: ${s.label} (pid ${s.pid}, type: ${s.processType}${s.group ? `, group: ${s.group}` : ""})` ).join("\n"); - pendingAlerts.push( + pushAlert(null, `${surviving.length} background process(es) from previous session still running:\n${summary}\n Note: These processes are outside bg_shell's control. Kill them manually if needed.` ); } diff --git a/src/resources/extensions/bg-shell/process-manager.ts b/src/resources/extensions/bg-shell/process-manager.ts index db707fb40..659f13e26 100644 --- a/src/resources/extensions/bg-shell/process-manager.ts +++ b/src/resources/extensions/bg-shell/process-manager.ts @@ -33,6 +33,8 @@ export const processes = new Map(); /** Pending alerts to inject into the next agent context */ export let pendingAlerts: string[] = []; +const MAX_PENDING_ALERTS = 50; + /** Replace the pendingAlerts array (used by the extension entry point) */ export function setPendingAlerts(alerts: string[]): void { pendingAlerts = alerts; @@ -58,8 +60,12 @@ export function addEvent(bg: BgProcess, event: Omit): } } -export function pushAlert(bg: BgProcess, message: string): void { - pendingAlerts.push(`[bg:${bg.id} ${bg.label}] ${message}`); +export function pushAlert(bg: BgProcess | null, message: string): void { + const prefix = bg ? `[bg:${bg.id} ${bg.label}] ` : ""; + pendingAlerts.push(`${prefix}${message}`); + if (pendingAlerts.length > MAX_PENDING_ALERTS) { + pendingAlerts.splice(0, pendingAlerts.length - MAX_PENDING_ALERTS); + } } export function getInfo(p: BgProcess): BgProcessInfo { From 4744e86c8f052957cc80a745fa8f312780aebf13 Mon Sep 17 00:00:00 2001 From: deseltrus Date: Mon, 6 Apr 2026 09:57:40 +0200 Subject: [PATCH 4/4] test: structural regression tests for session memory/CPU leak fixes Verifies that defensive guards (render-skip, chat cap, dispose, signal handler cleanup, alert cap, orphan kill) are present in source. These are structural tests because the leaks manifest over hours of real usage, not in unit test timescales. --- src/tests/session-memory-leaks.test.ts | 144 +++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 src/tests/session-memory-leaks.test.ts diff --git a/src/tests/session-memory-leaks.test.ts b/src/tests/session-memory-leaks.test.ts new file mode 100644 index 000000000..17a3590bb --- /dev/null +++ b/src/tests/session-memory-leaks.test.ts @@ -0,0 +1,144 @@ +/** + * Regression tests for CPU/memory leak fixes in long-running sessions. + * + * Structural tests that verify the fix patterns are present in source — + * NOT runtime integration tests. This approach is chosen because: + * - The leaks manifest over hours of real usage, not in unit test timescales + * - The fixes are defensive guards (caps, disposal, handler cleanup) + * - Structural verification catches regressions when code is refactored + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +// ── Helpers ────────────────────────────────────────────────────────── + +function readSource(relativePath: string): string { + return readFileSync(join(import.meta.dirname, "..", "..", relativePath), "utf-8"); +} + +function extractFunctionBody(src: string, name: string): string { + const fnStart = src.indexOf(name); + assert.ok(fnStart > -1, `${name} must exist in source`); + let depth = 0; + let fnEnd = -1; + for (let i = src.indexOf("{", fnStart); i < src.length; i++) { + if (src[i] === "{") depth++; + if (src[i] === "}") depth--; + if (depth === 0) { fnEnd = i; break; } + } + return src.slice(fnStart, fnEnd + 1); +} + +// ── TUI render-skip ───────────────────────────────────────────────── + +test("Container caches render output for stable-reference comparison", () => { + const src = readSource("packages/pi-tui/src/tui.ts"); + assert.ok( + src.includes("_prevRender"), + "Container must have _prevRender cache for render-skip optimization", + ); +}); + +test("TUI skips post-processing when component output is unchanged", () => { + const src = readSource("packages/pi-tui/src/tui.ts"); + assert.ok( + src.includes("_lastRenderedComponents"), + "TUI must track _lastRenderedComponents for reference-equality skip", + ); +}); + +// ── Loader frame isolation ────────────────────────────────────────── + +test("Loader does not call setText on every spinner tick", () => { + const src = readSource("packages/pi-tui/src/components/loader.ts"); + // The old pattern was: setText(`${frame} ${message}`) inside the interval + // The new pattern: only update Text when message changes, prepend frame in render() + assert.ok( + src.includes("_lastMessage"), + "Loader must track _lastMessage to avoid setText on every tick", + ); + // Verify the interval does NOT call setText or updateDisplay + const intervalMatch = src.match(/setInterval\s*\(\s*\(\)\s*=>\s*\{([^}]+)\}/s); + assert.ok(intervalMatch, "Loader must have a setInterval callback"); + const intervalBody = intervalMatch[1]; + assert.ok( + !intervalBody.includes("setText") && !intervalBody.includes("updateDisplay"), + "Loader interval must NOT call setText or updateDisplay — " + + "frame rotation should only trigger requestRender()", + ); +}); + +// ── Text cache guard ──────────────────────────────────────────────── + +test("Text.setText returns early when text is unchanged", () => { + const src = readSource("packages/pi-tui/src/components/text.ts"); + const setTextBody = extractFunctionBody(src, "setText("); + assert.ok( + setTextBody.includes("if (this.text === text) return"), + "setText must early-return when text is identical to prevent cache invalidation", + ); +}); + +// ── Chat component cap ────────────────────────────────────────────── + +test("InteractiveMode caps rendered chat components", () => { + const src = readSource("packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts"); + assert.ok( + src.includes("MAX_CHAT_COMPONENTS"), + "InteractiveMode must define MAX_CHAT_COMPONENTS to prevent unbounded growth", + ); + assert.ok( + src.includes("trimChatHistory"), + "InteractiveMode must call trimChatHistory to enforce the cap", + ); +}); + +// ── ToolExecution dispose ─────────────────────────────────────────── + +test("ToolExecutionComponent has dispose() to clear heavy references", () => { + const src = readSource("packages/pi-coding-agent/src/modes/interactive/components/tool-execution.ts"); + assert.ok( + src.includes("dispose()"), + "ToolExecutionComponent must have dispose() for GC of image maps, diff previews, etc.", + ); +}); + +// ── Orphan process prevention ─────────────────────────────────────── + +test("InteractiveMode kills descendant processes on shutdown", () => { + const src = readSource("packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts"); + assert.ok( + src.includes("listDescendants"), + "Shutdown must use listDescendants to find orphan child processes", + ); + assert.ok( + src.includes("SIGTERM") && src.includes("SIGKILL"), + "Shutdown must send SIGTERM then SIGKILL to descendants", + ); +}); + +// ── Signal handler accumulation ───────────────────────────────────── + +test("bg-shell removes signal handlers on session_shutdown", () => { + const src = readSource("src/resources/extensions/bg-shell/bg-shell-lifecycle.ts"); + assert.ok( + src.includes('process.off("SIGTERM"') || src.includes("process.off('SIGTERM'"), + "session_shutdown must remove SIGTERM handler to prevent accumulation", + ); + assert.ok( + src.includes('process.off("SIGINT"') || src.includes("process.off('SIGINT'"), + "session_shutdown must remove SIGINT handler to prevent accumulation", + ); +}); + +// ── Alert queue cap ───────────────────────────────────────────────── + +test("pendingAlerts has a maximum size cap", () => { + const src = readSource("src/resources/extensions/bg-shell/process-manager.ts"); + assert.ok( + src.includes("MAX_PENDING_ALERTS"), + "process-manager must cap pendingAlerts to prevent unbounded growth", + ); +});