From 2e2159a9c39cc4fa58df94a2d68d494877c4d665 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Tue, 10 Feb 2026 15:21:54 -0800 Subject: [PATCH] fix: address coderabbit review - clarify pre-attach guard, fix hardcoded port Improve the comment on the pre-attach debugger guard to explain why it only fires on the SIGUSR1 path (--inspect already has the debugger attached by JSC at startup). Replace hardcoded port 6499 assertion in Windows test with regex match. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/bun.js/bindings/BunDebugger.cpp | 4 ++++ .../bun/runtime-inspector/runtime-inspector-windows.test.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bun.js/bindings/BunDebugger.cpp b/src/bun.js/bindings/BunDebugger.cpp index 5ba768ab75..d2aa9ef4c1 100644 --- a/src/bun.js/bindings/BunDebugger.cpp +++ b/src/bun.js/bindings/BunDebugger.cpp @@ -136,6 +136,10 @@ public: // StopTheWorld (JSC's JSON parser needs GC heap operations that conflict with STW). // By attaching early, we enable the pause mechanism so messages can be // dispatched in the normal runWhilePaused loop after STW completes. + // Pre-attach the debugger so that notifyNeedDebuggerBreak() can trigger a pause + // for CDP message dispatch. This only fires on the SIGUSR1 path because --inspect + // and --inspect-wait already have the debugger attached by JSC at startup + // (globalObject->debugger() is non-null), so the condition is false for those paths. auto* controllerDebugger = globalObject->inspectorController().debugger(); if (controllerDebugger && !globalObject->debugger()) { controllerDebugger->attach(globalObject); diff --git a/test/js/bun/runtime-inspector/runtime-inspector-windows.test.ts b/test/js/bun/runtime-inspector/runtime-inspector-windows.test.ts index 90bc992d98..8f99817707 100644 --- a/test/js/bun/runtime-inspector/runtime-inspector-windows.test.ts +++ b/test/js/bun/runtime-inspector/runtime-inspector-windows.test.ts @@ -93,7 +93,7 @@ describe.skipIf(!isWindows)("Runtime inspector Windows file mapping", () => { // Verify inspector actually started expect(targetStderr).toContain("Bun Inspector"); - expect(targetStderr).toContain("ws://localhost:6499/"); + expect(targetStderr).toMatch(/ws:\/\/localhost:\d+\//); }); test("_debugProcess works with current process's own pid", async () => {