From 05eb7a53cb84d921e37e425958dfb3affd463406 Mon Sep 17 00:00:00 2001 From: andreinknv Date: Sun, 26 Apr 2026 01:27:34 -0400 Subject: [PATCH] fix: correctness bugs found in audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four user-visible bugs caught by an independent audit pass: 1. Svelte symbols reported on the wrong line src/extraction/svelte-extractor.ts:144 The script-block regex captures content starting with the leading newline that follows `>`, so the inner extractor sees that newline as line 1 of its 1-indexed input and the first real code on line 2. The previous `contentStartLine = scriptTagLine + openingTagLines + 1` was added to that 1-indexed line number, shifting every Svelte symbol's startLine / endLine off by 1. Drops the `+1`. Five regression tests added covering single-line, multi-line opening tag, template-offset, single-line no-newline, and dual module/instance script blocks. 2. Watcher silently dropped pending changes on sync failure src/sync/watcher.ts:177 `hasChanges = false` ran before the sync attempt, so a thrown sync (DB locked, transient FS error) left the pending batch forgotten until a NEW file event arrived. Re-set `hasChanges = true` in the catch path so a transient failure schedules a retry on its own. Regression test added (mocks fail-then-succeed, asserts the second call happens without a new file event). 3. Graph traversal default maxDepth was Infinity src/graph/traversal.ts:14, src/types.ts:301 `limit: 1000` capped returned nodes, but during traversal the visited set and BFS/DFS frontier can grow far beyond `limit` on highly connected graphs before the cap kicks in. Default is now 10. Callers who really need exhaustive traversal can still pass `maxDepth: Infinity` explicitly — the JSDoc documents this. This is a public-API behavior change; existing tests pass. Also caps `findPath`'s BFS queue at 100,000 entries (FIND_PATH_MAX_QUEUE) and returns null if exceeded — each entry holds a cloned path array, so on dense graphs the queue could otherwise consume gigabytes before either finding a path or exhausting the search. 4. `findRelevantContext` did not bound caller-supplied limits src/context/index.ts:284 `searchLimit` is multiplied by 5 in `findNodesByExactName` and feeds several other unbounded operations; a caller passing `searchLimit: 1_000_000` would pull millions of rows. Now clamped: searchLimit ∈ [1, 100], maxNodes ∈ [1, 1000], traversalDepth ∈ [0, 10]. Regression test asserts a 1e9 input is bounded. All 387 tests pass serialized; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- __tests__/context.test.ts | 13 ++++++ __tests__/extraction.test.ts | 69 ++++++++++++++++++++++++++++++ __tests__/watcher.test.ts | 30 +++++++++++++ src/context/index.ts | 8 ++++ src/extraction/svelte-extractor.ts | 10 +++-- src/graph/traversal.ts | 23 +++++++++- src/sync/watcher.ts | 12 +++++- src/types.ts | 7 ++- 8 files changed, 165 insertions(+), 7 deletions(-) diff --git a/__tests__/context.test.ts b/__tests__/context.test.ts index 52dae1fe..9a0614aa 100644 --- a/__tests__/context.test.ts +++ b/__tests__/context.test.ts @@ -210,6 +210,19 @@ export function validateEmail(email: string): boolean { expect(result.nodes.size).toBeLessThanOrEqual(5); }); + + it('should clamp absurd searchLimit/maxNodes values to safe upper bounds', async () => { + // Without clamping, the internal `findNodesByExactName` query would + // request `searchLimit * 5` rows — passing 1e9 here would blow out + // memory. The call should complete in normal time and not return more + // than the hard cap on maxNodes (1000). + const result = await cg.findRelevantContext('function', { + searchLimit: 1_000_000_000, + maxNodes: 1_000_000_000, + traversalDepth: 1_000, + }); + expect(result.nodes.size).toBeLessThanOrEqual(1000); + }); }); describe('buildContext()', () => { diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 8a70ffed..a6fd7687 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -3079,3 +3079,72 @@ describe('Directory Exclusion', () => { expect(files.every((f) => !f.includes('vendor'))).toBe(true); }); }); + +// ============================================================================= +// Svelte line-number regressions (audit fix) +// ============================================================================= + +describe('Svelte line numbering', () => { + it('reports symbol line numbers relative to the .svelte file, not the script content', () => { + // Line 1: + const code = `\n`; + const result = extractFromSource('Comp.svelte', code); + const fn = result.nodes.find((n) => n.kind === 'function' && n.name === 'add'); + expect(fn).toBeDefined(); + expect(fn?.startLine).toBe(2); + }); + + it('handles multi-line opening tags (script with attributes wrapped)', () => { + // Line 1: + const code = `\nfunction greet() { return "hi"; }\n\n`; + const result = extractFromSource('Comp.svelte', code); + const fn = result.nodes.find((n) => n.kind === 'function' && n.name === 'greet'); + expect(fn).toBeDefined(); + expect(fn?.startLine).toBe(3); + }); + + it('preserves correct line numbers when the script block is offset by template lines', () => { + // Line 1:

Hello

+ // Line 2: + // Line 3: + const code = `

Hello

\n\n\n`; + const result = extractFromSource('Comp.svelte', code); + const fn = result.nodes.find((n) => n.kind === 'function' && n.name === 'bottom'); + expect(fn).toBeDefined(); + expect(fn?.startLine).toBe(4); + }); + + it('handles a single-line script block with no internal newline', () => { + // Line 1: + const code = `\n`; + const result = extractFromSource('Comp.svelte', code); + const fn = result.nodes.find((n) => n.kind === 'function' && n.name === 'inline'); + expect(fn).toBeDefined(); + expect(fn?.startLine).toBe(1); + }); + + it('attributes each block correctly when a file has both module and instance scripts', () => { + // Line 1: + // Line 4: + // Line 5: + const code = + `\n` + + `\n\n`; + const result = extractFromSource('Comp.svelte', code); + const moduleFn = result.nodes.find((n) => n.kind === 'function' && n.name === 'moduleHelper'); + const instanceFn = result.nodes.find((n) => n.kind === 'function' && n.name === 'instanceHelper'); + expect(moduleFn?.startLine).toBe(2); + expect(instanceFn?.startLine).toBe(6); + }); +}); diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index f3638e6d..ee732df6 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -218,6 +218,36 @@ describe('FileWatcher', () => { watcher.stop(); }); + + it('should retry pending changes after a sync failure (no events lost)', async () => { + // First call rejects, subsequent calls resolve. After the initial + // failure, the watcher should retry the same batch on its own — without + // this, transient sync failures (DB locked etc.) would silently drop the + // changes until a new file event happened. + let calls = 0; + const syncFn = vi.fn().mockImplementation(() => { + calls++; + if (calls === 1) return Promise.reject(new Error('transient')); + return Promise.resolve({ filesChanged: 1, durationMs: 5 }); + }); + const onSyncError = vi.fn(); + const onSyncComplete = vi.fn(); + const watcher = new FileWatcher(testDir, baseConfig, syncFn, { + debounceMs: 100, + onSyncError, + onSyncComplete, + }); + + watcher.start(); + fs.writeFileSync(path.join(testDir, 'src', 'test.ts'), 'export const z = 3;'); + + await waitFor(() => onSyncComplete.mock.calls.length > 0, 5000); + expect(onSyncError).toHaveBeenCalledTimes(1); + expect(syncFn).toHaveBeenCalledTimes(2); + expect(onSyncComplete).toHaveBeenCalledWith({ filesChanged: 1, durationMs: 5 }); + + watcher.stop(); + }); }); describe('CodeGraph integration', () => { diff --git a/src/context/index.ts b/src/context/index.ts index 94192377..08f25657 100644 --- a/src/context/index.ts +++ b/src/context/index.ts @@ -286,6 +286,14 @@ export class ContextBuilder { options: FindRelevantContextOptions = {} ): Promise { const opts = { ...DEFAULT_FIND_OPTIONS, ...options }; + // Bound user-supplied limits — `searchLimit` is multiplied by 5 in + // findNodesByExactName (line 312) and feeds several other unbounded + // operations below, so a request with `searchLimit: 1_000_000` would + // pull millions of rows before any filtering. 100 is well above the + // largest legitimate use we've seen. + opts.searchLimit = Math.min(Math.max(1, opts.searchLimit), 100); + opts.maxNodes = Math.min(Math.max(1, opts.maxNodes), 1000); + opts.traversalDepth = Math.min(Math.max(0, opts.traversalDepth), 10); // Start with empty subgraph const nodes = new Map(); diff --git a/src/extraction/svelte-extractor.ts b/src/extraction/svelte-extractor.ts index 5586ee34..323cbe80 100644 --- a/src/extraction/svelte-extractor.ts +++ b/src/extraction/svelte-extractor.ts @@ -135,13 +135,17 @@ export class SvelteExtractor { // Detect module script const isModule = /context\s*=\s*["']module["']/.test(attrs); - // Calculate start line of the script content (line after