napi: apply ToObject coercion to element/property-name/prototype APIs

[robobun]

What does this PR do?

The N-API object-operating family diverged from Node's CHECK_TO_OBJECT contract across two groups of entry points.

Group 1 (element / property-name / prototype): napi_set_element, napi_has_element, napi_get_element, napi_delete_element, napi_get_property_names, napi_get_all_property_names, napi_get_prototype. These called getObject() / is_object() and rejected every primitive with napi_array_expected or napi_object_expected and no pending exception.

Group 2 (by-key property siblings): napi_set_property, napi_get_property, napi_has_property, napi_delete_property, napi_has_own_property, napi_set_named_property, napi_get_named_property, napi_has_named_property. These already coerced primitives via toObject but returned napi_pending_exception (10) instead of Node's napi_object_expected (2) on null/undefined.

// per call: bun status vs node status
napi_set_element(env, num42_5, 0, v);        // bun 8, no exc  | node 0 (coerced no-op)
napi_set_element(env, jsnull, 0, v);         // bun 8, NO exc  | node 2 + pending TypeError
napi_get_element(env, str_abc, 1, &r);       // bun 2          | node 0, r = "b"
napi_get_property_names(env, str_abc, &r);   // bun 2          | node 0 (index keys)
napi_get_prototype(env, num42_5, &r);        // bun 2          | node 0 (Number.prototype)
napi_get_property(env, jsnull, key, &r);     // bun 10         | node 2 + pending TypeError
napi_get_all_property_names(env, o, /*key_mode*/99, f, c, &r);       // bun 0 | node 1
napi_get_all_property_names(env, o, m, f, /*key_conversion*/99, &r); // bun 0 | node 1
// napi_key_keep_numbers on ["x"]-style index keys:
//   key0 typeof -> bun string | node number

Portable addons feature-detect against these status codes, and napi_array_expected is a status Node uses only for napi_get_array_length.

Cause

Node runs ToObject(context) (its CHECK_TO_OBJECT macro, js_native_api_v8.cc) on the target in every one of these entry points, which coerces strings/numbers/booleans/symbols to their wrapper object and only throws on null/undefined; when it throws, the status is napi_object_expected and the TypeError is left pending.

napi_get_all_property_names additionally never validated its key_mode / key_conversion enum arguments (Node rejects out-of-range values with napi_invalid_arg) and never read key_conversion at all, so napi_key_keep_numbers still returned array-index keys as JSStrings.

Fix

