fix: Node.js 24 compatibility — four test failures resolved

[dividedmind]

Node.js 24 introduced several behavioural changes that broke the test suite. This PR fixes all four failures. Two are functional bugs that affected correctness on all Node versions once properly diagnosed; the other two are test infrastructure issues.

1. Symbol properties in util.inspect output (test/helpers.ts)

Node 24 changed the serialisation format of enumerable symbol properties in util.inspect from [Symbol(foo)]: value to Symbol(foo): value (no brackets). The fixValue() normaliser in test helpers already stripped these to keep snapshots stable across Node versions, but its regex required the leading bracket.
Fix: \[Symbol → \[?Symbol (one character).

2. SQL return events out of order (src/hooks/sqlite.ts, src/hooks/mysql.ts)

Native sqlite3/mysql callbacks fire outside any AsyncLocalStorage context in Node 24, causing SQL return events to be appended to the root event buffer instead of their correct nested position. This made return events appear after all subsequent call events in the AppMap — inaccurate regardless of Node version.
Fix: capture Recording.getContext() at the time of the DB call and restore it inside the completion callback via Recording.run(). The sqlite snapshot is updated to reflect the now-correct interleaved event ordering.

3. Non-deterministic HTTP AppMap filename ordering (test/httpServer.test.ts)

Node 24 processes consecutive HTTP requests fast enough that two requests share the same millisecond timestamp prefix in their AppMap filenames, making glob ordering non-deterministic.
Fix: add 10 ms delays between requests in the test helper makeRequests(). Production code is unchanged.

4. ESM library instrumentation race condition (src/register.ts, src/loader.ts)

The mechanism for patching library modules under ESM (e.g. json5, node:console) relied on the loader thread sending a BroadcastChannel message to the main thread, which then require()d the module to pre-populate the shared CJS cache before the ESM import evaluated. In Node 24 the main thread consistently evaluates ESM module bodies before the event loop delivers the broadcast message, so the patched module was never seen.

This was always racy — Node 24 just made it consistently reproducible. The same race had already been acknowledged and worked around for node:http and node:https by pre-patching them eagerly in register.ts at startup. That pattern is now generalised:

• All module: entries in the user's appmap.yml are pre-patched at startup
• Prisma client module IDs are also pre-patched at startup
• The BroadcastChannel side-channel (esmRequireChannel) and its onmessage handler are removed
• The forceRequire() export is removed from register.ts
• The forceRequire() call sites in loader.ts (resolve() and load() hooks) are removed
• The now-empty resolve() hook is removed from loader.ts (Node only invokes exported hooks, so a passthrough adds overhead for no benefit)

register.ts runs via --require before any ESM evaluation, so these require() calls are unconditionally race-free on all Node versions.

Fixes #190, supersedes #189.

[Copilot]

Pull request overview

This PR addresses Node.js 24 compatibility regressions by stabilizing test snapshot normalization and fixing async-context-sensitive instrumentation so recorded AppMap event ordering remains correct across Node versions.

Changes:

• Stabilize test outputs across Node versions (ObjectId / symbol formatting, deterministic HTTP AppMap filename ordering).
• Fix sqlite/mysql hook event ordering by restoring captured AsyncLocalStorage context when emitting completion events; update affected snapshots.
• Remove the racy ESM “force-require” side-channel by pre-patching configured modules at startup; update CI to include Node 22/24.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/httpServer.test.ts	Adds small inter-request delays to make AppMap filename ordering deterministic under Node 24 timing.
test/helpers.ts	Updates snapshot normalizers for Node 24 util.inspect changes and MongoDB ObjectId formatting.
test/__snapshots__/sqlite.test.ts.snap	Updates sqlite snapshot to reflect corrected event interleaving/ordering.
test/__snapshots__/mongo.test.ts.snap	Updates Mongo snapshot to match the new ObjectId normalization.
src/register.ts	Pre-patches configured modules and Prisma module IDs at startup; removes BroadcastChannel-based ESM side-channel.
src/loader.ts	Removes now-unneeded resolve() hook and forceRequire() call sites.
src/hooks/sqlite.ts	Captures/restores recording async context so sqlite completion emits return events in causal order.
src/hooks/mysql.ts	Captures/restores recording async context so mysql completion emits return/exception events in causal order.
.github/workflows/publish.yml	Bumps action majors and pins publish job to Node 24.
.github/workflows/ci.yml	Expands Node matrix to include 22/24; bumps action majors; updates artifact upload action major.

Comments suppressed due to low confidence (1)

src/hooks/sqlite.ts:108

• The captured async context is restored only for emitting the SQL functionReturn events, but originalCompletionCallback is invoked outside Recording.run(...). If the user callback performs additional instrumented work, those events will still be recorded in the wrong async buffer/order. Consider invoking originalCompletionCallback inside the Recording.run(asyncContext, ...) block (and ensure it still runs even on error).

            ),
          );
        }
        originalCompletionCallback?.apply(this, args);
      };

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/appmap-node@191

commit: 0d31528

[appmap-releasebot]

🎉 This PR is included in version 2.26.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