fix(server): finished runs rescan the file index off the event loop#22603

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This targeted fix removes the post-run full file-index scan while retaining VCS and pull-request refreshes. Search freshness now relies entirely on the watcher, with documented stale-index cases during large bursts or lost events, creating a material runtime tradeoff for human review.

^{You can add or adjust custom eligibility rules. Learn more.}

Explain Change →

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Checked both CodeRabbit points against 1d6f7c7.

• No-call assertion: the layer type is the guard. layerObserver no longer requires WorkspaceEntries, and the tests no longer provide it. If you restore the old RunFinalizationService.ts against these tests, typecheck fails (TS377004 … missing … WorkspaceEntries), and 4 of 5 tests fail (Service not found: t3/workspace/WorkspaceEntries). If the suggested mock is added back, it satisfies a reintroduced dependency, and only the call assertion still guards. So I kept the tests as they are.
• Linked issues: only #22586 is a closing reference. The body mentions #11306 only to say that this PR leaves it open.

Thanks @mikeohuo. I kept your diagnosis and the complete finalization change, preserved your commit, merged current main, and pushed two scoped follow-ups to this branch. The diff against main is five files, +80/-10.

The added recovery repairs changes missed by fff's watcher: path listing, path search, and content search attempt a rescan on their next read after five minutes. Concurrent readers share the attempt. Failed attempts are also throttled and logged, and cached searches remain usable. Recovery happens before the content-search time budget starts. No timers, new RPCs, UI controls, dependencies, or test files were added. VCS/PR refresh and their branch/run guards remain unchanged.

Verification on 7f970597ec475836315ece16312e03eaeaea85e6:

• Blacksmith: 49 tests passed across RunFinalizationService, RunFinalizationQueue, WorkspaceEntries, WorkspaceFileSystem, and WorkspaceSearchIndex.
• Blacksmith vp run check:changed 9d7997afc5db09462e140de19f799c709655094c: format, lint, knip, generated GraphQL checks, affected typechecks, and adjacent tests passed.
• The missed-event regression failed before recovery was added. Failure throttling and content-search budgeting each failed before their corrections. Restoring main's finalization implementation against the contributor's test layers failed four tests with Service not found: t3/workspace/WorkspaceEntries. This also verifies the existing guard against restoring the dependency; I did not add the suggested redundant mock.
• Runtime with real fff 0.11.0, watchers deliberately disabled: a renamed file stayed missing before the deadline; after a five-minute clock advance, listing recovered the rename/deletion and content search recovered an edit. Repeated reads did not rescan.
• The local dev web client completed a real GPT-6-Astra provider turn and displayed the requested reply. This was a functional check, not a timing measurement.
• Two independent reviews, one round plus one follow-up, identified failure retry and content-budget issues; both are corrected with failing-then-passing regressions. No third review round was requested.

Unverified: the original Windows large-repository/concurrent-run freeze, macOS, packaged desktop/mobile, and remote/relay/tunnel execution. There is no UI layout change or before/after performance claim.

Remaining upstream limit, @mikeohuo: the occasional recovery scan still uses fff's synchronous native API and can block the event loop. This bounds the recovery frequency; it does not replace worker isolation or fix #11306. A failed repair may continue serving stale results until a later successful attempt.

GitHub CI and preview workflows currently report action_required, so this is waiting for maintainer intervention rather than fully green. CI run. CodeRabbit's latest automatic review is pending; its earlier attempt reported a review-capacity limit. No review bot was invoked manually. No labels were changed manually, and this PR remains open.

Verified in a real dev environment

verified exact head 7f970597ec475836315ece16312e03eaeaea85e6 on this linux boat vm using the repo's playwright-core, /usr/bin/google-chrome, and real codex gpt-6.1-sol through the local proxy. projects were added through the app. no mocks, replay, seeded database, source edits, commits, or pushes.

