Skip to content

test(node-suite): expand async hooks parity coverage#6448

Open
TheHypnoo wants to merge 16 commits into
mainfrom
test/expand-node-async-hooks-parity
Open

test(node-suite): expand async hooks parity coverage#6448
TheHypnoo wants to merge 16 commits into
mainfrom
test/expand-node-async-hooks-parity

Conversation

@TheHypnoo

@TheHypnoo TheHypnoo commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

Expands the granular node:async_hooks parity suite from 5 to 193 fixtures (+188) using Node.js 26.5.0 as the exact oracle, with corresponding Bun and Deno compatibility checks.

The suite is organized into small diagnostic cases covering:

  • AsyncResource construction, identifiers, scope execution, binding, destruction, receiver and argument preservation
  • AsyncLocalStorage nesting, enterWith, exit, disable, re-entry, values, static bind/snapshot, and propagation
  • promises, microtasks, next ticks, timers, timer promises, Web Locks, workers, HTTP, DNS, crypto, filesystem and other provider lifecycles
  • hook/resource option accessors, thrown getters, validation, constructor calls, cleanup, metadata, exports and descriptors
  • bounded raw provider relationships and execution-resource invariants without comparing runtime-specific numeric async IDs

The latest expansion adds 12 fixtures and strengthens 3 existing fixtures:

  • API function metadata and exact module export/descriptors/immutability
  • BLOBREADER and DNSCHANNEL resource lifecycles
  • inherited hook/resource option accessors and accessor-error short-circuiting
  • EventEmitter async-resource back-reference
  • Web Locks and Immediate self-clear ALS propagation
  • undefined, null, falsy and object store values
  • constructor-call validation
  • scheduler yield/wait and cancelled async interval propagation
  • bounded HTTP and worker provider counts, parent relationships, execution-resource mapping and lifecycle balance

Verification

Exact Node oracle:

  • Node.js v26.5.0
  • all 193 fixtures pass node --check
  • new and strengthened fixtures produced byte-identical output across three Node runs

Alternate runtimes:

  • Bun 1.2.18: every retained new fixture completed successfully
  • Deno 2.9.2: every retained new fixture completed successfully
  • differences are retained as diagnostic compatibility evidence rather than weakening Node assertions

Perry focused suite, repeated from clean executions:

python3 scripts/node_suite_run.py async_hooks
async_hooks: 78/193 matched, 114 output differences, 1 runtime timeout

The second focused run reproduced the same 78/193, 114 differences and single pre-existing TLS timeout. The granular baseline is updated from 76/181 to 78/193; the overall baseline is 2883/3051 (94.5%).

Additional checks:

prettier --check <changed async_hooks fixtures and documentation>
node --check <all 193 async_hooks fixtures>
git diff --check
cargo fmt --all -- --check
python3 -m py_compile scripts/node_suite_run.py scripts/node_suite_regression_check.py

All completed successfully.

Stable diagnostics

The expanded suite exposes stable Perry gaps including:

  • missing or incomplete hook callbacks and provider resources
  • incomplete AsyncResource prototype/constructor behavior
  • mutable/non-standard module namespace descriptors
  • constructor calls accepted without new
  • provider execution-resource and lifecycle mapping gaps
  • incomplete EventEmitter resource back-references

It also records compatible behavior where Perry already matches Node, including the new Web Locks and store-value propagation cases.

Deliberate exclusions

The README documents the stopping judgment and upstream mappings. Remaining cases are excluded when they are redundant, unsupported by the claimed surface, or unreliable as granular parity diagnostics:

  • raw callback ordering, recursion and scheduler-sensitive interleavings
  • GC/finalizer/destroy timing, stress and leak tests
  • worker/process shutdown and cross-thread propagation races
  • direct MessagePort lifecycle coverage: Bun 1.2.18 did not terminate even after listener removal, unref() and explicit close
  • Argon2 and prime callback providers: Perry does not settle those callback operations, so they belong to the corresponding crypto runtime work
  • unbounded HTTP/provider internals; retained HTTP and worker coverage uses explicit completion and cleanup barriers

