fix(watcher): retain pending files on zero-result sync (#450)
* fix(watcher): retain pending files on zero-result sync
* refactor(watcher): detect lock-unavailable at the wrapper
Replace the heuristic `(filesChanged === 0 && durationMs === 0)` check
inside `FileWatcher.flush()` with a typed `LockUnavailableError` thrown
by `CodeGraph.watch()`'s sync wrapper. The wrapper has access to the
full `SyncResult`, including `filesChecked` — which is **only** zero
when `sync()` failed to acquire the cross-process file lock (a real
empty sync always has `filesChecked > 0` because `scanDirectory` ran).
That eliminates the heuristic's edge case where a fast no-op sync
returns `durationMs === 0` by `Date.now()` rounding and gets mistaken
for a lock failure on tiny projects.
The watcher's `catch` block now distinguishes `LockUnavailableError`
from real errors: it logs at `logDebug` (not `logWarn`) and does NOT
call `onSyncError` — so a long-running external indexer holding the
lock doesn't spam stderr every debounce cycle via the MCP daemon's
`Auto-sync error` handler. The existing post-catch path already
preserves `pendingFiles` and reschedules, so no new control flow is
needed.
A/B validated end-to-end against the built dist on macOS with a
three-scenario repro (lock held, lock released mid-flight, real sync
error):
- main: lock-held silently clears pendingFiles (BUG);
lock-released never recovers (no real sync runs).
- PR-as-is: lock-held preserves pendingFiles; lock-released
drains. Same observable behavior as wrapper-level.
- wrapper-level: same outcomes; lock-failure goes through the catch
path silently (logDebug only, no onSyncError noise);
real errors still surface via onSyncError.
Updates the regression test to throw `LockUnavailableError` (the real
contract surfaced to `FileWatcher` by `CodeGraph.watch()`), and
asserts `onSyncError` stays quiet during the lock-held cycle.
Closes #449.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Colby McHenry <me@colbymchenry.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
Colby McHenry
parent
6015e4fdd2
commit
72c08c2bef
@@ -25,7 +25,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import { FileWatcher } from '../src/sync/watcher';
|
||||
import { FileWatcher, LockUnavailableError } from '../src/sync/watcher';
|
||||
import CodeGraph from '../src/index';
|
||||
import { triggerFileEvent } from './__helpers__/chokidar-mock';
|
||||
|
||||
@@ -274,17 +274,53 @@ describe('FileWatcher', () => {
|
||||
const after = watcher.getPendingFiles();
|
||||
expect(after.some((p) => p.path === 'src/will-fail.ts')).toBe(true);
|
||||
|
||||
// Schedule a retry by emitting the event again (production would do
|
||||
// this implicitly on the next file change; tests synthesize it).
|
||||
triggerFileEvent(testDir, 'change', 'src/will-fail.ts');
|
||||
|
||||
// Retry resolves; entry clears.
|
||||
// Retry resolves automatically; entry clears.
|
||||
await waitFor(
|
||||
() => !watcher.getPendingFiles().some((p) => p.path === 'src/will-fail.ts'),
|
||||
);
|
||||
|
||||
watcher.stop();
|
||||
});
|
||||
|
||||
it('should retain pending files and retry when syncFn throws LockUnavailableError (#449)', async () => {
|
||||
// CodeGraph.watch() converts the cross-process lock-failure no-op
|
||||
// into LockUnavailableError so the watcher's retry path picks it up
|
||||
// instead of falsely clearing pendingFiles. This test exercises the
|
||||
// contract directly.
|
||||
const syncFn = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new LockUnavailableError())
|
||||
.mockResolvedValueOnce({ filesChanged: 1, durationMs: 10 });
|
||||
const onSyncComplete = vi.fn();
|
||||
const onSyncError = vi.fn();
|
||||
const watcher = new FileWatcher(testDir, syncFn, {
|
||||
debounceMs: 100,
|
||||
onSyncComplete,
|
||||
onSyncError,
|
||||
});
|
||||
watcher.start();
|
||||
await watcher.waitUntilReady();
|
||||
|
||||
triggerFileEvent(testDir, 'add', 'src/locked.ts');
|
||||
|
||||
await waitFor(() => syncFn.mock.calls.length >= 1);
|
||||
expect(watcher.getPendingFiles().some((p) => p.path === 'src/locked.ts')).toBe(true);
|
||||
// A held-lock no-op is not a sync failure — onSyncError stays quiet
|
||||
// so a long-running external indexer doesn't spam stderr every cycle.
|
||||
expect(onSyncError).not.toHaveBeenCalled();
|
||||
expect(onSyncComplete).not.toHaveBeenCalled();
|
||||
|
||||
await waitFor(() => syncFn.mock.calls.length >= 2);
|
||||
await waitFor(
|
||||
() => !watcher.getPendingFiles().some((p) => p.path === 'src/locked.ts'),
|
||||
);
|
||||
|
||||
expect(onSyncComplete).toHaveBeenCalledTimes(1);
|
||||
expect(onSyncComplete).toHaveBeenCalledWith({ filesChanged: 1, durationMs: 10 });
|
||||
expect(onSyncError).not.toHaveBeenCalled();
|
||||
|
||||
watcher.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe('callbacks', () => {
|
||||
|
||||
Reference in New Issue
Block a user