test: close the handles and children Windows teardown trips over

[bompus]

The suite fails 23 tests on Windows and none on Linux or macOS. Every failure is a teardown removing a temp tree while something still holds it. Windows refuses to delete a file with an open handle, and refuses to delete a live process's working directory; POSIX unlinks both regardless, which is why CI has never shown any of this.

This is test-only. No file outside __tests__/ is touched, so the product code is byte-identical to main.

Three causes, each fixed at its own site

Cause	Fix	Files
A database handle open across fs.rmSync	close it in afterEach	arkts-resolution, frameworks-integration, resolution
A spawned child still owning its cwd	kill() only asks — await the exit event before removing	mcp-initialize, mcp-roots, mcp-subproject-adoption
waitForMarker returning on file existence	wait for content, not existence	cli-ui-command

The third is worth spelling out: shell redirection creates the marker file before the command writes to it, so the test could read an empty marker and fail on a comparison against nothing.

Where retrying is the honest tool

__tests__/rm-temp.ts adds a bounded retry, used only where the holder is genuinely unreachable. serve --mcp can start a detached daemon, which is not a child of the test process: there is no handle to close and no exit event to await, only the OS releasing its files once the process is gone. Everything with a reachable holder is closed or awaited at its own site instead.

mcp-daemon.test.ts additionally reaps by pid and requires consecutive quiet passes. A single snapshot of the daemon log is not enough — a second daemon can bind after the snapshot is taken and then live until its idle timeout.

watcher.test.ts uses the same retry for the same reason: fs.watch's directory handle is released a beat after unwatch() returns.

Two of these are pre-existing flakes, not Windows-only failures

waitForMarker's existence check and watcher.test.ts's synchronous teardown are unchanged from main. Both can lose their race on any platform under load; Windows just loses it reliably.

Browser suppression coverage

The CODEGRAPH_BROWSER=none CLI test now waits for the suppression message and rejects the launch message. Its former one-second wait checked a marker written only by a custom browser stub, but none replaces that stub path, so the marker did not establish suppression. A direct openBrowser() test intercepts spawn and requires zero calls for Windows, macOS, and Linux platform arguments. The 1.5-second --no-open observation and positive browser-launch integration test remain.

Validated on Windows with Node 24.16 against this PR's prior head, 263168d, plus the two-file follow-up: full build passed, focused UI server and CLI suites passed (53 passed, 3 skipped), and diff checks passed. Independent review found no issues. Mutation validation on the consolidated fork made the new assertion fail on three intercepted platform launch attempts when the environment setting was ignored; it passed after restoration.

This removes a fixed one-second wait in the successful suppression case; no suite-wide speedup is claimed. The full suite was not rerun for this follow-up.

Earlier full-suite verification

Windows 11, Node 26.8.1: 23 failures before, 0 after, across 10 consecutive full runs of the suite — 4,194 passed, 46 skipped, 0 failed, on a frozen tree.

The retry helper is load-bearing but is not hiding a slow path: across those 10 runs it retried 25 times, and every retry succeeded on its first attempt. Nothing legitimate needed a second one.

I also measured a daemon double-bind explicitly rather than inferring it from failed teardowns — 3 concurrent launchers × 30 trials produced exactly one bind every time and no double-bind. Earlier sightings of a second daemon all came from teardowns inspected because they had already failed, so that sample was conditioned on the outcome. There is no product-bug claim in this PR.

What I did not measure

I have no clean cross-runtime comparison. An attempt to baseline Node 24 alongside Node 26 turned out to be measuring Windows Defender rather than the runtime: on this host, Defender scans file I/O from binaries outside C:\Program Files, and the vendored Node 24 was in a scratch directory while Node 26 was installed. Both arms have to be in the same trust class for that comparison to mean anything, so I am reporting only the Node 26 numbers above, which come from the installed binary.

