diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9e447daa3..a8500e74e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -31,6 +31,9 @@ ### Fixed - Fixed `omp --smoke-test` recursively deleting unrelated directories in `os.tmpdir()` (tmux/ssh sockets, editor state, build trees). The smoke broker now keeps its runtime dir under a private parent, and the dead-scope reclaim refuses any root that is not the `daemons` container and only prunes entries named like a 16-hex daemon scope key ([#8721](https://github.com/can1357/oh-my-pi/issues/8721)). +### Fixed + +- Fixed high CPU during multi-subagent / workflowz / orchestrate sessions: each live tool block (streaming args, a running partial tool, or a `task` subagent) armed its own 80ms spinner `setInterval` driving `requestComponentRender`, so N concurrent live blocks created N unsynchronized repaint timers that kept the render scheduler awake near-continuously. The per-block timers are now consolidated into a single shared spinner ticker that repaints every live block in one coalesced frame per glyph step, independent of block count ([#8731](https://github.com/can1357/oh-my-pi/issues/8731)). ## [17.3.5] - 2026-08-16 diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index 37ccd198f..2cc73cb28 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -269,6 +269,39 @@ export function sharedSpinnerFrame(frameCount: number, now: number = performance return frameCount > 0 ? Math.floor(now / SPINNER_GLYPH_ADVANCE_MS) % frameCount : 0; } +/** Live tool blocks currently driving a spinner. A single shared ticker (below) + * advances and repaints every registered block per glyph step, so N concurrent + * live/streaming blocks — e.g. parallel `task` subagents — cost one 80ms timer + * and one coalesced render frame per tick instead of N unsynchronized timers + * each independently waking the render scheduler (issue #8731). */ +const liveSpinnerBlocks = new Set(); +let sharedSpinnerTimer: NodeJS.Timeout | undefined; + +/** Arm the shared spinner ticker if it is not already running. */ +function ensureSharedSpinnerTicker(): void { + if (sharedSpinnerTimer) return; + sharedSpinnerTimer = setInterval(() => { + const frame = sharedSpinnerFrame(theme.spinnerFrames.length); + // Deleting the current block mid-iteration (freeze path) is safe on a Set. + for (const block of liveSpinnerBlocks) block.tickSpinner(frame); + }, SPINNER_RENDER_INTERVAL_MS); +} + +/** Register a live block with the shared ticker, starting it on first use. */ +function registerSpinnerBlock(block: ToolExecutionComponent): void { + liveSpinnerBlocks.add(block); + ensureSharedSpinnerTicker(); +} + +/** Drop a block; stop the ticker once no live block remains. */ +function unregisterSpinnerBlock(block: ToolExecutionComponent): void { + if (!liveSpinnerBlocks.delete(block)) return; + if (liveSpinnerBlocks.size === 0 && sharedSpinnerTimer) { + clearInterval(sharedSpinnerTimer); + sharedSpinnerTimer = undefined; + } +} + // Stable per-instance counter so each tool execution's inline images get a // graphics id that survives child re-creation (the image budget keys off it). let toolExecutionInstanceSeq = 0; @@ -337,7 +370,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac #convertedImages: Map = new Map(); // Spinner animation for partial task results #spinnerFrame?: number; - #spinnerInterval?: NodeJS.Timeout; + #spinnerActive = false; // Todo write completion strikethrough reveal animation #todoStrikeInterval?: NodeJS.Timeout; // Track if args are still being streamed (for edit/write spinner) @@ -697,27 +730,16 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac !isBackgroundAsyncRunning && (pendingCallConsumesSpinner || partialResultConsumesSpinner); const needsSpinner = isStreamingArgs || isLivePartialTool || this.#displaceableByToolName === "hub"; - if (needsSpinner && !this.#spinnerInterval) { + if (needsSpinner && !this.#spinnerActive) { const frameCount = theme.spinnerFrames.length; const frame = sharedSpinnerFrame(frameCount); this.#spinnerFrame = frame; this.#renderState.spinnerFrame = frame; - this.#spinnerInterval = setInterval(() => { - // If a detached task interval from an older render path is still live, - // stop it the instant the block leaves the repaintable region. - if (this.#maybeFreezeBackgroundTask()) return; - const now = performance.now(); - const frameCount = theme.spinnerFrames.length; - this.#spinnerFrame = sharedSpinnerFrame(frameCount, now); - this.#renderState.spinnerFrame = this.#spinnerFrame; - // Component-scoped: a spinner tick only changes this tool block, so - // the TUI reuses every other root subtree instead of walking the - // whole tree (issue #4377). - this.#ui.requestComponentRender(this); - }, SPINNER_RENDER_INTERVAL_MS); - } else if (!needsSpinner && this.#spinnerInterval) { - clearInterval(this.#spinnerInterval); - this.#spinnerInterval = undefined; + this.#spinnerActive = true; + registerSpinnerBlock(this); + } else if (!needsSpinner && this.#spinnerActive) { + this.#spinnerActive = false; + unregisterSpinnerBlock(this); // Clear the last drawn frame so a non-live renderCall (e.g. a write whose // args just completed) stops showing a frozen spinner glyph. Skip when a // todo strike owns the frame — it sets its own value right after this. @@ -728,6 +750,20 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac } } + /** + * Advance to the shared spinner glyph and repaint just this block. Driven by + * the single shared spinner ticker (see `registerSpinnerBlock`); the tick is + * component-scoped so the TUI reuses every other root subtree (issue #4377). + */ + tickSpinner(frame: number): void { + // A detached task block that scrolled into native scrollback stops the + // instant it leaves the repaintable region. + if (this.#maybeFreezeBackgroundTask()) return; + this.#spinnerFrame = frame; + this.#renderState.spinnerFrame = frame; + this.#ui.requestComponentRender(this); + } + /** * Freeze a detached (`async.state === "running"`) task block once its rows * become native-scrollback history: the block left the transcript's live @@ -790,7 +826,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac clearInterval(this.#todoStrikeInterval); this.#todoStrikeInterval = undefined; } - if (!this.#spinnerInterval) { + if (!this.#spinnerActive) { this.#spinnerFrame = undefined; this.#renderState.spinnerFrame = undefined; } @@ -886,9 +922,9 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac * Stop spinner animation and cleanup resources. */ stopAnimation(): void { - if (this.#spinnerInterval) { - clearInterval(this.#spinnerInterval); - this.#spinnerInterval = undefined; + if (this.#spinnerActive) { + this.#spinnerActive = false; + unregisterSpinnerBlock(this); this.#spinnerFrame = undefined; this.#renderState.spinnerFrame = undefined; } diff --git a/packages/coding-agent/test/modes/components/tool-execution-spinner.test.ts b/packages/coding-agent/test/modes/components/tool-execution-spinner.test.ts index e4cc7a9be..6a8d95b2c 100644 --- a/packages/coding-agent/test/modes/components/tool-execution-spinner.test.ts +++ b/packages/coding-agent/test/modes/components/tool-execution-spinner.test.ts @@ -1,6 +1,9 @@ import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; import { stripVTControlCharacters } from "node:util"; -import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; +import { + SPINNER_RENDER_INTERVAL_MS, + ToolExecutionComponent, +} from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; import { TranscriptContainer } from "@oh-my-pi/pi-coding-agent/modes/components/transcript-container"; import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { TUI } from "@oh-my-pi/pi-tui"; @@ -222,4 +225,38 @@ describe("ToolExecutionComponent live preview spinners", () => { component.stopAnimation(); } }); + + // Regression (issue #8731): concurrent live tool blocks — e.g. parallel task + // subagents — must share ONE spinner timer, not one per block, or active-work + // CPU scales with block count. + it("drives every concurrent live block from a single shared spinner timer", () => { + vi.useFakeTimers(); + const setIntervalSpy = vi.spyOn(globalThis, "setInterval"); + const renders = [vi.fn(), vi.fn(), vi.fn()]; + const components = renders.map( + requestComponentRender => + new ToolExecutionComponent( + "eval", + { language: "py", code: "import time\ntime.sleep(10)" }, + {}, + undefined, + { requestRender: vi.fn(), requestComponentRender } as unknown as TUI, + process.cwd(), + ), + ); + + try { + const spinnerTimers = setIntervalSpy.mock.calls.filter(([, ms]) => ms === SPINNER_RENDER_INTERVAL_MS).length; + // One shared ticker for all three live blocks, not three. + expect(spinnerTimers).toBe(1); + + // A single tick repaints every registered block in lockstep. + vi.advanceTimersByTime(SPINNER_RENDER_INTERVAL_MS); + for (const requestComponentRender of renders) { + expect(requestComponentRender).toHaveBeenCalledTimes(1); + } + } finally { + for (const component of components) component.stopAnimation(); + } + }); });