Merge pull request #3608 from deseltrus/perf/session-memory-cpu-leaks

perf: fix CPU/memory leaks in long-running sessions

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) {

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);
 	}
 

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();
 		}

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;

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);

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.`
 				);
 			}

src/resources/extensions/bg-shell/process-manager.ts

@@ -33,6 +33,8 @@ export const processes = new Map<string, BgProcess>();
 /** 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<ProcessEvent, "timestamp">):
 	}
 }
 
-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 {

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",
+  );
+});