One test, #1557, is sensitive to the same effect — it writes 'const value = 1;\n'.repeat(70_000), which is close to a worst case for the scanner. I earlier wrote here that it "still times out on this host", and that was wrong: it times out only when the suite is driven by a Node binary in a user-writable directory, which is how I first ran it. Under the installed Node it passes.

Measured directly, since the first claim was not. Same test, byte-identical node.exe (cmp clean), interleaved across three rounds:

Round	C:\Program Files\nodejs\node.exe	the same binary copied to a scratch directory
1	201 ms, passes	12,948 ms, times out
2	196 ms, passes	times out
3	204 ms, passes	times out

So it is environmental and outside this PR's scope, and it is not a runtime or codegraph defect — but the honest statement is that it passes on this host, and the location of the interpreter is what decides it.

[bompus]

Pushed 263168d — two review fixes, plus one retraction that matters more than either.

Retracting a defect I reported against this branch. I earlier said mcp-roots.test.ts leaks a detached daemon that its teardown cannot reach, and that I had confirmed it empirically. That was wrong, and the way it was wrong is worth stating.

The reasoning was: daemon mode is opt-out, and mcp-roots alone among the three MCP suites sets no CODEGRAPH_NO_DAEMON. My probe then reproduced it — a surviving pid, a 2s retry failing with EPERM, removal succeeding only after a kill. But the probe ran init and index in the spawn cwd first, and mcp-roots never does: line 84's own comment says cwdDir "has NO .codegraph", and all three spawns use cwdDir, not projectDir. A root known at initialize takes the daemon-attach path; a root resolved later via roots/list does not. I copied spawnServer's arguments and not its fixture, and the fixture was the entire discriminator.

The probe had a positive control and still misled, because the control proved the detector could see a daemon — not that the arm under test matched the suite. A control that validates the detector is not a control that validates the setup. No change to mcp-roots in this push; it is unmodified from main.

resolution.test.ts — the db handle. db.close() sat in the try in all four hunks, after the query and the assertions, while the finally's own comment already said both holders have to go. A throw from .all() or from an expect() left the direct handle open, and the plain non-retrying rmSync below then failed with EPERM — so a genuine assertion failure came back dressed as a Windows permission error. let db is hoisted above each try and closed in the finally ahead of cg?.close().

Confirmed by reintroducing it rather than by the passing run: removing the inline close from all four hunks and restoring it to only two made exactly those two tests fail with EPERM at the rmSync line.

mcp-daemon.test.ts — reapDaemons could not report its own failure. It returned Promise<void>, so it returned identically whether the root went quiet or whether it burned all 200 passes with a daemon still alive. Those mean opposite things and the caller read the second as the first. It now returns whether it actually went quiet, and afterEach throws — after rmTempDir, so a failed reap still cleans up what it can and surfaces as itself rather than as the EPERM it would cause on the next test.

Verified the assertion can fire: with the pass budget starved to 0, all 10 tests fail with the new message. The void version passed that same run.

Still a follow-up, deliberately not in this PR. waitForMarker now waits for content rather than existence, which means a caller asserting that no launch happened burns its full timeout instead of returning early. That is correct but slower, and if any such assertion sits near the 5s limit it wants either a comment or a shorter explicit timeout. It is a timing question about existing callers, not part of the teardown fix, so I would rather it land separately than widen a test-only PR.

202/202 across mcp-daemon, resolution and mcp-roots; tsc clean.

[bompus]

@colbymchenry could you review this Windows teardown fix at 263168d?

GitHub currently reports it as conflict-free and awaiting one approving review. There are no reviews or checks listed.

This head is already included in our local consolidated fork, where the combined JavaScript/Rust validation passed with 4,284 tests passing, 44 skipped, and no failures. That is combined-fork validation, not a new standalone run of this PR.

The existing discussion records the failure-path cleanup checks and the correction to the earlier daemon-leak report. The negative-marker timeout speed improvement remains an optional follow-up. #1723 is closed as superseded by this PR.