Stop the lost-watch harness aborting the Windows unit job on an 8.3 short temp path

[kriszyp]

Unit Test (Windows, Node.js v24) has been red on main since e966f27 (#2364): the unitTests/utility/** group exits 7 because all seven cases of unitTests/utility/watcherFallback.test.js's lost native watch guard block fail (run 33592149944). Six abort the child with Windows exit 3221226505 (0xC0000409) and Assertion failed: !_wcsnicmp(filename, dir, dirlen), file src\win\fs-event.c, line 72; the seventh exits 0 but observes zero of the two warnings it expects. Both causes are in the harness that commit added — the guard under test is not involved in either — and both are fixed here without skipping, quarantining or relaxing anything.

The abort

Node 24 bundles libuv 1.52.1, whose Windows fs-event callback rebuilds each directory event's absolute path from the directory the watch was armed on, expands it with GetLongPathNameW, and then asserts the expansion still prefix-matches that directory (deps/uv/src/win/fs-event.c:72). An 8.3 short ancestor never survives the comparison, and the assertion aborts the process rather than failing the watch — so nothing in JavaScript, the guard included, can intervene. libuv v1.x has since replaced that assert with a -1 return whose comment names this case ("which can happen if the directory is a short path"), but the Node 24 line CI pins ships the assert.

On the GitHub Actions Windows runner the user is runneradmin, so os.tmpdir() is the 8.3 spelling C:\Users\RUNNER~1\AppData\Local\Temp, and the harness armed a chokidar watch on a raw mkdtempSync(join(os.tmpdir(), …)) path. That is the invariant #2309 established for #2234 — the same assertion, same file and line, raised from production watchers — and which every production native watch already honours through resolveWatchTarget(): EntryHandler.ts:549/:559, OptionsWatcher.ts:115, RootConfigWatcher.ts:32, keys.ts:386, manageThreads.js:1538, and blob.ts:844 for the fs.watch at blob.ts:263. The fixture was the one caller that did not, so it now resolves its temporary root with realpathSync.native before anything is watched; resolving the root alone is sufficient because every segment appended below it is spelled long by construction.

The one mode that creates no watcher (warn-threshold) is also the one that did not abort, which is what pins the abort to the watch rather than to any mode's own behaviour.

The silent warn cadence

initLogSettings() (utility/logging/harper_logger.ts:463) takes the logger's destination from the ambient install. With no boot properties file it falls into its catch branch, which sets the module-level log_to_file = false; logToStdstreams = true and then calls createLogger({ level: logLevel }) — with no stdStreams, which createLogger destructures into a local that shadows the module variable, so logStdErr writes nowhere at all. The Linux unit job runs Setup Harper first (unit-test.yml:60, DEFAULTS_MODE: dev); the Windows job has no such step. That is the whole of the platform difference, and it reproduces on Linux: run the fixture with HOME pointed at an empty directory and the case sees 0 warnings; with the ambient install present it sees 2. The child now writes a harper-config.yaml into its own temporary root and points ROOTPATH at it before the logger module loads, so the cadence is asserted against configuration the harness owns. That ordering is the load-bearing part: initLogSettings() runs at import.

Which framing the evidence supported

The task left open whether the fix belonged in (a) the production guard/watcher or (b) the harness. (b). The guard cannot reach either failure — an abort() inside libuv is not observable from JavaScript, and the production watchers already canonicalize — and both defects are in test code new in #2364: one bypassing an invariant #2309 established, one depending on log routing the Windows job never configures.

For the human reviewer

• The guardedWatch() funnel does not enforce the canonicalization invariant. It is documented as the mandatory funnel for every Harper chokidar watcher, but only for the lost-watch guard; each caller still has to remember resolveWatchTarget() separately, and the commit that introduced guardedWatch() is also the one whose new caller forgot. I did not move canonicalization into the funnel: callers keep their own copy of the watch path and compare event paths against it — EntryHandler builds normalizedDirectory/normalizedBases from the path it passed and uses them in its ignored predicate — so silently rewriting the path there would turn a forgotten canonicalization from a loud abort into a watcher that ignores every event. Worth a separate decision.
• The logger's no-boot-properties branch drops every line (harper_logger.ts:543-548). That is the second half of this failure and a real bug in its own right: a Harper process with no boot properties file — harper install itself, and every unit-test process in the Windows gate — logs nothing at all, contradicting the logToStdstreams = true two lines above it. Fixing it would also have fixed this test, but it turns logging on for every such process, and that blast radius belongs in its own change; the fixture supplies its own configuration instead.
• The cadence assertion is now on the occurrences, not the count. The planning round noted that two warnings across twelve claims was the same tally whether they landed at 1 and 10 or at 2 and 11, so it asserts the occurrence numbers are exactly [1, 10].
• Declined nit from the pre-push review, raised in both rounds: that // 12 claims => warnings at occurrence 1 and 10, and no others. narrates the assertion. It is the pre-existing comment from main, restored verbatim, and it says where the literal [1, 10] comes from rather than what the next line does.
• The red Format Check is not from this branch. unitTests/resources/query-array-scoping.test.js is unformatted on main itself and already has Format query-array-scoping test so the main Format Check passes again #2464 and fix(logging): stop the no-config window dropping every log line #2467 open against it, so it is deliberately untouched here rather than fixed a third time. Every other check is green, including Unit Test (Windows, Node.js v24).
• The Windows leg is only observable in CI. The Linux runs below prove the logging half and ordinary watcher behaviour; only Windows Node 24 with a short-form temp ancestor exercises the abort, so this PR's Unit Test (Windows, Node.js v24) job is the end-to-end evidence.

Verification

• npx mocha unitTests/utility/watcherFallback.test.js — 23 passing, 1 pending. Run twice: with this machine's ambient Harper install, and with HOME pointed at an empty directory, which is the Windows job's condition. Before the change, the second run reproduces the warn-cadence failure (0 warnings observed where 2 are expected).
• npx mocha "unitTests/utility/**/*test.*js" — 708 passing, 0 failing: the group the Windows gate reports as FAIL … exited 7 on main.
• npm run test:unit:main — 5224 passing, 1 failing; npm run test:unit:resources — 1922 passing, 0 failing. The single failure is configValidator › getDomainSocketPathLengthWarning › does not warn when a relative rootPath resolves within the limit, which resolves relative/root/operations-server against process.cwd() and so fails in any checkout whose path is long enough — including every .claude/worktrees/<name> worktree. Pre-existing and unrelated to this change.
• npm run test:integration:all not run: there are no production code changes for it to cover.
• Unit Test (Windows, Node.js v24) on this PR: pass (3m2s), with the gate reporting ok 21s 696 passing unitTests/utility/**/*test.*js where main reports FAIL 21s 689 passing … exited 7. All seven previously-failing cases now run on Windows, including the two that only assert there (survives deletion of a watched directory requires the guard to have claimed a real lost watch, and the prepend-ordering case skips off Windows entirely).
• Independent pre-push review: codex, 2 rounds, 1 finding (the nit above), 0 fixed, 1 open. The planning round recorded Framing-Verdict: chosen-approach-sound.

Refs #2364, #2309, #2234

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex; declined=gemini,cursor-grok,cursor-composer,domain; rounds=2 @ d949238}

