inspector: handle BigInt/Symbol in returnByValue instead of asserting - #36841
Open
robobun wants to merge 4 commits into
Open
inspector: handle BigInt/Symbol in returnByValue instead of asserting#36841robobun wants to merge 4 commits into
robobun wants to merge 4 commits into
Claude / Claude Code Review
completed
Aug 3, 2026 in 8m 38s
Code review found 1 potential issue
Found 2 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/cli/inspect/inspect.test.ts:330-334 |
spawnInspectee() leaks child process if URL discovery throws |
Annotations
Check warning on line 334 in test/cli/inspect/inspect.test.ts
claude / Claude Code Review
spawnInspectee() leaks child process if URL discovery throws
The hoist changed cleanup ordering: previously `metadataInspectee = spawn({...})` ran before the URL-discovery loop, so `afterEach` would kill the child even if this throw fired; now the child is a local that only escapes on success, so a throw here leaks the subprocess. Consider `child.kill()` before throwing (inspectee.js self-exits after 30s and the banner is written atomically, so this is low-probability — but it's a regression from ca97e16f and REVIEW.md asks for cleanup registered before t
Loading