Summary by CodeRabbit

  • Tests

    • Expanded async context parity coverage across promises, timers, resources, storage, streams, filesystem, networking, crypto, workers, and child processes.
    • Added validation for lifecycle events, context propagation, cleanup, isolation, error handling, and module/API behavior.
    • Updated focused coverage baseline to 78/193 and overall regression baseline to 2,883/3,051.
  • Documentation

    • Clarified deterministic test scope, comparison methodology, supported coverage, and excluded environment-sensitive scenarios.
  • Chores

    • Async context tests now run in a dedicated sequential lane for improved consistency.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR expands the node:async_hooks parity suite with deterministic propagation, lifecycle, hook, provider, integration, validation, documentation, scheduling, and baseline updates.

Changes

Async hooks parity coverage

Layer / File(s) Summary
AsyncLocalStorage propagation and storage
test-parity/node-suite/async_hooks/propagation/*, test-parity/node-suite/async_hooks/storage/*
Adds coverage for async boundaries, nested contexts, storage transitions, pending work, isolation, and cleanup.
AsyncResource and static context behavior
test-parity/node-suite/async_hooks/resource/*, test-parity/node-suite/async_hooks/static/*
Adds construction, scope, binding, snapshot, receiver, nesting, restoration, and destruction checks.
Hooks and validation
test-parity/node-suite/async_hooks/hooks/*, test-parity/node-suite/async_hooks/validation/*
Adds hook configuration, lifecycle, provider, promise-tracking, cancellation, argument, exception, and invalid-input tests.
Runtime integrations and providers
test-parity/node-suite/async_hooks/integrations/*, test-parity/node-suite/async_hooks/providers/*
Adds AsyncLocalStorage propagation fixtures for crypto, filesystem, streams, timers, compression, networking, child processes, DNS, TLS, events, and workers.
Suite scheduling and baselines
scripts/node_suite_run.py, test-parity/node-suite/async_hooks/README.md, test-parity/node_suite_baseline.json
Moves async_hooks to the sequential lane, documents expanded coverage and exclusions, and updates Node 26.5.0 baseline values.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • PerryTS/perry#5097: Updates related Node suite lane scheduling and differential runner behavior.
  • PerryTS/perry#5166: Updates related Node suite baseline floors and schema metadata.
  • PerryTS/perry#6484: Modifies the same SLOW_MODULES scheduling mechanism for another Node suite module.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning Required template sections are missing: Changes, Related issue, Test plan, Screenshots/output, and Checklist. Add the missing sections with concrete bullets, an issue reference or n/a, verification commands, optional output, and checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: expanded async hooks parity coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/expand-node-async-hooks-parity

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
test-parity/node-suite/async_hooks/storage/disable-one-pending-other.ts (1)

12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consistently format store values.

For consistency with line 28 and the preceding first.getStore() call, consider wrapping second.getStore() in String() as well.

♻️ Proposed refactor
           console.log(
             "pending isolated stores:",
             String(first.getStore()),
-            second.getStore(),
+            String(second.getStore()),
           );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-parity/node-suite/async_hooks/storage/disable-one-pending-other.ts`
around lines 12 - 16, Update the pending isolated stores log in the relevant
async-hooks test to wrap second.getStore() with String(), matching the
formatting already used for first.getStore() and the later log.
test-parity/node-suite/async_hooks/hooks/multiple-hooks.ts (1)

7-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract duplicate hook creation into a factory function.

The init, before, and after callbacks for both hooks are identical except for the target events array they append to. Consider extracting this configuration into a helper function to eliminate duplication.

♻️ Proposed refactor
-const first = createHook({
-  init(asyncId, type) {
-    if (type === "ParityMultipleHooks") {
-      targetId = asyncId;
-      firstEvents.push("init");
-    }
-  },
-  before(asyncId) {
-    if (asyncId === targetId) firstEvents.push("before");
-  },
-  after(asyncId) {
-    if (asyncId === targetId) firstEvents.push("after");
-  },
-}).enable();
-const second = createHook({
-  init(asyncId, type) {
-    if (type === "ParityMultipleHooks") {
-      targetId = asyncId;
-      secondEvents.push("init");
-    }
-  },
-  before(asyncId) {
-    if (asyncId === targetId) secondEvents.push("before");
-  },
-  after(asyncId) {
-    if (asyncId === targetId) secondEvents.push("after");
-  },
-}).enable();
+function createTrackingHook(events: string[]) {
+  return createHook({
+    init(asyncId, type) {
+      if (type === "ParityMultipleHooks") {
+        targetId = asyncId;
+        events.push("init");
+      }
+    },
+    before(asyncId) {
+      if (asyncId === targetId) events.push("before");
+    },
+    after(asyncId) {
+      if (asyncId === targetId) events.push("after");
+    },
+  });
+}
+
+const first = createTrackingHook(firstEvents).enable();
+const second = createTrackingHook(secondEvents).enable();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-parity/node-suite/async_hooks/hooks/multiple-hooks.ts` around lines 7 -
34, Extract the duplicated async hook configuration into a factory function that
accepts the target events array, preserves the shared targetId checks and
callback behavior, and returns an enabled hook. Replace the separate inline
configurations for first and second with factory calls using firstEvents and
secondEvents.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test-parity/node-suite/async_hooks/hooks/multiple-hooks.ts`:
- Around line 7-34: Extract the duplicated async hook configuration into a
factory function that accepts the target events array, preserves the shared
targetId checks and callback behavior, and returns an enabled hook. Replace the
separate inline configurations for first and second with factory calls using
firstEvents and secondEvents.

In `@test-parity/node-suite/async_hooks/storage/disable-one-pending-other.ts`:
- Around line 12-16: Update the pending isolated stores log in the relevant
async-hooks test to wrap second.getStore() with String(), matching the
formatting already used for first.getStore() and the later log.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ec7bb2d7-f866-4316-a925-8f8b54fc727d

📥 Commits

Reviewing files that changed from the base of the PR and between 0188613 and 011184d.

📒 Files selected for processing (26)
  • test-parity/node-suite/async_hooks/README.md
  • test-parity/node-suite/async_hooks/hooks/enable-disable.ts
  • test-parity/node-suite/async_hooks/hooks/multiple-hooks.ts
  • test-parity/node-suite/async_hooks/hooks/resource-argument.ts
  • test-parity/node-suite/async_hooks/hooks/scope-throw-lifecycle.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-derive.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-random-bytes.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-random-fill.ts
  • test-parity/node-suite/async_hooks/integrations/events-emitter.ts
  • test-parity/node-suite/async_hooks/integrations/fs-callback-chain.ts
  • test-parity/node-suite/async_hooks/integrations/fs-promises-chain.ts
  • test-parity/node-suite/async_hooks/integrations/fs-streams.ts
  • test-parity/node-suite/async_hooks/integrations/stream-finished.ts
  • test-parity/node-suite/async_hooks/integrations/stream-readable.ts
  • test-parity/node-suite/async_hooks/integrations/stream-transform.ts
  • test-parity/node-suite/async_hooks/integrations/stream-writable.ts
  • test-parity/node-suite/async_hooks/integrations/timers-promises.ts
  • test-parity/node-suite/async_hooks/integrations/util-promisify.ts
  • test-parity/node-suite/async_hooks/integrations/zlib-brotli.ts
  • test-parity/node-suite/async_hooks/integrations/zlib-gzip-gunzip.ts
  • test-parity/node-suite/async_hooks/resource/async-scope-storage.ts
  • test-parity/node-suite/async_hooks/static/bind-concurrent.ts
  • test-parity/node-suite/async_hooks/static/snapshot-async.ts
  • test-parity/node-suite/async_hooks/storage/disable-one-pending-other.ts
  • test-parity/node-suite/async_hooks/storage/exit-async-rejection.ts
  • test-parity/node_suite_baseline.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • test-parity/node_suite_baseline.json

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
test-parity/node-suite/async_hooks/propagation/dynamic-import.ts (1)

7-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using a finally block for global state cleanup.

While a failure in this test fixture will likely exit the process anyway, it's generally best practice to clean up temporary global mutations in a finally block to guarantee restoration if the import or execution throws.

🛠️ Proposed refactor
-const observed = await storage.run("dynamic-import", async () => {
-  const namespace = await import(source);
-  return namespace.store;
-});
-delete (globalThis as any).parityDynamicImportStore;
+let observed;
+try {
+  observed = await storage.run("dynamic-import", async () => {
+    const namespace = await import(source);
+    return namespace.store;
+  });
+} finally {
+  delete (globalThis as any).parityDynamicImportStore;
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-parity/node-suite/async_hooks/propagation/dynamic-import.ts` around
lines 7 - 11, Wrap the dynamic import and storage observation in a try/finally
construct so the temporary global state is always cleaned up. Keep the existing
storage.run behavior and delete parityDynamicImportStore in the finally block,
including when import(source) or the callback throws.
test-parity/node-suite/async_hooks/integrations/fs-access.ts (1)

6-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Ensure test file cleanup even if an error occurs.

If access returns an error and the promise rejects, the unlinkSync call will be bypassed, leaving the temporary file behind on the filesystem. Wrap the asynchronous operation in a try...finally block to guarantee cleanup.

♻️ Proposed refactor
-await storage.run(
-  "fs-access",
-  () =>
-    new Promise<void>((resolve, reject) => {
-      access(path, constants.R_OK, (error) => {
-        console.log("fs.access store:", storage.getStore());
-        if (error) return reject(error);
-        resolve();
-      });
-    }),
-);
-unlinkSync(path);
-console.log("fs.access outside:", String(storage.getStore()));
+try {
+  await storage.run(
+    "fs-access",
+    () =>
+      new Promise<void>((resolve, reject) => {
+        access(path, constants.R_OK, (error) => {
+          console.log("fs.access store:", storage.getStore());
+          if (error) return reject(error);
+          resolve();
+        });
+      }),
+  );
+} finally {
+  unlinkSync(path);
+}
+console.log("fs.access outside:", String(storage.getStore()));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-parity/node-suite/async_hooks/integrations/fs-access.ts` around lines 6
- 18, Wrap the await storage.run operation in a try...finally block so
unlinkSync(path) always executes, including when access rejects with an error.
Keep the existing access callback, logging, rejection, and post-cleanup store
logging behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test-parity/node-suite/async_hooks/integrations/fs-directory.ts`:
- Around line 1-5: Update the temporary path setup around the path constant and
realpath comparison to resolve the temporary directory base with realpathSync
before constructing the expected directory path. Import and use realpathSync
alongside the existing fs imports so resolved and expected paths match on macOS
and Linux.

In `@test-parity/node-suite/async_hooks/providers/http-client-events.ts`:
- Around line 15-42: Guarantee cleanup of servers and client/request handles
across rejected and socket-error paths. In
test-parity/node-suite/async_hooks/providers/http-client-events.ts:15-42, wrap
storage.run in try/finally and move server.close into finally; in
test-parity/node-suite/async_hooks/providers/http-request.ts:14-40, close the
server unconditionally after the request attempt; in
test-parity/node-suite/async_hooks/providers/net-connect.ts:12-29 and
test-parity/node-suite/async_hooks/providers/net-write-callbacks.ts:12-32,
destroy the client on failure and close the server in finally; in
test-parity/node-suite/async_hooks/providers/net-server-events.ts:9-27, retain
server and client handles and clean both up on rejection paths.

---

Nitpick comments:
In `@test-parity/node-suite/async_hooks/integrations/fs-access.ts`:
- Around line 6-18: Wrap the await storage.run operation in a try...finally
block so unlinkSync(path) always executes, including when access rejects with an
error. Keep the existing access callback, logging, rejection, and post-cleanup
store logging behavior unchanged.

In `@test-parity/node-suite/async_hooks/propagation/dynamic-import.ts`:
- Around line 7-11: Wrap the dynamic import and storage observation in a
try/finally construct so the temporary global state is always cleaned up. Keep
the existing storage.run behavior and delete parityDynamicImportStore in the
finally block, including when import(source) or the callback throws.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8afccbdb-ceaa-45e3-8403-a0951a852f72

📥 Commits

Reviewing files that changed from the base of the PR and between 011184d and 11227f4.

📒 Files selected for processing (65)
  • scripts/node_suite_run.py
  • test-parity/node-suite/async_hooks/README.md
  • test-parity/node-suite/async_hooks/hooks/cancelled-immediate-destroy.ts
  • test-parity/node-suite/async_hooks/hooks/cancelled-timeout-destroy.ts
  • test-parity/node-suite/async_hooks/hooks/disable-in-init.ts
  • test-parity/node-suite/async_hooks/hooks/enable-in-init.ts
  • test-parity/node-suite/async_hooks/hooks/multiple-hooks.ts
  • test-parity/node-suite/async_hooks/hooks/promise-resolve.ts
  • test-parity/node-suite/async_hooks/hooks/provider-crypto-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-fs-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-microtask-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-nexttick-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-timer-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/track-promises.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-generate-key-pair.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-generate-key.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-hkdf.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-prime.ts
  • test-parity/node-suite/async_hooks/integrations/crypto-random-int.ts
  • test-parity/node-suite/async_hooks/integrations/event-emitter-async-resource.ts
  • test-parity/node-suite/async_hooks/integrations/fs-access.ts
  • test-parity/node-suite/async_hooks/integrations/fs-append-file.ts
  • test-parity/node-suite/async_hooks/integrations/fs-copy-file.ts
  • test-parity/node-suite/async_hooks/integrations/fs-directory.ts
  • test-parity/node-suite/async_hooks/integrations/fs-fd-operations.ts
  • test-parity/node-suite/async_hooks/integrations/fs-metadata.ts
  • test-parity/node-suite/async_hooks/integrations/fs-mkdtemp.ts
  • test-parity/node-suite/async_hooks/integrations/fs-rename.ts
  • test-parity/node-suite/async_hooks/integrations/fs-truncate.ts
  • test-parity/node-suite/async_hooks/integrations/fs-watch-file.ts
  • test-parity/node-suite/async_hooks/integrations/fs-watch.ts
  • test-parity/node-suite/async_hooks/integrations/zlib-create-gzip.ts
  • test-parity/node-suite/async_hooks/integrations/zlib-deflate-inflate.ts
  • test-parity/node-suite/async_hooks/propagation/dynamic-import.ts
  • test-parity/node-suite/async_hooks/propagation/nested-nexttick.ts
  • test-parity/node-suite/async_hooks/propagation/timer-ref-unref.ts
  • test-parity/node-suite/async_hooks/propagation/vm-context.ts
  • test-parity/node-suite/async_hooks/providers/child-exec-file.ts
  • test-parity/node-suite/async_hooks/providers/child-exec.ts
  • test-parity/node-suite/async_hooks/providers/child-spawn-events.ts
  • test-parity/node-suite/async_hooks/providers/dgram-events.ts
  • test-parity/node-suite/async_hooks/providers/dgram-send.ts
  • test-parity/node-suite/async_hooks/providers/dns-lookup.ts
  • test-parity/node-suite/async_hooks/providers/dns-resolve4.ts
  • test-parity/node-suite/async_hooks/providers/events-on-async.ts
  • test-parity/node-suite/async_hooks/providers/fixtures/context-worker.cjs
  • test-parity/node-suite/async_hooks/providers/fixtures/tls-credentials.js
  • test-parity/node-suite/async_hooks/providers/http-agent-isolation.ts
  • test-parity/node-suite/async_hooks/providers/http-client-events.ts
  • test-parity/node-suite/async_hooks/providers/http-request.ts
  • test-parity/node-suite/async_hooks/providers/https-agent-isolation.ts
  • test-parity/node-suite/async_hooks/providers/https-request.ts
  • test-parity/node-suite/async_hooks/providers/net-connect.ts
  • test-parity/node-suite/async_hooks/providers/net-server-events.ts
  • test-parity/node-suite/async_hooks/providers/net-write-callbacks.ts
  • test-parity/node-suite/async_hooks/providers/readline-interface.ts
  • test-parity/node-suite/async_hooks/providers/stream-async-iterator.ts
  • test-parity/node-suite/async_hooks/providers/tls-connect.ts
  • test-parity/node-suite/async_hooks/providers/worker-message-events.ts
  • test-parity/node-suite/async_hooks/resource/bind-receiver-values.ts
  • test-parity/node-suite/async_hooks/resource/scope-receiver-values.ts
  • test-parity/node-suite/async_hooks/static/snapshot-throw-cleanup.ts
  • test-parity/node-suite/async_hooks/storage/disable-one-pending-other.ts
  • test-parity/node-suite/async_hooks/validation/track-promises.ts
  • test-parity/node_suite_baseline.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • test-parity/node-suite/async_hooks/hooks/multiple-hooks.ts
  • test-parity/node-suite/async_hooks/storage/disable-one-pending-other.ts

Comment thread test-parity/node-suite/async_hooks/integrations/fs-directory.ts
Comment thread test-parity/node-suite/async_hooks/providers/http-client-events.ts Outdated
@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
test-parity/node-suite/async_hooks/hooks/fs-readfile-trigger-chain.ts (1)

1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer cross-platform temporary directory path.

Using a hardcoded /tmp/ path could fail on Windows and is less robust than using Node's os.tmpdir(). Consider utilizing node:os and node:path to construct a platform-agnostic temporary file path.

♻️ Proposed refactor for cross-platform temporary path
 import { createHook, executionAsyncId } from "node:async_hooks";
 import { readFile, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 
-const path = "/tmp/perry-async-hooks-readfile-chain.txt";
+const path = join(tmpdir(), "perry-async-hooks-readfile-chain.txt");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-parity/node-suite/async_hooks/hooks/fs-readfile-trigger-chain.ts` around
lines 1 - 4, Update the temporary file path declaration in the async hook test
to use the platform-specific directory from node:os and construct the filename
with node:path, replacing the hardcoded /tmp path while preserving the existing
filename and file operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test-parity/node-suite/async_hooks/hooks/fs-readfile-trigger-chain.ts`:
- Around line 1-4: Update the temporary file path declaration in the async hook
test to use the platform-specific directory from node:os and construct the
filename with node:path, replacing the hardcoded /tmp path while preserving the
existing filename and file operations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1421cb97-6a5e-4308-a7d3-90e1ae22ba67

📥 Commits

Reviewing files that changed from the base of the PR and between 8fdead4 and 5c47b97.

📒 Files selected for processing (18)
  • test-parity/node-suite/async_hooks/README.md
  • test-parity/node-suite/async_hooks/hooks/fs-readfile-trigger-chain.ts
  • test-parity/node-suite/async_hooks/hooks/interval-repeat-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/mutate-during-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/nexttick-sibling-trigger.ts
  • test-parity/node-suite/async_hooks/hooks/provider-dns-lookup-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-dns-lookup-service-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-dns-query-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-random-bytes-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-zlib-lifecycle.ts
  • test-parity/node-suite/async_hooks/integrations/event-emitter-brand-validation.ts
  • test-parity/node-suite/async_hooks/integrations/util-promisify-custom.ts
  • test-parity/node-suite/async_hooks/providers/dgram-dual-context.ts
  • test-parity/node-suite/async_hooks/providers/dns-lookup-service.ts
  • test-parity/node-suite/async_hooks/providers/net-dual-context.ts
  • test-parity/node-suite/async_hooks/resource/static-bind-types.ts
  • test-parity/node-suite/async_hooks/storage/exit-instance-isolation.ts
  • test-parity/node_suite_baseline.json

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test-parity/node_suite_baseline.json`:
- Line 5: Update the provenance statement in the baseline note to replace the
stale clean Node 26 run totals and percentage, 2810/2863 (98.1%), with the
current overall-run values recorded on lines 7–10: 2881/3027 (95.2%). Preserve
the surrounding explanations and module-specific provenance unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a7cdc76-ef5b-4843-8364-7199416fe6fa

📥 Commits

Reviewing files that changed from the base of the PR and between 24d3fcb and c382d8e.

📒 Files selected for processing (9)
  • test-parity/node-suite/async_hooks/README.md
  • test-parity/node-suite/async_hooks/hooks/immediate-nested-lifecycle.ts
  • test-parity/node-suite/async_hooks/hooks/provider-crypto-request-lifecycles.ts
  • test-parity/node-suite/async_hooks/hooks/provider-fs-promises-lifecycles.ts
  • test-parity/node-suite/async_hooks/hooks/provider-fs-watcher-lifecycles.ts
  • test-parity/node-suite/async_hooks/hooks/provider-net-lifecycle-matrix.ts
  • test-parity/node-suite/async_hooks/hooks/provider-udp-socket-lifecycle.ts
  • test-parity/node-suite/async_hooks/resource/static-bind-types.ts
  • test-parity/node_suite_baseline.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • test-parity/node-suite/async_hooks/resource/static-bind-types.ts

Comment thread test-parity/node_suite_baseline.json Outdated
@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@TheHypnoo

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…c-hooks-parity

# Conflicts:
#	test-parity/node_suite_baseline.json
@proggeramlug proggeramlug added the ready PR triaged: CodeRabbit feedback + conflicts addressed label Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready PR triaged: CodeRabbit feedback + conflicts addressed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants