node:assert: compare constructors like node in deepStrictEqual, not [[Prototype]] identity

[robobun]

Problem

• assert.deepStrictEqual(Object.create({ x: 1 }), {}) throws in Bun 1.4.0. It passes in Node and in Bun 1.3.14. The error is "Values have same structure but are not reference-equal". This broke the es-sax test suite (assert.deepEqual regression in Bun v1.4.0 #40127, 13 failures).
• The cause is the [[Prototype]] identity check that node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660 added for the node:assert entry point in src/jsc/bindings/bindings.cpp. Node does not require prototype identity.

Fix

• Replace the identity check with node's rule (objectComparisonStart in lib/internal/util/comparisons.js). When val1.constructor is inherited, or is a well-known built-in constructor, compare the two constructor values with ===. Otherwise compare the [[Prototype]]s with ===. So Object.create({ x: 1 }) equals {}, while Object.create(null) against {}, class instances against literals, and subclass instances against base instances stay unequal.
• After the constructor rule, compare the Object.prototype.toString tags like node. This replaces the old calculatedClassName comparison and also applies in skipPrototype mode.
• isWellKnownConstructor mirrors node's wellKnownConstructors set by ClassInfo: the built-in constructors, the typed arrays, DataView, and Buffer. One property-slot walk supplies both the constructor value and the own-property check.
• Verified: test/js/node/assert/deep-equal.test.ts (new matrix cases, stock bun fails the new strict checks). Also the full test/js/node/assert/ suite, the vendored node assert tests, Bun.deepEquals and expect() suites, and the reporter's es-sax suite (117 pass, was 101).

Background

• Bun has one native deep-equality function with template flags. The checkPrototypes instantiation serves only node:assert and util.isDeepStrictEqual. Bun.deepEquals and expect() use prototype-blind instantiations and do not change here.
• Node's rule falls back to prototype identity in two cases: no constructor at all (null-prototype chains), and an own constructor property whose value is not a built-in (an untrustworthy constructor).
• The expectations in the new test cases are cross-checked against node v26.3.0.

Notes

Behavior table (node v26.3.0 = bun with this fix):

case	node	bun 1.4.0
Object.create({x:1}) vs {}	equal	not equal (the regression)
Object.create({x:1}) vs Object.create({z:2})	equal	not equal (the regression)
Object.create(null) vs {}	not equal	not equal
new A() vs {}	not equal	not equal
Map subclass vs new Map()	not equal	not equal
own constructor: Object, different prototypes	equal	not equal
own constructor: fn, different prototypes	not equal	not equal

The well-known check only changes the outcome when the object has an own constructor property: an inherited defined constructor always takes the constructor-comparison branch.

The tag comparison matches node's hasUnequalTag/slowHasUnequalTag: two objects on one prototype with different own constructor functions, or a Proxy of {} and {}, are equal in node and were reported unequal by the old calculatedClassName comparison.

val1.constructor is an ordinary [[Get]] in node too, so getters and proxy traps fire the same way in both implementations.

Suites run: test/js/node/assert/ (438 pass), test-assert.js, test-assert-checktag.js, test-assert-deep-with-error.js, test-assert-typedarray-deepequal.js, test-assert-class.js (vendored node tests), test/js/bun/bun-object/deep-equals.test.ts + test/js/bun/test/expect.test.js (460 pass), es-sax at gameroman-npm/es-sax#renovate/bun-1.x (117 pass, 0 fail).

---

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/bun-object/deep-equals.test.ts

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 27 days. After that, they cost $0.25 per reviewed file.}

Or wait 21 minutes for your next included review.

View limit details

Limit details: You’ve used the included review currently available. Your 68 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7fc9d146-338e-492b-90dc-2b7022c635a4

📥 Commits

Reviewing files that changed from the base of the PR and between f2fe7d3 and 4e18b48.

📒 Files selected for processing (5)

• src/jsc/bindings/JSBuffer.cpp
• src/jsc/bindings/JSBuffer.h
• src/jsc/bindings/bindings.cpp
• test/js/bun/bun-object/deep-equals.test.ts
• test/js/node/assert/deep-equal.test.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4db9d5cc-e9c9-4da4-9f25-0ef744c11198

📥 Commits

