[Patrick-Erichsen]

Summary

The descendant-cleanup fixture can pass its assertions yet fail the unit gate with an unhandled Node/Undici setTypeOfService EINVAL as its listener terminates. Give each HTTP probe its own socket, drain its response, and handle request and response errors through the existing promise.

-fetch(url)
+node:http.get(url, { agent: false })
+  drain response; handle request and response errors

All alive/dead, bootstrap, process-group ownership, disconnect and termination assertions remain.

Evidence

• Before: Node 24.22.1/macOS reproduced the unhandled error in 3/10 prior-main runs and 6/10 initial scanner-adoption runs. The original full unit gate failed despite all 8,268 assertions passing.
  After: 20 iterations of all three cleanup cases and a separate formatted final run pass without the error. Independent P0–P3 review found no actionable defect.
• Rebased once onto main 793dea96f26627f234ee1f305aca7d2581f91368; prerequisite head a31d58e2a96b02f23626af8f0d9a71b33cd88628 has a byte-identical fixture delta. Fresh combined-stack validation at descendant a27156f02eec6737ef0eec7fb6d03fc3528cb135: static; all 15 focused tests; full unit coverage (8,304 passed / 5 skipped, no unhandled errors); TypeScript/build; package verification — all pass..
• The original scanner-adoption browser failure and its single failed unchanged retry are retained. The new accepted base independently contains the inspector error boundary required for that failure; this fixture change does not claim to repair that browser path or an upstream Node/kernel bug.

Merge Danger

Door: two-way

Reversible test-client isolation.

Blast Radius: tests

The fixture still checks actual descendant termination.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
clawhub	Ready	Preview	Oct 28, 2026 12:26pm UTC

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR replaces Fetch probes with one-shot HTTP requests that consume responses and handle request and response errors.

Example: The cleanup fixture terminates its HTTP listener after bootstrap exits with code 7.

• Before: Fetch probes could reportedly produce an unhandled setTypeOfService EINVAL despite passing lifecycle assertions.
• After: Each probe owns its connection and routes transport errors through its promise while preserving the lifecycle assertions.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused patch is sound and preserves coverage; evidence for the intermittent platform-specific failure remains reported runs and CI rather than inspectable diagnostics.
Proof confidence	🦐 gold shrimp (3/6)	This maintainer-authored, test-only PR is outside the ordinary contributor proof gate; reported repeated fixture runs and passing exact-head CI support validation but do not independently prove the macOS/Undici cause.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Contributors can receive a failing unit gate from an unhandled transport error even when descendant-cleanup assertions pass.
Reason: The collaborator-authored direction is authorized, narrowly scoped, and preserves meaningful lifecycle coverage.

Merge readiness

✅ Ready for maintainer review

Keep open: this maintainer-authored PR contains useful test isolation work absent from current main, with no blocking findings.

Priority: P2
Reviewed head: 24fd31638ae21103772b8968ca4d8b067fc1db07

Before merge

None.

Findings

None.

Agent review details

How this fits together

The local-auth runner launches a persistent Convex process-group owner; this test substitutes a small HTTP backend and checks descendant termination after success, bootstrap failure, and IPC disconnection.

flowchart TD
  A[Lifecycle test] --> B[Persistent launcher]
  B --> C[Bootstrap fixture]
  C --> D[HTTP backend descendant]
  A --> E[One-shot HTTP probe]
  E --> D
  A --> F[Signal or IPC disconnect]
  F --> B

Loading

Technical review

Best possible solution:

Keep the transport repair inside this fixture while retaining its existing process-group owner and lifecycle assertions.

Do we have a high-confidence way to reproduce the issue?

The author reports repeated before/after runs on Node 24.22.1/macOS, corroborated by the earlier scanner-adoption report; source confirms the implicated Fetch probes, but this review did not execute tests or independently establish the intermittent cause.

Is this the best way to solve the issue?

Best-fix verdict: acceptable mitigation at the correct fixture boundary; changing production networking or suppressing unhandled errors would exceed the demonstrated problem, whereas isolated requests preserve the lifecycle contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 793dea96f266.

Provenance checked

• Descendant-cleanup HTTP probes keeps the original intent (db91415: The original PR added real descendant-liveness checks to verify cleanup after successful bootstrap, failed bootstrap, and parent disconnection.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Pinned introduction: The introduced diff changes only the test transport: one helper and four probe call sites; lifecycle cases and ownership assertions remain intact. (scripts/playwright-local-convex.test.ts:14, 24fd31638ae2)
• Current-main comparison: Current main retains Fetch at all four probe sites; the verified synthetic merge changes only this test file. (scripts/playwright-local-convex.test.ts:68, 793dea96f266)
• Original lifecycle intent: fix: disable scheduled jobs before local browser tests start #3710 established the persistent launcher and three descendant-cleanup cases; this PR preserves that contract. (scripts/playwright-local-convex.ts:8, db9141546cd4)
• Prior symptom report: The merged scanner-adoption PR independently records the same unchanged-fixture Darwin error, while explicitly leaving its cause unproven: fix: unblock publication checks for native JavaScript validators #4115. (016fea6baa71)
• Discussion and validation: The full body and comments were read; no additional diagnostic artifact or substantive review was supplied. Exact-head unit, static, types-build, package, and browser checks passed; the remaining non-success check was a cancelled auto-response job. Supplied sourceRevision: 6205097c8cb9a07b62dea6088aad15a0023e9997f05b2409bfb0e24752be595a. (24fd31638ae2)
• HTTP transport contract: The patch directly imports node:http. Node 24 HTTP documentation confirms agent:false avoids connection reuse and premature closure reports errors on the request or response, matching both installed handlers.

Likely related people:

• Ayaan Zaidi: Raw commit db91415 adds scripts/playwright-local-convex.ts:13 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: db9141546cd4; files: scripts/playwright-local-convex.ts)

Review metrics

Metric	Value	Why it matters
Patch size	Tests +21/-4; production +0/-0	One shared probe helper replaces four Fetch calls without expanding production behavior.

Labels

Label changes:

• add P2: A bounded test-transport improvement addresses a reported intermittent unit-gate failure without changing production behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P2: A bounded test-transport improvement addresses a reported intermittent unit-gate failure without changing production behavior.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 28, 2026, 8:01 AM ET / 12:01 UTC.