scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-08 03:08:13 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(watch): await a watcher re-arm barrier so gitignore reloads cannot drop events (#3159)

Browse source 
* Increase CI timeout budget for flaky watch-filesystem test

* test(watch): include elapsed budget in waitFor timeout errors (#3156)

Make CI flake timeouts self-describing without raising the 90s ceiling, and cite the mcp/server-startup 15s/5s convention in the helper comment.

* fix(watch): await a watcher re-arm barrier after an ignore-rule reload

An ignore-rule reload re-armed the watcher with `watcher.add(repoPath)`,
which returns before the rescan it starts has finished and offers no signal
for that completion. A file the reload had just unignored was therefore still
unregistered when the call returned, and since `ignoreInitial` suppresses the
`add` that the in-flight rescan would emit, an immediate rewrite of that file
was dropped permanently. A standalone reproduction missed the rewrite 40/40
times on both chokidar 4.0.3 and 5.0.0.

Re-arm by arming a replacement watcher and awaiting its `ready` instead, which
is the only completion signal chokidar exposes (`ready` never fires twice on
one instance). The re-arm runs before the refresh, so a write that lands while
the replacement arms is still read by that refresh; the outgoing instance keeps
reporting until the swap, so no event window is dropped; and a replacement that
fails to arm leaves the working instance in place for the queue to retry. The
transient-watcher-error path now requests the same awaited re-arm rather than
re-arming inline ahead of its catch-up refresh.

This replaces the CI timeout increase from #3156, which treated the symptom:
the test was not slow, it was waiting for an event that never came.

Co-authored-by: Cursor <cursoragent@cursor.com>

* refactor(watch): drop restating comments and duplicated waitFor state

The re-arm error now uses the same cause-wrapping shape as ignore-control
reload, and the instant-rewrite test waits for both paths in one poll.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Gergő Magyar 2026-09-03 12:25:51 +01:00 • committed by GitHub
parent 33936cbc50
commit 12763a40c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 119 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

115
gitnexus/src/cli/analyze-watch.ts
View file

@ -257,6 +257,8 @@ export async function startWatchFileLoop(
): Promise<WatchFileLoop> {
let ignorePath = await createWatchIgnorePredicate(repoPath);
let ignoreControlValid = true;
let rearmPending = false;
let closed = false;
const queue = new WatchRefreshQueue(
async (paths) => {
if (paths.some(isIgnoreControlPath) || !ignoreControlValid) {
@ -264,7 +266,7 @@ export async function startWatchFileLoop(
try {
ignorePath = await createWatchIgnorePredicate(repoPath);
ignoreControlValid = true;
watcher.add(repoPath);
rearmPending = true;
} catch (error) {
ignoreControlValid = false;
throw new WatchControlReloadError(
@ -279,6 +281,11 @@ export async function startWatchFileLoop(
);
}
}
// Re-arm before refreshing, never after: the refresh reads the whole
// repository, so a write that lands while the replacement watcher is
// arming is still picked up by this refresh, and a write that lands
// afterwards is reported by the armed watcher.
if (rearmPending) await rearmWatcher();
await refresh(paths);
},
onError,
@ -291,44 +298,87 @@ export async function startWatchFileLoop(
},
);
const watcher: FSWatcher = watch(repoPath, {
ignoreInitial: true,
atomic: true,
followSymlinks: false,
awaitWriteFinish: { stabilityThreshold: 100, pollInterval: 20 },
ignored: (candidate, stats) => {
const relative = repoRelativeWatchPath(repoPath, candidate);
if (relative !== null && isAnalyzerOwnedWatchPath(relative)) return true;
if (relative !== null && (isIgnoreControlPath(relative) || isConfigControlPath(relative))) {
return false;
const createWatcher = (): FSWatcher => {
const created: FSWatcher = watch(repoPath, {
ignoreInitial: true,
atomic: true,
followSymlinks: false,
awaitWriteFinish: { stabilityThreshold: 100, pollInterval: 20 },
ignored: (candidate, stats) => {
const relative = repoRelativeWatchPath(repoPath, candidate);
if (relative !== null && isAnalyzerOwnedWatchPath(relative)) return true;
if (relative !== null && (isIgnoreControlPath(relative) || isConfigControlPath(relative))) {
return false;
}
return ignorePath(candidate, stats?.isDirectory() ?? false);
},
});
// Events from an instance being retired are kept: they overlap with the
// replacement's coverage and the queue coalesces the duplicates.
created.on('all', (event, changedPath) => {
if (event !== 'add' && event !== 'change' && event !== 'unlink') return;
const relative = repoRelativeWatchPath(repoPath, changedPath);
if (relative && isRelevantWatchPath(relative) && !isAnalyzerOwnedWatchPath(relative)) {
queue.enqueue(relative);
}
return ignorePath(candidate, stats?.isDirectory() ?? false);
},
});
watcher.on('all', (event, changedPath) => {
if (event !== 'add' && event !== 'change' && event !== 'unlink') return;
const relative = repoRelativeWatchPath(repoPath, changedPath);
if (relative && isRelevantWatchPath(relative) && !isAnalyzerOwnedWatchPath(relative)) {
queue.enqueue(relative);
});
created.on('error', (error) => {
// Replacement failures before the swap are reported through `waitUntilReady`.
if (created !== watcher) return;
// Chokidar can surface a transient EPERM on Windows while an ignored
// analyzer-owned path is replaced. Re-arm the watcher and force one
// bounded catch-up refresh so a missed event cannot leave the graph
// stale. Other watcher errors may mean coverage was lost and stay fatal.
if (TRANSIENT_WATCH_ERROR_CODES.has((error as NodeJS.ErrnoException).code ?? '')) {
rearmPending = true;
queue.enqueue(WATCH_FULL_REFRESH_PATH);
return;
}
onWatcherError(error);
});
return created;
};
let watcher: FSWatcher = createWatcher();
// Chokidar emits `ready` once per instance and `add()` returns before the
// rescan it starts has finished, with no signal for that completion. A file
// an ignore-rule reload has just unignored is therefore still unregistered
// when `add()` returns, and because `ignoreInitial` suppresses the `add` the
// rescan would emit, an immediate rewrite of that file is dropped for good
// (reproduced on chokidar 4 and 5). So re-arm by arming a replacement
// watcher and awaiting its `ready` instead. The outgoing instance keeps
// reporting until the replacement is armed, so the swap has no blind window,
// and a replacement that fails to arm leaves the working instance in place.
const rearmWatcher = async (): Promise<void> => {
rearmPending = false;
if (closed) return;
const replacement = createWatcher();
try {
await waitUntilReady(replacement);
} catch (error) {
rearmPending = true;
try {
await replacement.close();
} catch {
// The instance never became live; the arm error is the one to report.
}
throw new WatchControlReloadError(
new Error('Unable to re-arm the filesystem watcher', { cause: error }),
);
}
});
watcher.on('error', (error) => {
// Chokidar can surface a transient EPERM on Windows while an ignored
// analyzer-owned path is replaced. Re-arm the root and force one bounded
// catch-up refresh so a missed event cannot leave the graph stale. Other
// watcher errors may mean coverage was lost and remain fatal.
if (TRANSIENT_WATCH_ERROR_CODES.has((error as NodeJS.ErrnoException).code ?? '')) {
watcher.add(repoPath);
queue.enqueue(WATCH_FULL_REFRESH_PATH);
return;
}
onWatcherError(error);
});
const retired = watcher;
watcher = replacement;
await retired.close();
// `close()` can land between arming the replacement and the swap above.
if (closed) await replacement.close();
};
try {
await waitUntilReady(watcher);
await queue.runInitial();
} catch (error) {
closed = true;
await watcher.close();
await queue.close();
throw error;
@ -337,6 +387,7 @@ export async function startWatchFileLoop(
return {
waitForIdle: () => queue.waitForIdle(),
close: async () => {
closed = true;
await watcher.close();
await queue.close();
},

37
gitnexus/test/integration/watch-filesystem.test.ts
View file

@ -12,7 +12,9 @@ const loops: WatchFileLoop[] = [];
async function waitFor(predicate: () => boolean, timeoutMs = 5_000): Promise<void> {
const deadline = Date.now() + timeoutMs;
while (!predicate()) {
if (Date.now() >= deadline) throw new Error('timed out waiting for watcher event');
if (Date.now() >= deadline) {
throw new Error(`timed out waiting for watcher event after ${timeoutMs}ms`);
}
await new Promise((resolve) => setTimeout(resolve, 25));
}
}
@ -181,6 +183,39 @@ describe('watch filesystem integration', () => {
await waitFor(() => batches.flat().includes('blocked.ts'));
});
it('reports writes issued the instant a gitignore reload re-arms the watcher', async () => {
const repo = await makeRepo();
await fs.writeFile(path.join(repo, '.gitignore'), 'blocked.ts\n', 'utf8');
await fs.writeFile(path.join(repo, 'blocked.ts'), 'export const blocked = 1;', 'utf8');
await fs.writeFile(path.join(repo, 'tracked.ts'), 'export const tracked = 1;', 'utf8');
const batches: string[][] = [];
let rewritten = false;
const loop = await startWatchFileLoop(
repo,
25,
async (paths) => {
batches.push([...paths]);
// Writing from inside the refresh puts these rewrites right after the
// re-arm returns. Polling from the test body instead would leave enough
// slack for a watcher that is not armed yet to look armed.
if (paths.includes('.gitignore') && !rewritten) {
rewritten = true;
await fs.writeFile(path.join(repo, 'blocked.ts'), 'export const blocked = 2;', 'utf8');
await fs.writeFile(path.join(repo, 'tracked.ts'), 'export const tracked = 2;', 'utf8');
}
},
(error) => {
throw error;
},
);
loops.push(loop);
await fs.writeFile(path.join(repo, '.gitignore'), '', 'utf8');
await waitFor(
() => batches.flat().includes('blocked.ts') && batches.flat().includes('tracked.ts'),
);
});
it('keeps the last valid ignore predicate after an oversized reload and later recovers', async () => {
const repo = await makeRepo();
await fs.writeFile(path.join(repo, '.gitignore'), 'blocked.ts\n', 'utf8');