Substantive new public API — module.registerHooks() is the recommended replacement for module.register() (DEP0205). The CJS-only scope is reasonable for a first cut and the design (LIFO chain, nextResolve/nextLoad delegation, shortCircuit validation, deregister) tracks Node's contract. A few items worth a look.

Possible bug — source ?? "" silently swallows file-not-found

let source;
try {
  const filePath = StringPrototypeStartsWith(loadUrl, "file://")
    ? url.fileURLToPath(loadUrl)
    : loadUrl;
  source = op_require_read_file(filePath);
} catch {
  source = undefined;
}
return {
  source: source ?? "",
  format: currentContext?.format ?? undefined,
  shortCircuit: true,
};

When op_require_read_file throws (file missing, permission error, EISDIR, etc.), the chain falls through to source: "". The user code then loads an empty module. Two issues:

1. The user has no way to tell "the file exists but is empty" from "the file doesn't exist."
2. Node's default load behavior surfaces the read error to the caller of require(). Returning "" papers over it.

Suggested: re-throw the caught error (wrap in ERR_CANNOT_LOAD_MODULE or similar) rather than coercing to empty string. The try/catch was likely there to handle "virtual modules" that have no real file — but those should already have been intercepted by an upstream hook returning a source before reaching the default branch. By the time we reach the catch, a missing file is a real error.

Misleading comment — auto-detect doesn't actually detect

} else {
  // Auto-detect format: try CJS, fall back to ESM
  const source = typeof result.source === "string"
    ? result.source
    : new TextDecoder().decode(result.source);
  this._compile(source, this.filename);
}

The comment promises auto-detection; the code unconditionally calls _compile (CJS path) with no ESM fallback. If the hook returns {source: "import x from 'y'", format: undefined}, this hits _compile and the import keyword throws. Either:

• Implement actual detection (a regex for import/export at module top, or invoke loadESMFromCJS if _compile throws a specific syntax error), or
• Fix the comment to "Default to CJS when format is unspecified."

registerHooks input validation

export function registerHooks(hooks) {
  const entry = {
    resolve: typeof hooks.resolve === "function" ? hooks.resolve : null,
    load: typeof hooks.load === "function" ? hooks.load : null,
    id: nextHookId++,
  };
  ...
}

• registerHooks(null) / registerHooks(undefined) throws a cryptic TypeError: Cannot read properties of null rather than Node's ERR_INVALID_ARG_TYPE. A validateObject(hooks, "hooks") up front would match Node behavior and give a better error.
• registerHooks({}) (no resolve, no load) silently registers a no-op hook. Probably should throw — Node's docs say at least one of the two must be a function.

Smaller things

nextHookId is dead code

const entry = {
  resolve: ...,
  load: ...,
  id: nextHookId++,
};

The id is assigned but never read. deregister uses ArrayPrototypeIndexOf(hookEntries, entry) — direct identity, not the id. Drop nextHookId and entry.id.

O(N) "has load/resolve hook" scan on every resolve/load

if (hookEntries.length > 0 && !insideLoadHook) {
  let hasLoadHook = false;
  for (let i = 0; i < hookEntries.length; i++) {
    if (hookEntries[i].load !== null) {
      hasLoadHook = true;
      break;
    }
  }
  if (hasLoadHook) { ... }
}

Repeated in _load, Module.prototype.load, and _resolveFilename (with the resolve variant). For typical short hook lists this is fine; if registerHooks usage proliferates, the scan adds up. A pair of integer counters (loadHookCount, resolveHookCount) maintained at register/deregister would make this O(1):

if (loadHookCount > 0 && !insideLoadHook) { ... }

new TextDecoder() per non-string source

new TextDecoder().decode(result.source)

Three call sites (commonjs, json, auto-detect). Hoist a module-level const utf8Decoder = new TextDecoder().

Reentrancy edge case worth documenting

The insideResolveHook / insideLoadHook flags prevent the default resolver/loader from re-entering the hook chain. But if a user hook calls Module._resolveFilename directly (instead of nextResolve), the flag is false and the hook chain re-fires — infinite loop.

Probably worth either:

• A doc comment in executeResolveHookChain saying "user hooks must use nextResolve, not Module._resolveFilename, to delegate."
• Or set the flag for the entire executeResolveHookChain body, not just the default branch — defends against the user-_resolveFilename foot-gun at the cost of preventing legitimate same-realm re-entry.

Test coverage — error paths

The four spec tests are happy-path. The 10 Node compat enrollments cover shortCircuit-required-{start,middle} and context-merged/context-optional, which is good. But there's nothing exercising:

• A hook that throws synchronously (does the throw propagate to the require() caller?).
• registerHooks(null) / registerHooks({}) — the validation gap above.
• Resolve hook returning a non-file://, non-node: URL (e.g., https://...) — the current _resolveFilename returns result.url as-is, which then flows into Module.prototype.load. Worth testing what happens.

