Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
## [Unreleased]

### Fixed
- **File watcher no longer marks edited files as fresh when another process holds the index lock.** When a second writer (concurrent `codegraph index`, a git hook, another MCP daemon) held `.codegraph/codegraph.lock`, `CodeGraph.sync()` returned a zero-shape no-op instead of throwing. The file watcher took that as a successful sync and cleared `pendingFiles` — so the per-file staleness signal MCP tools surface to agents (issue #403) dropped immediately, even though the edit was never indexed. `CodeGraph.watch()` now converts that no-op into a typed `LockUnavailableError` thrown into the watcher; the existing retry path preserves `pendingFiles` and reschedules until the lock becomes available. The error is logged at debug only (no `onSyncError` callback) so a long-running external indexer doesn't spam stderr every debounce cycle. Closes #449.
- **Watch sync no longer aborts with `FOREIGN KEY constraint failed`.** PR #62 plugged this FK violation at the extraction layer (empty-named nodes whose containment edges had no target), but the same violation kept reappearing on v0.9.5 during the daemon's *watch sync* — not on initial index. Once an agent's daemon had been running long enough to accumulate edits, a resolver lookup that crossed a framework-specific cache could hand back a node whose row had been removed by a recent file rewrite, and the FK check then aborted the entire resolution batch, leaving the user's daemon log filling with `Watch sync failed { error: 'FOREIGN KEY constraint failed' }`. `QueryBuilder.insertEdges` now validates every batch's endpoints against the `nodes` table directly (one fresh `SELECT id IN (...)` per batch, no cache) and silently skips edges with missing source or target — so a stale lookup result drops one edge instead of aborting the whole sync. Surfaces as a fresh `codegraph init`/`index` cycle now surviving its first watch-sync cycle without the FK error, and the daemon recovering naturally instead of compounding into further failures. Closes #455.
- **Hermes Agent: `codegraph install --target hermes` no longer corrupts `~/.hermes/config.yaml`.** Hermes serializes its config with PyYAML's default block style, which writes list items at the *same* indent as the parent mapping key (`cli:` and `- hermes-cli` both at column 2). The previous line-based YAML patcher mistook that first ` - hermes-cli` for the next sibling key, truncated the `cli:` block, and then spliced `- mcp-codegraph` at indent 4 *before* the existing items — leaving subsequent entries (`- browser`, `- clarify`, …) and even other platforms (`telegram:`, `discord:`) appearing at the `platform_toolsets:` level, which is no longer parseable YAML. The installer now recognizes the same-indent list style, finds the real end of the block at the next sibling key, and appends `- mcp-codegraph` at whatever indent the existing items already use. Re-installing on an already-corrupted file (or a 4-space-nested config that worked before) still produces a clean, parseable result. Closes #456.
- **NestJS: `RouterModule.register([...])` route prefixes now propagate to controller routes.** Previously a controller declared inside a module wired through NestJS's `RouterModule` (a common pattern for modular apps with nested route prefixes) was indexed with its raw `@Controller(...) + @Get(...)` path — so `UsersController` under `RouterModule.register([{ path: 'admin', module: AdminModule, children: [{ path: 'users', module: UsersModule }] }])` showed up as `GET /` instead of `GET /admin/users`. The new cross-file pass walks every `*.module.{ts,js}` for `RouterModule.register/forRoot/forChild([...])` (recursive `children`) and `@Module({ controllers: [...] })`, then prepends the correct prefix to each affected route — including non-empty `@Controller` paths and method-level params (`/admin/users/:id`). The route node's `id` is preserved across the update so existing route→handler edges stay intact, and the pass is idempotent so incremental sync recovers when `app.module.ts` itself is edited. Closes #459.
Expand Down
48 changes: 42 additions & 6 deletions __tests__/watcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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', () => {
Expand Down
12 changes: 10 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
import { GraphTraverser, GraphQueryManager } from './graph';
import { ContextBuilder, createContextBuilder } from './context';
import { Mutex, FileLock } from './utils';
import { FileWatcher, WatchOptions, PendingFile } from './sync';
import { FileWatcher, WatchOptions, PendingFile, LockUnavailableError } from './sync';

// Re-export types for consumers
export * from './types';
Expand Down Expand Up @@ -75,7 +75,7 @@ export {
defaultLogger,
} from './errors';
export { Mutex, FileLock, processInBatches, debounce, throttle, MemoryMonitor } from './utils';
export { FileWatcher, WatchOptions, PendingFile } from './sync';
export { FileWatcher, WatchOptions, PendingFile, LockUnavailableError } from './sync';
export { MCPServer } from './mcp';

/**
Expand Down Expand Up @@ -495,6 +495,14 @@ export class CodeGraph {
this.projectRoot,
async () => {
const result = await this.sync();
// sync() returns this exact zero-shape iff it failed to acquire the
// file lock (a real empty sync always has filesChecked > 0 because
// scanDirectory ran). Surface that to the watcher as a typed error
// so it keeps pendingFiles + reschedules instead of clearing them
// (#449).
if (result.filesChecked === 0 && result.durationMs === 0) {
throw new LockUnavailableError();
}
const filesChanged = result.filesAdded + result.filesModified + result.filesRemoved;
return { filesChanged, durationMs: result.durationMs };
},
Expand Down
2 changes: 1 addition & 1 deletion src/sync/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* - Incremental reindexing (in extraction module)
*/

export { FileWatcher, WatchOptions, PendingFile } from './watcher';
export { FileWatcher, WatchOptions, PendingFile, LockUnavailableError } from './watcher';
export { watchDisabledReason, detectWsl } from './watch-policy';
export {
installGitSyncHook,
Expand Down
29 changes: 26 additions & 3 deletions src/sync/watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ export interface WatchOptions {
onSyncError?: (error: Error) => void;
}

/**
* Thrown by a `syncFn` to signal that the underlying sync couldn't acquire
* the cross-process write lock (#449). The watcher treats this as "no
* progress" — preserves `pendingFiles`, skips `onSyncComplete`, and the
* `finally` block reschedules. Quiet (debug-only) because a long-running
* external indexer can hit this every debounce cycle.
*/
export class LockUnavailableError extends Error {
constructor(message = 'CodeGraph file lock unavailable; another process is writing') {
super(message);
this.name = 'LockUnavailableError';
}
}

/**
* Per-file pending entry — tracks a source file the watcher saw an event for
* but hasn't yet synced into the index. Exposed via {@link FileWatcher.getPendingFiles}
Expand Down Expand Up @@ -352,11 +366,20 @@ export class FileWatcher {
}
this.onSyncComplete?.(result);
} catch (err) {
const error = err instanceof Error ? err : new Error(String(err));
logWarn('Watch sync failed', { error: error.message });
if (err instanceof LockUnavailableError) {
// Lock-failure no-op (another writer holds the lock). pendingFiles
// stays intact and the `finally` block reschedules. Debug-only —
// a long external index would otherwise spam stderr every cycle.
logDebug('Watch sync skipped: file lock unavailable', {
pendingFiles: this.pendingFiles.size,
});
} else {
const error = err instanceof Error ? err : new Error(String(err));
logWarn('Watch sync failed', { error: error.message });
this.onSyncError?.(error);
}
// Failure: leave pendingFiles untouched. Every edit it tracks is
// still unindexed; the rescheduled sync sees the same set.
this.onSyncError?.(error);
} finally {
this.syncing = false;

Expand Down