Canonicalize watch paths so a Windows 8.3 short path cannot abort the process

[kriszyp]

Refs #2234.

Harper aborts on Windows when a watched path carries an 8.3 short component. libuv's fs-event callback rebuilds each event's absolute path, expands it with GetLongPathNameW, and asserts the expansion still starts with the directory it stored when the watch was armed — Assertion failed: !_wcsnicmp(filename, dir, dirlen), file src\win\fs-event.c, line 72. A short directory (C:\Users\RUNNER~1\...) never survives that comparison, and libuv aborts the process rather than failing the watch. There is no JS-observable seam, so the existing isWatcherExhaustionError → polling recovery never runs and nothing is logged; the process is simply gone.

That is what has been failing Integration Tests 2/6 (Windows, Node.js v24) on roughly half of all runs since 2026-08-24, on every branch. The job only shows Probe /SeoPageCache/ did not become ready within 120000ms (last error=ECONNREFUSED) from describe-metadata-upgrade.test.ts; the assertion is only in the server-log artifact. On the GitHub Windows runner os.tmpdir() is C:\Users\RUNNER~1\AppData\Local\Temp, so every Harper instance in that shard runs with a short-path data root.

The surface is wider than "watches on a single file", which is the shape libuv needs: chokidar v4 opens a per-file fs.watch for every file it discovers inside a watched tree, so one component-directory watch arms hundreds of them. It is also not CI-only — the TLS certificate/private-key reload watcher is one of the six sites, and an abort there lands during automated renewal on a live node.

• canonicalizeWatchPath resolves a path to its long form before it can reach a native watch, and returns undefined when it cannot — resolveWatchTarget turns that into mustPoll, which each caller feeds into its existing polling options. Polling stats the file instead of arming a native watch, so it cannot reach the abort.
• Every Windows path is resolved, not only the ones that look short. An earlier revision gated on a ~<digits> spelling test; GetLongPathNameW's documentation is explicit that a short name need not contain a tilde, and NTFS allows an explicitly assigned one, so the test had false negatives that left the abort reachable — and false positives that would have pushed a genuine archive~2024 install into permanent polling. realpathSync is not a substitute for the .native variant: it resolves symlinks but leaves 8.3 names intact.
• A leaf that does not exist yet resolves through its directory, because libuv stores and compares only the parent directory of a file target. Without that, a config watcher armed during the install window — the "file not written yet" case OptionsWatcher already documents — would fail closed to polling for the life of the process.
• Applied at all six sites: the component tree and component config watchers, the root config watcher, the TLS reload watcher, the WATCH_DIR dev reloader, and the incomplete-blob read watcher. fs.watchFile (utility/logging/readLog.ts) is stat polling with no fs-event handle and is outside the invariant.
• EntryHandler is the one caller where the canonical path is load-bearing past the fs.watch call: chokidar's ignored predicate receives absolute paths built from cwd, so its bases are now derived from the same directory spelling (and hoisted out of the per-entry callback, where they were being rebuilt for every discovered path). Event paths are relative to cwd, so reads stay on the configured component.directory.
• resources/blob.ts had no polling story of its own, so a degraded watch there polls readMore on the same 20 ms backoff and the same incompleteDeadline the in-progress-write stall already uses, rather than sitting out the full read timeout and returning a 503 on a healthy live upload.
• Two adjacent crash paths in that same call, both pre-existing on main and both surfaced by review: fs.watch throws synchronously when the OS watcher pool is exhausted (EMFILE/ENOSPC), and an FSWatcher that fails after registration emits 'error' — with no listener Node rethrows it out of the watcher callback. Both now route into the same poll fallback.
• That whole block is watchInProgressFile(), exported with an injectable watch, so the paths a normal read cannot reach without an exhausted OS watcher pool are covered by six focused unit tests. Extracting it also closed a hole: both callbacks now go through the same isLive identity check, where before only the 'error' listener did — so neither a late error nor a late change from a watcher the read has already replaced can close the live watcher and start a second read sharing its fd and position.
• The two chokidar recovery sites start their close() from a microtask rather than as the argument to Promise.resolve(…), which evaluated it synchronously: a synchronous throw from close() escaped the chained .catch() and left the 'error' listener as an uncaught exception. That ordering is now pinned by a test on each site — unitTests/security/keys.test.js and a new unitTests/server/threads/watchDirFallback.test.js, which is also the first coverage watchDir has had — and the reopen no longer swallows a synchronous openWatcher throw silently, so a permanently unwatched certificate or component directory is logged.
• resources/blob.ts's pull() retired its in-progress watcher from five places, each hand-rolling close-then-null. They now share one closeWatcher() that drops the handle before closing it — so a callback racing the close fails its isLive check — and tolerates a throwing close(), so no teardown (onError, resumeIfWriterFinished, onChange, the readSync shortcut, cancel()) can be abandoned partway. Net fewer lines than the five copies.
• EntryHandler hands chokidar component.commonPatternBase. chokidar resolves a relative base against the canonicalized cwd, but an absolute one reaches the native watch as spelled, so an absolute base now goes through resolveWatchTarget as well.

