fix(code-index): restore watcher events after restart - #1835

Security Blast Radius

• inferred — Filesystem events can affect the existing workspace index and its vector-store and cache writes. The changed notification routing does not establish a new file-access path or sensitive sink.

Trust Boundaries and Controls

• observed — Emitter capture occurs before start listeners run; subsequent progress and completion notifications use those captured emitters. Disposal removes the old listeners, while the orchestrator releases its old subscriptions.

Resilience and Maintainability Implications

• inferred — An old batch can still write after watcher disposal or alongside a new batch. The inspected change leaves that pre-existing index-state limitation in place rather than introducing cancellation or write-session isolation.

Hardening Proposals

• proposed — If clearing or replacing an index must exclude late writes, drain active batches or enforce write-session ownership before clearing shared index state. This is a separate safeguard, not a protection supplied by the notification change.

Explanation

The title "fix(code-index): restore watcher events after restart" matches the main change. The file-watcher.ts changes recreate disposed batch event emitters on reinitialization, and the new tests cover event delivery after restart. It is short, specific, and in active voice. It uses the conventional commit form, which fits a repository history scan.

Explanation

Changed behavior in src/services/code-index/processors/file-watcher.ts, verified from the reviewed diff: 1. Emitters are recreated on initialize() after dispose(), and dispose() clears fileWatcher and the debounce timer. 2. initialize() returns early when a native watcher already exists. 3. Each batch captures its session's progress and finished emitters and passes them through the deletion, upsert and finalize phases. Coverage in src/services/code-index/processors/__tests__/file-watcher.spec.ts, a unit-level spec at the lowest valid layer for this class: - "does not deliver an old batch into subscriptions created after restart" covers the per-session emitter capture. It blocks an in-flight batch, disposes and reinitializes, and asserts the old batch's progress and finished events are not delivered. It then asserts a new batch still delivers events. - "restores all batch events after disposal and reinitialization" covers emitter recreation and checks that the old listener is not called. - "does not create duplicate native watchers on repeated initialization" covers the early-return guard. - "discards queued events and the pending timer when disposed before a batch starts" covers timer and queue reset and the double-dispose path. - "reports progress and preserves cached hashes when batch deletion fails" covers the error branch of the deletion phase, including progress sequence, finished payload, and no hash updates or upserts. The mock emitter changes make disposed emitters drop listeners and events. This matches the real behavior that the new tests depend on. The ESLint suppression count change is mechanical. Gaps I noticed but judged non-blocking under the check's criteria. The upsert-phase error branch was not newly covered. Its behavior is unchanged apart from which emitter it fires on, and the stale-session test exercises the upsert path via processFile. The check does not require every branch without a plausible regression scenario. No changed behavior lacks focused coverage at the unit layer.

Explanation

The PR changes three files: src/eslint-suppressions.json (one suppression count, 25 to 22), src/services/code-index/processors/__tests__/file-watcher.spec.ts (test mocks and tests), and src/services/code-index/processors/file-watcher.ts. The file-watcher.ts diff covers event-emitter lifecycle only. It converts three public event properties to getters, recreates emitters on initialize() after disposal, clears the watcher and debounce timer on dispose(), and passes per-session emitters through processBatch and its helpers. I checked each failure condition against this diff: - Secrets or PII leak: No changed path logs, returns, or emits credentials or personal data. The emitters carry file paths, progress counts, and batch summaries, which the existing code already emitted. - Unvalidated input trusted or executed: No changed path adds or removes input parsing, path validation, or command execution. The watcher still processes the same file events through the same existing code. - Approval or allowlist bypass: No changed path touches approval, allowlist, or permission logic. The changes affect event delivery and lifecycle only, so the check passes.

Explanation

The PR changes three files: src/services/code-index/processors/file-watcher.ts, its spec, and src/eslint-suppressions.json. The only production change is in file-watcher.ts, and it is limited to event routing and watcher lifecycle: - Batch events are exposed through getters. Emitters are recreated in initialize() after disposal. - initialize() returns early when a watcher already exists. - dispose() clears fileWatcher and the debounce timer, and marks emitters disposed. - triggerBatchProcessing() captures the session's progress and finished emitters and passes them into processBatch(). The persistence code is unchanged in the diff. The vector delete and upsert calls, the cacheManager hash updates, and their await ordering are not modified. orchestrator.ts is also unchanged. stopWatcher() still calls fileWatcher.dispose() without awaiting an active batch, and clearIndexData() still runs as before. On the prior FAIL: the overlap it describes is real. dispose() does not track or await processBatch(), so an in-flight batch can still write after restart. That behavior predates this PR. In the base, initialize() had no guard and re-created the watcher after dispose(). The base dispose() also did not track or await active batches. The PR does not add the overlap path, and it does not change write ordering or awaits. The prior claim that the base "did not permit reinitializing" the watcher is factually wrong. The PR's own description states that the drain work was removed by scope decision and that the persistence-overlap finding remains open. That is a real gap, but it is not a changed-code defect under this check's criteria. The check requires a changed persistence path that loses or corrupts state through an unawaited operation, a non-atomic write, missing rollback, or a missing default. The diff does not add one. Prior failure refuted: The overlap of an in-flight processBatch with a restarted watcher predates this PR. Base dispose() did not track or await batches, and base initialize() had no guard, so it re-created the watcher after dispose. The diff (only file-watcher.ts changes in production code) leaves processBatch's vector delete/upsert and cacheManager writes and their await ordering unchanged. orchestrator.ts is uncha...

Explanation

The diff changes three files: src/eslint-suppressions.json, file-watcher.spec.ts, and src/services/code-index/processors/file-watcher.ts. I read the full changed file-watcher.ts lifecycle paths. Changed lifecycle paths and their cleanup behavior: - initialize() now returns early when this.fileWatcher already exists (line 116). This prevents a second native FileSystemWatcher from being created without disposing the first. This removes a leak path. - initialize() recreates the three emitters only when eventsDisposed is true (lines 117-126). Disposed emitters are replaced before a restart. - dispose() now disposes fileWatcher and sets it to undefined (lines 144-145). It clears batchProcessDebounceTimer and sets it to undefined (lines 146-149). It also marks eventsDisposed = true (line 153). Native watcher, timer, and emitter references are released. - triggerBatchProcessing() captures the current emitters in batchEvents (lines 205-214). Progress and finish events from an older batch go to its own captured emitters. This prevents stale batch events from reaching a restarted session. Residual behavior: dispose() does not cancel an in-flight processBatch. The old batch can still run its persistence work after restart. The author states this explicitly in the PR description. This behavior predates the change. The diff only changes notification routing and does not add new work, timers, or watchers. The debounce timer and native watcher are both cleared on disposal, and no new listener or timer is created on any changed path. No changed path leaks a listener, watcher, provider, or timer. No changed path duplicates work because of cancellation, disposal, or restart beyond the pre-existing in-flight batch behavior. The check passes.

• Create a new PR