• main first: installed github/main at 49582b7b2a in ../main. each of three completed provider turns caused a real FileFinder.scanFiles() call. this reproduces the unwanted trigger, not the original windows freeze.
• stopped main before starting the pr with isolated .t3 state. four codex turns created, renamed, edited, deleted, and cleaned up files. no finalization caused a scan. all four checkpoint effects succeeded. replies, diffs, and vcs change counts appeared in the client.
• warmed path and content indexes before edits. the composer found created and renamed files. authenticated websocket searches returned new content and dropped deleted paths and old content before recovery ran.
• projects.writeFile still refreshed both active indexes. its file and content were searchable immediately after saving.
• real five-minute interval: save at 05:47:02 utc; reads at 05:51:25 caused no scans; concurrent listing/path/content reads at 05:52:31 caused exactly one recovery scan per active index. all succeeded. repeated reads and the cleanup turn caused no more scans. no clock override.
• blacksmith: 49 focused tests passed across finalization, queue, entries, filesystem, and search index. vp run check:changed 9d7997afc5db09462e140de19f799c709655094c passed format, lint, both knip checks, graphql, affected typechecks, and 10 adjacent tests. an initial shallow-worker merge-base failure was resolved by fetching contributor-branch ancestors.

runtime transcript: native-call timestamps, persisted effects, and rpc results

main, same create prompt and file data:

main: real codex-created alpha.txt in the file picker

pr:

pr: alpha.txt stays searchable without a finalization rescan

animated recording of the real rename/edit/delete turn followed by file search. functional evidence, not a timing comparison. inspected the clip and verified uploaded playback to completion and seeking in t3's browser player:

pr: codex turn completes and gamma.txt appears in search

Not verified

• original windows large-repo, approximately 12-concurrent-thread lock-contention/disconnect workload. main reproduced the forced-call trigger, but not its 15–52 second freeze. no speedup or freeze-elimination claim.
• pr tab startup displayed a 35-second cpu-overload warning, load 14 on four cores, visible in the clip. no fff scan corresponded to it. its cause was not established; general responsiveness under load remains unverified.
• deliberately lost watcher events, watcher startup failure, and live recovery-scan failures. the real recovery deadline was exercised; fault injection was not. existing regression tests passed.
• native desktop shells, ios/android, macos/windows, other providers, relay/tunnel, and branch-guarded pr-status refresh. local vcs refresh and checkpoints were exercised.
• full github ci and preview workflows remain action_required (ci). scoped blacksmith checks passed; full ci is not claimed green.

no code fix was needed for the exercised behavior. the pr remains open. no labels were changed.

The automated pipeline stopped on this pull request and will not merge it:

the capture step could not show the problem on the base commit, so there is nothing to prove fixed.

On each commit: create and commit 272,000 synthetic C# files; register the project; open the composer @Asset0001 picker to initialize its index; launch 12 concurrent replay threads while the driver edits one tracked file per worker; issue workspace searches every 500 ms and HTTP probes every 100 ms; record the browser and inspect checkpoint, finalization, and index-refresh traces.

It stays open for a person to finish or close.

@juliusmarminge the capture step could not show the freeze on the base commit because of the workload, not because the bug is gone. I reproduced it on the base commit, checked the rework against the same workload, and pushed one small fix (0120d57).

Why the pipeline workload does not freeze

One edited file per worker never holds fff's picker lock for long. The freeze needs a writer that holds the lock for seconds. In fff 0.11.0 that is index_new_directory (background_watcher.rs:844-856): when a folder appears in the indexed tree, the watcher takes one write lock and calls handle_create_or_modify for every file in it, with a metadata call and a binary-sniff read per file. A refresh that arrives in that window waits in fff_scan_files or fff_get_scan_progress for a read lock.

Reproduction

Windows 11 Pro 10.0.26300, NTFS, Defender real-time scanning on, Node 22.23.4, fff-node 0.11.0, ffi-rs 1.3.2. This is a standalone Node harness, not the full server. It copies the FileFinder.create options and the refresh sequence from WorkspaceSearchIndex.ts for each commit, and uses the fff-node patch from that commit.