_{Human-Review-Need: 2 @ d949238}

[gemini-code-assist]

Code Review

This pull request updates the lostWatchHarness.cjs test fixture to resolve the root path using fs.realpathSync.native (addressing Windows CI runner issues) and configures logging via a temporary configuration file. It also refactors the assertion in watcherFallback.test.js to verify specific warning occurrences instead of just the total count. The review feedback suggests cleaning up the temporary directory created in the test harness on process exit to prevent resource leaks, and recommends using assert.deepStrictEqual instead of assert.deepEqual in the test assertions to enforce strict equality.

[gemini-code-assist]

The temporary directory root created for the test harness is never cleaned up, which can lead to a leak of temporary directories in the OS temp folder. Registering a process.on('exit') handler to synchronously remove the directory ensures that successful test runs clean up after themselves.

const root = fs.realpathSync.native(fs.mkdtempSync(path.join(os.tmpdir(), 'harper-lost-watch-')));
process.on('exit', () => {
	try {
		fs.rmSync(root, { recursive: true, force: true });
	} catch {}
});

References

1. Register resources and clean them up to prevent active watchers, locks, or temporary directories from leaking.

[gemini-code-assist]

Use assert.deepStrictEqual instead of assert.deepEqual to prevent type-coercion bugs from being masked in tests, in accordance with the repository style guide and general rules.

Suggested change

	assert.deepEqual(warned, [1, 10], `expected warnings at occurrences 1 and 10, got ${warned.join(', ')}`);
	assert.deepStrictEqual(warned, [1, 10], "expected warnings at occurrences 1 and 10, got " + warned.join(", "));

References

1. Use assert.strictEqual/assert.deepStrictEqual explicitly where strict semantics are needed. ^(link)
2. Use strict assertions like assert.strictEqual and assert.deepStrictEqual instead of loose equality assertions (e.g., assert.equal, assert.deepEqual) in tests to prevent type-coercion bugs (such as string-vs-number typing issues) from being masked.

[claude]

Reviewed; no blockers found.