Reviewing files that changed from the base of the PR and between 85deae0 and 17bfe85.

📒 Files selected for processing (4)

• src/jsc/bindings/JSBuffer.cpp
• src/jsc/bindings/JSBuffer.h
• src/jsc/bindings/bindings.cpp
• test/js/node/assert/deep-equal.test.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

Walkthrough

Changes

JSBuffer constructor refactor

Layer / File(s)	Summary
Declare and implement JSBufferConstructor src/jsc/bindings/JSBuffer.h, src/jsc/bindings/JSBuffer.cpp	The class now derives from InternalFunction. Its factory methods are defined out of line, and its private constructor no longer accepts JSGlobalObject*.

Node deep equality

Layer / File(s)	Summary
Classify constructors and compare prototypes src/jsc/bindings/bindings.cpp	Deep equality recognizes well-known constructors and compares constructors, prototypes, and Object.prototype.toString tags according to the comparison mode.
Apply mode-specific class checks src/jsc/bindings/bindings.cpp	Strict class-name checks and boxed-string checks now depend on prototype checking.
Cover constructor and prototype cases test/js/node/assert/deep-equal.test.ts	Tests cover inherited and own constructors, built-in and user-defined constructors, prototype differences, Map subclasses, and Proxy values.

Suggested reviewers: jarred-sumner, cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the main change: matching Node’s constructor comparison behavior in deepStrictEqual.
Description check	✅ Passed	The description explains the problem, fix, scope, behavior, and verification results, despite using different headings from the template.

---

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

[robobun]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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.

[robobun]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

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.

[claude]

I reviewed this PR and didn't find any bugs. Because it re-encodes Node's objectComparisonStart semantics in the JSC deep-equals path (new observable [[Get]]s on .constructor, a well-known-constructor set, exception-scope changes), a human look would still be worthwhile.

What was reviewed

• Exception handling: every get/getOwnPropertySlot/getPrototype/strictEqual has a RETURN_IF_EXCEPTION; isWellKnownConstructor is pure structure/pointer checks.
• Branch logic against the PR's stated Node rule — undefined → proto, inherited → ctor, own well-known → ctor, own non-well-known → proto — matches the code.
• Includes: FunctionConstructor.h and JSPromiseConstructor.h are already in the file; the new headers cover the rest.
• Ruled out: SharedArrayBuffer omission from the well-known set (verifier confirmed not a bug).

Extended reasoning...

Overview

Replaces the [[Prototype]]-identity check in the checkPrototypes instantiation of Bun__deepEquals (bindings.cpp) with Node's constructor-based rule from lib/internal/util/comparisons.js. Adds isWellKnownConstructor (17 inherits<> checks + typed-array/Buffer identity checks) and 9 new matrix cases in test/js/node/assert/deep-equal.test.ts. Scope is limited to the node:assert/util.isDeepStrictEqual entry point via the checkPrototypes && !skipPrototypeIdentity template guard — Bun.deepEquals and expect() are unchanged.

Security risks

None identified. The new code reads .constructor via ordinary [[Get]] (can trigger user getters/Proxy traps), but Node does the same and every call site has RETURN_IF_EXCEPTION. No new allocations sized from untrusted input; isWellKnownConstructor is side-effect-free apart from possibly lazy-initializing the Buffer constructor via JSBufferConstructor() (main-thread only, which this path always is).

Level of scrutiny

