fix(buffer): distinguish Uint8Array in Buffer.isBuffer

[proggeramlug]

Summary

• add a Node Buffer brand predicate that excludes constructor-created Uint8Array values
• preserve the broader shared-storage predicate used by native APIs that accept Buffer or Uint8Array
• cover direct and first-class Buffer.isBuffer calls

Fixes #9179

Testing

• RUST_TEST_THREADS=1 cargo test -p perry-runtime buffer_brand_tests -- --nocapture
• cargo test -p perry --test issue_9179_buffer_isbuffer_uint8array -- --nocapture

Summary by CodeRabbit

• Bug Fixes

  • Buffer.isBuffer() now correctly returns false for plain Uint8Array instances and objects, matching Node.js behavior.
  • Buffer-related APIs continue accepting supported Uint8Array values where applicable.

• Tests

  • Added regression coverage for direct and stored-function calls to Buffer.isBuffer().

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c33f319-40c5-4530-a038-62208276c71c

📥 Commits

Reviewing files that changed from the base of the PR and between 562b482 and ce754a2.

📒 Files selected for processing (7)

• changelog.d/9205-buffer-isbuffer-uint8array-brand.md
• crates/perry-codegen/src/expr/array_methods.rs
• crates/perry-codegen/src/runtime_decls/stdlib_ffi/third_party.rs
• crates/perry-runtime/src/buffer/mod.rs
• crates/perry-runtime/src/buffer/query.rs
• crates/perry-runtime/src/object/native_module_dispatch/dispatch_a_c.rs
• crates/perry/tests/issue_9179_buffer_isbuffer_uint8array.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.

---

📝 Walkthrough

Walkthrough

Buffer.isBuffer() now rejects plain Uint8Array values. The runtime adds a Node-specific predicate that excludes constructor-created Uint8Arrays, while native APIs retain the broader shared-storage predicate. Code generation, dispatch, and regression tests use the new predicate.

Changes

Buffer identity predicate

Layer / File(s)	Summary
Runtime Buffer identity predicate crates/perry-runtime/src/buffer/query.rs, crates/perry-runtime/src/buffer/mod.rs	The runtime adds js_buffer_is_node_buffer, documents the shared storage predicate, re-exports the new function, and tests Buffer versus Uint8Array identity.
Buffer.isBuffer wiring and regression coverage crates/perry-codegen/src/runtime_decls/stdlib_ffi/third_party.rs, crates/perry-codegen/src/expr/array_methods.rs, crates/perry-runtime/src/object/native_module_dispatch/dispatch_a_c.rs, crates/perry/tests/issue_9179_buffer_isbuffer_uint8array.rs, changelog.d/9205-buffer-isbuffer-uint8array-brand.md	Code generation and native dispatch call the Node-specific predicate. Regression tests cover direct and stored-function calls and assert true false false output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to ce754

Buffer.isBuffer now distinguishes constructor-created Uint8Array values from Node Buffers without changing broader APIs that accept either representation. No actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant main_ts
  participant BufferIsBuffer
  participant nm_dispatch_buffer
  participant js_buffer_is_node_buffer
  main_ts->>BufferIsBuffer: call Buffer.isBuffer directly or through predicate
  BufferIsBuffer->>nm_dispatch_buffer: dispatch Buffer.isBuffer
  nm_dispatch_buffer->>js_buffer_is_node_buffer: pass NaN-boxed value as I64
  js_buffer_is_node_buffer-->>nm_dispatch_buffer: return I32 identity result

Loading

Suggested reviewers: thehypnoo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary change: distinguishing constructor-created Uint8Array values from Buffer values in Buffer.isBuffer.
Description check	✅ Passed	The description explains the behavior change, implementation intent, linked issue, and test commands. It does not reproduce the template's separate Changes, Related issue, Test plan, and Checklist sec…
Linked Issues check	✅ Passed	The PR satisfies issue #9179. It makes Buffer.isBuffer return false for plain Uint8Array and unrelated objects, keeps true for Buffer instances, uses the existing Uint8Array constructor discriminator,…
Out of Scope Changes check	✅ Passed	The changes are limited to the Buffer.isBuffer brand predicate, related runtime and FFI wiring, documentation, and regression tests required by issue #9179. No unrelated code changes are identified.

Full details: Description check

Explanation

The description explains the behavior change, implementation intent, linked issue, and test commands. It does not reproduce the template's separate Changes, Related issue, Test plan, and Checklist sections, but it contains the required information and is mostly complete.

Full details: Linked Issues check

Explanation

The PR satisfies issue #9179. It makes Buffer.isBuffer return false for plain Uint8Array and unrelated objects, keeps true for Buffer instances, uses the existing Uint8Array constructor discriminator, preserves the broader shared-storage predicate, and adds direct and first-class function regression tests.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[proggeramlug]

Merged. This closes #9179, which I filed while auditing #9176, and it takes the approach the issue argued for rather than the tempting shortcut.

The tempting version is to narrow js_buffer_is_buffer itself. That would have been wrong: native APIs legitimately accept either representation, and Perry deliberately backs both Buffer and constructor-created Uint8Array with BufferHeader in one registry — so the storage question and the identity question are genuinely different questions that happened to share a predicate. Splitting them, keeping js_buffer_is_buffer as the broad storage test and adding js_buffer_is_node_buffer for the public brand check, is the version that doesn't break the callers that wanted the old meaning. The doc comments now say which is which, which is the part that keeps this from being re-collapsed later.

Nice that the discriminator reuses is_uint8array_buffer — the same probe #9176 and #9177 just made cheap, so the brand check costs a range compare in the common case rather than a hash.

Validation: the exact reproducer from #9179 now prints true false false, matching node 26.5.1, and the rest of that probe (hex/base64 round trips, concat, compare, DataView aliasing over a shared ArrayBuffer, element access, JSON.stringify(Array.from(u8))) still matches byte-for-byte. perry-runtime suite green at RUST_TEST_THREADS=1.

Heads-up on a collision: #9213 ("fix(runtime): distinguish Buffer from Uint8Array", closing #9173) is yours too and overlaps this heavily — same array_methods.rs, third_party.rs, buffer/mod.rs, buffer/query.rs, dispatch_a_c.rs — while also touching exotic_view.rs, node_api_host/buffers.rs, global_this/populate.rs and object_ops/prototype.rs. I merged this one because it was validated and narrower; #9213 will need a rebase, and it's worth checking whether what remains of it is still the change you want or whether this one already covers part of its ground.