inspector: share TestReporter test ID counter between live and retroactive paths

src/jsc/Debugger.rs

@@ -809,6 +809,9 @@ pub fn will_dispatch_async_call(global_object: &JSGlobalObject, call: AsyncCallT
 #[derive(Default)]
 pub struct TestReporterAgent {
     pub(crate) handle: *mut TestReporterHandle,
+    /// Shared `describe`/`test` ID counter for both the live
+    /// (`ScopeFunctions::call`) and retroactive reporting paths.
+    pub next_test_id: i32,
 }
 
 /// this enum is kept in sync with c++ InspectorTestReporterAgent.cpp `enum class BunTestStatus`
@@ -921,8 +924,11 @@ pub fn test_reporter_agent_enable(agent: *mut TestReporterHandle) {
         // — a forward-dep cycle. Dispatched through [`RuntimeHooks`].
         if let Some(hooks) = runtime_hooks() {
             // SAFETY: `handle` is the live C++ agent just stored above.
-            unsafe {
-                (hooks.retroactively_report_discovered_tests)(dbg.test_reporter_agent.handle)
+            dbg.test_reporter_agent.next_test_id = unsafe {
+                (hooks.retroactively_report_discovered_tests)(
+                    dbg.test_reporter_agent.handle,
+                    dbg.test_reporter_agent.next_test_id,
+                )
             };
         }
     }

src/jsc/VirtualMachine.rs

@@ -1803,15 +1803,17 @@ pub struct RuntimeHooks {
     /// path. The caller gates the call on `vm.standalone_module_graph`.
     pub load_standalone_sourcemap:
         fn(path: &[u8]) -> Option<std::sync::Arc<bun_sourcemap::ParsedSourceMap>>,
-    /// `TestReporterAgent.retroactivelyReportDiscoveredTests(agent)`.
+    /// `TestReporterAgent.retroactivelyReportDiscoveredTests(agent, next_test_id)`.
     /// Walks the active test file's
     /// scope tree and emits `reportTestFoundWithLocation` for every test
     /// discovered before the inspector connected. `Jest` / `DescribeScope`
     /// live in `bun_runtime::test_runner` (forward-dep cycle), so the body is
     /// hoisted to the high tier; low-tier `Bun__TestReporterAgentEnable`
-    /// dispatches here. No-op when `bun test` isn't running.
+    /// dispatches here. `next_test_id` / the return value thread
+    /// `TestReporterAgent::next_test_id` through by value; no-op returns it
+    /// unchanged.
     pub retroactively_report_discovered_tests:
-        unsafe fn(agent: *mut crate::debugger::TestReporterHandle),
+        unsafe fn(agent: *mut crate::debugger::TestReporterHandle, next_test_id: i32) -> i32,
     /// Cancel every `TimeoutObject` / `ImmediateObject` still in the calling
     /// thread's `timer::All` heap so their JS pins and in-heap `+1` refs drop
     /// before the GC sweep. `timer::All` lives in `bun_runtime` (forward-dep);

src/runtime/jsc_hooks.rs

@@ -1675,23 +1675,30 @@ pub(crate) fn close_isolation_handles(vm: &mut VirtualMachine) {
     }
 }
 
-/// `TestReporterAgent.retroactivelyReportDiscoveredTests(agent)`.
+/// `TestReporterAgent.retroactivelyReportDiscoveredTests(agent, next_test_id)`.
 /// When `TestReporter.enable` arrives after test
 /// collection has started, walk the already-discovered scope tree, assign
 /// debugger test IDs, and emit `reportTestFoundWithLocation` for each.
+/// Returns `next_test_id` advanced past every ID assigned (the caller writes
+/// it back into `TestReporterAgent::next_test_id`).
 ///
 /// # Safety
 /// `agent` is a live C++ `Inspector::TestReporterAgent::Handle*` (just stored
 /// into `debugger.test_reporter_agent.handle` by the caller). Called on the JS
 /// thread.
-unsafe fn retroactively_report_discovered_tests(agent: *mut bun_jsc::debugger::TestReporterHandle) {
+unsafe fn retroactively_report_discovered_tests(
+    agent: *mut bun_jsc::debugger::TestReporterHandle,
+    next_test_id: i32,
+) -> i32 {
     use crate::test_runner::bun_test::{DescribeScope, Phase, TestScheduleEntry};
     use crate::test_runner::jest::Jest;
     use bun_jsc::debugger::{TestReporterHandle, TestType};
 
-    let Some(runner) = Jest::runner() else { return };
+    let Some(runner) = Jest::runner() else {
+        return next_test_id;
+    };
     let Some(active_file) = runner.bun_test_root.active_file.as_ref() else {
-        return;
+        return next_test_id;
     };
     // SAFETY: single-threaded; `active_file` keeps the cell alive for this call.
     let active_file = unsafe { &mut *active_file.as_ptr() };
@@ -1700,7 +1707,7 @@ unsafe fn retroactively_report_discovered_tests(agent: *mut bun_jsc::debugger::T
     // discovered).
     match active_file.phase {
         Phase::Collection | Phase::Execution => {}
-        Phase::Done => return,
+        Phase::Done => return next_test_id,
     }
 
     // Get the file path for source location info.
@@ -1710,8 +1717,7 @@ unsafe fn retroactively_report_discovered_tests(agent: *mut bun_jsc::debugger::T
         .text();
     let mut source_url = bun_core::String::init(file_path);
 
-    // Track the maximum ID we assign.
-    let mut max_id: i32 = 0;
+    let mut max_id: i32 = next_test_id;
 
     // Recursively report all discovered tests starting from root scope.
     retroactively_report_scope(
@@ -1722,9 +1728,7 @@ unsafe fn retroactively_report_discovered_tests(agent: *mut bun_jsc::debugger::T
         &mut source_url,
     );
 
-    // A debug-only log of `max_id` was dropped here: `scoped_log!` only accepts
-    // an ident, so it can't name the scoped-logger static in `bun_jsc::debugger`.
-    let _ = max_id;
+    return max_id;
 
     fn retroactively_report_scope(
         agent: *mut TestReporterHandle,

src/runtime/test_runner/ScopeFunctions.rs

@@ -1,6 +1,5 @@
 use core::fmt;
 use crate::test_runner::expect::JSValueTestExt;
-use core::sync::atomic::{AtomicI32, Ordering};
 
 use bun_jsc::{CallFrame, JSGlobalObject, JSValue, JsClass, JsResult};
 use bun_core::String as BunString;
@@ -352,8 +351,8 @@ impl ScopeFunctions {
         let mut test_id_for_debugger: i32 = 0;
         if let Some(debugger) = (*vm).debugger.as_mut() {
             if debugger.test_reporter_agent.is_enabled() {
-                static MAX_TEST_ID_FOR_DEBUGGER: AtomicI32 = AtomicI32::new(0);
-                let id = MAX_TEST_ID_FOR_DEBUGGER.fetch_add(1, Ordering::Relaxed) + 1;
+                debugger.test_reporter_agent.next_test_id += 1;
+                let id = debugger.test_reporter_agent.next_test_id;
                 let mut name = BunString::init(description.unwrap_or(b"(unnamed)"));
                 let parent: &DescribeScope = bun_test.collection.active_scope();
                 let parent_id = if parent.base.test_id_for_debugger != 0 {

test/cli/inspect/test-reporter.test.ts

@@ -310,4 +310,149 @@ afterAll(async () => {
     const exitCode = await proc.exited;
     expect(exitCode).toBe(0);
   });
+
+  test("assigns unique test IDs when TestReporter.enable lands mid-collection", async () => {
+    // Regression: the live-registration path (ScopeFunctions::call) and the
+    // retroactive path (retroactively_report_discovered_tests) used to draw
+    // IDs from two independent counters. The existing test above enables
+    // TestReporter only after collection finishes, so the two paths never
+    // interleave there.
+    //
+    // Here we pause inside suite B's *async describe callback*. By that point
+    // collection has already run suite A's (sync) callback to completion
+    // (registering test A1/A2), while suite B's own test() has not yet been
+    // called. Enabling TestReporter in that window sends suite A + suite B's
+    // describe through the retroactive walk; test B1 is registered live once
+    // we release the gate.
+
+    using dir = tempDir("test-reporter-mid-collection", {
+      "mid-collection.test.ts": `
+import { afterAll, describe, test, expect } from "bun:test";
+import { existsSync } from "node:fs";
+
+describe("suite A", () => {
+  test("test A1", () => {
+    expect(1).toBe(1);
+  });
+  test("test A2", () => {
+    expect(2).toBe(2);
+  });
+});
+
+describe("suite B", async () => {
+  console.log("__COLLECTION_CHECKPOINT__");
+  while (!existsSync("collect-gate")) {
+    await Bun.sleep(5);
+  }
+  test("test B1", () => {
+    expect(3).toBe(3);
+  });
+});
+
+afterAll(async () => {
+  while (!existsSync("done-gate")) await Bun.sleep(10);
+});
+`,
+    });
+
+    const socketPath = join(String(dir), `inspector-${Math.random().toString(36).substring(2)}.sock`);
+    const collectGatePath = join(String(dir), "collect-gate");
+    const doneGatePath = join(String(dir), "done-gate");
+
+    const session = new TestReporterSession();
+    const framer = new SocketFramer((message: string) => {
+      session.onMessage(message);
+    });
+
+    // Track every raw `found` id in arrival order (no dedupe) so a collision
+    // shows up as a literal duplicate independent of the Map-keyed bookkeeping
+    // in TestReporterSession (which would silently coalesce same-id events).
+    const rawFoundIds: number[] = [];
+    session.addEventListener("TestReporter.found", (params: any) => {
+      rawFoundIds.push(params.id);
+    });
+
+    const socketPromise = connect(`unix://${socketPath}`).then(s => {
+      socket = s;
+      session.socket = s;
+      session.framer = framer;
+      s.data = {
+        onData: framer.onData.bind(framer),
+      };
+      return s;
+    });
+
+    proc = spawn({
+      cmd: [bunExe(), `--inspect-wait=unix:${socketPath}`, "test", "--timeout", "30000", "mid-collection.test.ts"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+
+    await socketPromise;
+
+    // Enable Inspector and Console (NOT TestReporter). Console lets us observe
+    // the collection checkpoint without disturbing collection itself.
+    session.enableInspector();
+    session.send("Console.enable");
+
+    const checkpointSeen = session.waitForConsoleMessage("__COLLECTION_CHECKPOINT__", 15000);
+
+    // Signal ready - this allows test collection to proceed.
+    session.initialize();
+
+    // Wait until suite A has fully registered and collection is paused inside
+    // suite B's async describe callback (test B1 not yet registered).
+    await checkpointSeen;
+
+    // Enable TestReporter now, genuinely mid-collection.
+    session.enableTestReporter();
+
+    // suite A's describe + 2 tests, plus suite B's (empty) describe: 4 events
+    // via the retroactive walk.
+    await session.waitForFoundTests(4, 15000);
+
+    // Release the gate so suite B's callback registers test B1 via the live
+    // path while the agent is enabled.
+    await write(collectGatePath, "go");
+
+    // test B1 brings the total to 5. Pre-fix its id collides with one the
+    // retroactive walk already used, so the Map stays at 4 keys and this
+    // wait times out.
+    const foundTests = await session.waitForFoundTests(5, 15000);
+    expect(foundTests.size).toBe(5);
+
+    const endedTests = await session.waitForEndedTests(3, 15000);
+    expect(endedTests.size).toBe(3);
+
+    // All `end` events received; release the afterAll hold so the subprocess
+    // can exit.
+    await write(doneGatePath, "go");
+
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    void stdout;
+
+    // Core assertion: every `found` id across both paths is unique.
+    expect(rawFoundIds.length).toBe(5);
+    expect(new Set(rawFoundIds).size).toBe(5);
+
+    // Every `end` event's id must correspond to an actual `test`.
+    for (const id of endedTests.keys()) {
+      const found = foundTests.get(id);
+      expect(found).toBeDefined();
+      expect(found.type).toBe("test");
+    }
+
+    const testNames = [...foundTests.values()]
+      .filter(t => t.type === "test")
+      .map(t => t.name)
+      .sort();
+    expect(testNames).toEqual(["test A1", "test A2", "test B1"]);
+
+    if (exitCode !== 0) {
+      expect(stderr).toBe("");
+    }
+    expect(exitCode).toBe(0);
+  });
 });