test(opencode): avoid Windows Node exit teardown in built harnesses

[Astro-Han]

Summary

Updates the built Node test harnesses that were still tripping Windows Node 24 teardown assertions after #494:

• built-node-skill-bootstrap requests /agent and /command with node:http instead of Node global fetch, while preserving the same built artifact bootstrap and endpoint assertions.
• built-node-webfetch still exercises the built WebFetch tool and parser path, but closes the test-local HTTP server connections explicitly before child shutdown.
• built-node-webfetch injects a test-local node:http/node:https implementation for Effect's FetchHttpClient.Fetch reference, so the test does not trip the Windows Node 24 global fetch teardown abort after the WebFetch assertions have already succeeded.

Why

Follow-up to #494 and #492.

After #494 merged, dev still failed the advisory Windows run:

• Workflow: windows-advisory, run 25496202877
• Job: unit-windows-opencode-server-tools, job 74816526956
• Failing test: test/server/built-node-skill-bootstrap.test.ts
• Observed result: child stdout showed the endpoint requests succeeded, but the child exited with code 9 after process.exit(exitCode)
• Windows stderr: Assertion failed: !(handle->flags & UV_HANDLE_CLOSING), file src\\win\\async.c, line 76

The first PR version fixed that bootstrap harness. A manual Windows advisory run on PR head 580534b then proved the bootstrap test passed, but exposed the same teardown class in built-node-webfetch:

• Workflow: windows-advisory, run 25498482049
• Job: unit-windows-opencode-server-tools, job 74824553484
• built-node-skill-bootstrap: passed
• built-node-webfetch: produced the expected JSON output, then failed at process.exit(0) with the same src\\win\\async.c assertion

Closing the test-local HTTP server sockets was necessary but not sufficient. A second manual Windows advisory run on bdac89a still failed built-node-webfetch with correct stdout and the same Node 24 global fetch teardown assertion. The final fix keeps the built WebFetch tool coverage but removes Node global fetch from this harness.

I also tested replacing process.exit() with process.exitCode. It avoids the explicit exit, but locally leaves the built Node child alive until the test times out, so it is not a usable fix for either harness.

Related Issue

Fixes #492

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Please focus on the harness boundary. This PR intentionally avoids changing product server shutdown behavior again. It keeps the built artifact coverage intact and only makes the test child processes stop tripping the Windows Node 24 exit bug after their assertions have already succeeded.

Risk Notes

Low product risk because this changes only tests. The main risk is test fidelity:

• built-node-skill-bootstrap no longer uses Node global fetch; that is intentional because this test is about built server bootstrap and endpoint availability, not WebFetch.
• built-node-webfetch still uses the built WebFetch tool and parser path; the only transport substitution is the test-local FetchHttpClient.Fetch implementation to avoid the Windows Node 24 global fetch teardown bug.
• built-node-webfetch still verifies the hostile HTML, raw <, whitespace, and extracted text assertions against the built artifact.

How To Verify

Focused bootstrap test after review cleanup: 1 passed
Command: bun --cwd packages/opencode test test/server/built-node-skill-bootstrap.test.ts --timeout 60000

Focused webfetch test: 1 passed
Command: bun --cwd packages/opencode test test/server/built-node-webfetch.test.ts --timeout 60000

Related built Node / adapter tests: 3 passed
Command: bun --cwd packages/opencode test test/server/built-node-skill-bootstrap.test.ts test/server/built-node-webfetch.test.ts test/server/adapter-node-shutdown.test.ts --timeout 60000

Server-tools shard local equivalent: 98 passed
Command: bun --cwd packages/opencode test --timeout 30000 test/server

Typecheck: ok
Command: bun --cwd packages/opencode typecheck

PR checks on 86d3559: all passed
Result: ci, codeql, desktop-smoke, e2e-artifacts, dependency-review, commit-lint, pr-title-lint, CodeRabbit all successful

Windows advisory manual run on 86d3559: #492 target shard passed
Run: 25501384611
Job: unit-windows-opencode-server-tools / 74835020276
Result: success

Windows advisory manual run on 86d3559: full matrix still has an unrelated setup failure
Run: 25501384611
Job: unit-windows-app / 74835020009
Result: failed in oven-sh/setup-bun before bun install or unit tests. No app unit test result was produced. Other Windows advisory jobs, including unit-windows-opencode-session and unit-windows-opencode-server-tools, passed.

Screenshots or Recordings

Not applicable. No visible UI changes.

Checklist

• Human review status is stated above as pending, approved, or not required
• I linked the related issue, or stated why there is no issue
• This PR has type, primary area, and priority labels, or I requested maintainer labeling
• I described the review focus and any meaningful risks
• I listed the relevant verification steps and the key result for each
• I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
• I manually checked visible UI or copy changes when needed, with screenshots or recordings
• I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
• I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
• I reviewed the final diff for unrelated changes and suspicious dependency changes
• I am targeting dev, and my PR title and commit messages use Conventional Commits in English

[coderabbitai]

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 38 minutes and 16 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: db3de8d3-e198-42b2-863d-489c7eb06461

📥 Commits

Reviewing files that changed from the base of the PR and between 86d3559 and bc2434f.

📒 Files selected for processing (1)

• packages/opencode/test/server/built-node-webfetch.test.ts

📝 Walkthrough

Walkthrough

Two test files are refactored to replace fetch-based HTTP clients with native Node http.request/https streaming and add explicit socket tracking during shutdown. These changes address Windows Node 24 server teardown aborts by eliminating fetch dependency and destroying leftover connections before process exit.