Builtin hook is observe-only — intentional deviation?

executeLoadHookChain(filename, context);
// Always load builtins normally regardless of hook result;
// hooks can observe but not replace builtin source.

Node's module.registerHooks() lets you replace builtin source (the docs explicitly call out the node:* interception case). The PR's "observe-only" choice is a real API deviation — worth either:

• Supporting source replacement for builtins (consume result.source if non-null and use that to compile instead of loadNativeModule).
• Or documenting this as a known limitation in the same way the PR description calls out the ESM-side gap.

CI

134 pass, 2 skipping, 0 fail. ✓ Substance is otherwise tracking Node well.

Holding to COMMENT — this is a public CJS API addition, so the items above (especially the source ?? "" silent-fallthrough and the misleading auto-detect comment) deserve a maintainer eye before landing. Substance is mostly clean and the test coverage is reasonable for a first cut.

Most of my prior review addressed cleanly in c0154d6. Remaining items are non-blocking. Walked the delta:

Fixed:

• File-read errors propagate instead of source ?? ""-coercing to empty — the try/catch and ?? "" are both gone, op_require_read_file(filePath) now throws normally to the caller. Existing spec tests (main_load.cjs, main_resolve.cjs, main_chained.cjs, main_deregister.cjs) all return source from a hook rather than falling through to the default loader, so they're unaffected.
• nextHookId dead code dropped.
• new TextDecoder() per call hoisted to a module-level utf8Decoder with ??= lazy-init across all three call sites (commonjs, json, default-CJS).
• Misleading "auto-detect" comment corrected to "Default to CJS when format is unspecified."
• registerHooks input validation added: ERR_INVALID_ARG_TYPE for non-object/null, ERR_INVALID_ARG_VALUE when neither resolve nor load is a function. No more cryptic Cannot read properties of null from hooks.resolve.

Still open from prior review (non-blocking):

• O(N) "has load/resolve hook" scan in _load / _resolveFilename / Module.prototype.load — could be O(1) with counters maintained at register/deregister.
• Reentrancy: user hook calling Module._resolveFilename directly bypasses insideResolveHook and infinite-loops. Edge case; doc comment would suffice.
• Builtin hook is observe-only — Node's registerHooks allows replacing builtin source. The PR description scopes this to CJS-only, but the builtin observe-only choice deserves either implementation parity or an explicit "known limitation" line in the docs.
• Error-path test coverage gap (hook throws, registerHooks validation, non-file/non-node URLs from resolve) — happy-path only today.

CI: 74 pass, 21 pending, 0 fail. Holding COMMENT pending CI completion. Substance is otherwise clean. cc @bartlomieju on the observe-only-builtin scope decision (you're the author so this is moot, but flagging since it's the one place this PR's behavior diverges from Node's and it's worth pinning in a review thread either way).

502eb62 addresses the v2 "builtin observe-only is a real Node deviation" concern. Walked the new code:

• Builtin-override path in Module._load: when the hook returns format !== "builtin" and source !== null, a fresh Module(filename, parent) is constructed with the hook's source, cached in Module._cache[filename], compiled via _compile (or JSONParse for "json"), and returned — bypassing loadNativeModule. ✓
• Test main_builtin_override.cjs: registers a hook that intercepts node:util and replaces with module.exports = { customUtil: true }. Asserts both the override (util.customUtil === true) and the displaced original (util.inspect === undefined). Tight regression coverage.
• Format support: commonjs, json, and default-CJS all handled. format === "module" (ESM) falls through to default-CJS, which would syntax-error on import keywords — but the PR is CJS-scoped per the original description, so out of scope here.
• Cache semantics: subsequent require("node:util") after override hits Module._cache and returns the override. To revert, user must delete Module._cache["node:util"] — standard CJS pattern, matches user expectation.

CI

133 pass, 2 fail, 1 skip. The two fails are:

• ci status (rollup of the wpt fail)
• wpt release linux-x86_64 — single failing assertion is /fetch/api/abort/general.any.worker.html - Underlying connection is closed when aborting after receiving response - no-cors (5871 passed, 1 failed, 2076 expected-failure across 5872 tests). That's an unrelated fetch-abort test, nothing to do with node:test mock implementation or node:module registerHooks. Looks like the recurring fetch-abort WPT flake.

All relevant tests for this PR (test node_compat (1/3), (2/3), (3/3) across linux/macos/windows × debug/release) pass. The WPT failure is in a different subsystem entirely.

Holding COMMENT strictly per the CI-must-be-fully-green rule rather than approving on faith. If the WPT job is the recurring fetch-abort flake (not this PR's regression), one re-run should clear it — happy to flip to APPROVE on green.