Commit Graph

1 Commits

Author SHA1 Message Date
Michael Ramos 192b026073 fix(annotate): stop the folder watcher freezing the server (#1314)
* fix(annotate): stop the folder watcher freezing the server (#1313)

The file-browser content watcher built a chokidar scan over the whole
workspace synchronously on the request path. Under Bun that scan
monopolizes the event loop (a 780-directory nested tree measured 79
seconds), and because teardown was immediate on the last unsubscribe,
every EventSource reconnect paid the scan again: the reconnect the
freeze itself provoked made the hang self-sustaining.

The watcher engine now lives once in
packages/shared/file-browser-watch-core and both runtimes keep only
their transport:

- construction is deferred off the request path, so the SSE ready event
  and concurrent API requests are served before any scan starts
- teardown gets a 30s reconnect grace; a reload reuses the warm watcher
- on macOS and Windows the content watcher is the platform's native
  recursive fs.watch (measured ~0ms for the same tree); chokidar stays
  the Linux backend and the runtime fallback, with a forced catch-up
  refresh on the swap so no events are lost
- server stop tears every watcher down immediately in both runtimes

The responsiveness regression test reproduces the reported freeze on
the pre-fix implementation (79s, fails) and passes in under a second on
the fix.

* docs: folder annotate sessions do write per-file version history

The PLANNOTATOR_ANNOTATE_HISTORY row claimed URL, folder, and
annotate-last sessions never write to the data dir. The folder /api/doc
path deliberately runs the per-file version-history pipeline (lazily,
memoized per resolved path, gated on the same flag) to power the
per-file version diff, and has since it shipped. The code is the
intended behavior; the sentence was stale. URL and annotate-last
sessions remain fully stateless, and submit records remain single-file
only.

* fix(annotate): review follow-ups for the watcher engine

Applied from the independent review of #1314:

- contentWatchBackend gains a forced 'native' mode and the fallback
  tests use it, so the native-to-chokidar paths (creation failure and
  runtime error) genuinely execute on Linux CI; the runtime-error test
  is no longer macOS-only
- a platform-agnostic responsiveness test pins that SSE ready is served
  before the scan starts on the chokidar backend, via the runtime test
  hooks; the tight full-scan bound stays macOS-only
- watcher construction failures and the native-to-chokidar swap now log
  one console.error each instead of stranding subscribers silently;
  the swap also increments the diagnostics start counter honestly
- closeEntry guards both watcher close() calls; the Bun annotate stop
  chain got the same try/finally shape as the plan server; all four
  stop chains close watchers ahead of throwable disposals so a failing
  dispose cannot strand a watcher keeping embedded hosts alive
- a broadcast that empties the subscriber map by deleting dead
  subscribers now schedules the teardown grace instead of leaving the
  entry live until closeAll
- bun.lock drift reverted: only the chokidar edge and the workspace
  version corrections remain (27 unrelated esbuild resolution entries
  dropped; frozen-lockfile install verified)
- stale never-write comments in both annotate servers corrected to
  match the folder per-file history reality documented in AGENTS.md;
  the engine header now states plainly that chokidar is a correctness
  fallback, not a performance one
2026-08-13 16:02:41 -07:00