fix: don't materialize a callable leaf's built-in members as child endpoints - #307
Merged
Conversation
…dpoints The wrapper get trap resolved a property read to `impl[prop]` and, for any function-valued result, wrapped it as a child endpoint and registered ownership. On a callable leaf, that meant merely READING an inherited Function.prototype member — `apply`, `call`, `bind`, `constructor` — (or the non-enumerable own `prototype` slot) rewrote the leaf's loader record: it flipped from kind "function" to "namespace" and grew a phantom child (`leaf.apply`, …). A later leaves() or re-composition then surfaced Function.prototype.apply in place of the leaf. `leaf.apply(thisArg, args)` — a common forwarding idiom — silently corrupted the served surface (Reflect.apply was the only safe workaround). The get trap now returns the function's own built-in members directly instead of wrapping them: for a function impl, a property that is not a user-added ENUMERABLE own property is returned as-is. Genuine enumerable own children of a callable still materialize as endpoints. Covered in eager and lazy modes, including a callable that carries a real user child. Fixes #304
There was a problem hiding this comment.
Pull request overview
This PR fixes a UnifiedWrapper get-trap behavior where reading built-in callable members (e.g. apply/call/bind/constructor/prototype) on a callable leaf could incorrectly materialize them as child endpoints, mutating ownership records (turning a function leaf into a namespace) and creating “phantom” children.
Changes:
- Adds a get-trap guard to return non-enumerable callable members directly instead of wrapping/materializing them as child endpoints.
- Introduces a new Vitest regression suite covering eager and lazy modes, ensuring built-in callable members don’t mutate leaf records while real enumerable children still materialize.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lib/handlers/unified-wrapper.mjs | Adds a guard in the get trap to avoid wrapping callable built-in members as child endpoints. |
| tests/vitests/suites/unified-wrapper/leaf-function-prototype-read.test.vitest.mjs | Adds regression coverage ensuring Function.prototype member reads don’t create phantom children and don’t mutate leaf kind/records. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…port Address Copilot review on #307: add an afterAll that removes the per-run temp fixture dir under tmp/ (it was left behind on every run), and drop the unused `dirname` import.
Shinrai
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 What's Changed
💥 Breaking Changes
No breaking changes
✨ Features
No new features
🐛 Bug Fixes
📦 Dependencies
No dependency updates
🔧 Other Changes
👥 Contributors