From de99219db09091dea34f70c316733dd8edc2f618 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 13 Aug 2026 20:56:15 +0200 Subject: [PATCH] test(coding-agent): revert fake-timer rewrite of session_shutdown cap test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The b279db1790 rewrite wrapped runner.emit() in vi.useFakeTimers() and hand-advanced the clock, but the runner registers its cap setTimeout after more microtask turns than the test advances (emit defers the timeout machinery to the first matching handler and hops through Bun.sleep(0)), so the cap timer never fires, emit never settles, and fake timers also neutralize bun's per-test timeout — the singleton/global-state CI bucket hung silently until the 600s watchdog SIGKILL (exit 137). - Restored the pre-refactor real-time version: it has no sleeps or polling loops, runs the hung handlers against a 100ms cap, and asserts bounded wall-clock plus the per-extension timeout warnings. - Verified the full 79-file singleton bucket passes (867 tests) and the restored file passes on Linux bun 1.3.14 in Docker. - Restored the original 17.3.1 status-line changelog bullet (released sections stay immutable). --- packages/coding-agent/CHANGELOG.md | 2 +- .../repro-issue-2600-shutdown-timeout.test.ts | 79 +++++++++++-------- 2 files changed, 46 insertions(+), 35 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4542af48d..d0b7ec695 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -7,7 +7,7 @@ ### Fixed - Fixed Claude Code user discovery ignoring CLAUDE_CONFIG_DIR for configuration, plugins, MCP servers, and imported sessions. -- Fixed the status-line git branch display freezing after switching branches. The original directory-watch fix still froze on Linux, where Bun's inotify-backed `fs.watch` permanently stops delivering events after git's atomic HEAD rename ([oven-sh/bun#24875](https://github.com/oven-sh/bun/issues/24875)); branch watching now stat-polls the HEAD path (`git.head.watch`) on every platform, which also fixes the footer's branch display dying after the first switch. +- Fixed the status-line git branch display freezing after switching branches. - Fixed Pi extension contexts omitting the runtime mode, which caused TUI guards to silently disable extension UI. - Fixed extension-registered tool names being rejected by the --tools flag before extension discovery, which prevented least-privilege sessions from allowlisting plugin tools. - Fixed omp plugin install failing with cloning errors for legacy Pi extensions whose tool schemas use legacy-typebox builders. diff --git a/packages/coding-agent/test/repro-issue-2600-shutdown-timeout.test.ts b/packages/coding-agent/test/repro-issue-2600-shutdown-timeout.test.ts index e604ec76a..027b7d98b 100644 --- a/packages/coding-agent/test/repro-issue-2600-shutdown-timeout.test.ts +++ b/packages/coding-agent/test/repro-issue-2600-shutdown-timeout.test.ts @@ -99,24 +99,16 @@ describe("issue #2600 - session_shutdown handler timeout", () => { it("runs multiple session_shutdown handlers within one cap", async () => { const { runner, hangExtensionPaths, cleanup } = await buildRunnerWithHangingShutdown(4); const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); - vi.useFakeTimers(); try { testSetSessionShutdownHandlerTimeoutMs(100); - let settled = false; - const emitted = runner.emit({ type: "session_shutdown" }).then(() => { - settled = true; - }); - await Promise.resolve(); - vi.advanceTimersByTime(99); - await Promise.resolve(); - expect(settled).toBe(false); + const startedAt = performance.now(); + await runner.emit({ type: "session_shutdown" }); + const elapsedMs = performance.now() - startedAt; - vi.advanceTimersByTime(1); - await Promise.resolve(); - vi.advanceTimersByTime(0); - await emitted; - expect(settled).toBe(true); + // Multiple hung shutdown handlers must share the cap. Sequential + // dispatch would consume roughly count × cap and keep `/exit` slow. + expect(elapsedMs).toBeLessThan(350); for (const hangExtensionPath of hangExtensionPaths) { expect(warnSpy).toHaveBeenCalledWith("Extension handler timed out", { extensionPath: hangExtensionPath, @@ -125,7 +117,6 @@ describe("issue #2600 - session_shutdown handler timeout", () => { }); } } finally { - vi.useRealTimers(); warnSpy.mockRestore(); cleanup(); } @@ -136,38 +127,58 @@ describe("issue #2600 - session_shutdown handler timeout", () => { expect(SESSION_SHUTDOWN_HANDLER_TIMEOUT_MS).toBeLessThan(EXTENSION_HANDLER_TIMEOUT_MS); }); + it("returns within the short cap when a session_shutdown handler hangs forever", async () => { + const { runner, hangExtensionPath, cleanup } = await buildRunnerWithHangingShutdown(); + try { + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); + + // Generic budget is left at the production default (30s). The + // shutdown cap is shortened to 100ms so this test stays under a + // second while still asserting the dispatch path uses the dedicated + // cap. + testSetSessionShutdownHandlerTimeoutMs(100); + + const startedAt = performance.now(); + await runner.emit({ type: "session_shutdown" }); + const elapsedMs = performance.now() - startedAt; + + // Loose upper bound to absorb CI scheduler jitter; the regression + // would expire at ~30_000ms. + expect(elapsedMs).toBeLessThan(1_000); + expect(warnSpy).toHaveBeenCalledWith("Extension handler timed out", { + extensionPath: hangExtensionPath, + event: "session_shutdown", + timeoutMs: 100, + }); + warnSpy.mockRestore(); + } finally { + cleanup(); + } + }); + it("session_shutdown cap is independent from the generic handler cap", async () => { const { runner, hangExtensionPath, cleanup } = await buildRunnerWithHangingShutdown(); - const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); - vi.useFakeTimers(); try { - // If dispatch reads the generic knob, advancing the shutdown budget - // cannot settle this handler. + const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {}); + + // Raise the *generic* timeout to a value the test would never + // tolerate (10s) while leaving the shutdown cap at 50ms. If the + // dispatcher pulls from the wrong knob the test wall-clock balloons. testSetExtensionHandlerTimeoutMs(10_000); testSetSessionShutdownHandlerTimeoutMs(50); - let settled = false; - const emitted = runner.emit({ type: "session_shutdown" }).then(() => { - settled = true; - }); - await Promise.resolve(); - vi.advanceTimersByTime(49); - await Promise.resolve(); - expect(settled).toBe(false); + const startedAt = performance.now(); + await runner.emit({ type: "session_shutdown" }); + const elapsedMs = performance.now() - startedAt; - vi.advanceTimersByTime(1); - await Promise.resolve(); - vi.advanceTimersByTime(0); - await emitted; - expect(settled).toBe(true); + expect(elapsedMs).toBeLessThan(500); expect(warnSpy).toHaveBeenCalledWith("Extension handler timed out", { extensionPath: hangExtensionPath, event: "session_shutdown", timeoutMs: 50, }); - } finally { - vi.useRealTimers(); warnSpy.mockRestore(); + } finally { cleanup(); } });