Changes

Node Server Teardown Stability

Layer / File(s)	Summary
Bootstrap Test HTTP Client packages/opencode/test/server/built-node-skill-bootstrap.test.ts	Embedded script imports node:http and replaces fetch with a Promise-based http.request implementation that buffers response chunks, returns { status, body }, and includes explicit error handling for request and response events.
Webfetch Test HTTP Client packages/opencode/test/server/built-node-webfetch.test.ts	New nodeFetch function converts node:http and node:https request/response streams into standard Response objects with status, headers, and buffered body, decoupling the Effect runtime from Bun's fetch environment.
Server Socket Lifecycle Management packages/opencode/test/server/built-node-skill-bootstrap.test.ts, packages/opencode/test/server/built-node-webfetch.test.ts	Both embedded servers now track open sockets on connection, remove them on close, and explicitly destroy all tracked sockets during shutdown before finalizing process exit.
Effect Runtime Binding packages/opencode/test/server/built-node-webfetch.test.ts	Effect runtime tool execution is provided with FetchHttpClient.Fetch bound to the new nodeFetch implementation, replacing reliance on Bun's fetch.
Test Assertion Formatting packages/opencode/test/server/built-node-skill-bootstrap.test.ts	Line position adjustments around the snapshot fixture assertion due to embedded script size changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A hop through Node's teardown dance,
No more fetch, just streams and chance,
Sockets tracked, then gently closed,
Windows Node now well-composed!
Exit clean, no races won,
Bootstrap tests are finally done! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: updating test harnesses to avoid a Windows Node exit teardown issue.
Linked Issues check	✅ Passed	The PR successfully addresses issue #492 by removing global fetch usage and explicitly managing HTTP connections to prevent the Windows Node 24 teardown assertion, while preserving test coverage and built artifact assertions.
Out of Scope Changes check	✅ Passed	All changes are limited to test harnesses (built-node-skill-bootstrap and built-node-webfetch) to resolve the Windows teardown issue; no product code or unrelated refactors are introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description is comprehensive and follows the template structure, including summary, why, related issue, review focus, risk notes, verification steps, and checklist completion.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/i492-windows-node-exit

---

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 and usage tips.}

[gemini-code-assist]

Code Review

This pull request replaces the fetch API with node:http's request in a test script to prevent issues on Windows Node 24 where fetch can abort during process termination. The reviewer recommended removing the redundant readEndpoint wrapper and calling the request function directly to streamline the code.

[coderabbitai]

🧹 Nitpick comments (1)

packages/opencode/test/server/built-node-webfetch.test.ts (1)

89-128: 💤 Low value

init.headers as a Headers instance would be silently dropped.

On Line 92, { ...(init.headers ?? {}) } uses object spread, which only copies own enumerable properties. A Headers object exposes its entries via the iterator protocol, not own properties, so any caller passing init.headers as a Headers instance (allowed by the standard fetch RequestInit shape) would have those headers silently dropped. The input instanceof Request branch already handles this via Object.fromEntries(input.headers); consider the same treatment for init.headers.

The Effect FetchHttpClient path may not exercise this today, but the harness is presented as a fetch polyfill, so making it tolerant of all standard inputs avoids a future-test foot-gun.

♻️ Proposed fix

-          const headers = { ...(input instanceof Request ? Object.fromEntries(input.headers) : {}), ...(init.headers ?? {}) }
+          const toRecord = (h) =>
+            h instanceof Headers
+              ? Object.fromEntries(h)
+              : Array.isArray(h)
+                ? Object.fromEntries(h)
+                : (h ?? {})
+          const headers = {
+            ...(input instanceof Request ? Object.fromEntries(input.headers) : {}),
+            ...toRecord(init.headers),
+          }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/test/server/built-node-webfetch.test.ts` around lines 89 -
128, The nodeFetch polyfill currently drops Headers instances because headers
are built with { ...(init.headers ?? {}) }; update the headers merge in the
nodeFetch function so init.headers is normalized like the Request branch (e.g.,
convert Headers or iterable headers into a plain object via Object.fromEntries
when init.headers is a Headers or iterable, otherwise use init.headers as-is),
then merge with the input headers and set connection: "close"; modify the header
construction around the headers = { ...(input instanceof Request ?
Object.fromEntries(input.headers) : {}), ...(init.headers ?? {}) } expression to
first detect and convert Headers/iterable init.headers (or arrays of pairs) into
an object before spreading.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/opencode/test/server/built-node-webfetch.test.ts`:
- Around line 89-128: The nodeFetch polyfill currently drops Headers instances
because headers are built with { ...(init.headers ?? {}) }; update the headers
merge in the nodeFetch function so init.headers is normalized like the Request
branch (e.g., convert Headers or iterable headers into a plain object via
Object.fromEntries when init.headers is a Headers or iterable, otherwise use
init.headers as-is), then merge with the input headers and set connection:
"close"; modify the header construction around the headers = { ...(input
instanceof Request ? Object.fromEntries(input.headers) : {}), ...(init.headers
?? {}) } expression to first detect and convert Headers/iterable init.headers
(or arrays of pairs) into an object before spreading.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d7e21fd2-a5d9-45f0-be04-e2edb8900ac4

📥 Commits

Reviewing files that changed from the base of the PR and between 95826de and 86d3559.

📒 Files selected for processing (2)

• packages/opencode/test/server/built-node-skill-bootstrap.test.ts
• packages/opencode/test/server/built-node-webfetch.test.ts