Medium-high. This is a user-observable semantic change to assert.deepStrictEqual that fixes a regression (#40127) but requires faithfully mirroring Node's objectComparisonStart. The C++ is in a hot, exception-sensitive JSC path. The implementation looks correct — the four-way branch (undefined/inherited/own-well-known/own-other) matches the described Node rule, and the test matrix covers each arm plus both directions of the reporter's case. But confirming the well-known set exactly matches Node's wellKnownConstructors, and that the observable side-effect ordering (Bun does [[Get]] before [[GetOwnProperty]]; Node does hasOwnProperty first) is acceptable, is worth a maintainer's eye.

Other factors

• Test coverage is strong: new cases are cross-checked against Node v26.3.0, existing null-prototype/subclass cases from #34660 are preserved, and the PR ran the vendored Node assert tests plus the reporter's es-sax suite.
• typedArrayConstructorConcurrently is used deliberately (no lazy init) with a sound justification in the comment.
• No prior human or bot reviews with substantive feedback; CodeRabbit was rate-limited.

[robobun]

On the side-effect ordering note: the order matches node. Node evaluates wellKnownConstructors.has(val1.constructor) before hasOwn(val1, "constructor"), so the [[Get]] of constructor comes first there too. The one difference is that node reads val1.constructor up to three times in that condition while this code caches the first read. That is only observable with an exotic getter that returns different values per call.

[gameroman]

That is only observable with an exotic getter that returns different values per call.

Is that not a common thing?

[robobun]

No, it is rare. For almost every object, constructor is a plain data property on the prototype. Classes, object literals, and built-ins all work that way. A read of a data property returns the same value every time, so one read and three reads give the same answer.

The difference only shows when constructor is an accessor, through Object.defineProperty(obj, "constructor", { get() {...} }) or a Proxy get trap, and the getter returns a different value on each call during one comparison. Code like that is pathological. Node itself does not guard against it either: it just happens to call the getter up to three times where this patch calls it once.

[robobun]

Thanks for the review and the follow-up commits.

[robobun]

^{Updated 2:45 AM PT - Aug 24th, 2026}

✅ @dylan-conway, your commit 4e18b48bc9c758e5bc458ce2d4e7b82382a30414 passed in Build #104792! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 40131

That installs a local version of the PR into your bun-40131 executable, so you can run:

bun-40131 --bun

[claude]

I re-reviewed after 0dddff9, fece231, and b31d7c3 — both earlier findings are addressed and this pass found no bugs. Given the change is a semantic port of node's objectComparisonStart into the C++ deep-equality path (property-slot walk, Proxy/isTaintedByOpaqueObject branch, the mono-proto structure shortcut, and the new objectPrototypeToString tag comparison), a human look at the algorithm fidelity would still be worthwhile.

Checked this run: exception propagation after every JS-entering call in the new block; both calculatedClassName guards now gate on !checkPrototypes && !skipPrototypeIdentity so Bun.deepEquals(a, b, true, true) stays prototype-blind; isWellKnownConstructor against node's set (SharedArrayBuffer is intentionally absent in node's list too); the JSBufferConstructor header move is a pure relocation.

Extended reasoning...

Overview

The PR replaces the [[Prototype]] identity check in Bun__deepEquals<…, checkPrototypes=true, …> (bindings.cpp) with node's constructor/prototype rule from lib/internal/util/comparisons.js objectComparisonStart, plus an Object.prototype.toString tag comparison. It adds isWellKnownConstructor (~30 ClassInfo checks), moves JSBufferConstructor from JSBuffer.cpp to JSBuffer.h so its info() is visible, and adds ~40 matrix cases to test/js/node/assert/deep-equal.test.ts plus a 4-arg Bun.deepEquals regression test.

Security risks

None. This is comparison-semantics logic in node:assert/util.isDeepStrictEqual. No auth, crypto, filesystem, or network surface. The new code reads .constructor via getPropertySlot (can fire getters/Proxy traps) but node does the same, and every JS-entering call has RETURN_IF_EXCEPTION.

Level of scrutiny

High. This is hand-written C++ in the JSC bindings layer implementing a subtle Node.js compat algorithm with several branches (well-known constructor vs inherited vs own, Proxy taint fallback, mono-proto structure shortcut). It affects assert.deepStrictEqual, util.isDeepStrictEqual, and every downstream test using them. The inheritedFromSharedChain optimization (same mono-proto structureID ⇒ same inherited data-slot value) is correct but non-obvious enough that a maintainer should confirm it. The checkPrototypes instantiation is also on the hot path for the vendored node test suite.

Other factors

Two prior claude[bot] findings were fixed in follow-up commits: the <true,false,false,true> guard regression (0dddff9, now covered by deep-equals.test.ts) and a duplicate test case (b31d7c3). The comment-cop bot flagged long comments which were trimmed (fece231). A maintainer (dylan-conway) is already shepherding the PR. The test matrix is comprehensive and cross-checked against node v26.3.0. Verifier agents ruled out the SharedArrayBuffer omission — node's wellKnownConstructors set does not include it either. Given the algorithm complexity and that it changes user-visible node:assert semantics, deferring rather than approving.