• Add a NAPI_CHECK_TO_OBJECT(env, globalObject, result, src) macro that mirrors Node's semantics: JSValue::toObject() to coerce primitives, and on exception return napi_object_expected with the TypeError still pending. Apply it at all fourteen C++ sites above.
• napi_get_prototype (Rust): replace the is_object() gate with an is_undefined_or_null() gate that throws the TypeError via to_object() and returns napi_object_expected; other primitives fall through to JSValue::getPrototype(), which already synthesizes the correct prototype (synthesizePrototype) without allocating a wrapper.
• napi_get_all_property_names: validate key_mode and key_conversion; when key_conversion == napi_key_keep_numbers, walk the result array and replace canonical array-index strings with jsNumber values (JSC's ownPropertyKeys/allPropertyKeys always emit string keys).
• napi_delete_element now performs the delete even when result == NULL, matching Node (previously the delete itself was skipped).

Verification

test/napi/napi-app/standalone_tests.cpp gains test_napi_object_coercion, which prints status / pending for every call above plus the key_conversion typeof check. The napi.test.ts case runs it through checkSameOutput, so Bun's output is asserted byte-for-byte against Node's, and additionally spot-checks the high-signal lines so the assertion fails under USE_SYSTEM_BUN=1 even if the Node comparison happened to produce matching (wrong) output.

Before / after

# system bun (before)               # this PR == node v26.3.0
set_element(number): status=8       set_element(number): status=0 pending=0
set_element(null): status=8         set_element(null): status=2 pending=1
get_element(string,1): status=2     get_element(string,1): status=0 pending=0
                                    get_element(string,1) value=b
get_prototype(number): status=2     get_prototype(number): status=0 pending=0
                                    get_prototype(number) is Number.prototype=1
get_prototype(null): status=2       get_prototype(null): status=2 pending=1
get_property(null): status=10       get_property(null): status=2 pending=1
get_all_property_names(key_mode=99): status=0    ... status=1 pending=0
keep_numbers key0 typeof=string     keep_numbers key0 typeof=number

Node's upstream js-native-api/test_object, test_array, and test_properties suites continue to pass.

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/napi/napi.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4955bb67-d694-4de3-b346-4e9c31e535ca

📥 Commits

Reviewing files that changed from the base of the PR and between 7721611 and 7d9d546.

📒 Files selected for processing (3)

• src/jsc/bindings/napi.cpp
• src/runtime/napi/napi_body.rs
• test/napi/napi-app/standalone_tests.cpp

Walkthrough

N-API object-taking functions now share exception-aware ToObject coercion. Property-name enumeration validates arguments and can restore numeric index keys. Prototype handling for null and undefined is adjusted, with new standalone and Node-parity tests.

Changes

N-API object coercion

Layer / File(s)	Summary
Shared object coercion and API adoption src/jsc/bindings/napi.cpp	Adds NAPI_CHECK_TO_OBJECT and applies it across property and element APIs.
Enumeration and prototype handling src/jsc/bindings/napi.cpp, src/runtime/napi/napi_body.rs	Validates property-name arguments, preserves numeric index keys, updates property-name collection, and adjusts prototype handling for null and undefined.
Object coercion regression coverage test/napi/napi-app/standalone_tests.cpp, test/napi/napi-app/js_test_helpers.cpp, test/napi/napi-app/module.js, test/napi/napi.test.ts	Adds helper updates, standalone coverage, and Node-parity assertions for coercion, pending exceptions, enum validation, and numeric keys.

Possibly related PRs

• oven-sh/bun#34137: Related N-API pending-exception handling changes involving napi_get_prototype and napi.cpp.
• oven-sh/bun#34141: Related updates to the N-API property-name test helper result shape.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: applying ToObject coercion across N-API object APIs.
Description check	✅ Passed	The description covers the PR purpose and includes verification details, matching the required template content.
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.

---

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

[robobun]

^{Updated 2:05 AM PT - Jul 14th, 2026}

@robobun, your commit 7d9d546 is building: #72827

[claude]

I didn't find any bugs, but this is a behavioral change across seven N-API entry points (ToObject coercion, pending-exception semantics, enum validation, and the new napi_key_keep_numbers rewrite loop) in native C++/Rust — worth a human look.

What was reviewed:

• NAPI_CHECK_TO_OBJECT macro: exception left pending via RETURN_IF_EXCEPTION on the preamble throw scope, matching Node's contract.
• napi_delete_element: delete now runs unconditionally (was previously skipped when result == NULL); exception check added after deletePropertyByIndex.
• napi_key_keep_numbers loop: asString(key)->value() has NAPI_RETURN_IF_EXCEPTION; parseIndex bounds match V8's array-index range.
• Rust napi_get_prototype: to_object result is discarded only for the throw side effect; get_prototype on primitives synthesizes the wrapper prototype without allocation.

Extended reasoning...

Overview

This PR aligns seven N-API functions (napi_set_element, napi_has_element, napi_get_element, napi_delete_element, napi_get_property_names, napi_get_all_property_names, napi_get_prototype) with Node's CHECK_TO_OBJECT contract: primitives are coerced via ToObject and succeed, while null/undefined return napi_object_expected with a pending TypeError. It also adds enum validation to napi_get_all_property_names, implements napi_key_keep_numbers (previously ignored), and fixes napi_delete_element to actually perform the delete when result == NULL. Changes span src/jsc/bindings/napi.cpp (~70 lines), src/runtime/napi/napi_body.rs (~7 lines), plus a new C++ standalone test and a checkSameOutput test case.

Security risks

None identified. This is Node.js compatibility work on argument coercion and status codes; no auth, crypto, or untrusted-input parsing paths are involved. The new parseIndex loop operates on JSC-generated property-name strings, not user-controlled data in a security-relevant sense.

Level of scrutiny

Medium-high. N-API is the FFI surface used by every native addon; changing status codes and whether exceptions are left pending is exactly the kind of thing portable addons feature-detect against. The changes look correct and are validated byte-for-byte against Node via checkSameOutput, but this is not a mechanical or config-level change — it introduces a new macro, a new post-processing loop over property keys, and alters error-path semantics at six C++ call sites plus one Rust site.

Other factors

• The bug hunter found no issues; one style-only candidate (comment length) was ruled out.
• Test coverage is thorough: a dedicated C++ standalone test prints status/pending for every affected call and is diffed against Node, with additional spot-check assertions so USE_SYSTEM_BUN=1 fails.
• The napi_delete_element change is a real behavioral fix (delete was silently skipped when result was null) bundled into this PR.
• The PR description notes tests were deferred to CI ("no test proof · Platform-specific test(s) that do not run on this machine"), so CI green is the actual verification signal.

Given the scope (native code, seven public API entry points, new exception-handling macro, behavioral semantics change), this exceeds the bar for auto-approval and should get a human review even though no defects were found.

[Jarred-Sumner]

is this correct? this looks wrong?

[robobun]

Yes, this is correct. The previous !object.is_object() gate rejected every primitive with napi_object_expected and no pending exception, which is the bug: Node coerces primitives via ToObject and only fails on null/undefined.

object.get_prototype(...) calls JSC__JSValue__getPrototype which is JSValue::getPrototype (JSObjectInlines.h:83):

ALWAYS_INLINE JSValue JSValue::getPrototype(JSGlobalObject* globalObject) const
{
    if (isObject())
        return asObject(asCell())->getPrototype(globalObject);
    return synthesizePrototype(globalObject);
}

synthesizePrototype returns numberPrototype() for numbers, stringPrototype() for strings, etc., without allocating a wrapper object, and only throws on null/undefined. So letting primitives fall through to get_prototype produces exactly what Node's CHECK_TO_OBJECT + GetPrototype() does, without the intermediate Number/String wrapper allocation.

The is_undefined_or_null() branch handles the remaining case: to_object() throws the TypeError (left pending, matching Node) and we return napi_object_expected.

The new test confirms byte-for-byte parity with Node:

get_prototype(number): status=0 pending=0
get_prototype(number) is Number.prototype=1
get_prototype(string): status=0 pending=0
get_prototype(bool): status=0 pending=0
get_prototype(null): status=2 pending=1
get_prototype(undefined): status=2 pending=1

[coderabbitai]

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 `@src/runtime/napi/napi_body.rs`:
- Around line 876-882: Update the prototype lookup flow around the visible
null/undefined handling and get_prototype call to first check whether env has a
stashed pending exception, returning napi_pending_exception immediately when
present. Perform this guard before to_object or get_prototype so prior
napi_throw* errors are preserved and cannot reach proxy traps or be replaced
with napi_object_expected.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7283eafb-16c3-453f-aa4f-ff7923a73c7d

📥 Commits

Reviewing files that changed from the base of the PR and between 644b0b6 and 376dd51.

📒 Files selected for processing (4)

• src/jsc/bindings/napi.cpp
• src/runtime/napi/napi_body.rs
• test/napi/napi-app/standalone_tests.cpp
• test/napi/napi.test.ts

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/jsc/bindings/napi.cpp:1830-1839 — The new enum checks for key_mode/key_conversion run before NAPI_CHECK_TO_OBJECT, but Node's napi_get_all_property_names runs CHECK_TO_OBJECT first and validates the enums after. So napi_get_all_property_names(env, jsnull, (napi_key_collection_mode)99, ...) returns status=1 pending=0 in Bun vs status=2 pending=1 in Node. Trivial reorder — move the two NAPI_RETURN_EARLY_IF_FALSE checks below NAPI_CHECK_TO_OBJECT — and the test doesn't exercise this combination so checkSameOutput currently misses it.

  Extended reasoning...

  What the issue is

  Node's napi_get_all_property_names (js_native_api_v8.cc, ~lines 1085-1136) validates arguments in this order:

  NAPI_PREAMBLE(env);
  CHECK_ARG(env, result);
  CHECK_TO_OBJECT(env, context, obj, object);   // ← ToObject FIRST
  // ... filter setup ...
  switch (key_mode) { ... default: return napi_set_last_error(env, napi_invalid_arg); }
  switch (key_conversion) { ... default: return napi_set_last_error(env, napi_invalid_arg); }

  This PR places the two new NAPI_RETURN_EARLY_IF_FALSE enum checks at lines 1830-1835, before NAPI_CHECK_TO_OBJECT at line 1839. So the enum validation runs first and short-circuits before ToObject ever sees the target.

  Code path that triggers it

  napi_value r;
  napi_status s = napi_get_all_property_names(
      env, v_null,
      (napi_key_collection_mode)99,   // out-of-range
      napi_key_all_properties,
      napi_key_numbers_to_strings,
      &r);

  • Node: CHECK_TO_OBJECT runs first → ToObject(null) throws → returns napi_object_expected (2) with a pending TypeError.
  • Bun (this PR): NAPI_RETURN_EARLY_IF_FALSE(env, key_mode == ... || ..., napi_invalid_arg) fires first → returns napi_invalid_arg (1) with no pending exception.

  Why the test doesn't catch it

  test_napi_object_coercion exercises the two failure modes independently:

  • get_all_property_names(key_mode=99) uses a valid obj
  • get_all_property_names(null) uses valid enum arguments

  The (null object + bad enum) combination is never sent through checkSameOutput, so byte-for-byte parity is never asserted for it.

  Impact

  Minimal — the trigger requires an addon to pass both null/undefined and an out-of-range C enum constant simultaneously. Enum arguments are compile-time constants in real addons, so this is a double-programmer-error scenario with essentially zero real-world likelihood. It's flagged only because it's exactly the class of status-code-ordering divergence this PR set out to eliminate, and the fix is a two-line reorder.

  Fix

  Move the two enum checks after NAPI_CHECK_TO_OBJECT, matching Node's order:

  NAPI_PREAMBLE(env);
  NAPI_CHECK_ARG(env, result);
  NAPI_CHECK_ARG(env, objectNapi);
  
  auto globalObject = toJS(env);
  auto objectValue = toJS(objectNapi);
  NAPI_CHECK_TO_OBJECT(env, globalObject, object, objectValue);
  
  NAPI_RETURN_EARLY_IF_FALSE(env,
      key_mode == napi_key_include_prototypes || key_mode == napi_key_own_only,
      napi_invalid_arg);
  NAPI_RETURN_EARLY_IF_FALSE(env,
      key_conversion == napi_key_keep_numbers || key_conversion == napi_key_numbers_to_strings,
      napi_invalid_arg);

  Optionally add one line to the fixture so checkSameOutput locks it in:

  report_status(env, "get_all_property_names(null,key_mode=99)",
      napi_get_all_property_names(env, v_null,
          static_cast<napi_key_collection_mode>(99),
          napi_key_all_properties, napi_key_numbers_to_strings, &out));

  Step-by-step proof

  1. Call napi_get_all_property_names(env, v_null, (napi_key_collection_mode)99, napi_key_all_properties, napi_key_numbers_to_strings, &out).
  2. NAPI_PREAMBLE passes (no pending exception). NAPI_CHECK_ARG(env, result) passes (&out non-null). NAPI_CHECK_ARG(env, objectNapi) passes (v_null is a non-null napi_value handle wrapping JS null).
  3. Line 1830: key_mode == napi_key_include_prototypes || key_mode == napi_key_own_only → 99 == 0 || 99 == 1 → false → return napi_set_last_error(env, napi_invalid_arg). Status = 1, pending = 0.
  4. In Node, step 3 doesn't exist yet; CHECK_TO_OBJECT runs on v_null → V8's ToObject throws TypeError: Cannot convert undefined or null to object → RETURN_STATUS_IF_FALSE(..., napi_object_expected). Status = 2, pending = 1.
  5. Observable divergence: {status: 1, pending: 0} vs {status: 2, pending: 1}.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/jsc/bindings/napi.cpp (1)

2477-2492: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a napi_delete_element(..., nullptr) test case
The existing coverage only calls napi_delete_element with a non-null bool*, so the regression where deletion was skipped when result was NULL still isn’t exercised.

🤖 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 `@src/jsc/bindings/napi.cpp` around lines 2477 - 2492, Add a test for
napi_delete_element that passes nullptr as the result argument, then verify the
targeted element is actually deleted and the call succeeds. Reuse the existing
napi_delete_element test setup and assertions, changing only the result-pointer
scenario needed to cover the null-output path.

🤖 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/napi/napi.test.ts`:
- Around line 552-585: Update the subprocess assertions in the
“napi_fatal_exception triggers uncaughtException for non-Error values” test so
stderr is checked only when exitCode is nonzero, matching the file’s existing
subprocess patterns; preserve the current stdout payload and successful
exit-code expectations.

---

Outside diff comments:
In `@src/jsc/bindings/napi.cpp`:
- Around line 2477-2492: Add a test for napi_delete_element that passes nullptr
as the result argument, then verify the targeted element is actually deleted and
the call succeeds. Reuse the existing napi_delete_element test setup and
assertions, changing only the result-pointer scenario needed to cover the
null-output path.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bcde39e2-927c-4d10-9a37-f9d80c0d74f2

📥 Commits

Reviewing files that changed from the base of the PR and between 376dd51 and 7721611.

📒 Files selected for processing (6)

• src/jsc/bindings/napi.cpp
• src/runtime/napi/napi_body.rs
• test/napi/napi-app/js_test_helpers.cpp
• test/napi/napi-app/module.js
• test/napi/napi-app/standalone_tests.cpp
• test/napi/napi.test.ts

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/jsc/bindings/napi.cpp (1)

2477-2492: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add a napi_delete_element(..., nullptr) test case
The existing coverage only calls napi_delete_element with a non-null bool*, so the regression where deletion was skipped when result was NULL still isn’t exercised.

🤖 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 `@src/jsc/bindings/napi.cpp` around lines 2477 - 2492, Add a test for
napi_delete_element that passes nullptr as the result argument, then verify the
targeted element is actually deleted and the call succeeds. Reuse the existing
napi_delete_element test setup and assertions, changing only the result-pointer
scenario needed to cover the null-output path.

🤖 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/napi/napi.test.ts`:
- Around line 552-585: Update the subprocess assertions in the
“napi_fatal_exception triggers uncaughtException for non-Error values” test so
stderr is checked only when exitCode is nonzero, matching the file’s existing
subprocess patterns; preserve the current stdout payload and successful
exit-code expectations.

---

Outside diff comments:
In `@src/jsc/bindings/napi.cpp`:
- Around line 2477-2492: Add a test for napi_delete_element that passes nullptr
as the result argument, then verify the targeted element is actually deleted and
the call succeeds. Reuse the existing napi_delete_element test setup and
assertions, changing only the result-pointer scenario needed to cover the
null-output path.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bcde39e2-927c-4d10-9a37-f9d80c0d74f2

📥 Commits

Reviewing files that changed from the base of the PR and between 376dd51 and 7721611.

📒 Files selected for processing (6)

• src/jsc/bindings/napi.cpp
• src/runtime/napi/napi_body.rs
• test/napi/napi-app/js_test_helpers.cpp
• test/napi/napi-app/module.js
• test/napi/napi-app/standalone_tests.cpp
• test/napi/napi.test.ts

🛑 Comments failed to post (1)

test/napi/napi.test.ts (1)

552-585: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Unconditional expect(stderr).toBe("") risks flakiness on ASAN/debug lanes.

This departs from the file's other subprocess-assertion patterns (gating on exitCode !== 0, or checking specific PASS/FAIL substrings). Prefer only asserting stderr when the run actually failed.

🧪 Proposed fix

       const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
-      expect(stderr).toBe("");
       expect(JSON.parse(stdout.trim())).toEqual({
         statuses: [0, 0, 0, 0],
         caught: ["addon says: something fatal", 42, { plain: "object" }, "Error: real error"],
       });
+      if (exitCode !== 0) {
+        expect(stderr).toBe("");
+      }
       expect(exitCode).toBe(0);

Based on learnings, "avoid unconditionally asserting expect(stderr).toBe(""). In ASAN/debug builds, benign warnings may be emitted to stderr" and the established convention to assert stderr only when exitCode !== 0.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
      expect(JSON.parse(stdout.trim())).toEqual({
        statuses: [0, 0, 0, 0],
        caught: ["addon says: something fatal", 42, { plain: "object" }, "Error: real error"],
      });
      if (exitCode !== 0) {
        expect(stderr).toBe("");
      }
      expect(exitCode).toBe(0);

🤖 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/napi/napi.test.ts` around lines 552 - 585, Update the subprocess
assertions in the “napi_fatal_exception triggers uncaughtException for non-Error
values” test so stderr is checked only when exitCode is nonzero, matching the
file’s existing subprocess patterns; preserve the current stdout payload and
successful exit-code expectations.

Source: Learnings

[robobun]

Re the CodeRabbit out-of-line findings:

• napi_delete_element(..., nullptr) coverage: added in 7d9d546 (system bun skips the delete, Node and this PR perform it; has[0]=0 vs has[0]=1).
• test/napi/napi.test.ts:552-585 (napi_fatal_exception stderr assertion): that test is from napi: accept non-Error values in napi_fatal_exception #34127 on main and is not touched by this PR; leaving it for a separate change.
• The duplicate get_all_property_names helper fix here overlaps with test(napi): remove duplicate get_all_property_names helper #34141; whichever lands first wins, the other becomes a no-op.

[robobun]

CI on 7d9d546 (build 72827): test/napi/napi.test.ts and the node-napi-tests suites pass on every lane. The one [new] failure is test/js/node/test/parallel/test-net-connect-memleak.js on alpine x64 (a net.connect FinalizationRegistry timing check, untouched by this diff and already being fixed in a separate session); the remaining lanes are known flaky tests (dev-and-prod, express, cron, install etc.).

Diff is ready for review.