For the human reviewer

• No Windows host was available, so nothing here has been executed against real 8.3 expansion or a real native watch. The unit tests drive the branch logic with a symlink named RUNNER~1 pointing at a long-named directory, which models what realpathSync.native does to an alias. That proves the algorithm, not libuv. The proof that matters is this PR's own Integration Tests (Windows, Node.js v24) matrix — please check whether all six Windows shards are green, and re-run once, because the failure it targets is ~50/50 per run. Refs rather than Fixes for exactly that reason.
• Declined — a Windows-only regression test. The strongest test would obtain a real 8.3 alias, arm both a chokidar file watch and a direct fs.watch under it in a child process, mutate the files, and assert both event delivery and child survival, isolated because unfixed it aborts the runner. I did not write it blind: unit tests are ubuntu-only in CI (.github/workflows/unit-test.yml), so it would never run there, and I cannot check it on the platform it exists for. @kriszyp has a Windows box for the follow-up.
• Declined — a lint tripwire for the invariant. "New watch sites must go through the helper" is enforced only by the DESIGN.md paragraph, and the failure it prevents is an unloggable process abort, so a reviewer suggested an oxlint no-restricted-imports rule with utility/watchPath.ts as the only allowed importer (.oxlintrc.json already bans node:assert/strict this way). It is a good idea and I'd support it as a follow-up; it needs a seven-file allowlist plus the test files that stub chokidar, which is scope growth on an already large change.
• Done, after this was declined once — a test seam for the new resources/blob.ts watcher branches. The earlier revision declined it on the grounds that a seam through the blob read path was a worse trade than the uncovered branch; @kriszyp chose the seam. It is one exported function with an injectable watch argument that production never passes, which is what the tests above drive. What it does not cover: the mustPoll decision inside resolveWatchTarget (win32-only), and the read loop's own no-watcher deadline branch, which still needs a FileBackedBlob stream to reach.
• Not done — reopening a watcher after a non-exhaustion error. A reviewer noted that chokidar can close the underlying watcher when it emits, so an EACCES on the TLS or dev-reload watcher logs and then silently never fires again. security/keys.ts is backstopped by its periodic re-read, and the dev reloader is WATCH_DIR-only, so this is a degraded fast path rather than a lost renewal — but re-arming on a delay after any error is a behavior decision I did not want to make inside a scoped follow-up.
• Not done — the 20 ms poll re-allocates its read buffer. Each re-entry into readMore allocates a fresh 256 KiB buffer before probing for data, so a read stuck in the fallback churns allocations on a host that is already under watcher pressure. The allocation is pre-existing and shared with the stall poll this fallback reuses; reusing a per-pull buffer is a separate change to that path.
• Not done — direct coverage for closeWatcher()'s catch (raised by the review bot after the last push). The watchInProgressFile tests drive that helper's own close-throw guard directly, but pull()'s closeWatcher() has no seam: watchInProgressFile's injectable watch argument is a parameter of the helper, and production pull() calls it with the default. Reaching the catch from a real read means adding a second injection point to pull() itself — the same design question as the read loop's no-watcher deadline branch, which is the other known gap below.
• Not done — a call-level watch-site tripwire. The scan asserts each listed file mentions the canonicalization helper, so a second raw fs.watch beside a canonicalized one in an already-listed file stays green; making it call-level means parsing rather than grepping.
• Not done — the dev reloader's queued restart is an async setTimeout callback. A throw from beforeRestart() or restartWorkers() in server/threads/manageThreads.js becomes an unhandled rejection. It is byte-identical on origin/main — the block is only in this diff because the watch site around it moved — and it is WATCH_DIR-only, so it is recorded rather than absorbed; this PR has already taken on three pre-existing blob crash paths.
• Considered and declined — resolveWatchTarget as synchronous I/O on the blob tail path. A reviewer read watchTarget ??= resolveWatchTarget(filePath) as a per-read realpathSync.native. Off Windows canonicalizeWatchPath returns the path with no syscall at all, and on Windows it runs once per stream (the ??= caches it) and only after a read has already stalled with no data to return.
• Not done — an absolute component pattern base is separately broken. EntryHandler now canonicalizes an absolute commonPatternBase too, so it cannot reach a native watch unexpanded. But such a component is already non-functional for a different reason: join(watchDirectory, base) in the ignored predicate produces garbage for an absolute base, so every discovered path is ignored. Component rejects a pattern that starts with /, which makes this unreachable on POSIX but leaves a Windows drive-letter pattern (C:\...) through. Rejecting absolute patterns outright is a user-facing validation change and belongs in its own PR.
• Declined — extracting the reopen-on-exhaustion sequence into a shared helper. security/keys.ts and manageThreads.js's watchDir now hand-roll the same usingPolling / liveWatcher !== opened guard, warnWatcherFallback, close-and-reopen sequence that EntryHandler, OptionsWatcher and RootConfigWatcher already carry as private methods — five copies of one invariant, and a reviewer asked for a shared utility/watcherFallback.ts helper. It is the right end state and I'd support it as a follow-up alongside the lint tripwire above; @kriszyp confirmed the deferral when this follow-up was scoped. Three of the five copies predate this PR, and the two shapes differ (private class fields with a #openWatcher method vs. closure locals), so a common helper is a cross-cutting refactor of five files rather than part of this fix.
• Symlink and junction resolution on Windows. realpathSync.native also resolves reparse points, so a watched path that is a junction is now watched at its target on Windows; retargeting the junction without touching the target would not be seen. Two reviewers raised it and the adjudicating pass dropped it both rounds on the grounds that fs.watch/inotify already follow a symlinked file to its target inode, so the same retarget is already invisible on Linux and macOS — this makes Windows match, rather than diverge. security/keys.ts also keeps its periodic re-read as a backstop. Flagging it because it is the one deliberate platform-behavior change in the diff.
• A watcher that degrades to polling stays there for its lifetime, and the warning is one-shot process-wide, so a second, later degradation logs nothing. That matches how #usingPolling already latches for ENOSPC/EMFILE — it is now the same field, so _usingPollingForTests reports it — and it is a one-line change if you want it re-armed per watcher.
• Rebased onto latest main (760f5ffc9) at 8d0c6ce45, then two further review rounds fixed a config-watcher gap the rebase's wider CI surface exposed. OptionsWatcher/RootConfigWatcher still called the failed watcher's close() directly inside the chokidar 'error' listener — unlike blob.ts/keys.ts/manageThreads.js, hardened in earlier rounds — so a synchronous throw there would escape as an uncaught exception; fixed by starting close() from a microtask there too. The same round surfaced a second gap in the same two files: the reopen was chained via .finally() with no trailing .catch(), so a synchronous throw from #openWatcher() on reopen would become an unhandled rejection — added the same trailing .catch() pattern keys.ts/manageThreads.js already use. Both are now covered by a mirrored test in each file, using the sinon-free chokidar.watch reassignment pattern from watchDirFallback.test.js (per AGENTS.md's no-new-sinon rule) — which also replaced this branch's own earlier sinon.stub(chokidar, 'watch') additions in keys.test.js with the same pattern.
• CI at 8d0c6ce45: all six Windows shards ran (after one re-run); Integration Tests 2/6 (Windows) — the shard this fix targets — passed, and 6/6 failed on its first two attempts for the same unrelated known reason as before, then passed on a third attempt. Both failures were set_configuration tests hitting EPERM: operation not permitted, rename ...harper-config.yaml.tmp -> harper-config.yaml from server/operationsServer.ts:345 — a different test each time (replicated: true rejects explicitly, then writes an operator-named component _package entry), matching the same flake documented independently on Pin the dispatched test workflows to a read-only token on v5.2 #2318, feat(server): let liveSubscriptionAuth revoke a single subscriber without ending its subscription #2039, and Exclude transient npm/git artifacts from component file watcher #809 (all unrelated PRs, none touching watch paths or config writing) and tracked at Windows: set_configuration returns 500 because the config write's rename retry blocks the thread that would release the handle #2313 with a fix already up in fix(config): read root config synchronously and bound rename retries by wall clock #2191. All other checks (Build, Unit Tests ×3 Node versions, Integration Tests 1–5/6 across Bun/Node/uWS/Windows, Next.js adapter, smoke, lint, format, coverage) passed on the first attempt.
• CI at 485414892: all six Windows shards ran; Integration Tests 2/6 (Windows) — the shard this fix targets — passed again, and 6/6 failed for the same unrelated known reason as before (set_configuration → EPERM ... harper-config.yaml.tmp -> harper-config.yaml). All three Unit Test jobs failed on the first attempt and passed on re-run with no code change; the two failures were HNSW vector-index assertions (greedy descent changed the result set, bulk delete severed survivors) in unitTests/resources/vectorIndex*, on a code path this diff does not touch, and they pass locally at this head.
• CI at the earlier head 31cfbbbe: all six Windows shards ran; Integration Tests 2/6 (Windows) — the shard this fix targets — passed, and 6/6 failed for an unrelated known reason. That failure is Configuration → set_configuration returning 500 from EPERM: operation not permitted, rename ...harper-config.yaml.tmp -> harper-config.yaml, which is Windows: set_configuration returns 500 because the config write's rename retry blocks the thread that would release the handle, already under fix in fix(config): read root config synchronously and bound rename retries by wall clock. It reproduces identically on main with no part of this diff present — same suite, same rename, same message — in this main Integration Tests run. Nothing in this diff touches config writing.

Verification

• Rebase round (8d0c6ce45, onto main@760f5ffc9): npm run build, oxlint --deny-warnings on all changed files, and every affected suite together — unitTests/resources/blob.test.js, unitTests/utility/watchPath.test.js, unitTests/server/threads/watchDirFallback.test.js, unitTests/components/OptionsWatcher*.test.js, unitTests/config/rootConfigWatcher.test.js, unitTests/security/keys.test.js — 234 passing, 0 failing, re-run after each of the two review-round fixes.

• Follow-up round (485414892): npm run build, changed-file prettier + oxlint clean, and each affected suite run on its own — unitTests/resources/blob.test.js 110 passing, unitTests/security/keys.test.js 62 passing, unitTests/utility/watchPath.test.js 13 passing, unitTests/components/EntryHandler.test.js 40 passing, unitTests/server/threads/*.test.js 36 passing. 0 failing.

• The synchronous-close()-throw fix is now mutation-pinned on both sites: reverting either reopen chain to Promise.resolve(opened.close()) fails unitTests/security/keys.test.js and unitTests/server/threads/watchDirFallback.test.js, re-measured after the suites were rewritten to node:assert.

• Running unitTests/resources alongside unitTests/security in one mocha process reports 4 failures in two unrelated createTLSSelector cases, from an analytics-writer timer firing across suites. Identical, same two cases, on this branch's prior head 31cfbbbe2 with these files reverted — pre-existing cross-directory pollution, and not a shape CI runs (test:unit:main excludes unitTests/resources).

• Re-verified at the pushed head 31cfbbbe: npm run build, then unitTests/resources/blob.test.js + unitTests/utility/watchPath.test.js + unitTests/security/keys.test.js — 183 passing, 0 failing.

• Follow-up review fix: non-ENOENT native realpath failures now fail closed to polling; npm run build, changed-file oxlint, and unitTests/utility/watchPath.test.js all pass (13 tests).

• npm run test:integration:all completed 1,816 passing and 20 skipped; 6 real-Ollama children were cancelled by an unrelated JSON import-attribute startup error in that optional suite.

• npm run build, npm run lint:required, npm run test:types — all pass.

• unitTests/utility/watchPath.test.js — 13 passing: the non-Windows identity return, directory and file resolution through a short-form ancestor, the not-yet-written leaf resolving through its directory, fail-closed when not even the directory resolves, and both resolveWatchTarget outcomes.

• Every suite covering a changed file — unitTests/components, unitTests/config, unitTests/security, unitTests/utility, unitTests/resources/blob* — 3195 passing, 44 pending, 0 failing. The two resources/blob.ts commits after that run were re-covered by unitTests/resources/blob* + unitTests/utility/watchPath.test.js (111 passing) and unitTests/config/rootConfigWatcher.test.js (6 passing).

• npm run test:unit:resources — 1684 passing, 16 pending, 3 failing; the three (randomAccessFields ×2, replayStructures) are this machine's pre-existing baseline failures.

• npm run test:unit:main cannot run on this machine: it aborts at module load against a stale local Harper install, identically on a pristine origin/main checkout, so it is environmental. The suites above cover its files for this diff.

• End-to-end route: this PR's own Windows CI matrix, as described above. Not reproducible locally — no Windows host.

• The watcher-seam follow-up: unitTests/resources/blob.test.js 109 passing, unitTests/security 664 passing, unitTests/utility 530 passing, npm run build, npm run format:write and npm run lint:required clean. npm run test:unit:resources — 1688 passing, 16 pending, 8 failing, and the same 8 fail identically on this branch's prior head with these files reverted, so they are this machine's baseline, not the change. unitTests/components and unitTests/server cannot start on this machine at all (they abort at module load resolving a storage root, on the unmodified tree too).

• Each of the five helper behaviors was mutation-checked rather than assumed: dropping the isLive guard from either callback, dropping the mustPoll latch, watching the uncanonicalized path, and registering unconditionally each fail at least one of the new tests.

Independent review: seventeen rounds (prepush-review.mjs, receipt count) — codex, Gemini, Cursor Grok and the Harper-domain adjudicator; the round numbers below are the substantive ones. The four rounds after 31cfbbbe ran Gemini alone, which is why the receipt reads declined=codex: the adjudicating and codex legs had already covered this diff four times, and each of those rounds reviewed a small delta that came directly out of Gemini's own prior findings. Round 1: 5 findings, 5 fixed. Round 2 found the one that changed the design — an earlier ~<digits> gate had false negatives, so the abort survived for a non-tilde alias — plus the resources/blob.ts fallback gap; both fixed. Rounds 4–6 each surfaced one further pre-existing crash path in the blob watcher call, all fixed above. Round 7 (the seam commit) raised the unguarded change callback as major — fixed in the next commit. Round 8 raised the synchronous close() throw — fixed. Round 9 is LGTM from the graded leg with one repeat comment-style nit. Round 10 (codex + Gemini, on the new regression coverage): 3 findings, 3 fixed — the new watchDir suite moved to bare node:assert per AGENTS.md, and both suites that fire an exhaustion error now reset warnWatcherFallback's process-global one-time gate. Round 11 raised the uncanonicalized absolute commonPatternBase and the unguarded close() in watchInProgressFile's error listener — both fixed. Round 12 raised the five hand-rolled blob teardowns and the two silently-swallowed reopen throws — both fixed. Round 13 produced no new actionable finding: one pre-existing main defect, one misreading of the lazy ??= watch-target cache, and two repeats. After the rebase onto latest main, further rounds found and fixed the config-watcher microtask/reopen gap described above, plus a new-sinon house-style violation in the tests added to cover it; subsequent rounds converged with no further actionable findings beyond the pre-existing/deferred items already listed above. Everything raised and not taken is a Not done / Declined entry above.

Complexity: moderate — a small path-normalization helper, but it changes what every file watcher in the process is handed, and one caller (resources/blob.ts) needed a new fallback inside a delicate stall path.

— Claude Opus

🤖 Generated with Claude Code

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

_{Human-Review-Need: 4 @ 8d0c6ce}

[gemini-code-assist]

Code Review

This pull request introduces a path canonicalization mechanism (utility/watchPath.ts) to prevent libuv from aborting the process on Windows when encountering 8.3 short directory paths during native file watching. It updates various watch sites—including EntryHandler, OptionsWatcher, RootConfigWatcher, FileBackedBlob, keys.ts, and manageThreads.js—to resolve watch targets and gracefully fall back to polling when canonicalization is not possible. Unit tests and design documentation have also been added to support this change. There are no review comments, and I have no feedback to provide.

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Reviewed e61454d4 — no open issues. This PR looks good, nice job!

Both Mediums from the last round are closed, and I re-ran them as mutations rather than reading the assertions:

• .native coverage. Re-ran the exact mutation that survived last time — realpathSync.native → plain realpathSync at utility/watchPath.ts:29, rebuilt with tsc -p tsconfig.build.json, exactly one isolated marker in dist/utility/watchPath.js. Last round: 8 passing / 0 failing (survived). Now: 10 passing / 3 failing — killed by all three of the new recordingNativeCalls cases. The call target is genuinely pinned now.
• The watch-site invariant. The native watch sites block is the executable form of the DESIGN.md prose. Both halves kill their mutant: adding a rogue node:fs watch() in a new file fails "are exactly the files that canonicalize their watch path", and renaming resolveWatchTarget out of security/keys.ts fails "all route their path through the canonicalization helper". Worth noting it scans source, not dist/, so it is immune to the #src/* → dist/*.js build indirection that hides .ts edits from mocha.
• keys.ts 'error' listener (the Low). Now present, with the exhaustion reopen-on-polling latch, and covered by "reopens on polling when the watcher reports exhaustion" — including the assertion that repeated ENOSPC emissions do not open a third watcher.

The four invariants I said had to stay true still do, re-measured rather than assumed:

• All six production native-watch sites canonicalize (EntryHandler.ts:542, OptionsWatcher.ts:105, RootConfigWatcher.ts:22, keys.ts:360, manageThreads.js:1235, blob.ts:772), with readLog.ts:634 correctly outside the invariant as fs.watchFile.
• canonicalizeWatchPath still returns at platform !== 'win32' before the try. Runtime probe after module load: realpathSync.native calls = 0, realpathSync calls = 0 for canonicalizeWatchPath/resolveWatchTarget on darwin and linux. No symlink resolution, no new syscalls off Windows.
• No identity clobber in blob.ts — open() at :558 and every error message still use filePath; only watch() at :778 sees watchTarget.path.
• The unresolvable fallback cannot reach the abort. Instrumented chokidar 4.0.3: usePolling: true on a single file gives fs.watch = 0, fs.watchFile = 1 (change events still fire); on a directory tree, fs.watch = 0, fs.watchFile = 5. Without it, 1 and 5 fs.watch calls respectively. blob.ts's mustPoll path arms no watcher at all.

Two caveats worth stating plainly:

1. None of this verifies Windows. Every measurement above is from macOS, where the helper is a hard no-op by construction. The 8.3 behavior itself — that realpathSync.native expands a short component and plain realpathSync does not — is unverifiable from here and is taken from the GetLongPathNameW contract, not from observation.
2. Six green Windows shards is one run against a roughly 50/50 failure, which is close to no evidence. The re-run request from the last round still stands.

Local runs vs. the true merge base f4314e76: unitTests/utility 669 pass / 0 fail (base 656 / 0, the +13 being the new file), unitTests/security 662 / 2 (base 661 / 2 — the same two jsLoader SES failures, pre-existing and environmental), unitTests/config 258 / 0 both sides, and components + server 2270 / 13 against base 2260 / 23, where head's failures are an exact subset of base's. The red Unit Test checks on CI are the known unitTests/resources HNSW flake (vectorIndex.test.js:1009), not this change.

—
Generated by Barber AI

[cb1kenobi]

Reviewed 48541489 (was 31cfbbbe) — no issues found. This PR looks good, nice job!

Same merge-base (f4314e76), four new commits (rounds 10–12 plus the synchronous-close() coverage). CI green (44 success, 4 skipped), all five prior threads resolved.

EntryHandler.ts — canonicalizing an absolute pattern base. This closes a real hole in the PR's own fix: cwd was already canonicalized, but chokidar resolves a relative pattern base against cwd while an absolute one reaches the native watch as spelled — so an absolute commonPatternBase bypassed the 8.3 short-path canonicalization entirely. Propagating patternTarget.mustPoll into #usingPolling is right too, since the pattern base can sit on a different volume than the component directory and independently require polling. Verified the symbols are actually in scope rather than assuming: resolveWatchTarget is imported at EntryHandler.ts:18, #usingPolling is declared at :108, and isAbsolute was added to the existing node:path import.

blob.ts — the closeWatcher() consolidation. Null-then-close is the correct order and the comment states why precisely. watchInProgressFile's error listener gates on handlers.isLive(watcher), which is (candidate) => watcher === candidate against the outer binding — so nulling first means a callback racing the close fails its liveness check instead of running against a retired watcher. Five call sites now funnel through it, and the try/catch means a throwing close() can no longer abandon the teardown it was part of.

I checked the one site that deliberately does not call it: onFailure sets watcher = null directly. That is correct and not an oversight — watchInProgressFile already closed the handle immediately before invoking onFailure, so routing it through closeWatcher() would double-close. Changing it from undefined to null also makes the sentinel consistent across every path.

keys.ts / manageThreads.js — logging a failed reopen. Both previously ended the polling-fallback chain in .catch(() => {}), so a watcher that failed to reopen left the process silently un-watched with no trace. Each now logs, and each uses the logging facility its own file already uses (logger.error?. vs console.error) rather than importing a new one.

Coverage tracks the change: watchDirFallback.test.js (+77), keys.test.js (+41), blob.test.js (+15).

—
Generated by Barber AI

[kriszyp]

Windows verification on real hardware

This PR shipped with "No Windows host was available, so nothing here has been executed against real 8.3 expansion or a real native watch" and asked a human to check the Windows matrix. Both PRs have now been run on a real Windows 11 machine (Node v24.14.0, libuv 1.51.0) whose os.tmpdir() is C:\Users\KRISZY~1\AppData\Local\Temp — the same 8.3 shape as the runner.

The premise is confirmed, from CI artifacts rather than a local repro

Being precise about what was and was not reproduced:

• Not reproduced locally. I could not trip the abort by hand: real generated 8.3 aliases, length-mismatched names (AVERYL~1 → averylongdirectorynamehere), directory / file / recursive / trailing-separator / forward-slash / dot-suffixed watch shapes, and an ACL intended to make GetLongPathNameW fail — all survived, with correct event filenames.
• Confirmed from CI. Downloading the harper-server-logs-windows-shard-2 artifacts, Assertion failed: !_wcsnicmp(filename, dir, dirlen), file src\win\fs-event.c, line 72 appears in stderr.log in three separate recent main runs (32905597531, 32898256258, 32877287726) — always the describe_all metadata upgrade phase 1 instance, data root C:\Users\RUNNER~1\..., immediately after packGitReferenceWithoutScripts / extractApplication, i.e. during component extract/install directory churn.
• The assertion is live in release Node. src\win\fs-event.c is present as a UTF-16 string inside node.exe, so libuv's asserts are compiled into Windows release builds. Not a debug-build artifact.

Why the fix is sufficient regardless of the unknown trigger

Reading libuv 1.51.0 src/win/fs-event.c: line 72 is reachable only from uv__fs_event_cb's handle->dirw branch, comparing the per-event GetLongPathNameW expansion against whatever uv_fs_event_start stored. handle->dirw is assigned only on the directory branch (line 320) and stays NULL for file targets — so the exposed surface is directory watches, exactly as this PR's chokidar note describes.

That yields a stronger sufficiency argument than the PR currently makes, and one that does not depend on knowing why GetLongPathNameW failed on the runner: once libuv is handed an already-long path, dirw is long, so GetLongPathNameW(dirw + "\" + name) always starts with dirw and the assertion cannot fire. The mustPoll branch is safe by a different route — chokidar's usePolling path uses fs.watchFile stat polling and never arms an fs-event handle at all. Both branches are structurally closed, not probabilistically improved.

A real defect in this PR's own test — fixed in this push

unitTests/utility/watchPath.test.js could not run on Windows at all: 0 passing, failing in its before hook with

Error: EEXIST: file already exists, symlink '...\watch-path-XXXX\runneradmin' -> '...\watch-path-XXXX\RUNNER~1'

The fixture creates a directory runneradmin and then a symlink named RUNNER~1 beside it — but on a volume with 8.3 name creation enabled (the default, and the exact configuration this helper exists for) NTFS has already assigned RUNNER~1 to runneradmin, so symlinkSync collides. The one suite written to model Windows behaviour was the one suite that could not run on Windows, and unit tests are ubuntu-latest-only, so nothing caught it.

Fixed by renaming the alias link, with a comment recording the constraint. 13/13 passing on Windows — the first time canonicalizeWatchPath / resolveWatchTarget have been exercised against real realpathSync.native 8.3 expansion rather than only the Linux symlink model.

CI on this push

All six Windows shards pass on the first attempt, including 2/6, the shard this fix targets:

shard	result
Integration Tests 1/6 (Windows)	pass 4m00s
Integration Tests 2/6 (Windows)	pass 13m23s
Integration Tests 3/6 (Windows)	pass 5m47s
Integration Tests 4/6 (Windows)	pass 3m59s
Integration Tests 5/6 (Windows)	pass 3m18s
Integration Tests 6/6 (Windows)	pass 4m22s

Two attempt-1 failures, both explained, neither related to this diff:

• Integration Tests 2/6 (Node.js v24) failed in the Relink bin scripts setup step, before any test ran.
• Integration Tests 6/6 (Windows) failed on set_configuration still writes an operator-named component _package entry with EPERM: operation not permitted, rename ...harper-config.yaml.tmp -> harper-config.yaml. That is Fix intermittent set_configuration 500s on Windows caused by the config write's rename retry #2339's failure, not this one — this branch does not carry the descriptor-lifetime fix. The same run showing shard 2/6 green and shard 6/6 red is a clean demonstration that the two PRs fix disjoint bugs and neither subsumes the other. On the stacked Fix intermittent set_configuration 500s on Windows caused by the config write's rename retry #2339, all six shards are green.

Local Windows results

• unitTests/utility/watchPath.test.js — 13 passing (after the fix above)
• unitTests/resources/blob.test.js — 115 passing
• unitTests/server/threads/watchDirFallback.test.js — 2 passing
• integrationTests/deploy/redeploy-runtime-equivalence.test.ts — 18/18
• integrationTests/apiTests/describe-metadata-upgrade.test.ts — 2/2
• Clean npm run build, npm run typecheck, npm run lint:required, prettier --check

Two suites could not be run to completion on this machine. Both fail identically on main, so they are environmental rather than regressions — but it does mean the two new security/keys.ts tests in this PR remain unverified on Windows:

• unitTests/security/keys.test.js — the before all hook exceeds its own this.timeout(10000). Not mkcert (measured 565 ms for createCA + createCert); it stalls in the testUtils.preTestPrep() / setupTestDBPath() storage-root setup.
• unitTests/components/EntryHandler.test.js — 39 passing / 2 failing; the after each hook dies with an uncaught EPERM: operation not permitted, watch. See below.

One gap this PR does not close

main's Windows failures sort into three buckets, and only two are covered:

shard	cause	fixed by
2/6	libuv 8.3 abort	this PR
6/6	EPERM rename on config write	#2339
4/6	uncaught EPERM: operation not permitted, watch	neither

chokidar 4.0.3 attaches its 'error' listener to the underlying Node FSWatcher — including its Windows EPERM workaround for nodejs/node-v0.x-archive#4337 — only in the persistent: true branch of setFsWatchListener:

if (!options.persistent) {
    watcher = createFsWatchInstance(path, options, listener, errHandler, rawEmitter);
    if (!watcher) return;
    return watcher.close.bind(watcher);      // returns WITHOUT watcher.on('error', ...)
}

Every Harper chokidar watcher passes persistent: false, so when libuv delivers an async watch error it lands on an emitter with no listener and throws. Attaching .on('error') to the chokidar wrapper — which this PR does at several sites — does not help, because the native watcher's error never reaches the wrapper's error route in that branch. resources/blob.ts is the one site that is covered, precisely because this PR attaches a listener to its raw fs.watch handle.

Measured on Windows (arm the watch, then delete the watched directory):

shape	result
chokidar.watch(dir, {persistent:false})	uncaught EPERM watch
chokidar.watch(file, {persistent:false})	uncaught EPERM watch
chokidar.watch('.', {cwd, persistent:false})	uncaught EPERM watch
chokidar.watch('**', {cwd, persistent:false, ignored})	survived
chokidar.watch(dir, {persistent:true})	survived
raw fs.watch(dir, {persistent:false}) + .on('error')	survived

This is pre-existing (it reproduces on main), it is what kills Integration Tests 4/6 (Windows) during Redeploy runtime-equivalence proof, and it is tracked separately rather than expanded into this PR. It is worth noting here because it sits directly adjacent to this PR's new DESIGN.md invariant: a watch site can canonicalize its path correctly and still take the process down.

On two of the declined follow-ups

• The Windows-only regression test is worth revisiting now that a Windows host exists — but it still would not run in CI, because .github/workflows/unit-test.yml pins ubuntu-latest. Adding Windows unit-test coverage is filed separately; the pre-existing Windows failures above are the obstacles.
• The repo-wide watch-site scan does pass on Windows and enumerates correctly: an independent grep for every chokidar.watch / fs.watch call site in the tree returns exactly the six files the test lists.

🤖 Verification performed with Claude Code on Windows 11 / Node v24.14.0