1. Create and commit a git repository of 272,000 small C# files. Create the finder and wait until the index is ready.
2. Build a folder of 250,000 files of 1 KB next to the repository, then move it into the repository with one rename.
3. Run one refresh every 1 to 1.5 s for the next 30 s, as finished runs do.
4. Measure two things: how late a 20 ms timer fires (event loop), and how long fs.promises.stat takes every 100 ms (libuv pool).

The last row ran the same source change applied by hand to the patched package, before I committed it.

So the rework removes the event-loop freeze on the finalization path. In the f3bc16a paths run, 11 refreshes waited up to 15.2 s for the lock and the event loop kept running.

The defect in f3bc16a, fixed in 0120d57

ffi-rs 1.3.2 does not start a thread for runInNewThread: true. It calls napi Env::spawn, which queues the call on the libuv pool (4 threads by default, shared with fs, dns.lookup, crypto and zlib). A standalone check with 8 calls that each block for 4 s: 4 calls finished at 4.0 s, 4 at 8.0 s, and an fs.promises.readFile started in between waited 7.8 s.

scanFilesAsync already shares one call per finder. The progress poll in waitForIndexReady did not: each concurrent refresh made its own fff_get_scan_progress call. With a content index, each refresh polls for seconds, so several polls wait on the lock at the same time and fill the pool. The event loop answers pings, but asynchronous file system calls wait until the lock is released.

0120d57 adds FileFinder.getScanProgressAsync(), which returns the pending read when one exists, and waitForIndexReady polls through it. One finder now holds at most two pool threads. The new test in WorkspaceEntries.test.ts runs 8 concurrent refreshes on a real finder and expects at most one read in flight. It fails on f3bc16a, where the method does not exist, and it fails with expected 8 to be 1 when the sharing is switched off.

Checks on 0120d57: the 5 workspace and finalization test files pass (57 passed, 7 skipped), vp run --filter t3 typecheck exits 0, and the pre-commit vp fmt, vp lint and knip:check pass.

Question: the cost of a rescan after every run

The rework keeps the explicit rescan after every finished run, and an explicit rescan skips fff's rescan throttle (shared.rs:234). I measured that cost on the repository from #22586 (a Unity project, about 275,000 tracked files and 1,141,152 files in the fff index, paths index only), two finders side by side for 120 s, one sample each:

That is about 10 GB read and 35 s of CPU for each rescan. In the 16-session workload from the issue a run finished about every 40 s, so the load does not stop. This is consistent with the 99 % fff-git-status core and the 1.7 GB/s read rate reported there.

The rescan is there to recover events the watcher dropped, and the burst measurements in the earlier PR body show that this recovery is needed. Now that the call no longer blocks the event loop, would you accept a minimum interval between forced rescans for each index, in this PR or in a follow-up? I did not change this, because it is a product decision.

Not covered

• fileSearch, directorySearch, mixedSearch, grep and isScanning still take the same read lock on the event loop, so a file picker keystroke or a content search inside a long writer hold still blocks the server. destroy() takes the write lock on the event loop, also from the 15-minute idle expiry. That is #11306.
• Four finders that wait on their locks at the same time can still fill the pool, for example nested project roots that each have both indexes. I did not reproduce this. A process-wide cap would close it, at the cost that one busy workspace delays refreshes in the others.
• waitForIndexReady checks its deadline only after a poll returns. A lock held for minutes makes that refresh, and the finalization that waits for it, last for minutes.
• I did not rerun the 12 to 16 session workload on a packaged build, and I did not test macOS or Linux.

Thanks for this. Main has the change since #35059.

Main now runs rescan and readiness polling inside each finder's worker thread, preserving refresh behavior while keeping native lock waits off the server event loop. I compared your refresh change, asynchronous FFI patch, concurrent-progress protection and disposal handling against the merged worker implementation; its dedicated threads also avoid the libuv-pool exhaustion your call sharing prevents.

Closing as superseded.