diff --git a/CHANGELOG.md b/CHANGELOG.md index 37b6c711e..3e52b2c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -402,6 +402,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Python, a call on a module imported from outside your project, like `subprocess.run(…)` after `import subprocess`, no longer links to a project method that happens to share its name. Thanks @Egrn for the report. (#2599) - `codegraph_explore`, the CLI `codegraph explore` and the prompt hook no longer stall for a minute or more on a large project after a full index; the step that finds the project's busiest file no longer slows to a crawl when a few files are huge, like a generated Android `R.java` (#2571). Thanks @yangfengfan2011. - `codegraph_explore`, the CLI `codegraph explore` and the Claude Code prompt hook now answer a long query, like a pasted log, document or source file, in seconds on a large project instead of minutes. Every word of such a query used to search the whole index several times over, so the prompt hook ran out of time on long prompts and added nothing. Those searches now reuse what they read, and a very long query is searched by its code names and its opening words rather than by every word in it. Answers to ordinary questions and lists of symbol names are unchanged. Thanks @retsamekohC for the report and the first fix. (#2620) +- A search with a `path:` or `name:` filter, in `codegraph query`, the `codegraph_search` tool or the library's `searchNodes()`, now finds the symbols it asks for at any limit, and an offset pages through those symbols alone. Before, the filter was applied only after the results had been cut to the limit, so `codegraph query "authenticate path:src/api"` could find nothing while a much larger `--limit` showed the match. Thanks @rudycelekli. +- A search in `codegraph query`, the `codegraph_search` tool or the library's `searchNodes()` now finds the symbols a pasted call names, like `res.send(body)` or `leaf(entry)`: a parenthesis, quote, `*`, `^` or `:` between two words now separates them, as it does in the index. Before, those characters were dropped, which glued the words around them into one that no symbol has, so such a search came back empty or missed the names it was given. Thanks @inth3shadows for the report and @kjgggggg for the first fix. (#2971) - The Claude Code prompt hook now goes by what you typed. Claude Code's desktop app puts status notes, like which other sessions are linked, in front of your prompt, and the hook read them as part of it, so every turn, even a one-word "yes", could get code context about the note's words, and so could a task notification, a subagent's report or a message from another session that came after one. The notes are now skipped, and a message from another session gets no context, as task notifications and subagent reports already didn't. Thanks @WonderMr for the report and @yang0228 for the first fix. (#2635) - `codegraph_explore` and `codegraph context` now find your project's busiest file, and favor the code around it, even when twenty or more test or generated files are busier still, such as a package of protobuf stubs. Before, the search looked only at the 20 busiest files and then set the tests and generated ones aside, so it could find no file at all, or judge one far ahead of the next when it was not. `getTopRouteFile()` in the library counts every file's routes the same way. Thanks @inth3shadows for the report. (#2578) - In JavaScript and TypeScript, a call written as the body of an `if`, `while`, `for` or `do` without braces, like `if (e.key === "Escape") setOpen(false);`, no longer links to a same-named property or method in another file when the name is the file's own, such as a `setOpen` from `useState` (#2585). @@ -452,10 +454,14 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph index` no longer stops with advice to run `codegraph daemon stop`, a command that doesn't exist, when the project's background-server lock names a process that isn't CodeGraph, such as a lock left by a crash whose process number the system has since given to another program: it now clears that stale lock, leaving the process alone, and rebuilds, and `codegraph unlock` clears it too. When a CodeGraph process that doesn't answer holds the lock, such as an older background server, the message names that process and the command that stops it, and the message about another server already serving a project now points to `codegraph daemon`. Thanks @inth3shadows for the report and @danusha2345 for the first fix. (#2770) - A program that uses CodeGraph as a library and indexes chosen files with `indexFiles()` now keeps other CodeGraph processes, like a `codegraph sync` run by a git hook, out of the index until those files are stored. Before, it let go of the write lock that keeps two writers apart as soon as indexing started, so another process could write to the index at the same time. +- When several CodeGraph processes start at the same moment after one of them crashed, like the MCP server, a git hook's `codegraph sync` and `codegraph index`, only one of them now takes over the write lock the crashed process left behind. Before, two of them could both take it, the second one deleting the first one's lock as it did, and both went on to write to the index at once. A write lock held by another user's CodeGraph, or on Windows by one running as administrator, is no longer mistaken for a crashed one's and taken over either. Thanks @inth3shadows for the report and @vedantb7 for the first fix. (#2979) +- A program that uses CodeGraph as a library and runs `indexAll()` again on a project it has already indexed now keeps every symbol findable by the words in its name, which the prompt hook and `codegraph_explore` use to match a question like "how does the order state machine work" to `OrderStateMachine`. Before, only the files changed since the last index kept their names' words, and a cancelled run lost them all; `codegraph index` was not affected. Thanks @rudycelekli. - The shared background server is no longer killed with "Main thread unresponsive" while it brings another project's index up to date, such as one a session names with `projectPath`: only its own project's index counted as progress, so a long update of any other looked like a hang, and the server was stopped and restarted again and again. Every index the server keeps up to date now counts, and so does the one a session updates on its own when the background server is unavailable. Thanks @mradwankhalil for the report and the first fix. (#2404) - A project a session queries through `projectPath` is now let go once it has gone its own 10 minutes without a query. Before, when its index was still being brought up to date as the session went on to query another project, it was held, locked, until that other project's 10 minutes were up, so its own session and `codegraph index` could wait nearly twice as long to take over. Thanks @bompus. (#2087) - An index left incomplete by an indexing run that was stopped partway, by a crash, an out-of-memory kill or the server's own hang check, no longer gets the background server killed with "Main thread unresponsive" every time it starts: on a large project, restoring the database indexes the run had set aside took longer than the hang check allows, so each restart died at the same point and the index was never repaired. They are now restored in the background, and the project is served once they are back. Thanks @cbeaulieu-gt for the report and the first fix. (#1887) - A new agent session no longer waits up to about 12 minutes for its first answer when the background server is stuck: a server whose process still holds its connection open but never responds made the session wait out a long series of retries. The session now gives up within seconds and answers on its own, and switches back to the background server once that has been restarted. +- An MCP session whose agent stops reading its answers, while keeping the session open, now shuts down the normal way the next time it answers: it stops its work, frees the project for the next session and exits cleanly, and other sessions sharing the background server carry on. Before, that answer crashed it with "Uncaught exception: Error: EPIPE". Thanks @iiijiashu for the report and the first fix. (#2991) +- A tool call whose arguments aren't an object, like `"arguments": "alpha"` from a hand-written MCP client or script, now gets the standard invalid-params error saying the arguments must be an object, and no tool runs. Before, a string or a number made most tools fail and come back as an internal CodeGraph error that told the agent to retry and then to continue without CodeGraph, and an array was run as if it were the arguments. Thanks @inth3shadows for the report and @kjgggggg for the first fix. (#2981) - The bundled Node runtime moves to 24.22.1, Node's October security release, and every release now checks the Node it bundles against the checksums nodejs.org publishes. The new runtime also fixes a crash on Windows as a command like `codegraph sync` exits, after its work is done, printing `Assertion failed: !(handle->flags & UV_HANDLE_CLOSING)`, which made a script or CI run that had succeeded fail. Thanks @Ancienttwo for the report and the first fix, and @IdealBoy709 for the report on the security release. (#2617, #2780) - Building CodeGraph from source with bun, as in `bun run build`, no longer runs until it is stopped: bun ran one step of the build as the whole build again, so it kept starting itself over. Building with npm is unchanged. Thanks @bompus. - When CodeGraph can't open a project's database, it now says what went wrong: SQLite's error, the file's path, and what to check, such as another program holding the file open, a folder you can't write to, or a network share. Before, every such failure read "CodeGraph requires node:sqlite (Node.js 22.5+)", and an agent was told no project was loaded. That hint now appears only when Node really can't load its built-in SQLite, a case that used to end in an unhandled error instead. @@ -465,6 +471,9 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - For Hermes Agent, `codegraph install` and `upgrade` no longer add `mcp-codegraph` to `platform_toolsets.cli`. Hermes up to 0.22.2 reported that name as an unknown toolset on every start, and Hermes 0.22.4 reads it as the only MCP server to load, hiding your others from CLI sessions; an entry an earlier install added is taken out again. A refresh also no longer resets your own `mcp_servers.codegraph` settings, such as an absolute command path, timeouts or `enabled: false`, and a config indented or written differently from Hermes's default no longer gets a duplicate or unreadable entry. Thanks @Huy3ko for the report and @conifercone for confirming it. (#2485) - `codegraph upgrade` now upgrades a global npm install when run from your home folder, which holds npm's global folder on Windows and with tools like nvm: before, it ran `npm install` without `-g` there, which left the global copy on the old version and put a second copy in your home folder. A copy installed in a project is now upgraded in that project wherever you run the command, with a warning if npm left it on the old version, and a copy installed with pnpm, yarn or bun gets the command that upgrades it instead of npm. Thanks @timlincool for the report and @maxmilian for the first fix. (#2477) - `codegraph status` no longer says the index is up to date while indexed files are missing their symbols: it now names files the parser couldn't read and files stored without their symbols (which `codegraph sync` repairs), `status --json` counts both, and `codegraph files --json` lists each file's recorded errors. Thanks @lipchey for the report. (#2336) +- Running `codegraph status` while `codegraph index` rebuilds a project no longer makes the rebuild fail with "database is locked": status now only reads the index, and it says when a rebuild is running in another process, in its output and as `rebuildInProgress` in `status --json`. Thanks @sealad886 for the report, and @danusha2345 and @KissFeng for the fixes. (#2913) +- Running `codegraph query`, `explore`, `node`, `callers`, `callees`, `impact`, `context`, `files` or `affected` while `codegraph index` rebuilds a project no longer makes the rebuild fail with "database is locked", and neither does the context hook Claude Code runs on its own when you ask about your code: like `codegraph status`, they now only read the index and say when a rebuild is running, and the hook stays quiet until the rebuild is done. They still answer from an index an older CodeGraph version built. Thanks @sealad886 for the report. (#2913) +- Opening a project whose index needs a schema update no longer fails when several CodeGraph processes open it at the same moment, like agent sessions starting right after an upgrade, or the MCP server and a git hook: one of them updates the index while the others wait for it and then carry on. Before, the ones that lost the race applied the same update again, and failed with "UNIQUE constraint failed" or "database is locked". Thanks @inth3shadows for the report and @johnhldiver-beep for the first fix. (#2980) - A qualified name given to `codegraph callers`, `callees` or `impact`, or to `codegraph_node`, like `Format.Price`, now names only the symbol whose scope is spelled that way, so its answer no longer merges in `LegacyFormat.Price` or any other class whose name only ends with `Format`, nor a class that just sits in a file named `Format.cs`. In C#, `BasketService.AddItemToBasket` no longer also selects the `IBasketService` interface's method; ask for `IBasketService.AddItemToBasket` for that one. A file or folder still names the module in Rust, Python and Go, and a PHP namespace can be written with its own backslashes, like `Billing\Format::price`. No re-index needed. Thanks @Gobakabana for the report and @yang0228. (#2908) - `codegraph affected --filter` now matches its glob against whole names: `tests/**/*.py` also finds the tests directly inside `tests/`, including a changed test file itself, `tests/*.py` no longer counts a changed `tests/x.py.bak` as a test, and `e2e/*.spec.ts` no longer matches a folder that only ends in `e2e`, like `xe2e/`. A pattern with a folder in it still matches that folder at any depth, so `e2e/*.spec.ts` keeps finding `apps/web/e2e/login.spec.ts`; start it with `/` to match from the project root only. Thanks @Jseca666. - `codegraph init` and `codegraph index` no longer report a file you deleted as one that "could not be read" while git still lists it, which it does until the deletion is staged: such a file is now skipped, and so is a symlink that points at nothing. A file that is there but can't be read is still reported. Thanks @xianzhi-xzw for the report and the first fix. (#2904) @@ -477,6 +486,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Vue and Nuxt components, calls written in the template (`{{ formatDate(post) }}`, `:to="getAccountRoute(account)"`, `@click="save(item)"`) and calls in a destructuring declaration at the top of ` +`; + const result = extractFromSource('Page.astro', code); + + expect(result.errors).toEqual([]); + const ready = result.nodes.find((n) => n.name === 'ready'); + expect(ready).toBeDefined(); + expect(ready?.startLine).toBe(8); + // The markup between the two scripts is still scanned. + const call = result.unresolvedReferences.find( + (ref) => ref.referenceKind === 'calls' && ref.referenceName === 'formatDate' && ref.line === 6 + ); + expect(call).toBeDefined(); + }); + + it('should not let a self-closing +`; + const result = extractFromSource('Styled.astro', code); + + const call = result.unresolvedReferences.find( + (ref) => ref.referenceKind === 'calls' && ref.referenceName === 'formatDate' && ref.line === 2 + ); + expect(call).toBeDefined(); + }); + + it("still reads a call in a self-closing +`; + const result = extractFromSource('Schema.astro', code); + + expect(result.errors).toEqual([]); + expect(result.nodes.find((n) => n.name === 'ready')?.startLine).toBe(7); + const call = result.unresolvedReferences.find( + (ref) => ref.referenceKind === 'calls' && ref.referenceName === 'serializeJsonLd' && ref.line === 5 + ); + expect(call).toBeDefined(); + }); }); describe('Instantiates + Decorates edge extraction', () => { diff --git a/__tests__/filelock-stale-takeover.test.ts b/__tests__/filelock-stale-takeover.test.ts new file mode 100644 index 000000000..fba7755a9 --- /dev/null +++ b/__tests__/filelock-stale-takeover.test.ts @@ -0,0 +1,224 @@ +/** + * Taking over a stale `codegraph.lock` must leave exactly one holder (#2979). + * + * `FileLock.acquire()` used to read the lock, see a dead PID, unlink the file + * and create its own. Two processes that both read the same dead PID could + * then both succeed: the first replaced the stale lock, and the second, acting + * on what it had read a moment earlier, unlinked the first one's live lock and + * created its own. Both then wrote to the index as its only writer. + * + * The two-process cases pin that interleaving with a barrier rather than + * timing: the slow contender stops right after reading the stale lock and + * resumes only once the fast one holds it (see fixtures/filelock-contender.cjs). + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { spawn, type ChildProcess } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { FileLock } from '../src/utils'; + +const UTILS = path.resolve(__dirname, '../dist/utils.js'); +const CONTENDER = path.resolve(__dirname, 'fixtures/filelock-contender.cjs'); +/** Above every platform's PID range (and odd, which no Windows PID is): never a live process. */ +const DEAD_PID = '99999999'; + +interface Contender { + child: ChildProcess; + name: string; + stderr: () => string; + exited: Promise; +} + +describe('FileLock stale takeover (#2979)', () => { + let dir: string; + let lockPath: string; + let markers: string; + let contenders: Contender[] = []; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-filelock-')); + lockPath = path.join(dir, 'codegraph.lock'); + markers = path.join(dir, 'markers'); + fs.mkdirSync(markers); + }); + + afterEach(async () => { + for (const { child, exited } of contenders) { + if (child.exitCode === null && child.signalCode === null) child.kill(); + await exited; + } + contenders = []; + fs.rmSync(dir, { recursive: true, force: true }); + }); + + function track(name: string, args: string[]): Contender { + const child = spawn(process.execPath, args, { + // Off the fixture, so a child still exiting cannot hold it (Windows EBUSY). + cwd: os.tmpdir(), + stdio: ['ignore', 'ignore', 'pipe'], + windowsHide: true, + }); + let stderr = ''; + child.stderr!.on('data', (chunk) => { stderr += String(chunk); }); + const exited = new Promise((resolve) => child.once('exit', (code) => resolve(code))); + const contender = { child, name, stderr: () => stderr, exited }; + contenders.push(contender); + return contender; + } + + const start = (name: string, mode: 'pause' | 'run') => + track(name, [CONTENDER, UTILS, lockPath, markers, name, mode]); + /** A live process that never touches the lock: its PID names a holder that is alive. */ + const idle = () => track('idle', ['-e', 'setInterval(() => {}, 1000)']); + + /** The first of `names` (marker files) to appear, and its content. */ + async function firstMarker(names: string[], from: Contender[]): Promise<{ name: string; text: string }> { + const deadline = Date.now() + 20_000; + for (;;) { + for (const name of names) { + const file = path.join(markers, name); + if (fs.existsSync(file)) return { name, text: fs.readFileSync(file, 'utf8') }; + } + if (Date.now() > deadline) { + const detail = from.map((c) => `${c.name} (exit ${c.child.exitCode}): ${c.stderr()}`).join('\n'); + throw new Error(`none of ${names.join(', ')} appeared\n${detail}`); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + } + + const release = (who: Contender) => fs.writeFileSync(path.join(markers, `${who.name}.release`), ''); + const go = () => fs.writeFileSync(path.join(markers, 'go'), ''); + + it('lets only one of two processes take over a stale lock', async () => { + fs.writeFileSync(lockPath, DEAD_PID); + + // The slow process reads the dead PID, then stalls before acting on it. + const slow = start('slow', 'pause'); + const read = await firstMarker(['slow.read'], [slow]); + expect(read.text.trim()).toBe(DEAD_PID); + + // Meanwhile another process takes the stale lock over, and holds it. + const fast = start('fast', 'run'); + expect((await firstMarker(['fast.acquired', 'fast.refused'], [fast])).name).toBe('fast.acquired'); + + // The slow one resumes with the dead PID it read: it must not replace a live lock. + go(); + const slowOutcome = await firstMarker(['slow.acquired', 'slow.refused'], [slow]); + expect(slowOutcome.name, 'both processes held the lock at once').toBe('slow.refused'); + expect(slowOutcome.text).toMatch(/locked by another process/); + expect(fs.readFileSync(lockPath, 'utf8').trim()).toBe(String(fast.child.pid)); + + release(fast); + expect(await fast.exited).toBe(0); + expect(await slow.exited).toBe(0); + expect(fs.existsSync(lockPath)).toBe(false); + }, 60_000); + + it('still lets the slow process in once the other one has released the lock', async () => { + fs.writeFileSync(lockPath, DEAD_PID); + + const slow = start('slow', 'pause'); + await firstMarker(['slow.read'], [slow]); + const fast = start('fast', 'run'); + expect((await firstMarker(['fast.acquired', 'fast.refused'], [fast])).name).toBe('fast.acquired'); + release(fast); + await firstMarker(['fast.released'], [fast]); + expect(await fast.exited).toBe(0); + + // The lock is free now: what the slow one read is out of date in the other direction. + go(); + expect((await firstMarker(['slow.acquired', 'slow.refused'], [slow])).name).toBe('slow.acquired'); + expect(fs.readFileSync(lockPath, 'utf8').trim()).toBe(String(slow.child.pid)); + release(slow); + expect(await slow.exited).toBe(0); + expect(fs.existsSync(lockPath)).toBe(false); + }, 60_000); + + it('leaves a stale lock to the process already taking it over', async () => { + // A live process holds the takeover sentinel: it is replacing the stale lock right now. + const breaker = idle(); + fs.writeFileSync(lockPath, DEAD_PID); + fs.writeFileSync(`${lockPath}.break`, String(breaker.child.pid)); + + const lock = new FileLock(lockPath); + expect(() => lock.acquire()).toThrow(new RegExp(`locked by another process \\(PID ${breaker.child.pid}\\)`)); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(DEAD_PID); + expect(fs.readFileSync(`${lockPath}.break`, 'utf8')).toBe(String(breaker.child.pid)); + }); + + it('takes over a stale lock whose takeover sentinel a dead process left behind', () => { + fs.writeFileSync(lockPath, DEAD_PID); + fs.writeFileSync(`${lockPath}.break`, DEAD_PID); + + const lock = new FileLock(lockPath); + lock.acquire(); + try { + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + expect(fs.readdirSync(dir).sort()).toEqual(['codegraph.lock', 'markers']); + } finally { + lock.release(); + } + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it('treats an empty lock file its creator may still be writing as held, and an old one as stale', () => { + // A filesystem without hard links creates the lock first and writes the PID after. + fs.writeFileSync(lockPath, ''); + const lock = new FileLock(lockPath); + expect(() => lock.acquire()).toThrow(/locked by another process/); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(''); + + // Left empty by a crash long ago: stale. + const old = new Date(Date.now() - 60_000); + fs.utimesSync(lockPath, old, old); + lock.acquire(); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + lock.release(); + expect(fs.existsSync(lockPath)).toBe(false); + }); + + it('treats a PID it may not signal (EPERM) as a live holder', () => { + // Another user's process, or an elevated one on Windows: alive, just not ours to signal. + const foreign = 4242; + fs.writeFileSync(lockPath, String(foreign)); + const kill = vi.spyOn(process, 'kill').mockImplementation(((pid: number, signal?: string | number) => { + if (pid === foreign && signal === 0) throw Object.assign(new Error('kill EPERM'), { code: 'EPERM' }); + return true; + }) as typeof process.kill); + try { + expect(() => new FileLock(lockPath).acquire()).toThrow(/locked by another process \(PID 4242\)/); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(foreign)); + } finally { + kill.mockRestore(); + } + }); + + it.runIf(process.platform === 'win32')('keeps a lock whose holder Windows will not let us open, like an elevated process', () => { + // PID 4 is always the System process: opening it to signal is refused (EPERM), as for an elevated one. + fs.writeFileSync(lockPath, '4'); + expect(() => new FileLock(lockPath).acquire()).toThrow(/locked by another process \(PID 4\)/); + expect(fs.readFileSync(lockPath, 'utf8')).toBe('4'); + }); + + it.runIf(process.platform !== 'win32')('does not write through a symlink planted at the private file path', async () => { + const { threadId } = await import('worker_threads'); + const victim = path.join(dir, 'victim.txt'); + fs.writeFileSync(victim, 'keep me'); + fs.symlinkSync(victim, `${lockPath}.${process.pid}.${threadId}.tmp`); + const lock = new FileLock(lockPath); + lock.acquire(); + expect(fs.readFileSync(victim, 'utf8')).toBe('keep me'); + expect(fs.readFileSync(lockPath, 'utf8')).toBe(String(process.pid)); + lock.release(); + }); + + it('leaves nothing but the lock file while held, and nothing after release', () => { + const lock = new FileLock(lockPath); + lock.acquire(); + expect(fs.readdirSync(dir).sort()).toEqual(['codegraph.lock', 'markers']); + lock.release(); + expect(fs.readdirSync(dir)).toEqual(['markers']); + }); +}); diff --git a/__tests__/fixtures/filelock-contender.cjs b/__tests__/fixtures/filelock-contender.cjs new file mode 100644 index 000000000..cfd68ac8f --- /dev/null +++ b/__tests__/fixtures/filelock-contender.cjs @@ -0,0 +1,57 @@ +/** + * One contender for a `FileLock`, run as its own process by + * filelock-stale-takeover.test.ts (#2979). + * + * node filelock-contender.cjs + * + * It takes the lock with the built `FileLock`, writes `.acquired` (and + * holds the lock until `.release` appears) or `.refused`, then + * exits. With `` = `pause`, it stops right after its first read of the + * lock file, with whatever that read returned in hand, writes `.read`, + * and waits for `go`: the moment a slow process has decided a lock is stale + * but has not acted on it yet. Every wait is on a marker file the test writes, + * so the interleaving is the same on every run. A contender gives up after + * 30 s of waiting, so none outlives a test that stopped driving it. + */ +const fs = require('fs'); +const path = require('path'); + +const [utilsJs, lockPath, markers, name, pause] = process.argv.slice(2); +const marker = (suffix) => path.join(markers, `${name}.${suffix}`); +const mark = (suffix, text = String(process.pid)) => fs.writeFileSync(marker(suffix), text); +const sleep = (ms) => Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +const waitFor = (file) => { + const deadline = Date.now() + 30_000; + while (!fs.existsSync(file)) { + if (Date.now() > deadline) process.exit(3); + sleep(5); + } +}; + +if (pause === 'pause') { + const target = path.resolve(lockPath); + const read = fs.readFileSync; + let paused = false; + fs.readFileSync = function readThenPause(file, ...rest) { + const content = read.call(this, file, ...rest); + if (!paused && typeof file === 'string' && path.resolve(file) === target) { + paused = true; + mark('read', String(content)); + waitFor(path.join(markers, 'go')); + } + return content; + }; +} + +const { FileLock } = require(utilsJs); +const lock = new FileLock(lockPath); +try { + lock.acquire(); +} catch (err) { + mark('refused', err instanceof Error ? err.message : String(err)); + process.exit(0); +} +mark('acquired'); +waitFor(marker('release')); +lock.release(); +mark('released'); diff --git a/__tests__/gitignore-pattern-parity.test.ts b/__tests__/gitignore-pattern-parity.test.ts new file mode 100644 index 000000000..9d7e3932a --- /dev/null +++ b/__tests__/gitignore-pattern-parity.test.ts @@ -0,0 +1,68 @@ +/** + * codegraph applies .gitignore rules itself in two places: on the filesystem + * walk (a project without git), for every .gitignore it meets, and in a git + * project on top of `git ls-files`, for its built-in defaults merged with the + * root .gitignore. Both go through the `ignore` package, which before 7.0.14 + * read several pattern forms differently from git: + * - a backslash escape: `\d` was a regex digit, not a literal `d`; + * - a POSIX class such as `[[:digit:]]`, and a `]` or `!` first in a bracket; + * - a run of three asterisks, which git reads as a globstar; + * - a lone `!` line, which matched every path, so it re-included everything + * the .gitignore and codegraph's own defaults (node_modules) leave out. + * Each fixture file is kept or dropped the way git decides, except + * node_modules, which codegraph's defaults always leave out. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { scanDirectory } from '../src/extraction'; + +const roots: string[] = []; + +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); + +function project(mode: 'walk' | 'git', gitignore: string, files: string[]): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), `cg-gitignore-parity-${mode}-`)); + roots.push(root); + for (const file of files) { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), 'export const value = 1;\n'); + } + fs.writeFileSync(path.join(root, '.gitignore'), gitignore); + if (mode === 'git') execFileSync('git', ['init', '-q'], { cwd: root, stdio: 'ignore' }); + return root; +} + +function scanned(root: string): string[] { + return scanDirectory(root).map((f) => f.replace(/\\/g, '/')).sort(); +} + +describe.each(['walk', 'git'] as const)('.gitignore patterns follow git (%s)', (mode) => { + it('reads escapes, POSIX classes, brackets, *** and trailing spaces as git does', () => { + const root = project( + mode, + [ + '\\d.ts', // a literal "d" + '[[:digit:]]x.ts', // a POSIX class + '[!a]neg.ts', // a negated bracket + '[]]bracket.ts', // "]" first in a bracket is a member + 'stars/***/z.ts', // three asterisks are a globstar + 'trail.ts ', // trailing spaces are trimmed + ].join('\n') + '\n', + [ + '1.ts', 'd.ts', '1x.ts', 'ax.ts', 'aneg.ts', 'bneg.ts', ']bracket.ts', + 'stars/z.ts', 'stars/b/z.ts', 'stars/b/c/z.ts', 'trail.ts', 'other.ts', + ], + ); + expect(scanned(root)).toEqual(['1.ts', 'aneg.ts', 'ax.ts', 'other.ts']); + }); + + it('treats a lone "!" line as no pattern, so ignored files and node_modules stay out', () => { + const root = project(mode, 'skipped.ts\n!\n', ['kept.ts', 'skipped.ts', 'node_modules/pkg/index.ts']); + expect(scanned(root)).toEqual(['kept.ts']); + }); +}); diff --git a/__tests__/go-package-value-exported.test.ts b/__tests__/go-package-value-exported.test.ts new file mode 100644 index 000000000..24b805b84 --- /dev/null +++ b/__tests__/go-package-value-exported.test.ts @@ -0,0 +1,211 @@ +/** + * A Go package-level variable or constant is exported by its own name's case, + * as a function or a type is: + * + * var Hook = func() int { return 1 } // exported + * var ( + * GroupedHook = func() int { return 2 } // exported + * grouped = 0 // package-local + * ) + * const Limit = 3 // exported + * + * Both extractors read exportedness off the declaration node, which has no + * name (the names sit on its `var_spec` / `const_spec` children), so every + * var and const was unexported. Another package's `a.Hook()` then found no + * exported `Hook` in package `a` and linked to nothing, though a call from + * inside the package linked to the variable. + * + * Runs against the native kernel (when built) and the wasm extractor, which + * must agree. + */ +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; +import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel'; +import type { ExtractionResult } from '../src/types'; + +const KERNEL_PATH = path.join( + __dirname, + '..', + 'codegraph-kernel', + 'prebuilds', + `${process.platform}-${process.arch}`, + 'codegraph-kernel.node' +); +const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1'; + +const PACKAGE_A = `package a + +// Hook is a package-level function value. +var Hook = func() int { return 1 } + +var ( + // GroupedHook sits in a var group. + GroupedHook = func() int { return 2 } + grouped = 0 +) + +var hidden = func() int { return 3 } + +var Typed, second int + +const Limit = 3 + +const ( + ModeA = iota + modeB +) + +type Client struct{} + +func (c *Client) Do() int { return 4 } + +func helper() int { return Hook() + hidden() + grouped } +`; + +const PACKAGE_B = `package b + +import "example.com/app/a" + +func Use() int { + return a.Hook() + a.GroupedHook() + a.Limit + a.hidden() +} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +describe('Go package-level variables and constants are exported by their name', () => { + let savedEnv: Record = {}; + + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['go']); + }); + + beforeEach(() => { + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + resetKernelForTests(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + resetKernelForTests(); + }); + + function extract(backend: 'kernel' | 'wasm', source: string): ExtractionResult { + if (backend === 'wasm') { + process.env.CODEGRAPH_KERNEL = '0'; + return extractFromSource('a/a.go', source, 'go'); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract('a/a.go', source, 'go'); + expect(result, 'kernel extraction').not.toBeNull(); + return result!; + } + + const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const); + + for (const crlf of [false, true]) { + it.each(backends)(`%s${crlf ? ' (CRLF)' : ''}`, (backend) => { + const result = extract(backend, crlf ? PACKAGE_A.replace(/\n/g, '\r\n') : PACKAGE_A); + const exported = Object.fromEntries( + result.nodes + .filter((n) => n.kind === 'variable' || n.kind === 'constant') + .map((n) => [`${n.kind} ${n.name}`, n.isExported]) + ); + // Every spec's node says, as a boolean, whether its name is exported. + expect(exported).toEqual({ + 'variable Hook': true, + 'variable GroupedHook': true, + 'variable grouped': false, + 'variable hidden': false, + 'variable Typed': true, + 'constant Limit': true, + 'constant ModeA': true, + 'constant modeB': false, + }); + // Functions, methods and types keep theirs. + const flag = (kind: string, name: string) => result.nodes.find((n) => n.kind === kind && n.name === name)?.isExported; + expect(flag('struct', 'Client')).toBe(true); + expect(flag('method', 'Do')).toBe(true); + expect(flag('function', 'helper')).toBe(false); + }); + } +}); + +describe.each([ + ['default', 'LF'], + ['wasm', 'LF'], + ['default', 'CRLF'], + ['wasm', 'CRLF'], +])('a call through another package reaches its exported variable (%s, %s)', (backend, eol) => { + let root = ''; + let cg: CodeGraph | undefined; + let kernel: string | undefined; + + beforeAll(async () => { + kernel = process.env.CODEGRAPH_KERNEL; + if (backend === 'wasm') process.env.CODEGRAPH_KERNEL = '0'; + else delete process.env.CODEGRAPH_KERNEL; + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-go-value-exported-')); + const files: Record = { + 'go.mod': 'module example.com/app\n\ngo 1.21\n', + 'a/a.go': PACKAGE_A, + 'b/b.go': PACKAGE_B, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), eol === 'CRLF' ? content.replace(/\n/g, '\r\n') : content); + } + cg = await CodeGraph.init(root, { index: true }); + }, 60_000); + + afterAll(() => { + cg?.destroy(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + if (kernel === undefined) delete process.env.CODEGRAPH_KERNEL; + else process.env.CODEGRAPH_KERNEL = kernel; + }); + + const nodeNamed = (file: string, name: string) => { + const nodes = cg!.getNodesInFile(file).filter((n) => n.name === name && n.kind !== 'import'); + expect(nodes, `${name} in ${file}`).toHaveLength(1); + return nodes[0]!; + }; + + /** `kind file:name` of each call out of `name` in `file`, sorted. */ + const callsFrom = (file: string, name: string) => + cg!.getOutgoingEdgesFrom([nodeNamed(file, name).id]) + .filter((e) => e.kind === 'calls') + .map((e) => `${e.kind} ${cg!.getNode(e.target)!.filePath}:${cg!.getNode(e.target)!.name}`) + .sort(); + + it('a.Hook() and a.GroupedHook() call the variables package a exports', () => { + expect(callsFrom('b/b.go', 'Use')).toEqual([ + 'calls a/a.go:GroupedHook', + 'calls a/a.go:Hook', + ]); + }); + + it('an unexported variable stays inside its package', () => { + const unresolved = cg!.getUnresolvedReferencesFrom(nodeNamed('b/b.go', 'Use').id) + .filter((r) => r.referenceKind === 'calls') + .map((r) => r.referenceName); + expect(unresolved).toEqual(['a.hidden']); + }); + + it('calls inside the package still reach its variables', () => { + expect(callsFrom('a/a.go', 'helper')).toEqual([ + 'calls a/a.go:Hook', + 'calls a/a.go:hidden', + ]); + }); +}); diff --git a/__tests__/mcp-stdout-failure.test.ts b/__tests__/mcp-stdout-failure.test.ts new file mode 100644 index 000000000..0a0e1a904 --- /dev/null +++ b/__tests__/mcp-stdout-failure.test.ts @@ -0,0 +1,237 @@ +/** + * A host that stops reading an MCP server's output, while keeping its stdin + * open and staying alive itself, must get the server's normal shutdown, not a + * crash (#2991). + * + * The next response the server wrote failed (EPIPE), and with no listener on + * stdout Node handed the error to the last-resort fatal handler, which exits 1 + * at once. That skipped the shutdown that stops the engine, releases the + * writer lock and waits for query workers still starting, which on Windows can + * crash a process that exits under them. Stdin, the host's PID and the + * liveness watchdog all still look healthy, so nothing else notices. + * + * Each case runs a real server or proxy in a child process, destroys the + * test's end of the child's stdout, then sends one more request so its + * response meets the broken pipe. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import { spawn, type ChildProcessWithoutNullStreams } from 'child_process'; +import * as fs from 'fs'; +import * as net from 'net'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); +const PROXY = path.resolve(__dirname, '../dist/mcp/proxy.js'); + +const line = (msg: object): string => JSON.stringify({ jsonrpc: '2.0', ...msg }) + '\n'; +const INITIALIZE = line({ + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-06-18', capabilities: {}, clientInfo: { name: 'test', version: '0.0.0' } }, +}); +const call = (id: number, name = 'codegraph_status'): string => + line({ id, method: 'tools/call', params: { name, arguments: {} } }); +/** No watchdogs, prompt hook or handshake timer: only the stdout failure can end these processes. */ +const QUIET = { + CODEGRAPH_TELEMETRY: '0', DO_NOT_TRACK: '1', CODEGRAPH_NO_PROMPT_HOOK: '1', + CODEGRAPH_NO_WATCHDOG: '1', CODEGRAPH_PPID_POLL_MS: '0', CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0', +}; + +interface Child { + child: ChildProcessWithoutNullStreams; + out: () => string; + err: () => string; + exit: Promise; + write: (s: string) => void; +} + +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +async function until(check: () => boolean, what: string, timeout = 20_000): Promise { + const deadline = Date.now() + timeout; + while (!check()) { + if (Date.now() > deadline) throw new Error(`Timed out waiting for ${what}`); + await delay(25); + } +} + +/** Remove `dir` once nothing holds it: Windows can keep it busy a moment after a child exits. */ +async function removeWhenReleased(dir: string, timeoutMs = 5_000): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code ?? ''; + if (!['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code) || Date.now() > deadline) throw error; + await delay(50); + } + } +} + +function answered(c: Child, id: number): boolean { + return c.out().split('\n').some((l) => { + try { return JSON.parse(l).id === id; } catch { return false; } + }); +} + +describe('MCP stdio output failure (#2991)', () => { + const children: Child[] = []; + const dirs: string[] = []; + const cleanups: Array<() => Promise | void> = []; + + afterEach(async () => { + for (const { child, exit } of children.splice(0)) { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); + await exit; + } + for (const cleanup of cleanups.splice(0)) await cleanup(); + for (const dir of dirs.splice(0)) await removeWhenReleased(dir); + }); + + function start(args: string[], env: Record = {}): Child { + const child = spawn(process.execPath, args, { + // Off every fixture, so a child still exiting cannot hold one (Windows EBUSY). + cwd: os.tmpdir(), + env: { ...process.env, ...QUIET, ...env }, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }) as ChildProcessWithoutNullStreams; + let out = ''; + let err = ''; + child.stdout.on('data', (d) => { out += d; }); + child.stdout.on('error', () => { /* destroyed by the test on purpose */ }); + child.stderr.on('data', (d) => { err += d; }); + child.stdin.on('error', () => { /* the child is gone */ }); + const exit = new Promise((resolve) => child.once('exit', (code) => resolve(code))); + const handle: Child = { + child, out: () => out, err: () => err, exit, + write: (s) => { if (child.exitCode === null) child.stdin.write(s); }, + }; + children.push(handle); + return handle; + } + + /** The host stops reading; the next response meets the broken pipe. Stdin stays open. */ + async function breakOutputThenCall(c: Child, id: number, name?: string): Promise { + c.child.stdout.destroy(); + c.write(call(id, name)); + let timer: NodeJS.Timeout | undefined; + const timedOut = new Promise<'timeout'>((resolve) => { timer = setTimeout(() => resolve('timeout'), 30_000); }); + const result = await Promise.race([c.exit, timedOut]); + clearTimeout(timer); + if (result === 'timeout') throw new Error(`still running 30 s after its output failed:\n${c.err()}`); + return result; + } + + it('stops a direct-mode server through its normal shutdown, which releases the writer lock', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-stdout-direct-')); + dirs.push(dir); + CodeGraph.initSync(dir).close(); + const writerPid = path.join(dir, '.codegraph', 'writer.pid'); + + // The runtime flags make this child the server itself, not a relauncher. + const server = start([BIN, 'serve', '--mcp', '--path', dir], { CODEGRAPH_NO_DAEMON: '1' }); + server.write(INITIALIZE); + await until(() => answered(server, 1), 'the initialize response'); + server.write(line({ method: 'notifications/initialized' })); + server.write(call(2)); + await until(() => answered(server, 2), 'the first tool call'); + expect(JSON.parse(fs.readFileSync(writerPid, 'utf8')).pid).toBe(server.child.pid); + + expect(await breakOutputThenCall(server, 3), server.err()).toBe(0); + expect(server.err()).not.toMatch(/Uncaught exception/); + expect(fs.existsSync(writerPid)).toBe(false); + }, 60_000); + + it('stops the local proxy\'s in-process engine before the proxy exits', async () => { + const proxy = start(['-e', ` + const fs = require('fs'); + const { runLocalHandshakeProxy } = require(${JSON.stringify(PROXY)}); + runLocalHandshakeProxy({ + root: process.cwd(), + getDaemonSocket: async () => null, + makeEngine: () => ({ + ensureInitialized: async () => {}, + isReadOnly: () => true, + getToolHandler: () => ({ execute: async () => ({ content: [{ type: 'text', text: 'ok' }] }) }), + stop: () => new Promise((resolve) => setTimeout(resolve, 100)).then(() => fs.writeSync(2, 'ENGINE STOPPED\\n')), + }), + }); + `], { CODEGRAPH_DAEMON_RETRY_MS: '0' }); + proxy.write(INITIALIZE); + proxy.write(call(2, 'test')); + await until(() => answered(proxy, 2), 'the first tool call'); + + expect(await breakOutputThenCall(proxy, 3, 'test'), proxy.err()).toBe(0); + expect(proxy.err()).toContain('ENGINE STOPPED'); + expect(proxy.err()).not.toMatch(/Uncaught exception/); + }, 60_000); + + it('detaches the local proxy from the shared daemon and leaves the daemon\'s other clients alone', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-stdout-daemon-')); + dirs.push(dir); + const socketPath = process.platform === 'win32' + ? `\\\\.\\pipe\\cg-stdout-daemon-${process.pid}-${Date.now()}` + : path.join(dir, 'd.sock'); + // Stands in for the shared daemon: answers every request on every connection. + const connections = new Set(); + const daemon = net.createServer((socket) => { + connections.add(socket); + socket.on('close', () => connections.delete(socket)); + socket.on('error', () => { /* only this fixture's clients */ }); + let buffer = ''; + socket.on('data', (chunk) => { + buffer += String(chunk); + let newline: number; + while ((newline = buffer.indexOf('\n')) >= 0) { + const request = buffer.slice(0, newline); + buffer = buffer.slice(newline + 1); + try { + const msg = JSON.parse(request); + if (msg.id !== undefined) socket.write(line({ id: msg.id, result: { content: [{ type: 'text', text: 'daemon' }] } })); + } catch { /* not a request */ } + } + }); + }); + await new Promise((resolve) => daemon.listen(socketPath, resolve)); + cleanups.push(() => new Promise((resolve) => { + for (const socket of connections) socket.destroy(); + daemon.close(() => resolve()); + })); + const other = net.createConnection(socketPath); + cleanups.unshift(() => { other.destroy(); }); + await new Promise((resolve, reject) => { other.once('connect', () => resolve()); other.once('error', reject); }); + await until(() => connections.size === 1, 'the other client to connect'); + + const proxy = start(['-e', ` + const net = require('net'); + const { runLocalHandshakeProxy } = require(${JSON.stringify(PROXY)}); + runLocalHandshakeProxy({ + root: process.cwd(), + getDaemonSocket: async () => { + const socket = net.createConnection(${JSON.stringify(socketPath)}); + await new Promise((resolve, reject) => { socket.once('connect', resolve); socket.once('error', reject); }); + return socket; + }, + makeEngine: () => { throw new Error('no in-process engine while the daemon serves'); }, + }); + `], { CODEGRAPH_DAEMON_RETRY_MS: '0' }); + proxy.write(INITIALIZE); + proxy.write(call(2, 'test')); + await until(() => answered(proxy, 2), 'a tool call answered through the daemon'); + expect(connections.size).toBe(2); + + expect(await breakOutputThenCall(proxy, 3, 'test'), proxy.err()).toBe(0); + expect(proxy.err()).not.toMatch(/Uncaught exception/); + await until(() => connections.size === 1, 'only the proxy\'s own connection to close'); + expect(daemon.listening).toBe(true); + let reply = ''; + other.on('data', (chunk) => { reply += String(chunk); }); + other.write(call(9, 'test')); + await until(() => reply.includes('"id":9'), 'the daemon to answer its other client'); + }, 60_000); +}); diff --git a/__tests__/mcp-tool-call-arguments.test.ts b/__tests__/mcp-tool-call-arguments.test.ts new file mode 100644 index 000000000..4ae753a7e --- /dev/null +++ b/__tests__/mcp-tool-call-arguments.test.ts @@ -0,0 +1,142 @@ +/** + * A `tools/call` whose `arguments` is not an object is a malformed request, + * answered with a JSON-RPC invalid-params error (-32602) like a call that + * names no tool, and never run (#2981). + * + * Before, the session handed a string or number straight to the tool handler, + * where `'_cgExploreSession' in args` threw, and the call came back as a tool + * result flagged isError saying "This is an internal codegraph error — retry + * the call once; if it persists, continue without codegraph": a client's bug + * reported as codegraph's own, with advice to stop using it. An array went + * through as if it were the arguments. No arguments, or null, still mean none. + * + * Both ways a call is served are covered: the session (the direct stdio server + * and every client of the shared daemon), and the stdio proxy serving in + * process while the daemon is unavailable. + */ +import { describe, it, expect } from 'vitest'; +import { spawn } from 'child_process'; +import * as path from 'path'; +import { MCPSession } from '../src/mcp/session'; +import type { MCPEngine } from '../src/mcp/engine'; +import type { JsonRpcTransport, JsonRpcRequest, JsonRpcNotification } from '../src/mcp/transport'; + +const MALFORMED: unknown[] = ['alpha', 42, true, [1], [{ query: 'q' }]]; +const NONE: unknown[] = [undefined, null, '']; + +describe('a tools/call whose arguments are not an object (#2981)', () => { + function session() { + let handle: ((m: JsonRpcRequest | JsonRpcNotification) => Promise) | null = null; + const results: Array<{ id: unknown; result: unknown }> = []; + const errors: Array<{ id: unknown; code: number; message: string }> = []; + const executed: unknown[] = []; + const transport: JsonRpcTransport = { + start(h) { handle = h as typeof handle; }, + stop() { /* nothing to tear down */ }, + send() { /* unused */ }, + notify() { /* unused */ }, + async request() { return {}; }, + sendResult(id, result) { results.push({ id, result }); }, + sendError(id, code, message) { errors.push({ id, code, message }); }, + }; + const engine = { + ensureInitialized: async () => { /* already open */ }, + hasDefaultCodeGraph: () => true, + getProjectPath: () => '/repo', + retryInitializeSync: () => { /* nothing to retry */ }, + getToolHandler: () => ({ + getTools: () => [], + execute: async (_name: string, args: unknown) => { + executed.push(args); + return { content: [{ type: 'text' as const, text: 'ok' }] }; + }, + }), + } as unknown as MCPEngine; + new MCPSession(transport, engine).start(); + const call = (id: number, args: unknown) => + handle!({ + jsonrpc: '2.0', id, method: 'tools/call', + params: args === undefined ? { name: 'codegraph_explore' } : { name: 'codegraph_explore', arguments: args }, + } as JsonRpcRequest); + return { call, results, errors, executed }; + } + + it('is answered with invalid params, and never reaches a tool', async () => { + const s = session(); + for (const [i, args] of MALFORMED.entries()) await s.call(i, args); + expect(s.errors).toEqual(MALFORMED.map((_, i) => ({ id: i, code: -32602, message: 'Tool arguments must be an object' }))); + expect(s.executed).toEqual([]); + expect(s.results).toEqual([]); + }); + + it('runs a call with an object, or with no arguments at all', async () => { + const s = session(); + await s.call(1, { query: 'q' }); + for (const [i, args] of NONE.entries()) await s.call(10 + i, args); + expect(s.errors).toEqual([]); + expect(s.executed).toEqual([{ query: 'q' }, {}, {}, {}]); + }); + + it('is answered the same way by the proxy while the daemon is unavailable', async () => { + const proxyModule = path.resolve(__dirname, '../dist/mcp/proxy.js'); + // The real proxy entry point, with no daemon and an engine that says when + // a tool runs. + const script = ` + const { runLocalHandshakeProxy } = require(${JSON.stringify(proxyModule)}); + const say = (s) => require('fs').writeSync(1, s + '\\n'); + runLocalHandshakeProxy({ + root: process.cwd(), + getDaemonSocket: async () => null, + makeEngine: () => ({ + ensureInitialized: async () => {}, + isReadOnly: () => true, + getToolHandler: () => ({ execute: async (name, args) => { + say('EXECUTED ' + JSON.stringify(args)); + return { content: [{ type: 'text', text: 'ok' }] }; + } }), + stop: async () => {}, + }), + }); + `; + const child = spawn(process.execPath, ['-e', script], { + env: { + ...process.env, + CODEGRAPH_TELEMETRY: '0', DO_NOT_TRACK: '1', CODEGRAPH_NO_PROMPT_HOOK: '1', + CODEGRAPH_NO_WATCHDOG: '1', CODEGRAPH_PPID_POLL_MS: '0', + CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0', CODEGRAPH_DAEMON_RETRY_MS: '60000', + }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + let out = ''; + let err = ''; + child.stdout!.on('data', (d) => { out += d; }); + child.stderr!.on('data', (d) => { err += d; }); + const exited = new Promise((resolve) => child.once('exit', () => resolve())); + const send = (msg: object) => child.stdin!.write(JSON.stringify({ jsonrpc: '2.0', ...msg }) + '\n'); + const reply = (id: number) => + out.split('\n').filter((l) => l.startsWith('{')).map((l) => JSON.parse(l)).find((m) => m.id === id); + const until = async (check: () => boolean) => { + const deadline = Date.now() + 15_000; + while (!check()) { + if (Date.now() > deadline) throw new Error(`no reply: ${out}\n${err}`); + await new Promise((r) => setTimeout(r, 25)); + } + }; + try { + send({ id: 1, method: 'initialize', params: {} }); + await until(() => !!reply(1)); + send({ id: 2, method: 'tools/call', params: { name: 'codegraph_explore', arguments: 'alpha' } }); + send({ id: 3, method: 'tools/call', params: { name: 'codegraph_status', arguments: [1] } }); + send({ id: 4, method: 'tools/call', params: { name: 'codegraph_status' } }); + await until(() => !!reply(2) && !!reply(3) && !!reply(4)); + expect(reply(2).error).toEqual({ code: -32602, message: 'Tool arguments must be an object' }); + expect(reply(3).error).toEqual({ code: -32602, message: 'Tool arguments must be an object' }); + expect(reply(4).result).toEqual({ content: [{ type: 'text', text: 'ok' }] }); + expect(out.match(/^EXECUTED .*$/gm)).toEqual(['EXECUTED {}']); + } finally { + child.stdin!.end(); + child.kill('SIGKILL'); + await exited; + } + }, 30_000); +}); diff --git a/__tests__/migration-concurrent-opens.test.ts b/__tests__/migration-concurrent-opens.test.ts new file mode 100644 index 000000000..13042530e --- /dev/null +++ b/__tests__/migration-concurrent-opens.test.ts @@ -0,0 +1,202 @@ +/** + * Opening an index that still needs a schema migration, while another process + * opens it too, must not fail (#2980). + * + * The open read the schema version, then ran each pending migration in its own + * deferred transaction. A process that read the version just before another + * one migrated went on to apply the same migration again: it either failed + * recording a version that was already recorded ("UNIQUE constraint failed: + * schema_versions.version"), or, for a migration that reads before it writes, + * hit SQLITE_BUSY at once, since SQLite does not wait on a lock for a + * transaction that has already read. Either way the open failed. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawn, type ChildProcess } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src'; +import { DatabaseConnection } from '../src/db'; +import { createDatabase } from '../src/db/sqlite-adapter'; +import { CURRENT_SCHEMA_VERSION, getCurrentVersion } from '../src/db/migrations'; + +const DIST_INDEX = path.resolve(__dirname, '../dist/index.js'); +/** The index migration 22 adds (upstream's migration 16, numbered 22 here because the fork's own migrations sit between): the one the fixture leaves pending first. */ +const V22_INDEX = 'idx_unresolved_failed_module_member'; + +/** Opens the project once the test writes a byte to its stdin, and says how it went. */ +const OPENER = ` +const fs = require('fs'); +const CodeGraph = require(process.argv[1]).default; +process.stdout.write('READY\\n'); +try { fs.readSync(0, Buffer.alloc(1), 0, 1, null); } catch (_) {} +try { + CodeGraph.openSync(process.argv[2]).close(); + process.stdout.write('OPENED\\n'); +} catch (err) { + process.stdout.write('FAILED ' + String((err && err.message) || err).replace(/\\s+/g, ' ') + '\\n'); +} +`; + +/** + * Another process in the middle of migrating: it holds the write lock with + * version 22 recorded but not committed, and commits on its own after a while. + * Until then any other connection still reads version 21. + */ +const MIGRATING = ` +const { DatabaseSync } = require('node:sqlite'); +const db = new DatabaseSync(process.argv[1]); +db.exec('PRAGMA busy_timeout = 5000'); +db.exec('BEGIN IMMEDIATE'); +db.prepare("INSERT INTO schema_versions (version, applied_at, description) VALUES (22, ?, 'another process')").run(Date.now()); +process.stdout.write('HOLDING\\n'); +setTimeout(() => { db.exec('COMMIT'); db.close(); process.stdout.write('COMMITTED\\n'); }, Number(process.argv[2])); +`; + +/** A writer holding the lock on a current index, until the test ends it. */ +const WRITING = ` +const fs = require('fs'); +const { DatabaseSync } = require('node:sqlite'); +const db = new DatabaseSync(process.argv[1]); +db.exec('BEGIN IMMEDIATE'); +db.exec("INSERT INTO project_metadata (key, value, updated_at) VALUES ('cg2980', '1', 0) ON CONFLICT(key) DO UPDATE SET value = excluded.value"); +process.stdout.write('HOLDING\\n'); +try { fs.readSync(0, Buffer.alloc(1), 0, 1, null); } catch (_) {} +db.exec('ROLLBACK'); +db.close(); +`; + +interface Child { + child: ChildProcess; + output: () => string; + exited: Promise; +} + +describe('opening an index with a pending migration from several processes (#2980)', () => { + let dir: string; + let dbPath: string; + let children: Child[] = []; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-migrate-')); + CodeGraph.initSync(dir).close(); + dbPath = path.join(dir, '.codegraph', 'codegraph.db'); + }); + + afterEach(async () => { + for (const { child, exited } of children) { + if (child.exitCode === null && child.signalCode === null) child.kill(); + await exited; + } + children = []; + fs.rmSync(dir, { recursive: true, force: true }); + }); + + /** Take the index back to version 21, the way an index built before migration 22 is. */ + function pendingV22(refs = 0): void { + const { db } = createDatabase(dbPath); + try { + db.exec('PRAGMA foreign_keys = OFF'); + db.exec(`DROP INDEX IF EXISTS ${V22_INDEX}; + DELETE FROM schema_versions WHERE version >= 22; + INSERT OR IGNORE INTO schema_versions (version, applied_at, description) VALUES (21, 0, 'fixture');`); + if (refs > 0) { + // Enough rows that building the index takes a while, so openers overlap. + db.exec(`WITH RECURSIVE n(i) AS (SELECT 1 UNION ALL SELECT i + 1 FROM n WHERE i < ${refs}) + INSERT INTO unresolved_refs (from_node_id, reference_name, reference_kind, line, col, file_path, language, status, name_tail) + SELECT 'n' || (i % 500), 'store.get' || i, 'calls', i % 900, 1, 'src/f' || (i % 300) || '.ts', 'typescript', + CASE WHEN i % 3 = 0 THEN 'failed' ELSE 'pending' END, + CASE WHEN i % 3 = 0 THEN 'module:./m' || (i % 50) ELSE 'get' || i END + FROM n`); + } + } finally { + db.close(); + } + } + + function state(): { version: number; v22Records: number; index: boolean } { + const { db } = createDatabase(dbPath, { readOnly: true }); + try { + return { + version: getCurrentVersion(db), + v22Records: (db.prepare('SELECT COUNT(*) AS n FROM schema_versions WHERE version = 22').get() as { n: number }).n, + index: db.prepare("SELECT 1 FROM sqlite_master WHERE type = 'index' AND name = ?").get(V22_INDEX) !== undefined, + }; + } finally { + db.close(); + } + } + + function run(args: string[]): Child { + const child = spawn(process.execPath, args, { + stdio: ['pipe', 'pipe', 'ignore'], + // Off the fixture, so a child still exiting cannot hold it (Windows EBUSY). + cwd: os.tmpdir(), + windowsHide: true, + }); + let out = ''; + child.stdout!.on('data', (chunk) => { out += String(chunk); }); + const exited = new Promise((resolve) => child.once('exit', (code) => resolve(code))); + const handle = { child, output: () => out, exited }; + children.push(handle); + return handle; + } + + async function says(who: Child, word: string): Promise { + const deadline = Date.now() + 30_000; + while (!who.output().includes(word)) { + if (who.child.exitCode !== null || Date.now() > deadline) { + throw new Error(`expected "${word}", got: ${who.output()}`); + } + await new Promise((resolve) => setTimeout(resolve, 10)); + } + } + + it('waits for a process already migrating, then skips the migration it applied', async () => { + pendingV22(); + const migrating = run(['-e', MIGRATING, dbPath, '1500']); + await says(migrating, 'HOLDING'); + + // Version 22 is not committed yet: this open reads 21, so it has a migration to run. + const started = Date.now(); + const conn = DatabaseConnection.open(dbPath); + const waited = Date.now() - started; + try { + expect(getCurrentVersion(conn.getDb())).toBe(CURRENT_SCHEMA_VERSION); + } finally { + conn.close(); + } + await says(migrating, 'COMMITTED'); + expect(waited).toBeGreaterThanOrEqual(500); + // Recorded once, by the other process, and not run again here: its index was never built. + expect(state()).toEqual({ version: CURRENT_SCHEMA_VERSION, v22Records: 1, index: false }); + }, 60_000); + + it('opens in every one of three processes that start at the same moment', async () => { + for (let round = 0; round < 2; round++) { + pendingV22(round === 0 ? 150_000 : 0); + const openers = [0, 1, 2].map(() => run(['-e', OPENER, DIST_INDEX, dir])); + await Promise.all(openers.map((opener) => says(opener, 'READY'))); + for (const opener of openers) opener.child.stdin!.write('g'); + await Promise.all(openers.map((opener) => opener.exited)); + for (const opener of openers) expect(opener.output(), `round ${round}`).toContain('OPENED'); + expect(state()).toEqual({ version: CURRENT_SCHEMA_VERSION, v22Records: 1, index: true }); + } + }, 120_000); + + it('still applies a pending migration in a single process', () => { + pendingV22(); + DatabaseConnection.open(dbPath).close(); + expect(state()).toEqual({ version: CURRENT_SCHEMA_VERSION, v22Records: 1, index: true }); + }); + + it('opens a current index without waiting for another process that is writing', async () => { + const writing = run(['-e', WRITING, dbPath]); + await says(writing, 'HOLDING'); + const started = Date.now(); + DatabaseConnection.open(dbPath).close(); + expect(Date.now() - started).toBeLessThan(2_000); + writing.child.stdin!.write('g'); + expect(await writing.exited).toBe(0); + }, 60_000); +}); diff --git a/__tests__/node-overload-spread.test.ts b/__tests__/node-overload-spread.test.ts new file mode 100644 index 000000000..757ad27db --- /dev/null +++ b/__tests__/node-overload-spread.test.ts @@ -0,0 +1,157 @@ +/** + * codegraph_node on a name with more definitions than its caps hold (#1657). + * + * The index lists a name's definitions by path, and the multi-definition + * answer rendered and listed them in that order until its caps ran out: 16 + * bodies, 20 list lines, then a bare "+N more". So hundreds of definitions + * under an early-sorting folder hid the only one in another language or a + * late folder, and nothing in the answer said another existed. Now a language + * with no body gets one on top of the overloads that fit, each language and + * top-level folder gets an early list line, and the overflow names what it + * leaves out. The overloads that rendered before still render. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { ToolHandler, definitionCapOrder, describeOmittedDefinitions } from '../src/mcp/tools'; +import type { Node } from '../src/types'; + +const MAX_OUTPUT_LENGTH = 15000; + +function write(root: string, rel: string, body: string): void { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), body); +} + +async function indexed(root: string): Promise<{ cg: CodeGraph; handler: ToolHandler }> { + const cg = CodeGraph.initSync(root); + await cg.indexAll(); + return { cg, handler: new ToolHandler(cg) }; +} + +async function nodeText(handler: ToolHandler, args: Record): Promise { + const res = await handler.execute('codegraph_node', args); + expect(res.isError).toBeFalsy(); + return res.content?.[0]?.text ?? ''; +} + +describe('codegraph_node: definitions past the caps (#1657)', () => { + let root: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeAll(async () => { + // 400 TypeScript `num` methods under an early-sorting folder, one more in + // another top-level folder, and the only Python `num` in a late one. + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-node-spread-')); + for (let f = 0; f < 40; f++) { + const classes = Array.from({ length: 10 }, (_, c) => + `export class C${f}_${c} {\n num(v: number): string {\n return v.toFixed(${c});\n }\n}\n`).join('\n'); + write(root, `aaa_client/m${String(f).padStart(2, '0')}.ts`, classes); + } + write(root, 'bbb_lib/helper.ts', 'export function num(v: number): number {\n return Math.round(v);\n}\n'); + write(root, 'zzz_server/format.py', 'def num(value):\n return round(value, 2)\n'); + ({ cg, handler } = await indexed(root)); + }, 120_000); + + afterAll(() => { + handler?.closeAll(); + cg?.destroy(); + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('renders the only Python definition beside the TypeScript overloads that fit', async () => { + const text = await nodeText(handler, { symbol: 'num', includeCode: true }); + expect(text).toContain('**402 definitions named "num"**'); + expect(text).toContain('Returning 17 in full; 385 more listed below'); + // main: the Python body, and the file, appeared nowhere. + expect(text).toContain('**Location:** zzz_server/format.py:1'); + expect(text).toContain('def num(value):'); + // The same sixteen TypeScript bodies as before still render, in index order. + const tsBodies = [...text.matchAll(/^\*\*Location:\*\* (aaa_client\/\S+)/gm)].map((m) => m[1]); + expect(tsBodies).toEqual([ + ...Array.from({ length: 10 }, (_, c) => `aaa_client/m00.ts:${2 + 6 * c}`), + ...Array.from({ length: 6 }, (_, c) => `aaa_client/m01.ts:${2 + 6 * c}`), + ]); + expect(text.length).toBeLessThanOrEqual(MAX_OUTPUT_LENGTH); + }); + + it('gives another top-level folder an early list line and names the overflow', async () => { + const text = await nodeText(handler, { symbol: 'num', includeCode: true }); + const listed = [...text.slice(text.indexOf('**Other definitions**')).matchAll(/^- `num` \(\w+\) — (\S+)$/gm)].map((m) => m[1]); + expect(listed).toHaveLength(20); + // main: bbb_lib/helper.ts was behind a bare "- … +365 more". + expect(listed).toContain('bbb_lib/helper.ts:1'); + expect(text).toContain('- … +365 more: 365 typescript in aaa_client/'); + expect(text).not.toMatch(/^- … \+\d+ more$/m); + }); + + it('keeps every language and folder in a list too long for the output cap', async () => { + const text = await nodeText(handler, { symbol: 'num' }); + // main: the output cap cut the list at "... (output truncated)", past + // which both late folders fell. + expect(text).not.toContain('(output truncated)'); + expect(text.length).toBeLessThanOrEqual(MAX_OUTPUT_LENGTH); + expect(text).toContain('- `num` (function) — zzz_server/format.py:1'); + expect(text).toContain('- `num` (function) — bbb_lib/helper.ts:1'); + expect(text).toMatch(/^- … \+\d+ more: \d+ typescript in aaa_client\/$/m); + }); +}); + +describe('codegraph_node: a set that fits every cap reads as before (#1657)', () => { + let root: string; + let cg: CodeGraph; + let handler: ToolHandler; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-node-fits-')); + write(root, 'a/x.ts', 'export function num(v: number): string {\n return String(v);\n}\n'); + write(root, 'b/y.py', 'def num(value):\n return str(value)\n'); + write(root, 'c/z.ts', 'export function num(v: number): number {\n return v;\n}\n'); + ({ cg, handler } = await indexed(root)); + }, 60_000); + + afterAll(() => { + handler?.closeAll(); + cg?.destroy(); + fs.rmSync(root, { recursive: true, force: true }); + }); + + it('renders every definition in index order with nothing listed', async () => { + const text = await nodeText(handler, { symbol: 'num', includeCode: true }); + expect(text).toContain('Returning 3 in full — pick the one you need'); + expect([...text.matchAll(/^\*\*Location:\*\* (\S+)/gm)].map((m) => m[1])).toEqual(['a/x.ts:1', 'b/y.py:1', 'c/z.ts:1']); + expect(text).not.toContain('**Other definitions**'); + }); +}); + +describe('definitionCapOrder / describeOmittedDefinitions (#1657)', () => { + const def = (filePath: string, language: string, startLine = 1): Node => + ({ id: `${filePath}:${startLine}`, name: 'num', kind: 'function', filePath, language, startLine } as unknown as Node); + + it('puts each language first, then each top-level folder, then the rest in order', () => { + const nodes = [ + def('a/1.ts', 'typescript'), def('a/2.ts', 'typescript'), def('b/1.ts', 'typescript'), + def('b/2.ts', 'typescript'), def('root.py', 'python'), def('z/1.py', 'python'), + ]; + expect(definitionCapOrder(nodes).map((n) => n.filePath)).toEqual([ + 'a/1.ts', 'root.py', // the first in each language + 'b/1.ts', 'z/1.py', // the first in each other folder within a language + 'a/2.ts', 'b/2.ts', // the rest + ]); + const oneFolder = [def('a/1.ts', 'typescript'), def('a/2.ts', 'typescript', 9), def('a/3.ts', 'typescript')]; + expect(definitionCapOrder(oneFolder)).toEqual(oneFolder); + }); + + it('names omitted definitions by language and folder, largest first', () => { + expect(describeOmittedDefinitions([ + def('z/1.py', 'python'), def('a/1.ts', 'typescript'), def('a/2.ts', 'typescript'), def('main.go', 'go'), + ])).toBe('2 typescript in a/, 1 python in z/, 1 go at the project root'); + const many = Array.from({ length: 8 }, (_, i) => def(`d${i}/x.ts`, 'typescript')); + expect(describeOmittedDefinitions(many)).toBe( + '1 typescript in d0/, 1 typescript in d1/, 1 typescript in d2/, 1 typescript in d3/, 1 typescript in d4/, 1 typescript in d5/, 2 elsewhere', + ); + }); +}); diff --git a/__tests__/search-special-characters.test.ts b/__tests__/search-special-characters.test.ts new file mode 100644 index 000000000..5fc17906c --- /dev/null +++ b/__tests__/search-special-characters.test.ts @@ -0,0 +1,62 @@ +/** + * A character full-text search can't take literally separates the words on + * either side of it, as it separates them in the index (#2971). + * + * The index splits text into words at anything that isn't a letter or digit, + * so `leaf(entry)` in a signature is stored as `leaf` and `entry`. The query + * side dropped `( ) ^ " ' * :` instead, gluing `leaf(entry)` into `leafentry`, + * a word no symbol has: a pasted call like `render(props)` or `getUser(id)` + * came back "No results found" while `render props` found both. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-search-special-')); + fs.mkdirSync(path.join(root, 'src')); + fs.writeFileSync( + path.join(root, 'src', 'a.ts'), + [ + 'export function entry() { mid1(); mid2(); }', + 'export function leaf() { return 1; }', + 'function mid1() { return leaf(); }', + 'function mid2() { return 2; }', + 'export function render(props: { title: string }) { return props.title; }', + '', + ].join('\n'), + ); + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const names = (query: string) => [...new Set(cg.searchNodes(query).map((r) => r.node.name))].sort(); + +describe('a character search cannot take literally', () => { + it.each(['leaf(entry)', 'leaf^entry', 'leaf"entry', "leaf'entry", 'leaf*entry', 'leaf:entry'])( + 'separates the words around it: %s', + (query) => { + expect(names(query)).toEqual(expect.arrayContaining(['entry', 'leaf'])); + }, + ); + + it('finds the function a pasted call names', () => { + expect(names('render(props)')).toContain('render'); + expect(names('mid1()')).toContain('mid1'); + }); + + it('reads as the words alone did, and a query of nothing else finds nothing', () => { + expect(names('leaf(entry)')).toEqual(names('leaf entry')); + expect(names('leaf()')).toEqual(names('leaf')); + expect(names('()')).toEqual([]); + }); +}); diff --git a/__tests__/stdout-teardown.test.ts b/__tests__/stdout-teardown.test.ts new file mode 100644 index 000000000..ec2041a77 --- /dev/null +++ b/__tests__/stdout-teardown.test.ts @@ -0,0 +1,57 @@ +/** + * #2991 — a stdout that fails (the host stopped reading it) must shut the + * server down through its own shutdown, not the fatal handler. + * treatStdoutFailureAsShutdown is the shared guard; the real processes are + * covered by mcp-stdout-failure.test.ts. + */ +import { describe, it, expect } from 'vitest'; +import { PassThrough } from 'stream'; +import { treatStdoutFailureAsShutdown } from '../src/mcp/stdout-teardown'; + +describe('treatStdoutFailureAsShutdown (#2991)', () => { + it.each(['EPIPE', 'ECONNRESET', 'ERR_STREAM_DESTROYED'])("treats a stdout 'error' (%s) as a shutdown signal", (code) => { + const s = new PassThrough(); + let calls = 0; + treatStdoutFailureAsShutdown(() => { calls++; }, s); + + // Without the guard's listener this emit would throw. + s.emit('error', Object.assign(new Error(`write ${code}`), { code })); + expect(calls).toBe(1); + }); + + it("also fires on 'close'", () => { + const s = new PassThrough(); + let calls = 0; + treatStdoutFailureAsShutdown(() => { calls++; }, s); + s.emit('close'); + expect(calls).toBe(1); + }); + + it('destroys the stream so later writes are dropped', () => { + const s = new PassThrough(); + treatStdoutFailureAsShutdown(() => { /* noop */ }, s); + s.emit('error', new Error('write EPIPE')); + expect(s.destroyed).toBe(true); + }); + + it('fires onTerminal at most once, and absorbs a write that fails during the shutdown', async () => { + const s = new PassThrough(); + let calls = 0; + treatStdoutFailureAsShutdown(() => { calls++; }, s); + s.emit('error', new Error('write EPIPE')); // fire() also destroys → emits 'close' + s.write('a response queued before the failure'); + await new Promise((resolve) => setImmediate(resolve)); + s.emit('close'); + expect(calls).toBe(1); + }); + + it('leaves a stream under backpressure alone', async () => { + const s = new PassThrough({ highWaterMark: 1 }); + let calls = 0; + treatStdoutFailureAsShutdown(() => { calls++; }, s); + expect(s.write('a response the host has not read yet')).toBe(false); + await new Promise((resolve) => setImmediate(resolve)); + expect(calls).toBe(0); + expect(s.destroyed).toBe(false); + }); +}); diff --git a/__tests__/svelte-self-closing-script.test.ts b/__tests__/svelte-self-closing-script.test.ts new file mode 100644 index 000000000..e5c26a652 --- /dev/null +++ b/__tests__/svelte-self-closing-script.test.ts @@ -0,0 +1,60 @@ +/** + * A self-closing ``: the component's real + * script was parsed with the markup in between, failed, and contributed no + * symbols, and the template scan skipped that markup. The same fix as #2989 + * for Astro. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; + +beforeAll(async () => { + await initGrammars(); + await loadAllGrammars(); +}); + +describe('Svelte self-closing +`; + const result = extractFromSource('src/routes/+page.svelte', code); + + expect(result.errors).toEqual([]); + const ready = result.nodes.find((n) => n.name === 'ready'); + expect(ready).toBeDefined(); + expect(ready?.startLine).toBe(7); + // The markup between the two tags is still scanned. + const call = result.unresolvedReferences.find( + (ref) => ref.referenceKind === 'calls' && ref.referenceName === 'formatDate' && ref.line === 4 + ); + expect(call).toBeDefined(); + expect(result.unresolvedReferences.some((ref) => ref.referenceName === 'Header' && ref.line === 5)).toBe(true); + }); + + it("still reads a call in a self-closing +`; + const result = extractFromSource('src/routes/+layout.svelte', code); + + expect(result.errors).toEqual([]); + expect(result.nodes.find((n) => n.name === 'ready')?.startLine).toBe(5); + expect(result.unresolvedReferences.some( + (ref) => ref.referenceKind === 'calls' && ref.referenceName === 'assetUrl' && ref.line === 2 + )).toBe(true); + }); +}); diff --git a/__tests__/ui-highlight.test.ts b/__tests__/ui-highlight.test.ts index 9f46268d6..3b9dfab43 100644 --- a/__tests__/ui-highlight.test.ts +++ b/__tests__/ui-highlight.test.ts @@ -96,6 +96,13 @@ describe('which languages classify', () => { expect(regions?.[0]?.language).toBe('typescript'); }); + it('skips a self-closing \n'; + const regions = syntaxRegionsFor(source, 'astro'); + expect(regions).toHaveLength(1); + expect(source.slice(regions![0]!.start, regions![0]!.end)).toBe('\nconst ready = true;\n'); + }); + it('has no grammar for the formats that only have file-level extraction', () => { for (const language of ['yaml', 'xml', 'properties', 'twig', 'unknown']) { expect(grammarFor(language)).toBeNull(); diff --git a/codegraph-kernel/src/go/mod.rs b/codegraph-kernel/src/go/mod.rs index 49a2e26e7..b6a70041d 100644 --- a/codegraph-kernel/src/go/mod.rs +++ b/codegraph-kernel/src/go/mod.rs @@ -279,7 +279,8 @@ impl<'t> Walker<'t> { Some(sig) } - /// goExtractor.isExported: uppercase first letter of the name field. + /// goExtractor.isExported: uppercase first letter of the name field (a + /// var_spec / const_spec's first name included). fn is_exported(&self, node: Node) -> bool { if let Some(name_node) = node.child_by_field_name("name") { let text = self.text(name_node); @@ -625,11 +626,14 @@ impl<'t> Walker<'t> { // own doc comment beside it in the parentheses; one without // keeps the group's. let doc = self.docstring_of(spec).or_else(|| docstring.clone()); + // Exported by its own name's case, read from the spec (the + // declaration around it has no name). + let is_exported = Some(self.is_exported(spec)); var_row = self.create_node( if is_const_decl { "constant" } else { "variable" }, &name, spec, - Extra { docstring: doc, signature, ..Extra::default() }, + Extra { docstring: doc, signature, is_exported, ..Extra::default() }, ); } } diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 000000000..f4ee493e2 --- /dev/null +++ b/docs/README.md @@ -0,0 +1,26 @@ +# Documentation + +To install and use CodeGraph, start with the [project README](../README.md). To build it and send a change, see [Contributing](../CONTRIBUTING.md). + +## Guides + +| Topic | Where to start | +|-------|----------------| +| Architecture, build rules and platform validation | [Project guide](../AGENTS.md) | +| Distribution and release packaging | [Bundling](../BUNDLING.md) | +| What telemetry collects, and how to turn it off | [Telemetry](../TELEMETRY.md) | +| Release history | [Changelog](../CHANGELOG.md) | +| Entries held for the viewer's launch | [Viewer changelog](viewer-launch-changelog.md) | +| What languages and frameworks cover, and what's left | [Framework coverage](design/framework-coverage.md) | +| Validating a language or framework on real repositories | [Validation guide](AGENTS.md) and [coverage playbook](design/dynamic-dispatch-coverage-playbook.md) | +| Checking a language by hand | [Language verification guide](SEARCH_QUALITY_LOOP.md) | +| The browser viewer's design | [Viewer design spec](design/codegraph-ui-design-spec.md) | +| The native extraction kernel | [Kernel design](design/native-extraction-kernel.md) | + +## Supporting material + +- [Design](design/): architecture notes and feature proposals. Some describe planned work rather than what has shipped. +- [Benchmarks](benchmarks/): measurements and how they were taken. Read each against the revision and setup it records. +- [Grammars](grammars/): notes and patches for the parser grammars CodeGraph builds itself. +- [Plans](plans/): implementation plans. +- [Discussion notes](from-chat/): supporting notes from design discussions. diff --git a/package-lock.json b/package-lock.json index b17828134..34290cab9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17,7 +17,7 @@ "entities": "6.0.1", "fast-string-width": "^3.0.2", "fast-wrap-ansi": "^0.2.0", - "ignore": "^7.0.5", + "ignore": "^7.0.14", "jsonc-parser": "^3.3.1", "picomatch": "^4.0.3", "sisteransi": "^1.0.5" @@ -28,7 +28,6 @@ "devDependencies": { "@ast-grep/cli": "^0.45.3", "@sveltejs/vite-plugin-svelte": "^4.0.4", - "@types/better-sqlite3": "^7.6.0", "@types/node": "^20.19.30", "@types/picomatch": "^4.0.2", "jsdom": "^25.0.1", @@ -1331,16 +1330,6 @@ "vite": "^5.0.0" } }, - "node_modules/@types/better-sqlite3": { - "version": "7.6.13", - "resolved": "https://registry.npmjs.org/@types/better-sqlite3/-/better-sqlite3-7.6.13.tgz", - "integrity": "sha512-NMv9ASNARoKksWtsq/SHakpYAYnhBrQgGD8zkLYk/jaK8jUGn08CfEdTRgYhMypUQAfzSP8W6gNLe0q19/t4VA==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/d3-color": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/@types/d3-color/-/d3-color-3.1.3.tgz", @@ -1930,9 +1919,9 @@ } }, "node_modules/devalue": { - "version": "5.9.1", - "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.9.1.tgz", - "integrity": "sha512-+17vil3EVQRzvtDJSFuTWEb8XJRvXqAiV3qZyQWD398QeXUa6CxsUyMdD1fxzEhUrd4FojitFz7lhIHBTlV4fw==", + "version": "5.9.4", + "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.9.4.tgz", + "integrity": "sha512-sPAT4pztbu6586/hrhOnMKS17IJrvg12mXiSPSS3W5qDeN2RGgvZ0diZCm31dBbnevfVmujNO3IM2wrS4Y2Rhg==", "license": "MIT" }, "node_modules/dunder-proto": { @@ -2333,9 +2322,9 @@ } }, "node_modules/ignore": { - "version": "7.0.5", - "resolved": "https://registry.npmjs.org/ignore/-/ignore-7.0.5.tgz", - "integrity": "sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg==", + "version": "7.0.14", + "resolved": "https://registry.npmjs.org/ignore/-/ignore-7.0.14.tgz", + "integrity": "sha512-wCBiZ77u1tuWde6G/nfOtLaCTYcFDU5yCzARzK10y+rLhzYoL7a7yEemcopcc2Mzow7gZOIjnV2ym1NYmdiJOQ==", "license": "MIT", "engines": { "node": ">= 4" @@ -2494,9 +2483,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.11", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.11.tgz", - "integrity": "sha512-N8SpfPUnUp1bK+PMYW8qSWdl9U+wwNWI4QKxOYDy9JAro3WMX7p2OeVRF9v+347pnakNevPmiHhNmZ2HbFA76w==", + "version": "3.3.20", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.20.tgz", + "integrity": "sha512-uKdg2G3GNCKQn9byYOpxbGqrT2fGO5KRt5J/8b3pok8rT6qxGWF6hxMyJiEYtAf+FVyYuD9hRaDqX5uPFYJ4ZQ==", "dev": true, "funding": [ { @@ -2583,9 +2572,9 @@ } }, "node_modules/postcss": { - "version": "8.5.6", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.6.tgz", - "integrity": "sha512-3Ybi1tAuwAP9s0r1UQ2J4n5Y0G05bJkpUIO0/bI9MhwmD70S5aTWbXGBwxHrelT+XM1k6dM0pk+SwNkpTRN7Pg==", + "version": "8.5.32", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.32.tgz", + "integrity": "sha512-LJlKCCAUFKOuZxipUdYw1NzIITEsPcgyOG9LkvRJvuKj/bRzbuRY0ARXvL0pWGpsZvn8jku+LMep2eU0seHp8g==", "dev": true, "funding": [ { @@ -2603,9 +2592,9 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.11", + "nanoid": "^3.3.20", "picocolors": "^1.1.1", - "source-map-js": "^1.2.1" + "source-map-js": "^1.2.2" }, "engines": { "node": "^10 || ^12 || >=14" @@ -2754,9 +2743,9 @@ "license": "MIT" }, "node_modules/source-map-js": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.1.tgz", - "integrity": "sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.2.tgz", + "integrity": "sha512-KGj/8Y43x35aZVDtt+J4mK1hoLGHULMYfSkODJNQjNDC3oW1PqPoxMwo0pLUsWM/UEGzON/NxeHywEfNXNP3Vw==", "dev": true, "license": "BSD-3-Clause", "engines": { diff --git a/package.json b/package.json index 09dbf4fe1..7213e89e0 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "entities": "6.0.1", "fast-string-width": "^3.0.2", "fast-wrap-ansi": "^0.2.0", - "ignore": "^7.0.5", + "ignore": "^7.0.14", "jsonc-parser": "^3.3.1", "picomatch": "^4.0.3", "sisteransi": "^1.0.5" @@ -63,7 +63,6 @@ "devDependencies": { "@ast-grep/cli": "^0.45.3", "@sveltejs/vite-plugin-svelte": "^4.0.4", - "@types/better-sqlite3": "^7.6.0", "@types/node": "^20.19.30", "@types/picomatch": "^4.0.2", "jsdom": "^25.0.1", diff --git a/site/package-lock.json b/site/package-lock.json index 442666fc2..fa0977595 100644 --- a/site/package-lock.json +++ b/site/package-lock.json @@ -1916,7 +1916,6 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.16.0.tgz", "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2003,7 +2002,6 @@ "resolved": "https://registry.npmjs.org/astro/-/astro-6.3.7.tgz", "integrity": "sha512-zIeDRrI0qNgN1lcCjNqt6/IVCVej7VwSa326cO8uP9BOk1cg4QuffhLnOn2gCgWQr32/wxpSRFfXiLKHglu1Tw==", "license": "MIT", - "peer": true, "dependencies": { "@astrojs/compiler": "^4.0.0", "@astrojs/internal-helpers": "0.9.1", @@ -2457,9 +2455,9 @@ } }, "node_modules/devalue": { - "version": "5.8.1", - "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.8.1.tgz", - "integrity": "sha512-4CXDYRBGqN+57wVJkuXBYmpAVUSg3L6JAQa/DFqm238G73E1wuyc/JhGQJzN7vUf/CMphYau2zXbfWzDR5aTEw==", + "version": "5.9.4", + "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.9.4.tgz", + "integrity": "sha512-sPAT4pztbu6586/hrhOnMKS17IJrvg12mXiSPSS3W5qDeN2RGgvZ0diZCm31dBbnevfVmujNO3IM2wrS4Y2Rhg==", "license": "MIT" }, "node_modules/devlop": { @@ -4642,9 +4640,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.12", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.12.tgz", - "integrity": "sha512-ZB9RH/39qpq5Vu6Y+NmUaFhQR6pp+M2Xt76XBnEwDaGcVAqhlvxrl3B2bKS5D3NH3QR76v3aSrKaF/Kiy7lEtQ==", + "version": "3.3.20", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.20.tgz", + "integrity": "sha512-uKdg2G3GNCKQn9byYOpxbGqrT2fGO5KRt5J/8b3pok8rT6qxGWF6hxMyJiEYtAf+FVyYuD9hRaDqX5uPFYJ4ZQ==", "funding": [ { "type": "github", @@ -4905,9 +4903,9 @@ } }, "node_modules/postcss": { - "version": "8.5.15", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.15.tgz", - "integrity": "sha512-FfR8sjd4em2T6fb3I2MwAJU7HWVMr9zba+enmQeeWFfCbm+UOC/0X4DS8XtpUTMwWMGbjKYP7xjfNekzyGmB3A==", + "version": "8.5.32", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.32.tgz", + "integrity": "sha512-LJlKCCAUFKOuZxipUdYw1NzIITEsPcgyOG9LkvRJvuKj/bRzbuRY0ARXvL0pWGpsZvn8jku+LMep2eU0seHp8g==", "funding": [ { "type": "opencollective", @@ -4923,11 +4921,10 @@ } ], "license": "MIT", - "peer": true, "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.20", "picocolors": "^1.1.1", - "source-map-js": "^1.2.1" + "source-map-js": "^1.2.2" }, "engines": { "node": "^10 || ^12 || >=14" @@ -5385,7 +5382,6 @@ "resolved": "https://registry.npmjs.org/rollup/-/rollup-4.60.4.tgz", "integrity": "sha512-WHeFSbZYsPu3+bLoNRUuAO+wavNlocOPf3wSHTP7hcFKVnJeWsYlCDbr3mTS14FCizf9ccIxXA8sGL8zKeQN3g==", "license": "MIT", - "peer": true, "dependencies": { "@types/estree": "1.0.8" }, @@ -5562,9 +5558,9 @@ } }, "node_modules/source-map-js": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.1.tgz", - "integrity": "sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.2.tgz", + "integrity": "sha512-KGj/8Y43x35aZVDtt+J4mK1hoLGHULMYfSkODJNQjNDC3oW1PqPoxMwo0pLUsWM/UEGzON/NxeHywEfNXNP3Vw==", "license": "BSD-3-Clause", "engines": { "node": ">=0.10.0" @@ -6049,7 +6045,6 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.3.tgz", "integrity": "sha512-/4XH147Ui7OGTjg3HbdWe5arnZQSbfuRzdr9Ec7TQi5I7R+ir0Rlc9GIvD4v0XZurELqA035KVXJXpR61xhiTA==", "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.27.0", "fdir": "^6.5.0", diff --git a/src/db/migrations.ts b/src/db/migrations.ts index 4b5f44e70..d5aa6cbc7 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -476,6 +476,16 @@ function recordMigration(db: SqliteDatabase, version: number, description: strin /** * Run all pending migrations + * + * Another process may be opening the same index right now and running them + * too (#2980), so `fromVersion`, read before, can be out of date by the time a + * migration runs. Each one runs in a transaction that takes the write lock up + * front, waiting out a process already migrating, and reads the version again + * there, skipping a migration that process has applied. Before, the second + * process applied it again and its open failed: recording the version broke + * the version's uniqueness, or a migration that reads first got SQLITE_BUSY. + * `fromVersion` stays a floor: a database built from schema.sql records only + * v1 and the current version, and a rewound one replays what is above it. */ export function runMigrations(db: SqliteDatabase, fromVersion: number): void { const pending = migrations.filter((m) => m.version > fromVersion); @@ -490,9 +500,10 @@ export function runMigrations(db: SqliteDatabase, fromVersion: number): void { // Run each migration in a transaction for (const migration of pending) { db.transaction(() => { + if (Math.max(fromVersion, getCurrentVersion(db)) >= migration.version) return; migration.up(db); recordMigration(db, migration.version, migration.description); - })(); + }, { immediate: true })(); } } diff --git a/src/db/queries.ts b/src/db/queries.ts index f75735acc..38162791f 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -2226,9 +2226,14 @@ export class QueryBuilder { // so treat it as whitespace before the strip step. Otherwise queries // like `stage_apply::run` collapse to `stage_applyrun` (the colons // are stripped without splitting) and find nothing. See #173. + // + // The other FTS5 syntax characters separate words too, as they do in + // the index, which splits text at anything but letters and digits: + // `leaf(entry)` is stored as `leaf` and `entry`. Dropping them glued a + // pasted call into one word no symbol has (`leafentry`, #2971). const ftsQuery = query .replace(/::/g, ' ') // Rust/C++/Ruby qualifier separator - .replace(/['"*():^]/g, '') // Remove FTS5 special chars + .replace(/['"*():^]/g, ' ') // FTS5 syntax characters separate words .split(/\s+/) .filter(term => term.length > 0) // Strip FTS5 boolean operators to prevent query manipulation diff --git a/src/db/sqlite-adapter.ts b/src/db/sqlite-adapter.ts index 9fee70835..3e7136638 100644 --- a/src/db/sqlite-adapter.ts +++ b/src/db/sqlite-adapter.ts @@ -27,11 +27,22 @@ export interface SqliteStatement { iterate(...params: any[]): IterableIterator; } +export interface TransactionOptions { + /** + * Begin with `BEGIN IMMEDIATE`, taking the write lock up front, instead of a + * deferred `BEGIN`. A connection that finds the lock taken then waits it out + * (busy_timeout); a deferred transaction that has already read gets + * SQLITE_BUSY at once when it comes to write, since SQLite will not wait for + * a lock there. No effect when nested in an open transaction. + */ + immediate?: boolean; +} + export interface SqliteDatabase { prepare(sql: string): SqliteStatement; exec(sql: string): void; pragma(str: string, options?: { simple?: boolean }): any; - transaction(fn: (...args: any[]) => T): (...args: any[]) => T; + transaction(fn: (...args: any[]) => T, options?: TransactionOptions): (...args: any[]) => T; close(): void; readonly open: boolean; /** Undefined on a runtime without `isTransaction`; callers must then not memoize. */ @@ -158,7 +169,7 @@ class NodeSqliteAdapter implements SqliteDatabase { return row; } - transaction(fn: (...args: any[]) => T): (...args: any[]) => T { + transaction(fn: (...args: any[]) => T, options?: TransactionOptions): (...args: any[]) => T { return (...args: any[]) => { // Nested call (a transaction()-wrapped helper invoked from inside another // transaction): run the body directly inside the enclosing transaction. @@ -173,7 +184,7 @@ class NodeSqliteAdapter implements SqliteDatabase { this._txDepth--; } } - this._db.exec('BEGIN'); + this._db.exec(options?.immediate ? 'BEGIN IMMEDIATE' : 'BEGIN'); this._txDepth = 1; try { const result = fn(...args); diff --git a/src/extraction/astro-extractor.ts b/src/extraction/astro-extractor.ts index c43baa53e..0d048dae5 100644 --- a/src/extraction/astro-extractor.ts +++ b/src/extraction/astro-extractor.ts @@ -161,11 +161,14 @@ export class AstroExtractor { private extractScriptBlocks(): Array<{ content: string; startLine: number }> { const blocks: Array<{ content: string; startLine: number }> = []; - const scriptRegex = /]*)?>(?[\s\S]*?)<\/script>/g; + // A self-closing ``. + const scriptRegex = /]*?)?(?:\/>|>(?[\s\S]*?)<\/script>)/g; let match; while ((match = scriptRegex.exec(this.source)) !== null) { - const content = match.groups?.content || match[2] || ''; + if (match.groups?.content === undefined) continue; + const content = match.groups.content; // Calculate the 0-indexed line where the content begins. The content // starts right after the opening tag's `>` — its leading `\n` is part @@ -224,9 +227,12 @@ export class AstroExtractor { coveredRanges.push([frontmatter.startLine - 1, frontmatter.endLine]); } - const tagRegex = /<(script|style)(\s[^>]*)?>[\s\S]*?<\/\1>/g; + const tagRegex = /<(script|style)(\s[^>]*?)?(?:\/>|>[\s\S]*?<\/\1>)/g; let tagMatch; while ((tagMatch = tagRegex.exec(this.source)) !== null) { + // A self-closing tag has no body to skip, and its attributes + // (`set:html={serializeJsonLd(schema)}`) are template expressions. + if (tagMatch[0].endsWith('/>')) continue; const startLine = (this.source.substring(0, tagMatch.index).match(/\n/g) || []).length; const endLine = startLine + (tagMatch[0].match(/\n/g) || []).length; coveredRanges.push([startLine, endLine]); diff --git a/src/extraction/languages/go.ts b/src/extraction/languages/go.ts index fba3726dc..b19aeaaab 100644 --- a/src/extraction/languages/go.ts +++ b/src/extraction/languages/go.ts @@ -203,8 +203,10 @@ export const goExtractor: LanguageExtractor = { }, isExported: (node, source) => { // Go: a symbol is exported when its identifier starts with an uppercase letter. - // Look at the `name` field directly (works for function_declaration, - // method_declaration, type_spec, and var_spec / const_spec via extractor flow). + // Look at the `name` field directly: function_declaration, + // method_declaration, type_spec, and a var_spec / const_spec, whose first + // name is the one the extractor gives a node (never the declaration + // around a spec, which has no name). const nameNode = getChildByField(node, 'name'); if (nameNode) { const text = getNodeText(nameNode, source); diff --git a/src/extraction/liquid-extractor.ts b/src/extraction/liquid-extractor.ts index 4de1826c8..3536ec64e 100644 --- a/src/extraction/liquid-extractor.ts +++ b/src/extraction/liquid-extractor.ts @@ -464,16 +464,20 @@ export class LiquidExtractor { * declarations as variables */ private extractAssignments(fileNodeId: string): void { + // A name is word characters and `-`, as Liquid reads it: `title-text` is one + // name. One that goes on with `.` gets no node rather than a cut-short one. + const name = `\\w[\\w-]*`; // Both spellings. Most of a modern theme's assigns are the bare kind. - for (const match of this.findTagOccurrences('assign', `(\\w+)\\s*=`)) { + for (const match of this.findTagOccurrences('assign', `(${name})\\s*=`)) { this.addVariable(fileNodeId, match.groups[1]!, match); } // A capture declares its name as an assign does, and Liquid reads a quoted - // name (`{% capture 'title' %}`) as the name inside the quotes. A name that - // goes on with `-` or `.`, which the assign rule doesn't read either, gets - // no node rather than a cut-short one. Captures come after assigns, so an - // assign keeps its id when a capture of the same name shares its line. - for (const match of this.findTagOccurrences('capture', `(?:'(\\w+)'|"(\\w+)"|(\\w+))(?=\\s|-?%\\}|$)`)) { + // name (`{% capture 'title' %}`) as the name inside the quotes. A bare name + // ends at the first whitespace or at the tag's end, `-%}` included: in + // `{% capture title-%}` the `-` trims whitespace, so the name is `title`. + // Captures come after assigns, so an assign keeps its id when a capture of + // the same name shares its line. + for (const match of this.findTagOccurrences('capture', `(?:'(${name})'|"(${name})"|(${name}?))(?=\\s|-?%\\}|$)`)) { const [, singleQuoted, doubleQuoted, bare] = match.groups; this.addVariable(fileNodeId, (singleQuoted ?? doubleQuoted ?? bare)!, match); } diff --git a/src/extraction/svelte-extractor.ts b/src/extraction/svelte-extractor.ts index fc1f4e5cd..afacfb5cc 100644 --- a/src/extraction/svelte-extractor.ts +++ b/src/extraction/svelte-extractor.ts @@ -158,12 +158,16 @@ export class SvelteExtractor { isTypeScript: boolean; }> = []; - const scriptRegex = /]*)?>(?[\s\S]*?)<\/script>/g; + // A self-closing ``. + const scriptRegex = /]*?)?(?:\/>|>(?[\s\S]*?)<\/script>)/g; let match; while ((match = scriptRegex.exec(this.source)) !== null) { + if (match.groups?.content === undefined) continue; const attrs = match[1] || ''; - const content = match.groups?.content || match[2] || ''; + const content = match.groups.content; // Detect TypeScript from lang attribute const isTypeScript = sfcScriptLanguage(attrs, 'svelte') === 'typescript'; @@ -236,9 +240,12 @@ export class SvelteExtractor { const coveredRanges: Array<[number, number]> = []; // Find all and ranges - const tagRegex = /<(script|style)(\s[^>]*)?>[\s\S]*?<\/\1>/g; + const tagRegex = /<(script|style)(\s[^>]*?)?(?:\/>|>[\s\S]*?<\/\1>)/g; let tagMatch; while ((tagMatch = tagRegex.exec(this.source)) !== null) { + // A self-closing tag has no body to skip, and its attributes + // (`src={assetUrl('/analytics.js')}`) are template expressions. + if (tagMatch[0].endsWith('/>')) continue; const startLine = (this.source.substring(0, tagMatch.index).match(/\n/g) || []).length; const endLine = startLine + (tagMatch[0].match(/\n/g) || []).length; coveredRanges.push([startLine, endLine]); @@ -318,9 +325,11 @@ export class SvelteExtractor { private extractTemplateComponents(componentNodeId: string): void { // Build ranges covered by `. +const SCRIPT_BLOCK = /]*?)?(?:\/>|>([\s\S]*?)<\/script>)/gi; /** Astro's frontmatter: a `---` fence at the very top of the file. */ const ASTRO_FRONTMATTER = /^(---\r?\n)([\s\S]*?)\r?\n---/; diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 1e5fc3ff8..e4c043e63 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -4291,9 +4291,14 @@ export class TreeSitterExtractor { // A member of a `const ( … )` or `var ( … )` group has its own doc // comment beside it in the parentheses; one without keeps the group's. + // Exported by its own name's case, read from the spec: the + // declaration around it has no name, so `var Hook = …` and + // `const Limit = 3` read as unexported and `a.Hook()` from another + // package linked to nothing. varNode = this.createNode(node.type === 'const_declaration' ? 'constant' : 'variable', name, spec, { docstring: this.docstringFor(spec) ?? docstring, signature: initSignature, + isExported: this.extractor.isExported?.(spec, this.source), }); if (varNode) { this.extractMarkdownPathReferencesFromSubtree(valueNode, varNode.id); diff --git a/src/mcp/proxy.ts b/src/mcp/proxy.ts index df40ef534..188738bcc 100644 --- a/src/mcp/proxy.ts +++ b/src/mcp/proxy.ts @@ -26,8 +26,9 @@ import { EARLY_PPID } from './early-ppid'; import { supervisionLostReason } from './ppid-watchdog'; import { armStartupHandshakeTimeout } from './startup-handshake'; import { treatStdinFailureAsShutdown } from './stdin-teardown'; +import { treatStdoutFailureAsShutdown } from './stdout-teardown'; import { CodeGraphPackageVersion, isOlderRelease } from './version'; -import { SERVER_INFO, PROTOCOL_VERSION, initializeInstructions } from './session'; +import { SERVER_INFO, PROTOCOL_VERSION, initializeInstructions, toolCallArguments, TOOL_ARGUMENTS_NOT_OBJECT } from './session'; import { SERVER_INSTRUCTIONS } from './server-instructions'; import { getStaticTools } from './tools'; import { ErrorCodes } from './transport'; @@ -360,10 +361,15 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< let msg: JsonRpc; try { msg = JSON.parse(line) as JsonRpc; } catch { return; } const id = msg.id; if (msg.method === 'tools/call' && id !== undefined) { + const params = (msg.params || {}) as { name: string; arguments?: unknown }; + const args = toolCallArguments(params); + if (!args) { + writeClient({ jsonrpc: '2.0', id, error: { code: ErrorCodes.InvalidParams, message: TOOL_ARGUMENTS_NOT_OBJECT } }); + return; + } try { const local = await ensureEngine(); - const params = (msg.params || {}) as { name: string; arguments?: Record }; - const result = await local.getToolHandler().execute(params.name, params.arguments || {}, exploreSession); + const result = await local.getToolHandler().execute(params.name, args, exploreSession); writeClient({ jsonrpc: '2.0', id, result }); getTelemetry().recordUsage('mcp_tool', params.name, !result.isError, telemetryClient); } catch (err) { @@ -445,6 +451,10 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< // instead of a clean close; destroying the stream stops a hung fd from // busy-spinning the event loop (#799). treatStdinFailureAsShutdown(shutdown); + // A host can also stop reading our output while it keeps stdin open and + // stays alive: shut down the same way, which closes only this session's + // daemon connection and stops its own engines (#2991). + treatStdoutFailureAsShutdown(shutdown); startPpidWatchdogNoSocket(shutdown); // Backstop for a launch abandoned before any of the above can see it: killed // launcher + held-open pipes + reparent that beat the EARLY_PPID capture diff --git a/src/mcp/session.ts b/src/mcp/session.ts index 0b46cc4a9..50fbcafe1 100644 --- a/src/mcp/session.ts +++ b/src/mcp/session.ts @@ -63,6 +63,23 @@ export function initializeInstructions(base: string, notice: string | null = get /** MCP Protocol Version (latest the server claims). */ export const PROTOCOL_VERSION = '2024-11-05'; +/** The invalid-params message for {@link toolCallArguments}' null. */ +export const TOOL_ARGUMENTS_NOT_OBJECT = 'Tool arguments must be an object'; + +/** + * The arguments a `tools/call` passes, or null when they are not an object. + * A string, number or array there is a malformed request, the client's to + * fix, so it is answered with a JSON-RPC invalid-params error, as a call that + * names no tool is, and never run: run, a string made the tool handler throw, + * which came back as an internal codegraph error to retry (#2981). Exported so + * the proxy, serving in process while the daemon is unavailable, answers it + * the same way. No arguments (missing, null or another empty value) are none. + */ +export function toolCallArguments(params: { arguments?: unknown }): Record | null { + const args = params.arguments || {}; + return typeof args === 'object' && !Array.isArray(args) ? (args as Record) : null; +} + /** * How long to wait for the client's `roots/list` response before giving up * and falling back to the process cwd. @@ -330,7 +347,11 @@ export class MCPSession { } const toolName = params.name; - const toolArgs = params.arguments || {}; + const toolArgs = toolCallArguments(params); + if (!toolArgs) { + this.transport.sendError(request.id, ErrorCodes.InvalidParams, TOOL_ARGUMENTS_NOT_OBJECT); + return; + } const tool = tools.find((t) => t.name === toolName); if (!tool) { diff --git a/src/mcp/stdout-teardown.ts b/src/mcp/stdout-teardown.ts new file mode 100644 index 000000000..aee0020e4 --- /dev/null +++ b/src/mcp/stdout-teardown.ts @@ -0,0 +1,42 @@ +/** + * Treat a failed stdout as a shutdown signal — issue #2991. + * + * A host can stop reading an MCP stdio server's output while it keeps the + * server's stdin open and stays alive itself. The next response the server + * writes then fails (EPIPE on a pipe, ECONNRESET on a socket), and with no + * `'error'` listener on stdout Node hands it to the last-resort fatal handler, + * which exits 1 at once (`src/bin/fatal-handler.ts`). That skips the + * shutdown that stops the engine, releases the writer lock and waits for query + * workers still starting (a process that exits under one of those can crash + * on Windows). Nothing else notices this failure: stdin is still open, the + * host's PID is alive and the event loop is healthy, so the stdin teardown + * (#799), the PPID watchdog and the liveness watchdog all stay quiet. + * + * So listen for `'error'`, and for the `'close'` that follows a failed stream, + * destroy stdout so later writes are dropped, and run the owner's shutdown, + * at most once. The listener stays attached: a write queued before the + * failure can still fail while the shutdown runs. A `write()` that returns + * false is backpressure, not failure, and is left alone. A reader that goes + * away while the server is silent is noticed at its next write. + * + * The twin of {@link treatStdinFailureAsShutdown} (`stdin-teardown.ts`). + * `stream` is injectable for tests; it defaults to `process.stdout`. + */ +export function treatStdoutFailureAsShutdown( + onTerminal: () => void, + stream: NodeJS.WritableStream = process.stdout +): void { + let fired = false; + const fire = (): void => { + if (fired) return; + fired = true; + try { + (stream as Partial<{ destroy(): void }>).destroy?.(); + } catch { + /* already gone */ + } + onTerminal(); + }; + stream.on('error', fire); + stream.on('close', fire); +} diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 30a3dedce..67025426f 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -2253,6 +2253,66 @@ export const tools: ToolDefinition[] = [ * explore's `['query']` becomes `['query', 'projectPath']`, and a tool with no * `required` list (status/files) gains `['projectPath']`. */ +/** A path's first folder, or '' for a file at the project root. */ +function topLevelFolder(filePath: string): string { + const slash = filePath.indexOf('/'); + return slash > 0 ? filePath.slice(0, slash) : ''; +} + +/** + * The order in which codegraph_node's multi-definition answer fills its + * capped list (#1657): the first definition in each language, then the first + * in each top-level folder within a language, then the rest, each tier in the + * order given. The index lists a name's definitions by path, so forty under an + * early-sorting folder used to take every slot, and a late folder's or another + * language's definition showed up nowhere. A set within one language and + * folder keeps its order. + */ +export function definitionCapOrder(nodes: readonly Node[]): Node[] { + const languages = new Set(); + const folders = new Set(); + const languageFirsts: Node[] = []; + const folderFirsts: Node[] = []; + const rest: Node[] = []; + for (const n of nodes) { + const folder = `${n.language}\0${topLevelFolder(n.filePath)}`; + const newFolder = !folders.has(folder); + folders.add(folder); + if (!languages.has(n.language)) { + languages.add(n.language); + languageFirsts.push(n); + } else if (newFolder) { + folderFirsts.push(n); + } else { + rest.push(n); + } + } + return [...languageFirsts, ...folderFirsts, ...rest]; +} + +/** + * What a capped definition list left out, by language and top-level folder, + * largest first: "35 typescript in client/, 2 python at the project root". + */ +export function describeOmittedDefinitions(nodes: readonly Node[]): string { + const groups = new Map(); + for (const n of nodes) { + const folder = topLevelFolder(n.filePath); + const key = `${n.language}\0${folder}`; + const group = groups.get(key); + if (group) group.count++; + else groups.set(key, { language: n.language, folder, count: 1 }); + } + const MAX_NAMED = 6; + const ranked = [...groups.values()].sort((a, b) => b.count - a.count); + const parts = ranked + .slice(0, MAX_NAMED) + .map((g) => `${g.count} ${g.language} ${g.folder ? `in ${g.folder}/` : 'at the project root'}`); + const elsewhere = ranked.slice(MAX_NAMED).reduce((sum, g) => sum + g.count, 0); + if (elsewhere > 0) parts.push(`${elsewhere} elsewhere`); + return parts.join(', '); +} + function withRequiredProjectPath(defs: ToolDefinition[]): ToolDefinition[] { return defs.map((tool) => { // A tool that names its own projects (codegraph_sessions `projects`) works without projectPath. @@ -9239,12 +9299,49 @@ export class ToolHandler { // FULL bodies as fit a char budget (the agent gets the one it needs in this // one call, no follow-up parameter to learn), and list any remainder by // file:line so a large overload set can't overflow the per-tool cap. + // + // The index lists a name's definitions by path, so forty under an + // early-sorting folder used to take every body and list slot, and the only + // one in a late folder or another language showed up nowhere, past a bare + // "+N more" (#1657). Now a language with no body gets one on top of the + // overloads that fit, and the list's slots go first to each language and + // top-level folder (`definitionCapOrder`); an overflow names the languages + // and folders it leaves out. Overloads that fit before still render, and + // what is shown keeps index order, so a set that fits every cap reads + // exactly as before. const header = `**${matches.length} definitions named "${symbol}"**`; + const indexOrder = new Map(matches.map((n, i) => [n, i] as const)); + const inIndexOrder = (items: T[], nodeOf: (item: T) => Node): T[] => + [...items].sort((a, b) => indexOrder.get(nodeOf(a))! - indexOrder.get(nodeOf(b))!); + const isGenerated = cg.generatedFilePredicate(matches.map((n) => n.filePath)); + // Generated stubs stay behind every hand-written definition (findSymbolMatches). + const capOrderOf = (nodes: Node[]): Node[] => [ + ...definitionCapOrder(nodes.filter((n) => !isGenerated(n.filePath))), + ...definitionCapOrder(nodes.filter((n) => isGenerated(n.filePath))), + ]; + const listLine = (n: Node) => `- \`${n.name}\` (${n.kind}) — ${n.filePath}:${n.startLine}`; + const omittedLine = (omitted: Node[]) => `- … +${omitted.length} more: ${describeOmittedDefinitions(omitted)}`; if (!includeCode) { - const list = matches.map((n) => `- \`${n.name}\` (${n.kind}) — ${n.filePath}:${n.startLine}`); - return this.textResult(this.truncateOutput( - [header, '', 'Re-query with `includeCode: true` to get every body in one call — no need to pick one first.', '', ...list].join('\n'), - )); + const prefix = [header, '', 'Re-query with `includeCode: true` to get every body in one call — no need to pick one first.', '']; + const prefixChars = prefix.join('\n').length + 1; + let list = matches.map(listLine); + if (prefixChars + list.join('\n').length > MAX_OUTPUT_LENGTH) { + // Too long for the output cap, which would cut the tail off unnamed: + // fill what fits in cap order, keeping room to name the rest. + const OMITTED_LINE_RESERVE = 1000; + const budget = MAX_OUTPUT_LENGTH - prefixChars - OMITTED_LINE_RESERVE; + const capOrder = capOrderOf(matches); + const shown: Node[] = []; + let listChars = 0; + for (const n of capOrder) { + const lineChars = listLine(n).length + 1; + if (listChars + lineChars > budget) break; + shown.push(n); + listChars += lineChars; + } + list = [...inIndexOrder(shown, (n) => n).map(listLine), omittedLine(capOrder.slice(shown.length))]; + } + return this.textResult(this.truncateOutput([...prefix, ...list].join('\n'))); } const BODY_BUDGET = 12000; // leaves room under MAX_OUTPUT_LENGTH for the header + list @@ -9253,42 +9350,63 @@ export class ToolHandler { // render in full rather than relegating the one the agent wanted to a // bodiless list. Only a set of many LARGE bodies hits the char budget first. const HARD_CAP = 16; - const rendered: string[] = []; - const listed: Node[] = []; + type Body = { node: Node; section: string }; + let rendered: Body[] = []; + let notRendered: Node[] = []; let used = 0; for (const n of matches) { - if (rendered.length >= HARD_CAP) { listed.push(n); continue; } + if (rendered.length >= HARD_CAP) { notRendered.push(n); continue; } const section = await this.renderNodeSection(cg, n, true); // Always emit the first; emit the rest only while within the char budget. if (rendered.length === 0 || used + section.length <= BODY_BUDGET) { - rendered.push(section); + rendered.push({ node: n, section }); used += section.length; } else { - listed.push(n); + notRendered.push(n); } } - const out: string[] = [ - header, - `Returning ${rendered.length} in full${listed.length ? `; ${listed.length} more listed below` : ''} — pick the one you need (no Read required).`, - '', - rendered.join('\n\n---\n\n'), - ]; - if (listed.length) { - const LIST_CAP = 20; - const shownList = listed.slice(0, LIST_CAP); - out.push( + const LIST_CAP = 20; + const compose = (bodies: Body[], rest: Node[]): string => { + const restInCapOrder = capOrderOf(rest); + const listed = inIndexOrder(restInCapOrder.slice(0, LIST_CAP), (n) => n); + const omitted = restInCapOrder.slice(LIST_CAP); + const out: string[] = [ + header, + `Returning ${bodies.length} in full${rest.length ? `; ${rest.length} more listed below` : ''} — pick the one you need (no Read required).`, '', - '**Other definitions**', - ...shownList.map((n) => `- \`${n.name}\` (${n.kind}) — ${n.filePath}:${n.startLine}`), - ); - if (listed.length > LIST_CAP) out.push(`- … +${listed.length - LIST_CAP} more`); - out.push( - '', - `> Need one of these in full? Call codegraph_node again with \`file\` (e.g. \`"${listed[0]!.filePath.split('/').pop()}"\`) or \`line\` — do NOT Read it.`, - ); - } - return this.textResult(this.truncateOutput(out.join('\n'))); + inIndexOrder(bodies, (b) => b.node).map((b) => b.section).join('\n\n---\n\n'), + ]; + if (listed.length) { + out.push( + '', + '**Other definitions**', + ...listed.map(listLine), + ); + if (omitted.length) out.push(omittedLine(omitted)); + out.push( + '', + `> Need one of these in full? Call codegraph_node again with \`file\` (e.g. \`"${listed[0]!.filePath.split('/').pop()}"\`) or \`line\` — do NOT Read it.`, + ); + } + return out.join('\n'); + }; + + // Each language still without a body gets its first hand-written + // definition rendered on top, while the whole answer stays under the + // output cap; one that doesn't fit keeps its early line in the list. + const languagesWithBody = new Set(rendered.map((b) => b.node.language)); + for (const n of [...notRendered]) { + if (languagesWithBody.has(n.language) || isGenerated(n.filePath)) continue; + languagesWithBody.add(n.language); + const bodies = [...rendered, { node: n, section: await this.renderNodeSection(cg, n, true) }]; + const rest = notRendered.filter((m) => m !== n); + if (compose(bodies, rest).length <= MAX_OUTPUT_LENGTH) { + rendered = bodies; + notRendered = rest; + } + } + return this.textResult(this.truncateOutput(compose(rendered, notRendered))); } /** diff --git a/src/mcp/transport.ts b/src/mcp/transport.ts index 3a2458bb1..78c515215 100644 --- a/src/mcp/transport.ts +++ b/src/mcp/transport.ts @@ -16,6 +16,7 @@ import * as readline from 'readline'; import type { Socket } from 'net'; +import { treatStdoutFailureAsShutdown } from './stdout-teardown'; /** * JSON-RPC 2.0 Request @@ -312,6 +313,9 @@ export class StdioTransport extends LineBasedJsonRpcTransport { }; this.rl.on('close', onStreamEnd); process.stdin.on('error', onStreamEnd); + // A host that stops reading our output, with stdin still open, gets the + // same shutdown rather than the fatal handler's exit 1 (#2991). + treatStdoutFailureAsShutdown(onStreamEnd); } stop(): void { diff --git a/src/utils.ts b/src/utils.ts index 905a4df8c..707c1f7b0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -31,6 +31,7 @@ import * as fs from 'fs'; import * as path from 'path'; +import { threadId } from 'worker_threads'; // ============================================================ // SECURITY UTILITIES @@ -275,6 +276,26 @@ export function escapeRegExp(text: string): string { return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } +/** + * An empty lock file younger than this is still being written by the process + * that created it (#2979): where hard links are missing, the lock is created + * first and the PID written a moment later. Only a crash leaves one empty for + * long, and FAT's 2 s timestamps stay well inside this. + */ +const EMPTY_LOCK_GRACE_MS = 10_000; + +/** + * How many `.break` levels a takeover may climb. Each level is needed only if + * a process died inside the level above it, within a few file operations. + */ +const MAX_LOCK_BREAK_DEPTH = 3; + +/** What a lock file says about its holder. */ +type LockHolder = + | { state: 'free' } + | { state: 'held'; pid?: number } + | { state: 'stale' }; + /** * Cross-process file lock using a lock file with PID tracking. * @@ -291,47 +312,115 @@ export class FileLock { /** * Acquire the lock. Throws if the lock is held by another live process. + * + * A free lock costs one create. A stale one, whose PID is dead or missing, + * is replaced only by the process holding `.break` (itself a + * FileLock), after reading it again there (#2979). Before, two processes + * that read the same dead PID both unlinked the lock and created their own, + * the second one deleting the first one's live lock, and both went on to + * write as the only writer. */ acquire(): void { - // Check for existing lock - if (fs.existsSync(this.lockPath)) { - try { - const content = fs.readFileSync(this.lockPath, 'utf-8').trim(); - const pid = parseInt(content, 10); - // A long sync can hold the lock for hours. Its age says nothing about - // whether the writer is still alive; only a dead PID is stale. - if (!isNaN(pid) && this.isProcessAlive(pid)) { - throw new Error( - `CodeGraph database is locked by another process (PID ${pid}). ` + - `If this is stale, run 'codegraph unlock' or delete ${this.lockPath}` - ); - } + const refusal = this.tryAcquire(0); + if (refusal) { + const who = refusal.state === 'held' && refusal.pid !== undefined ? ` (PID ${refusal.pid})` : ''; + throw new Error( + `CodeGraph database is locked by another process${who}. ` + + `If this is stale, run 'codegraph unlock' or delete ${this.lockPath}` + ); + } + } - // Stale lock (dead process or malformed PID) - remove it - fs.unlinkSync(this.lockPath); - } catch (err) { - if (err instanceof Error && err.message.includes('locked by another')) { - throw err; - } - // Other errors reading lock file - try to remove it - try { fs.unlinkSync(this.lockPath); } catch { /* ignore */ } + /** Null once the lock is ours; otherwise what kept us out. */ + private tryAcquire(depth: number): LockHolder | null { + if (this.create()) return this.taken(); + let holder = this.readHolder(); + if (holder.state === 'free') { + // Released between our create and our read. + if (this.create()) return this.taken(); + holder = this.readHolder(); + } + // A long sync can hold the lock for hours. Its age says nothing about + // whether the writer is still alive; only a dead PID is stale. + if (holder.state !== 'stale') return holder; + if (depth >= MAX_LOCK_BREAK_DEPTH) return { state: 'held' }; + const breaker = new FileLock(`${this.lockPath}.break`); + // Refused: a live process is replacing this stale lock right now. + const breaking = breaker.tryAcquire(depth + 1); + if (breaking) return breaking; + try { + // What we read before holding the break lock may be out of date: another + // process may have replaced the stale lock, or released its own, since. + const current = this.readHolder(); + if (current.state === 'held') return current; + // Nobody else replaces a stale lock while we hold the break lock, and no + // create succeeds while the file exists, so this unlinks the stale lock. + if (current.state === 'stale') { + try { fs.unlinkSync(this.lockPath); } catch { /* gone, or the create below says who has it */ } } + return this.create() ? this.taken() : this.readHolder(); + } finally { + breaker.release(); + } + } + + private taken(): null { + this.held = true; + return null; + } + + /** + * Create the lock file with our PID already in it, so no reader ever finds + * it empty: write a private file, then hard-link it into place, which fails + * if the lock exists. Where hard links are missing, fall back to an + * exclusive create, whose PID lands a moment later (see + * EMPTY_LOCK_GRACE_MS). False when the lock already exists. + */ + private create(): boolean { + const pid = String(process.pid); + const tmp = `${this.lockPath}.${process.pid}.${threadId}.tmp`; + try { + // Clear a leftover (or planted) entry first; 'wx' then refuses to write through a link. + try { fs.unlinkSync(tmp); } catch { /* nothing there */ } + fs.writeFileSync(tmp, pid, { flag: 'wx' }); + fs.linkSync(tmp, this.lockPath); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') return false; + // No hard links here (or no private file): create in place instead. + } finally { + try { fs.unlinkSync(tmp); } catch { /* never written */ } } + try { + fs.writeFileSync(this.lockPath, pid, { flag: 'wx' }); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') return false; + throw err; + } + } - // Write our PID to the lock file using exclusive create flag + private readHolder(): LockHolder { + let content: string; try { - fs.writeFileSync(this.lockPath, String(process.pid), { flag: 'wx' }); - this.held = true; - } catch (err: any) { - if (err.code === 'EEXIST') { - // Race condition: another process grabbed the lock between our check and write - throw new Error( - 'CodeGraph database is locked by another process. ' + - `If this is stale, run 'codegraph unlock' or delete ${this.lockPath}` - ); + content = fs.readFileSync(this.lockPath, 'utf-8').trim(); + } catch (err) { + // Unreadable is not proof the holder is gone: leave it to `codegraph unlock`. + return (err as NodeJS.ErrnoException).code === 'ENOENT' ? { state: 'free' } : { state: 'held' }; + } + if (content === '') { + let age: number; + try { + age = Date.now() - fs.statSync(this.lockPath).mtimeMs; + } catch { + return { state: 'free' }; } - throw err; + return age < EMPTY_LOCK_GRACE_MS ? { state: 'held' } : { state: 'stale' }; } + const pid = parseInt(content, 10); + // Malformed: no process owns it. + if (!(pid > 0)) return { state: 'stale' }; + return this.isProcessAlive(pid) ? { state: 'held', pid } : { state: 'stale' }; } /** @@ -382,8 +471,9 @@ export class FileLock { try { process.kill(pid, 0); return true; - } catch { - return false; + } catch (err) { + // EPERM: it exists but is not ours to signal (another user's, or elevated on Windows). + return (err as NodeJS.ErrnoException).code === 'EPERM'; } } }