diff --git a/.claude/skills/codegraph-build-validation/SKILL.md b/.claude/skills/codegraph-build-validation/SKILL.md index 4a71704954..5c1f8494fd 100644 --- a/.claude/skills/codegraph-build-validation/SKILL.md +++ b/.claude/skills/codegraph-build-validation/SKILL.md @@ -29,7 +29,7 @@ Under Bun the suite runs green via `__tests__/bun-homedir.setup.ts` (an `os.home `copy-assets` (called from `build`) copies `src/db/schema.sql` into `dist/`. **Any new SQL asset must be copied or it won't ship.** -The native kernel (`codegraph-kernel/`, Rust) is the **only parser**: every grammar is compiled into it (crates.io pins plus the vendored C under `codegraph-kernel/grammars/`, provenance in `grammars/PROVENANCE.md`). There is no wasm fallback. A source checkout needs a prebuild at `codegraph-kernel/prebuilds/-/codegraph-kernel.node`; `npm test` builds it through `scripts/ensure-kernel.mjs` when it is missing or was built from other kernel sources (needs a Rust toolchain), release bundles ship it, and a platform without a prebuild is unsupported (the CLI says so at startup). Languages with a bespoke walker are extracted in Rust; the rest are parsed by the kernel and walked by the generic TypeScript extractor over the serialized tree (`src/extraction/parse-tree.ts`). The whole-graph golden dumps (`__tests__/kernel-golden-dumps.test.ts`) are the regression gate for any extraction change; see `docs/design/kernel-only-extraction-plan.md`. +The native kernel (`codegraph-kernel/`, Rust) is the **only parser**: every grammar is compiled into it (crates.io pins plus the vendored C under `codegraph-kernel/grammars/`, provenance in `grammars/PROVENANCE.md`). There is no wasm fallback. A source checkout needs a prebuild at `codegraph-kernel/prebuilds/-/codegraph-kernel.node`; `npm test` builds it through `scripts/ensure-kernel.mjs` when it is missing or was built from other kernel sources (needs Rust 1.89 or newer), release bundles ship it, and a platform without a prebuild is unsupported (the CLI says so at startup). Languages with a bespoke walker are extracted in Rust; the rest are parsed by the kernel and walked by the generic TypeScript extractor over the serialized tree (`src/extraction/parse-tree.ts`). The whole-graph golden dumps (`__tests__/kernel-golden-dumps.test.ts`) are the regression gate for any extraction change; see `docs/design/kernel-only-extraction-plan.md`. One other build step writes into `dist/` and is subject to the same rule: `build:ui` builds the browser viewer into `dist/viewer/` (never `dist/ui/` — that's the terminal ui). diff --git a/.github/workflows/daemon-lifecycle.yml b/.github/workflows/daemon-lifecycle.yml new file mode 100644 index 0000000000..214a9669c1 --- /dev/null +++ b/.github/workflows/daemon-lifecycle.yml @@ -0,0 +1,57 @@ +name: Daemon installation lifecycle +on: + pull_request: + branches: [fork/consolidated] + paths: + - 'src/mcp/**' + - 'src/extraction/**' + - 'src/codegraph.ts' + - 'codegraph-kernel/**' + - '__tests__/daemon-*.test.ts' + - '__tests__/mcp-*.test.ts' + - '__tests__/writer-lock.test.ts' + - '__tests__/writer-handover-ordering.test.ts' + - '__tests__/fixtures/writer-lock-ordering.cjs' + - '__tests__/query-pool.test.ts' + - '__tests__/status-index-health.test.ts' + - '__tests__/fixtures/older-daemon.cjs' + - '.github/workflows/daemon-lifecycle.yml' +permissions: + contents: read +jobs: + lifecycle: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} + timeout-minutes: 25 + env: + CARGO_BUILD_JOBS: 2 + CARGO_PROFILE_RELEASE_DEBUG: 0 + steps: + - name: Check out the tested commit + shell: bash + run: | + git init . + git config core.autocrlf false + git remote add origin "https://github.com/$GITHUB_REPOSITORY.git" + git fetch --no-tags --depth=1 origin "$GITHUB_SHA" + git checkout --detach FETCH_HEAD + - name: Use the runner's Node 24 installation + shell: pwsh + run: | + $versions = Get-ChildItem "$env:RUNNER_TOOL_CACHE/node" -Directory + $version = $versions | Where-Object Name -like '24.*' | Sort-Object { [version]$_.Name } -Descending | Select-Object -First 1 + if (!$version) { throw 'Node 24 is missing from the runner tool cache' } + $arch = [System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture.ToString().ToLowerInvariant() + $nodeDir = Join-Path $version.FullName $arch + if (!$IsWindows) { $nodeDir = Join-Path $nodeDir 'bin' } + $nodeExe = if ($IsWindows) { 'node.exe' } else { 'node' } + if (!(Test-Path (Join-Path $nodeDir $nodeExe))) { throw 'Node 24 executable is missing' } + Add-Content $env:GITHUB_PATH $nodeDir + - run: node -e "if (process.versions.node.split('.')[0] !== '24') process.exit(1)" + - run: npm ci --ignore-scripts + - run: npm run build:kernel + - run: npm run build + - run: npx vitest run __tests__/daemon-install-check.test.ts __tests__/daemon-older-version.test.ts __tests__/cli-version.test.ts __tests__/daemon-registry.test.ts __tests__/daemon-pid-reuse.test.ts __tests__/daemon-lock-refresh.test.ts __tests__/mcp-daemon.test.ts __tests__/daemon-client-liveness.test.ts __tests__/daemon-bind-failure.test.ts __tests__/daemon-socket-fallback.test.ts __tests__/daemon-rendezvous-case.test.ts __tests__/writer-lock.test.ts __tests__/writer-handover-ordering.test.ts __tests__/mcp-writer-lock.test.ts __tests__/mcp-projectpath-lifecycle.test.ts __tests__/query-pool.test.ts __tests__/status-index-health.test.ts --maxWorkers=2 --minWorkers=1 diff --git a/CHANGELOG.md b/CHANGELOG.md index f022b16535..1759dd2257 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Breaking Changes +- Source builds require Rust 1.89 or newer for native writer coordination. + - **The native engine is now the only parser; the older WebAssembly parser is gone.** Every supported language is built into the native engine, so there is no fallback for a platform without a native build. Release bundles cover macOS (Intel and Apple silicon), Linux (x64 and arm64, glibc) and Windows (x64 and arm64); other platforms need a from-source build with a Rust toolchain, and CodeGraph now says so at startup instead of failing later. Installs are about 40% smaller and the MCP server starts one process lighter. `CODEGRAPH_KERNEL_LANGS` and `CODEGRAPH_NO_RELAUNCH` no longer do anything; `CODEGRAPH_KERNEL=0` now only switches the per-language native walkers off for debugging (every file is still parsed natively). - **Node.js 25 and newer are no longer refused.** The block existed for a V8 bug in compiling the old parser's grammars. Those versions are untested rather than unsupported. - **Node.js 22.13 or Bun 1.4.0 is now the minimum.** Both are the first releases where the built-in `node:sqlite` loads without a flag; older ones failed at the first database open. CodeGraph now says so at startup (`CODEGRAPH_ALLOW_UNSAFE_NODE=1` skips the check). `package.json` declares both in `engines`. @@ -87,6 +89,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Writer handovers, stale-lock cleanup, readiness updates and releases now share an OS file lock, preventing a competing launcher from overwriting a newer writer. The OS releases coordination locks on process exit. Contended retirement retries release while the owner stays alive; legacy daemons without coordinated handover stay running until their old sessions are restarted. + - Python member calls no longer keep a receiver's original type after it is reassigned on a later line. - Source builds now rebuild the native kernel after edits to new, untracked source files instead of using an older prebuild. @@ -286,6 +290,18 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Local JavaScript and TypeScript calls stay connected through linked packages and imports configured by a nested `baseUrl` (#1715). - Reading indexed files on Windows no longer produces false edit warnings when NTFS last-access updates are enabled. Thanks @JJordan0C. (#1451, #1472) + +- In VB.NET, every member of a `Structure` is now indexed, including its fields, properties, methods, constructors and nested enums. Before, only the first member was, so the rest could not be found and their callers looked empty. + +- In VB.NET and C#, what a property's `Get` and `Set` code calls, creates and reads now belongs to that property, as do C#'s `get => …` accessors and `=> …` property bodies. Before, it was dropped, so a method used only from a property looked unused. + +- In VB.NET, a field or property initializer like `= Compute()` or `As New List(Of Order)` now links what it calls and creates, and so do a `Custom Event`'s `AddHandler`, `RemoveHandler` and `RaiseEvent` blocks. Re-index VB.NET and C# projects after upgrading. + +- Upgrading CodeGraph while an agent session is open no longer leaves the old version's background server in charge of your project: the first session started from the new install stops it and starts a current one in its place, even while sessions opened before the upgrade are still running. That old server could no longer load the language parsers the upgrade removed, so it saved every file it re-indexed with no symbols, while new sessions could only read the index beside it without keeping it up to date. A background server from a newer install is never stopped, and sessions opened before the upgrade keep the old version until you restart them. Thanks @lipchey for the report. (#2335) + +- A file is no longer saved with no symbols when its language parser can't be loaded, which is what happened to every file a background server re-indexed after an upgrade removed its install: the file keeps what it had and is indexed again once the parser loads, and files an earlier version emptied this way are re-indexed by the next sync. A background server also exits on its own once its install is upgraded or removed, so the next session starts one from the current install. Thanks @lipchey for the report. (#2335) + +- `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) #### MCP / indexing - Full indexes of large projects no longer sometimes run two to three times slower than usual. A timing clash between two background database maintenance steps could switch reference resolution from several threads to one partway through. diff --git a/README.md b/README.md index 096caab828..5c494744f5 100644 --- a/README.md +++ b/README.md @@ -52,13 +52,13 @@ Follow [@getcodegraph](https://x.com/getcodegraph) on X for updates. ## About this fork -This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`6560052a`](https://github.com/colbymchenry/codegraph/commit/6560052a6f856855d3f71eee838fd66ccfa4285d) (v1.6.2, merged 2026-10-03) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. +This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`dea076fd`](https://github.com/colbymchenry/codegraph/commit/dea076fd1e9fa3e236e93fa915bb0feebe73579d) (after v1.6.2, merged 2026-10-05) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. The fork publishes no releases. The install scripts, npm package, badges and `codegraph upgrade` further down this page install **upstream's** releases. To run the fork, build it from source (below). ### Install the fork from source -You need Node.js 22.13 or newer (or Bun 1.4.0 or newer), git, and a [Rust toolchain](https://rustup.rs/) for the native kernel. +You need Node.js 22.13 or newer (or Bun 1.4.0 or newer), git, and a [Rust toolchain](https://rustup.rs/) 1.89 or newer for the native kernel. ```bash git clone https://github.com/bompus/codegraph.git @@ -74,7 +74,7 @@ Then run `codegraph init` in each project, as in [Get Started](#get-started). In ### What the fork adds -Compared with upstream `main` at `6560052` (v1.6.2). Each item here and in the dispatch and framework lists below was checked against upstream's tree at that commit. +Compared with upstream `main` at `dea076fd` (after v1.6.2). Each item here and in the dispatch and framework lists below was checked against upstream's tree at that commit. | Feature | Upstream | Fork | What it does | |---|:-:|:-:|---| @@ -106,6 +106,12 @@ Compared with upstream `main` at `6560052` (v1.6.2). Each item here and in the d The other languages are the same in both, listed under [Supported Languages](#supported-languages). +C# property accessors and expression-bodied properties contribute calls and references owned by the property. VB.NET member bodies and field initializers also contribute their calls and references. + +`codegraph status` reports files that need re-indexing and files with recorded parse errors. `status --json` includes `index.filesNeedingReindex` and `index.filesWithParseErrors`; `files --json` includes each file's extraction errors. A transient parser failure preserves the previous graph and retries on the next sync. + +The MCP launcher can replace a daemon from an older release when its hello confirms coordinated writer handover. Legacy daemons stay running while new sessions serve reads without auto-sync; stop the old MCP sessions and daemon, then reconnect with the current install. A daemon exits when its installation is deleted or its package version changes. Different managed builds of the same release retain the fork's version-identity checks. + Dispatch and framework coverage the fork adds, by kind: **Dispatch links** @@ -1102,7 +1108,7 @@ Framework routing is validated the same way, on a canonical app per framework: E **MCP server not connecting** — Your agent starts the server itself, so you don't launch it by hand. Make sure the project is initialized and indexed (`codegraph status`) and that the path in your MCP config is correct. If it still won't connect, re-run `codegraph install` to rewrite the config. -**Two `codegraph serve --mcp` on one project fight over the index / auto-sync stops** — CodeGraph allows one live MCP *writer* per project (the shared background daemon, or a single direct-mode process). Extra clients should proxy to that daemon. If you set `CODEGRAPH_NO_DAEMON=1`, run only one `serve --mcp` for that project; a second instance exits with a clear writer-lock error (see `writer.pid` under `.codegraph/`). Prefer leaving the daemon enabled so multiple MCP hosts share one watcher. +**Two `codegraph serve --mcp` on one project fight over the index / auto-sync stops** — CodeGraph allows one live MCP *writer* per project (the shared background daemon, or a single direct-mode process). Extra clients should proxy to that daemon. If you set `CODEGRAPH_NO_DAEMON=1`, run only one `serve --mcp` for that project; a second instance exits with a clear writer-lock error (see `writer.pid` under `.codegraph/`; its persistent `writer.pid.mutation.lock` coordinates ownership changes and needs no stale-lock deletion). Prefer leaving the daemon enabled so multiple MCP hosts share one watcher. **MCP tool calls fail with `Transport closed` while `codegraph status`/`sync` are healthy** — almost always WSL2 with the project on a Windows drive (a `/mnt/c` or `/mnt/d` path), where the local socket CodeGraph uses to share one background server across sessions is unreliable. CodeGraph now falls back to serving the session in-process instead of dropping the connection, and keeps trying to reach the shared server in the background: first after 5 seconds, backing off to every 5 minutes. `CODEGRAPH_DAEMON_RETRY_MS` and `CODEGRAPH_DAEMON_RETRY_MAX_MS` set those two delays, and `CODEGRAPH_DAEMON_RETRY_MS=0` stops retrying. If you still hit it, set `CODEGRAPH_NO_DAEMON=1` in your MCP server's environment to skip the shared server entirely (each session runs in its own process). Moving the project onto the Linux-native filesystem (e.g. under `~/` instead of `/mnt/`) restores the shared server. diff --git a/__tests__/csharp-property-accessors.test.ts b/__tests__/csharp-property-accessors.test.ts new file mode 100644 index 0000000000..6e9cc6ae4a --- /dev/null +++ b/__tests__/csharp-property-accessors.test.ts @@ -0,0 +1,91 @@ +/** + * C# property accessor bodies belong to the property. `get { … }`, + * `set { … }`, `get => …` and an expression-bodied `=> …` property used to + * be skipped by both extractors, so the calls, instantiations and static + * reads written there were lost and a getter's callee looked unused. + * + * Runs against the native walker and the generic walker over native trees, + * which must agree. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import type { Node } from '../src/types'; + +const FILES: Record = { + 'Helper.cs': `public static class Helper { + public static int Compute(int n) => n; + public static void Store(int n) {} + public static int Arrow() => 1; + public static int GetA() => 1; + public static void SetA(int v) {} +} +public class Widget {} +public static class Defaults { public const string Name = "x"; } +`, + 'Box.cs': `public class Box { + private int _x; + public int Value { + get { return Helper.Compute(_x); } + set { Helper.Store(value); _x = value; } + } + public int Arrow => Helper.Arrow(); + public int Both { get => Helper.GetA(); set => Helper.SetA(value); } + public Widget Made { get { return new Widget(); } } + public string Label { get { return Defaults.Name; } } + private void Handle(int v) { } + public System.Action Handler { get { return Pick(Handle); } } +} +`, +}; + +describe('C# property accessor bodies', () => { + let root = ''; + let cg: CodeGraph | undefined; + let kernel: string | undefined; + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cs-accessors-')); + for (const [rel, content] of Object.entries(FILES)) fs.writeFileSync(path.join(root, rel), content); + kernel = process.env.CODEGRAPH_KERNEL; + }); + + afterEach(() => { + cg?.close(); + cg = undefined; + fs.rmSync(root, { recursive: true, force: true }); + if (kernel === undefined) delete process.env.CODEGRAPH_KERNEL; + else process.env.CODEGRAPH_KERNEL = kernel; + }); + + it.each(['default', 'generic'])('are walked as the property (%s)', async (backend) => { + if (backend === 'generic') process.env.CODEGRAPH_KERNEL = '0'; + else delete process.env.CODEGRAPH_KERNEL; + cg = await CodeGraph.init(root, { index: true }); + const graph = cg; + const member = (qualifiedName: string): Node => { + const node = graph.getNodesInFile('Box.cs').find((n) => n.qualifiedName === qualifiedName); + expect(node, qualifiedName).toBeDefined(); + return node!; + }; + const targets = (node: Node, kind: string): string[] => + graph + .getOutgoingEdgesFrom([node.id]) + .filter((e) => e.kind === kind) + .map((e) => graph.getNode(e.target)!.qualifiedName) + .sort(); + + expect(targets(member('Box::Value'), 'calls')).toEqual(['Helper::Compute', 'Helper::Store']); + expect(targets(member('Box::Arrow'), 'calls')).toEqual(['Helper::Arrow']); + expect(targets(member('Box::Both'), 'calls')).toEqual(['Helper::GetA', 'Helper::SetA']); + expect(targets(member('Box::Made'), 'instantiates')).toEqual(['Widget']); + expect(targets(member('Box::Label'), 'references')).toContain('Defaults'); + // A method passed as a value inside an accessor is the property's + // reference, captured once — not the class's as well. + expect(targets(member('Box::Handler'), 'references')).toContain('Box::Handle'); + expect(targets(member('Box'), 'references')).not.toContain('Box::Handle'); + expect(targets(member('Box'), 'calls')).toEqual([]); + }); +}); diff --git a/__tests__/daemon-install-check.test.ts b/__tests__/daemon-install-check.test.ts new file mode 100644 index 0000000000..614afb6e4a --- /dev/null +++ b/__tests__/daemon-install-check.test.ts @@ -0,0 +1,83 @@ +/** + * A daemon whose install was upgraded or deleted underneath it exits (#2335). + * + * The daemon loads grammars (and the native kernel, and worker scripts) lazily + * from the directory it was started from. An upgrade replaces or deletes that + * directory while the daemon keeps running, so every later load fails; the + * 1.6.1 daemon in #2335 went on re-indexing files as empty for as long as it + * lived. It now checks its own package.json periodically and makes way for a + * daemon started from the current install. The check is pure and the Daemon's + * reaction is tested with it injected; neither starts a real daemon. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { Daemon, installChangedReason } from '../src/mcp/daemon'; + +describe('installChangedReason', () => { + let dir: string; + let pkg: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-install-check-')); + pkg = path.join(dir, 'package.json'); + fs.writeFileSync(pkg, JSON.stringify({ name: '@colbymchenry/codegraph', version: '1.6.2' })); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('is null while the install still carries the running version', () => { + expect(installChangedReason(pkg, '1.6.2')).toBeNull(); + }); + + it('reports an install upgraded in place', () => { + fs.writeFileSync(pkg, JSON.stringify({ version: '1.6.3' })); + expect(installChangedReason(pkg, '1.6.2')).toMatch(/replaced by v1\.6\.3/); + }); + + it('reports an install that was deleted, file or whole directory', () => { + fs.rmSync(pkg); + expect(installChangedReason(pkg, '1.6.2')).toMatch(/removed/); + fs.rmSync(dir, { recursive: true, force: true }); + expect(installChangedReason(pkg, '1.6.2')).toMatch(/removed/); + }); + + it('waits for a package.json caught mid-write instead of exiting on it', () => { + fs.writeFileSync(pkg, '{"name": "@colbymchenry/codeg'); + expect(installChangedReason(pkg, '1.6.2')).toBeNull(); + fs.writeFileSync(pkg, ''); + expect(installChangedReason(pkg, '1.6.2')).toBeNull(); + }); +}); + +describe('Daemon.checkInstall', () => { + // A Daemon is safe to construct (nothing binds until start()); stop() is + // replaced so a positive check can't tear down the test process. + const makeDaemon = () => { + const d = new Daemon('/tmp/codegraph-install-check-unit-test', { idleTimeoutMs: 0 }) as any; + d.stop = vi.fn(async () => {}); + return d; + }; + + it('stops the daemon when its install changed', () => { + const d = makeDaemon(); + expect(d.checkInstall(() => 'Install removed')).toBe(true); + expect(d.stop).toHaveBeenCalledWith('install changed'); + }); + + it('keeps running while the install is unchanged', () => { + const d = makeDaemon(); + expect(d.checkInstall(() => null)).toBe(false); + expect(d.stop).not.toHaveBeenCalled(); + }); + + it('does nothing once the daemon is already stopping', () => { + const d = makeDaemon(); + d.stopping = true; + expect(d.checkInstall(() => 'Install removed')).toBe(false); + expect(d.stop).not.toHaveBeenCalled(); + }); +}); diff --git a/__tests__/daemon-older-version.test.ts b/__tests__/daemon-older-version.test.ts new file mode 100644 index 0000000000..ead655dde0 --- /dev/null +++ b/__tests__/daemon-older-version.test.ts @@ -0,0 +1,799 @@ +/** + * A launcher replaces a daemon from an older install (#2335). + * + * A daemon keeps running the code it started with. The 1.6.1 daemon in #2335 + * outlived an upgrade to 1.6.2, went on loading grammars from the install the + * upgrade removed, and wrote every file its watcher re-indexed as empty. A + * launcher that found it could only serve its own session in-process, read-only: + * the old daemon held the project's writer lock and file watcher, so no daemon + * from the new install could start. Daemons from before the fix cannot notice + * the upgrade themselves, so the launcher acts: a daemon of an OLDER release is + * stopped the way `codegraph daemon` stops one (identity-checked SIGTERM, a + * graceful shutdown on POSIX) and one from the launcher's own install takes its + * place. The launcher hears an older daemon's hello on its own socket, or — + * when it listens elsewhere (on Windows, daemons before #2278 named their pipe + * after the root as typed) — finds it by the project lock. A daemon of the + * same, a newer or an unknown version is never touched, so two installed + * versions cannot fight over the daemon. + * + * The end-to-end cases run real daemons of other versions: this build's dist/ + * linked into a temp "install" whose package.json carries another version. A + * daemon resolves its version, and every grammar it loads, from its own files, + * so that install's daemon advertises that version in its lock and its hello. + */ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ChildProcessWithoutNullStreams, execFileSync, spawn } from 'child_process'; +import { once } from 'events'; +import * as fs from 'fs'; +import * as net from 'net'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { encodeLockInfo, getDaemonPidPath, getDaemonSocketPath } from '../src/mcp/daemon-paths'; +import { isProcessAlive, stopOlderDaemon } from '../src/mcp/daemon-registry'; +import { connectWithHello } from '../src/mcp/proxy'; +import { CodeGraphPackageVersion, isOlderRelease } from '../src/mcp/version'; +import { getWriterPidPath, readWriterLock, swapWriterLock } from '../src/mcp/writer-lock'; +import { recordSpawns, removeSpawnLog, settleLosingCandidates } from './daemon-candidates'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); +const DIST = path.resolve(__dirname, '../dist'); +const NODE_MODULES = path.resolve(__dirname, '../node_modules'); +const OLDER_DAEMON = path.resolve(__dirname, 'fixtures/older-daemon.cjs'); + +/** Plain releases always below / above any version this repo will carry. */ +const OLDER = '0.0.1'; +const NEWER = '999.0.0'; + +describe('isOlderRelease', () => { + it('orders plain releases numerically', () => { + expect(isOlderRelease('1.6.1', '1.6.2')).toBe(true); + expect(isOlderRelease('1.6.9', '1.6.10')).toBe(true); + expect(isOlderRelease('1.9.0', '1.10.0')).toBe(true); + expect(isOlderRelease('0.99.99', '1.0.0')).toBe(true); + expect(isOlderRelease('1.6.2', '1.6.2')).toBe(false); + expect(isOlderRelease('1.6.3', '1.6.2')).toBe(false); + expect(isOlderRelease('2.0.0', '1.99.99')).toBe(false); + }); + + it('never calls a prerelease, a build or an unknown version older, in either position', () => { + for (const odd of ['0.0.0-unknown', '0.0.0-mismatch', '1.6.1-beta.1', '1.6.1+local', 'v1.6.1', 'unknown', 'test', '', '1.6']) { + expect(isOlderRelease(odd, '1.6.2'), odd).toBe(false); + expect(isOlderRelease('1.6.1', odd), odd).toBe(false); + } + }); + + it('orders managed revisions by release while refusing same-release replacement', () => { + const first = 'a'.repeat(40); + const second = 'b'.repeat(40); + expect(isOlderRelease('1.6.1', `1.6.2+${first}`)).toBe(true); + expect(isOlderRelease(`1.6.1+${first}`, `1.6.2+${second}`)).toBe(true); + expect(isOlderRelease(`1.6.2+${first}`, '1.6.1')).toBe(false); + expect(isOlderRelease(`1.6.2+${first}`, `1.6.2+${second}`)).toBe(false); + expect(isOlderRelease(`1.6.2+${second}`, `1.6.2+${first}`)).toBe(false); + }); + + it('lets at most one of two versions replace the other', () => { + const versions = ['0.9.0', '1.6.1', '1.6.2', '1.6.10', '1.7.0', '2.0.0', '1.6.2-rc.1', '0.0.0-unknown']; + for (const a of versions) { + for (const b of versions) { + expect(isOlderRelease(a, b) && isOlderRelease(b, a), `${a} / ${b}`).toBe(false); + } + } + }); +}); + +/** A path a test server can listen on: a named pipe on Windows, a socket file elsewhere. */ +function listenPath(dir: string, name: string): string { + return process.platform === 'win32' + ? `\\\\.\\pipe\\${path.basename(dir)}-${name}` + : path.join(dir, `${name}.sock`); +} + +async function listen(server: net.Server, socketPath: string): Promise { + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(socketPath, resolve); + }); +} + +describe('connectWithHello', () => { + let dir: string; + let server: net.Server | null = null; + + beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-hello-')); }); + afterEach(async () => { + if (server) await new Promise((resolve) => server!.close(() => resolve())); + server = null; + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it.each([ + [OLDER, 'older-version', false], + [NEWER, 'version-mismatch', true], + ['0.0.0-unknown', 'version-mismatch', true], + ])('reports a %s daemon as %s', async (version, expected, logged) => { + const socketPath = listenPath(dir, 'daemon'); + server = net.createServer((socket) => { + socket.write(JSON.stringify({ codegraph: version, pid: process.pid, socketPath, protocol: 1 }) + '\n'); + }); + await listen(server, socketPath); + const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + try { + expect(await connectWithHello(socketPath, '1.6.2')).toBe(expected); + // An older daemon is the launcher's to report; it is not a mismatch to serve around. + expect(stderr.mock.calls.some(([line]) => String(line).includes('differs from ours'))).toBe(logged); + } finally { + stderr.mockRestore(); + } + }); +}); + +describe('swapWriterLock', () => { + let root: string; + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-writer-swap-')); + fs.mkdirSync(path.join(root, '.codegraph')); + }); + afterEach(() => { fs.rmSync(root, { recursive: true, force: true }); }); + + const record = (pid: number, mode: string) => ({ pid, mode, startedAt: 1, ready: false }); + + it('replaces the record of the given pid, and only that record', () => { + const file = getWriterPidPath(root); + fs.writeFileSync(file, JSON.stringify(record(111, 'daemon')) + '\n'); + expect(swapWriterLock(root, 222, record(333, 'handover'))).toBe(false); + expect(readWriterLock(root)).toMatchObject({ pid: 111, mode: 'daemon' }); + expect(swapWriterLock(root, 111, record(333, 'handover'))).toBe(true); + expect(readWriterLock(root)).toMatchObject({ pid: 333, mode: 'handover' }); + // A slot nobody holds is not handed over: it is acquired the usual way. + fs.rmSync(file); + expect(swapWriterLock(root, 333, record(444, 'daemon'))).toBe(false); + expect(fs.existsSync(file)).toBe(false); + // The permanent guard inode remains; no PID record or swap temporary does. + expect(fs.readdirSync(path.join(root, '.codegraph'))).toEqual(['writer.pid.mutation.lock']); + }); +}); + +/** A live process to stand in for a daemon; signals reach it, never the test. */ +function startDetachedProcess(): number { + const source = [ + "const { spawn } = require('child_process');", + "const child = spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { detached: true, stdio: 'ignore' });", + 'child.unref();', + 'console.log(child.pid);', + ].join(' '); + const pid = Number(execFileSync(process.execPath, ['-e', source], { encoding: 'utf8' }).trim()); + if (!Number.isInteger(pid) || pid <= 0) throw new Error('could not start daemon fixture'); + return pid; +} + +async function deadPid(): Promise { + const child = spawn(process.execPath, ['-e', 'process.exit(0)']); + await once(child, 'exit'); + await new Promise((r) => setTimeout(r, 50)); + return child.pid!; +} + +describe('stopOlderDaemon', () => { + let root: string; + let pidPath: string; + let socketPath: string; + let server: net.Server | null; + let pids: number[]; + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-older-daemon-')); + fs.mkdirSync(path.join(root, '.codegraph')); + pidPath = getDaemonPidPath(root); + socketPath = getDaemonSocketPath(root); + server = null; + pids = []; + }); + + afterEach(async () => { + vi.restoreAllMocks(); + for (const pid of pids) if (isProcessAlive(pid)) process.kill(pid, 'SIGKILL'); + if (server) await new Promise((resolve) => server!.close(() => resolve())); + fs.rmSync(root, { recursive: true, force: true }); + }); + + /** A daemon of `version`: its lock, and a socket answering the hello as `helloPid`. */ + async function fakeDaemon(version: string, helloPid?: number, writerProtocol: unknown = 1): Promise<{ pid: number; lock: string }> { + const pid = startDetachedProcess(); + pids.push(pid); + server = net.createServer((socket) => { + socket.end(JSON.stringify({ protocol: 1, codegraph: version, pid: helloPid ?? pid, socketPath, ...(writerProtocol === null ? {} : { writerProtocol }) }) + '\n'); + }); + await listen(server, socketPath); + const lock = encodeLockInfo({ pid, version, socketPath, startedAt: Date.now() }); + fs.writeFileSync(pidPath, lock); + return { pid, lock }; + } + + /** Signals sent to `pid`, other than the signal-0 liveness probe. */ + function signalsTo(kill: ReturnType, pid: number): unknown[] { + return kill.mock.calls.filter(([target, signal]) => target === pid && signal !== 0).map(([, signal]) => signal); + } + + /** The writer record, decoded. */ + const writerRecord = (): any => { + try { return JSON.parse(fs.readFileSync(getWriterPidPath(root), 'utf8')); } catch { return null; } + }; + + it.each([null, 2])('preserves a legacy writer without verified coordination capability (%s)', async (writerProtocol) => { + const { pid, lock } = await fakeDaemon('1.6.1', undefined, writerProtocol); + const record = { pid, mode: 'daemon', startedAt: 1, ready: false }; + fs.writeFileSync(getWriterPidPath(root), JSON.stringify(record)); + const kill = vi.spyOn(process, 'kill'); + expect(await stopOlderDaemon(root, '1.6.2')).toMatchObject({ pid, outcome: 'legacy-writer' }); + expect(signalsTo(kill, pid)).toEqual([]); + expect(writerRecord()).toEqual(record); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock); + expect(isProcessAlive(pid)).toBe(true); + }); + + it('stops a verified daemon of an older release and clears its lock', async () => { + const { pid } = await fakeDaemon('1.6.1'); + const result = await stopOlderDaemon(root, '1.6.2'); + expect(result).toMatchObject({ pid, outcome: 'term', version: '1.6.1' }); + expect(isProcessAlive(pid)).toBe(false); + expect(fs.existsSync(pidPath)).toBe(false); + // Kept for the daemon the caller starts next. + expect(writerRecord()).toMatchObject({ pid: process.pid, mode: 'handover' }); + }, 15000); + + it('holds the writer slot from before the signal, so it is never free or stale', async () => { + const { pid } = await fakeDaemon('1.6.1'); + fs.writeFileSync(getWriterPidPath(root), JSON.stringify({ pid, mode: 'daemon', startedAt: 1, ready: true }) + '\n'); + let atSignal: unknown = null; + const originalKill = process.kill.bind(process); + vi.spyOn(process, 'kill').mockImplementation((target, signal) => { + if (target === pid && signal === 'SIGTERM') atSignal = writerRecord(); + return originalKill(target, signal); + }); + expect(await stopOlderDaemon(root, '1.6.2')).toMatchObject({ pid, outcome: 'term' }); + // A session of the old daemon serving itself the moment it went found the + // slot held, and so served reads only. + expect(atSignal).toMatchObject({ pid: process.pid, mode: 'handover' }); + expect(writerRecord()).toMatchObject({ pid: process.pid, mode: 'handover' }); + expect(fs.existsSync(pidPath)).toBe(false); + }, 15000); + + it('gives the writer slot back to an older daemon that does not exit', async () => { + const { pid, lock } = await fakeDaemon('1.6.1'); + const record = { pid, mode: 'daemon', startedAt: 1, ready: true }; + fs.writeFileSync(getWriterPidPath(root), JSON.stringify(record) + '\n'); + // The signal reaches nothing: the daemon closes its socket, as its own + // shutdown does first, and then never exits. + const originalKill = process.kill.bind(process); + vi.spyOn(process, 'kill').mockImplementation((target, signal) => { + if (target === pid && signal === 'SIGTERM') { server?.close(); return true; } + return originalKill(target, signal); + }); + expect(await stopOlderDaemon(root, '1.6.2', { shutdownGraceMs: 300 })).toMatchObject({ pid, outcome: 'still-running' }); + expect(writerRecord()).toEqual(record); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock); + expect(isProcessAlive(pid)).toBe(true); + }, 15000); + + it('leaves the daemon to another launcher already holding the slot to replace it', async () => { + const { pid, lock } = await fakeDaemon('1.6.1'); + const other = startDetachedProcess(); + pids.push(other); + const claim = { pid: other, mode: 'handover', startedAt: 1, ready: false }; + fs.writeFileSync(getWriterPidPath(root), JSON.stringify(claim) + '\n'); + const kill = vi.spyOn(process, 'kill'); + expect(await stopOlderDaemon(root, '1.6.2')).toBeNull(); + expect(signalsTo(kill, pid)).toEqual([]); + expect(writerRecord()).toEqual(claim); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock); + }); + + it.each(['1.6.2', '1.6.3', '2.0.0', '1.6.1-beta.1', '0.0.0-unknown', 'test'])( + 'never signals a daemon of version %s', + async (version) => { + const { pid, lock } = await fakeDaemon(version); + const kill = vi.spyOn(process, 'kill'); + expect(await stopOlderDaemon(root, '1.6.2')).toBeNull(); + expect(signalsTo(kill, pid)).toEqual([]); + expect(isProcessAlive(pid)).toBe(true); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock); + }, + ); + + it('leaves an older lock alone when its socket answers for another process (#1553)', async () => { + const { pid, lock } = await fakeDaemon('1.6.1', process.pid); + const kill = vi.spyOn(process, 'kill'); + expect(await stopOlderDaemon(root, '1.6.2')).toMatchObject({ pid, outcome: 'unverified' }); + expect(signalsTo(kill, pid)).toEqual([]); + expect(signalsTo(kill, process.pid)).toEqual([]); + expect(isProcessAlive(pid)).toBe(true); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock); + }); + + it('reports an older daemon that already exited, leaving its lock to its successor', async () => { + const lock = encodeLockInfo({ pid: await deadPid(), version: '1.6.1', socketPath, startedAt: Date.now() }); + fs.writeFileSync(pidPath, lock); + expect(await stopOlderDaemon(root, '1.6.2')).toMatchObject({ outcome: 'not-running', version: '1.6.1' }); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(lock); + }); + + it('has nothing to stop without a lock, and leaves a listening socket alone', async () => { + server = net.createServer((socket) => { + socket.end(JSON.stringify({ protocol: 1, codegraph: '1.6.1', pid: process.pid, socketPath }) + '\n'); + }); + await listen(server, socketPath); + expect(await stopOlderDaemon(root, '1.6.2')).toBeNull(); + expect(await connectWithHello(socketPath, '1.6.2')).toBe('older-version'); + }); + + it('does not act on an unreadable lock', async () => { + fs.mkdirSync(pidPath); + expect(await stopOlderDaemon(root, '1.6.2')).toEqual({ root, pid: null, outcome: 'unverified' }); + }); +}); + +/** + * A CodeGraph install of `version`: this build's dist/, hard-linked file by + * file (copied where a link is refused), next to a package.json of that version + * and a link to this checkout's node_modules. + */ +function makeInstall(version: string, revision?: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-install-${version}-`)); + const linkTree = (from: string, to: string): void => { + fs.mkdirSync(to, { recursive: true }); + for (const entry of fs.readdirSync(from, { withFileTypes: true })) { + if (entry.name === 'build-revision.json') continue; + const source = path.join(from, entry.name); + const target = path.join(to, entry.name); + if (entry.isDirectory()) linkTree(source, target); + else { + try { fs.linkSync(source, target); } catch { fs.copyFileSync(source, target); } + } + } + }; + linkTree(DIST, path.join(dir, 'dist')); + fs.symlinkSync(NODE_MODULES, path.join(dir, 'node_modules'), 'junction'); + fs.symlinkSync(path.resolve(__dirname, '../codegraph-kernel'), path.join(dir, 'codegraph-kernel'), 'junction'); + if (revision) fs.writeFileSync(path.join(dir, 'dist', 'build-revision.json'), JSON.stringify({ revision })); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: '@colbymchenry/codegraph', version }) + '\n'); + return dir; +} + +interface Session { + child: ChildProcessWithoutNullStreams; + stdout: string[]; + stderr: string[]; +} + +function lines(stream: NodeJS.ReadableStream, into: string[]): void { + let buffer = ''; + stream.on('data', (chunk: Buffer) => { + buffer += chunk.toString('utf8'); + let idx: number; + while ((idx = buffer.indexOf('\n')) !== -1) { + into.push(buffer.slice(0, idx)); + buffer = buffer.slice(idx + 1); + } + }); +} + +function findResponse(stdout: string[], id: number): any | null { + for (const line of stdout) { + try { + const parsed = JSON.parse(line); + if (parsed && parsed.id === id && (parsed.result !== undefined || parsed.error !== undefined)) return parsed; + } catch { /* not JSON */ } + } + return null; +} + +function waitFor(predicate: () => T | undefined | null | false, timeoutMs: number, label: string): Promise { + return new Promise((resolve, reject) => { + const started = Date.now(); + const tick = (): void => { + let value: T | undefined | null | false; + try { value = predicate(); } catch (e) { reject(e); return; } + if (value) { resolve(value as T); return; } + if (Date.now() - started > timeoutMs) { reject(new Error(`Timed out after ${timeoutMs}ms waiting for: ${label}`)); return; } + setTimeout(tick, 25); + }; + tick(); + }); +} + +function isAlive(pid: number): boolean { + try { process.kill(pid, 0); return true; } catch { return false; } +} + +async function waitProcessExit(pid: number, timeoutMs: number): Promise { + return waitFor(() => !isAlive(pid), timeoutMs, `pid ${pid} to exit`).then(() => true, () => false); +} + +function readJson(file: string): any | null { + try { return JSON.parse(fs.readFileSync(file, 'utf8')); } catch { return null; } +} + +describe('a launcher meeting a daemon of another version (#2335)', () => { + let olderInstall: string; + let newerInstall: string; + let managedInstall: string; + const managedVersion = `${CodeGraphPackageVersion.split('+')[0]}+${'a'.repeat(40)}`; + let tempDir: string; + let realRoot: string; + const sessions: Session[] = []; + const daemonPids = new Set(); + + beforeAll(() => { + olderInstall = makeInstall(OLDER); + newerInstall = makeInstall(NEWER); + managedInstall = makeInstall(CodeGraphPackageVersion.split('+')[0], 'a'.repeat(40)); + }); + + afterAll(async () => { + for (const dir of [olderInstall, newerInstall, managedInstall]) { + if (!dir) continue; + // The link first: removing an install must never reach this checkout's modules. + try { fs.unlinkSync(path.join(dir, 'node_modules')); } catch { /* not created */ } + await fs.promises.rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + beforeEach(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-older-version-')); + fs.writeFileSync(path.join(tempDir, 'app.ts'), 'export function appMain() { return 1; }\n'); + const cg = await CodeGraph.init(tempDir); + try { await cg.indexAll(); } finally { cg.close(); } + realRoot = fs.realpathSync(tempDir); + }); + + afterEach(async () => { + // Every launcher first (with the runtime flags there is no relaunch child), + // then the losing daemon candidates, then the daemons themselves — and only + // then the fixture: on Windows a live process inside it blocks the removal. + await Promise.all(sessions.map(async ({ child }) => { + if (child.exitCode !== null || child.signalCode !== null) return; + const exited = once(child, 'exit'); + child.kill('SIGKILL'); + await exited; + })); + sessions.length = 0; + const lockPid = (): number | null => readJson(path.join(realRoot, '.codegraph', 'daemon.pid'))?.pid ?? null; + await settleLosingCandidates(tempDir, lockPid); + const holder = lockPid(); + if (holder) daemonPids.add(holder); + for (const pid of daemonPids) { + if (pid === process.pid || !isAlive(pid)) continue; + try { process.kill(pid, 'SIGKILL'); } catch { /* raced to exit */ } + await waitProcessExit(pid, 5000); + } + daemonPids.clear(); + removeSpawnLog(tempDir); + await fs.promises.rm(tempDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + }, 45_000); + + /** An agent session: `install`'s launcher, `serve --mcp` in the project. */ + function startSession(install: string | null, env: NodeJS.ProcessEnv = {}): Session { + const bin = install ? path.join(install, 'dist', 'bin', 'codegraph.js') : BIN; + const recorder = recordSpawns(tempDir); + const child = spawn(process.execPath, [...recorder.args, bin, 'serve', '--mcp'], { + cwd: tempDir, + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...process.env, + CODEGRAPH_MCP_LOG_ATTACH: '1', + CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', + ...recorder.env, + ...env, + }, + }) as ChildProcessWithoutNullStreams; + child.on('error', () => { /* ignore */ }); + child.stdin.on('error', () => { /* ignore */ }); + const session: Session = { child, stdout: [], stderr: [] }; + lines(child.stdout, session.stdout); + lines(child.stderr, session.stderr); + sessions.push(session); + send(session, { + jsonrpc: '2.0', id: 1, method: 'initialize', + params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'test', version: '0.0.0' }, rootUri: `file://${tempDir}` }, + }); + return session; + } + + function send(session: Session, msg: unknown): void { + try { session.child.stdin.write(JSON.stringify(msg) + '\n'); } catch { /* gone */ } + } + + async function status(session: Session, id: number, label: string): Promise { + send(session, { jsonrpc: '2.0', id, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + const reply = await waitFor(() => findResponse(session.stdout, id), 20000, label).catch((err: Error) => { + throw new Error(`${err.message}\n--- stderr ---\n${session.stderr.join('\n')}\n--- daemon.log ---\n${daemonLog()}`); + }); + expect(reply.error, label).toBeUndefined(); + expect(reply.result?.isError, label).not.toBe(true); + expect(JSON.stringify(reply.result), label).toContain('CodeGraph Status'); + return reply; + } + + const attachedLine = (session: Session): string | undefined => + session.stderr.find((l) => l.includes('Attached to shared daemon')); + const daemonLock = (): any => readJson(path.join(realRoot, '.codegraph', 'daemon.pid')); + const writerLock = (): any => readJson(path.join(realRoot, '.codegraph', 'writer.pid')); + const daemonLog = (): string => { + try { return fs.readFileSync(path.join(realRoot, '.codegraph', 'daemon.log'), 'utf8'); } catch { return ''; } + }; + + /** A session of `install` attached to a daemon of its own, which owns the project. */ + async function sessionWithDaemon(install: string, version: string, env: NodeJS.ProcessEnv = {}): Promise<{ session: Session; pid: number }> { + const session = startSession(install, env); + await waitFor(() => attachedLine(session), 20000, `the ${version} session to attach`); + // Attached means listening, not initialized: let the engine open the + // database before anything stops this daemon. + await status(session, 2, `a ${version} tool call`); + const lock = daemonLock(); + expect(lock).toMatchObject({ version }); + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + daemonPids.add(lock.pid); + return { session, pid: lock.pid }; + } + + /** A daemon process for the project, started the way a launcher starts one. */ + function startDaemon(env: NodeJS.ProcessEnv): { child: ChildProcessWithoutNullStreams; log: string[] } { + const child = spawn(process.execPath, [BIN, 'serve', '--mcp', '--path', realRoot], { + cwd: tempDir, + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, CODEGRAPH_DAEMON_INTERNAL: '1', CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', ...env }, + }) as ChildProcessWithoutNullStreams; + child.on('error', () => { /* ignore */ }); + child.stdout.resume(); + const log: string[] = []; + lines(child.stderr, log); + daemonPids.add(child.pid!); + return { child, log }; + } + + /** The slot as a launcher that just stopped an older daemon holds it: this test process stands in. */ + function holdWriterSlotForSuccessor(): string { + const claim = JSON.stringify({ pid: process.pid, mode: 'handover', startedAt: Date.now(), ready: false }) + '\n'; + fs.writeFileSync(path.join(realRoot, '.codegraph', 'writer.pid'), claim); + return claim; + } + + it('keeps an unchanged managed daemon alive through its install checks', async () => { + const { session, pid } = await sessionWithDaemon(managedInstall, managedVersion, { + CODEGRAPH_DAEMON_INSTALL_CHECK_MS: '20', + }); + await new Promise((resolve) => setTimeout(resolve, 100)); + await status(session, 3, 'managed daemon after periodic install checks'); + expect(daemonLock()).toMatchObject({ pid, version: managedVersion }); + expect(writerLock()).toMatchObject({ pid, mode: 'daemon' }); + }); + + it('a daemon takes over the writer slot held for it, and only that one', async () => { + const claim = holdWriterSlotForSuccessor(); + + // A daemon the slot is not held for finds it held, as by any live writer. + const stranger = startDaemon({}); + const [strangerCode] = await once(stranger.child, 'exit'); + expect(strangerCode).toBe(1); + expect(stranger.log.join('\n')).toContain(`writer lock held by PID ${process.pid} (handover mode)`); + expect(fs.readFileSync(path.join(realRoot, '.codegraph', 'writer.pid'), 'utf8')).toBe(claim); + + const successor = startDaemon({ CODEGRAPH_DAEMON_HANDOVER: String(process.pid) }); + await waitFor( + () => daemonLock()?.pid === successor.child.pid && successor.log.some((l) => l.includes('Listening on')), + 20000, 'the successor to listen', + ); + expect(writerLock()).toMatchObject({ pid: successor.child.pid, mode: 'daemon' }); + expect(successor.log.join('\n')).toContain(`Took over the writer lock from launcher pid ${process.pid}`); + }, 60000); + + it('a daemon the slot is held for outwaits a candidate still starting in its way', async () => { + holdWriterSlotForSuccessor(); + // A racing launcher's candidate holds the daemon lock, still starting; it + // cannot get the writer slot, so it will give up. + const candidate = startDetachedProcess(); + daemonPids.add(candidate); + const pidPath = path.join(realRoot, '.codegraph', 'daemon.pid'); + const candidateLock = encodeLockInfo({ + pid: candidate, version: CodeGraphPackageVersion, socketPath: getDaemonSocketPath(realRoot), startedAt: Date.now(), + }); + fs.writeFileSync(pidPath, candidateLock); + + // Any other daemon yields to it at once. + const yielding = startDaemon({}); + const [yieldingCode] = await once(yielding.child, 'exit'); + expect(yieldingCode).toBe(0); + expect(yielding.log.join('\n')).toContain(`Another daemon (pid ${candidate}) already holds the lock`); + + const successor = startDaemon({ CODEGRAPH_DAEMON_HANDOVER: String(process.pid) }); + await waitFor( + () => successor.log.some((l) => l.includes(`Waiting for daemon candidate pid ${candidate}`)), + 20000, 'the successor to wait for the candidate', + ); + expect(successor.child.exitCode).toBeNull(); + expect(fs.readFileSync(pidPath, 'utf8')).toBe(candidateLock); + // The candidate gives up: its lock goes, then so does it. + fs.rmSync(pidPath); + process.kill(candidate, 'SIGKILL'); + await waitFor( + () => daemonLock()?.pid === successor.child.pid && successor.log.some((l) => l.includes('Listening on')), + 20000, 'the successor to listen', + ); + expect(writerLock()).toMatchObject({ pid: successor.child.pid, mode: 'daemon' }); + }, 60000); + + it('keeps the older daemon\'s sessions read-only through the replacement, even mid-call', async () => { + const before = await sessionWithDaemon(olderInstall, OLDER); + // The older session calls every 20ms throughout, so it serves itself + // in-process the instant its daemon goes. Finding the writer slot free + // then, it would claim it — with the code of the install being replaced. + let next = 100; + const calling = setInterval(() => { + send(before.session, { jsonrpc: '2.0', id: next++, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + }, 20); + let after: Session; + try { + after = startSession(null); + await waitFor(() => attachedLine(after), 30000, 'the new session to attach'); + await new Promise((r) => setTimeout(r, 500)); + } finally { + clearInterval(calling); + } + const lock = daemonLock(); + daemonPids.add(lock.pid); + expect(lock.version).toBe(CodeGraphPackageVersion); + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + await status(after, 2, 'a tool call through the new daemon'); + + const last = next - 1; + await waitFor(() => findResponse(before.session.stdout, last), 20000, 'the older session to answer every call'); + for (let id = 100; id <= last; id++) expect(findResponse(before.session.stdout, id), `call ${id}`).toBeTruthy(); + const stderr = before.session.stderr.join('\n'); + expect(stderr).toContain('Serving reads in-process without auto-sync'); + expect(stderr).not.toContain('File watcher active'); + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + }, 120000); + + it('stops a daemon from an older install and starts one from its own', async () => { + const before = await sessionWithDaemon(olderInstall, OLDER); + + const after = startSession(null); + const stopped = await waitFor( + () => after.stderr.find((l) => l.includes('[CodeGraph MCP] Stopped')), + 30000, 'the older daemon to be stopped', + ); + expect(stopped).toContain(`the CodeGraph ${OLDER} daemon (pid ${before.pid})`); + expect(stopped).toContain(`from this install (${CodeGraphPackageVersion})`); + expect(await waitProcessExit(before.pid, 10000)).toBe(true); + + const attached = await waitFor(() => attachedLine(after), 20000, 'the new session to attach'); + const lock = daemonLock(); + daemonPids.add(lock.pid); + expect(lock.version).toBe(CodeGraphPackageVersion); + expect(lock.pid).not.toBe(before.pid); + expect(attached).toContain(`(pid ${lock.pid}, v${CodeGraphPackageVersion})`); + await status(after, 2, 'a tool call through the new daemon'); + // The replacement owns updates for the project: writer lock and watcher, + // handed to it by the launcher that held the slot through the stop. + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + expect(daemonLog()).toContain(`Took over the writer lock from launcher pid ${after.child.pid}`); + if (process.platform !== 'win32') { + // SIGTERM ran the old daemon's own shutdown. (On Windows it is + // TerminateProcess, and the stop clears what is left.) + expect(daemonLog()).toContain(`Shutting down (SIGTERM`); + } + + // The session from before the upgrade keeps answering, read-only, and + // leaves the new daemon its writer lock. + await waitFor( + () => before.session.stderr.some((l) => l.includes('Shared daemon connection lost')), + 10000, 'the older session to lose its daemon', + ); + await status(before.session, 3, 'a tool call in the older session'); + expect(before.session.stderr.some((l) => l.includes('Serving reads in-process without auto-sync'))).toBe(true); + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + expect(daemonLock()).toEqual(lock); + expect(isAlive(lock.pid)).toBe(true); + }, 120000); + + it('finds an older daemon its probe cannot reach by the project lock, and stops it', async () => { + // On Windows a 1.6.1 daemon named its pipe after the root as typed (#2278), + // so a newer launcher's probe never hears its hello; only its lock says + // where it listens. Stand one up on a socket no launcher probes. + const socketDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sock-')); + try { + const socketPath = listenPath(socketDir, 'older'); + const older = spawn(process.execPath, [OLDER_DAEMON, realRoot, OLDER, socketPath], { detached: true, stdio: 'ignore', env: { ...process.env, CODEGRAPH_TEST_WRITER_MODULE: path.resolve(__dirname, '../dist/mcp/writer-lock.js') } }); + older.unref(); + daemonPids.add(older.pid!); + await waitFor(() => daemonLock()?.pid === older.pid, 10000, 'the older daemon to hold the project'); + expect(writerLock()).toMatchObject({ pid: older.pid, mode: 'daemon' }); + + const session = startSession(null); + const stopped = await waitFor( + () => session.stderr.find((l) => l.includes('[CodeGraph MCP] Stopped')), + 30000, 'the older daemon to be stopped', + ); + expect(stopped).toContain(`the CodeGraph ${OLDER} daemon (pid ${older.pid})`); + expect(await waitProcessExit(older.pid!, 10000)).toBe(true); + const attached = await waitFor(() => attachedLine(session), 20000, 'the session to attach'); + const lock = daemonLock(); + daemonPids.add(lock.pid); + expect(lock.version).toBe(CodeGraphPackageVersion); + expect(attached).toContain(`(pid ${lock.pid}, v${CodeGraphPackageVersion})`); + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + expect(daemonLog()).toContain(`Took over the writer lock from launcher pid ${session.child.pid}`); + await status(session, 2, 'a tool call through the new daemon'); + } finally { + fs.rmSync(socketDir, { recursive: true, force: true }); + } + }, 90000); + + it('never stops a daemon from a newer install', async () => { + const retry = { CODEGRAPH_DAEMON_RETRY_MS: '300', CODEGRAPH_DAEMON_RETRY_MAX_MS: '600' }; + const newer = await sessionWithDaemon(newerInstall, NEWER); + const lock = daemonLock(); + + const current = startSession(null, retry); + // The first probe and the retries that follow all find the newer daemon. + await waitFor( + () => current.stderr.filter((l) => l.includes(`version (${NEWER}) differs from ours`)).length >= 3, + 20000, 'repeated probes of the newer daemon', + ); + await status(current, 2, 'a tool call served in-process'); + expect(current.stderr.some((l) => l.includes('Serving reads in-process without auto-sync'))).toBe(true); + expect(current.stderr.some((l) => l.includes('[CodeGraph MCP] Stopped'))).toBe(false); + + expect(isAlive(newer.pid)).toBe(true); + expect(daemonLock()).toEqual(lock); + expect(writerLock()).toMatchObject({ pid: newer.pid, mode: 'daemon' }); + await status(newer.session, 3, 'a tool call through the newer daemon'); + }, 90000); + + it('a session whose own install was upgraded serves reads only once its daemon exits', async () => { + // An upgrade in place: the daemon notices its package.json changed and + // exits (its install check), and its session falls back in-process. As + // the project's writer that session — running code the upgrade replaced — + // would only keep a daemon from the new install from starting. + const install = makeInstall('0.0.2'); + try { + const before = await sessionWithDaemon(install, '0.0.2', { + CODEGRAPH_DAEMON_INSTALL_CHECK_MS: '200', + CODEGRAPH_DAEMON_RETRY_MS: '0', + }); + fs.writeFileSync(path.join(install, 'package.json'), JSON.stringify({ name: '@colbymchenry/codegraph', version: '0.0.3' }) + '\n'); + expect(await waitProcessExit(before.pid, 10000)).toBe(true); + expect(daemonLog()).toContain('Install replaced by v0.0.3'); + await waitFor( + () => before.session.stderr.some((l) => l.includes('Shared daemon connection lost')), + 10000, 'the session to lose its daemon', + ); + + await status(before.session, 3, 'a tool call after the daemon exited'); + const stderr = before.session.stderr.join('\n'); + expect(stderr).toContain("this session's CodeGraph install was upgraded or removed"); + expect(stderr).not.toContain('File watcher active'); + expect(writerLock()).toBeNull(); + + // So a session from the current install gets a daemon straight away. + const after = startSession(null); + await waitFor(() => attachedLine(after), 20000, 'the new session to attach'); + const lock = daemonLock(); + daemonPids.add(lock.pid); + expect(writerLock()).toMatchObject({ pid: lock.pid, mode: 'daemon' }); + } finally { + // On Windows a process still running from the install blocks its removal. + for (const { child } of sessions) { + if (child.exitCode === null && child.signalCode === null) { + const exited = once(child, 'exit'); + child.kill('SIGKILL'); + await exited; + } + } + try { fs.unlinkSync(path.join(install, 'node_modules')); } catch { /* not created */ } + await fs.promises.rm(install, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }, 90000); +}); diff --git a/__tests__/fixtures/golden/torture-multilang.dump b/__tests__/fixtures/golden/torture-multilang.dump index 7cdaa71673..47f082b226 100644 --- a/__tests__/fixtures/golden/torture-multilang.dump +++ b/__tests__/fixtures/golden/torture-multilang.dump @@ -3402,7 +3402,7 @@ {"source":"variable:36e608fb368763b7f8a74a69f477cec7","target":"function:692077921d04b315880bc35a0bc872a9","kind":"references","metadata":"{\"confidence\":0.95,\"resolvedBy\":\"function-ref\",\"refName\":\"view\",\"refKind\":\"function_ref\",\"fnRef\":true}","line":49,"col":24,"provenance":null} {"source":"variable:6b7262d2883839c6f601126f4dd59e9b","target":"function:d8f9c30c4ee99a9302a150e8eed67acf","kind":"references","metadata":"{\"confidence\":0.95,\"resolvedBy\":\"function-ref\",\"refName\":\"TargetCb\",\"refKind\":\"function_ref\",\"fnRef\":true}","line":14,"col":9,"provenance":null} {"source":"variable:788d75a828c79f9847e0be0e2b73a196","target":"variable:8fef25ea8abe486ee94190ed47c3644b","kind":"references","metadata":"{\"valueRef\":true}","line":null,"col":null,"provenance":null} -== refs (1299) +== refs (1301) {"from_node_id":"class:01a3945fa12793f1af9ac531aedcc8cd","reference_name":"Mix2","reference_kind":"implements","line":34,"col":37,"candidates":null,"file_path":"TortureMini.dart","language":"dart","status":"failed","name_tail":"Mix2"} {"from_node_id":"class:01a3945fa12793f1af9ac531aedcc8cd","reference_name":"Paint","reference_kind":"implements","line":34,"col":59,"candidates":null,"file_path":"TortureMini.dart","language":"dart","status":"failed","name_tail":"Paint"} {"from_node_id":"class:066eb79e83cf3105cebd14593747068f","reference_name":"BaseService","reference_kind":"extends","line":13,"col":14,"candidates":null,"file_path":"torture.py","language":"python","status":"failed","name_tail":"BaseService"} @@ -4624,6 +4624,8 @@ {"from_node_id":"property:13da6cd7fb88d8425ef100ac799b81ef","reference_name":"Text","reference_kind":"calls","line":122,"col":33,"candidates":null,"file_path":"torture.swift","language":"swift","status":"failed","name_tail":"Text"} {"from_node_id":"property:13da6cd7fb88d8425ef100ac799b81ef","reference_name":"VStack","reference_kind":"calls","line":122,"col":24,"candidates":null,"file_path":"torture.swift","language":"swift","status":"failed","name_tail":"VStack"} {"from_node_id":"property:20f555c9e314ec2302940348c2df150c","reference_name":"Action","reference_kind":"references","line":40,"col":15,"candidates":null,"file_path":"Torture.cs","language":"csharp","status":"failed","name_tail":"Action"} +{"from_node_id":"property:2eba142ae17bb518a27a2345a0cc2620","reference_name":"Lookup","reference_kind":"calls","line":39,"col":45,"candidates":null,"file_path":"Torture.cs","language":"csharp","status":"failed","name_tail":"Lookup"} +{"from_node_id":"property:2eba142ae17bb518a27a2345a0cc2620","reference_name":"Store","reference_kind":"calls","line":39,"col":64,"candidates":null,"file_path":"Torture.cs","language":"csharp","status":"failed","name_tail":"Store"} {"from_node_id":"property:36d0b68644ae5cbe11d8a493727a8a5a","reference_name":"Set","reference_kind":"instantiates","line":239,"col":11,"candidates":null,"file_path":"torture.tsx","language":"tsx","status":"failed","name_tail":"Set"} {"from_node_id":"property:3f76120a6d12e45f619241f30c648d15","reference_name":"renderDescription","reference_kind":"calls","line":48,"col":28,"candidates":null,"file_path":"torture.swift","language":"swift","status":"failed","name_tail":"renderDescription"} {"from_node_id":"property:42cbcdbb0dffcc35a50326fe730508ab","reference_name":"Widget","reference_kind":"references","line":38,"col":15,"candidates":null,"file_path":"Torture.cs","language":"csharp","status":"failed","name_tail":"Widget"} diff --git a/__tests__/fixtures/older-daemon.cjs b/__tests__/fixtures/older-daemon.cjs new file mode 100644 index 0000000000..cd162c8e1c --- /dev/null +++ b/__tests__/fixtures/older-daemon.cjs @@ -0,0 +1,39 @@ +/** + * A daemon of an older release, listening where a current launcher does not + * look (#2335). On Windows, daemons before #2278 named their pipe after the + * project root as typed, so a newer launcher's probe never reaches them and + * only the project lock says where they are. Like a real daemon it holds the + * project's daemon and writer locks and answers the hello; on SIGTERM it lets + * go of whichever of the two locks still names it and exits, the shape of a + * real daemon's shutdown. (On Windows SIGTERM is TerminateProcess, and the + * stop clears what is left.) + * + * node older-daemon.cjs + */ +const fs = require('fs'); +const net = require('net'); +const path = require('path'); +const writer = require(process.env.CODEGRAPH_TEST_WRITER_MODULE); + +const [root, version, socketPath] = process.argv.slice(2); +const dir = path.join(root, '.codegraph'); +const writeLock = (name, record) => fs.writeFileSync(path.join(dir, name), JSON.stringify(record) + '\n'); + +const server = net.createServer((socket) => { + socket.end(JSON.stringify({ codegraph: version, pid: process.pid, socketPath, protocol: 1, writerProtocol: 1 }) + '\n'); +}); +server.listen(socketPath, () => { + writer.tryAcquireWriterLock(root, 'daemon'); + writer.markWriterReady(root); + writeLock('daemon.pid', { pid: process.pid, version, socketPath, startedAt: Date.now() }); +}); +process.on('SIGTERM', () => { + writer.releaseWriterLock(root); + for (const name of ['daemon.pid']) { + const file = path.join(dir, name); + try { + if (JSON.parse(fs.readFileSync(file, 'utf8')).pid === process.pid) fs.unlinkSync(file); + } catch { /* already gone */ } + } + server.close(() => process.exit(0)); +}); diff --git a/__tests__/fixtures/writer-lock-ordering.cjs b/__tests__/fixtures/writer-lock-ordering.cjs new file mode 100644 index 0000000000..df12d44378 --- /dev/null +++ b/__tests__/fixtures/writer-lock-ordering.cjs @@ -0,0 +1,113 @@ +const fs = require('node:fs'); +const { spawn, spawnSync } = require('node:child_process'); +const writer = require(process.argv[2]); +const root = process.argv[3]; +const operation = process.argv[4]; +const record = (pid, mode = 'daemon') => ({ pid, mode, startedAt: 1, ready: false }); + +if (operation === 'hold') { + const path = require('node:path'); + const kernel = require(path.join(path.dirname(process.argv[2]), '../extraction/kernel/loader.js')).requireKernel(); + const held = kernel.tryWriterMutationLock(`${writer.getWriterPidPath(root)}.mutation.lock`); + if (!held) process.exit(2); + process.send('held'); + process.on('message', () => { held.release(); process.exit(0); }); +} else if (operation === 'retire' || operation.startsWith('contended-ready')) { + (async () => { + writer.tryAcquireWriterLock(root, 'direct'); + const holder = spawn(process.execPath, [__filename, process.argv[2], root, 'hold'], { stdio: ['ignore', 'ignore', 'inherit', 'ipc'] }); + try { + await new Promise((resolve, reject) => { + holder.once('message', resolve); + holder.once('error', reject); + holder.once('exit', () => reject(new Error('holder exited before readiness'))); + }); + let failedRenames = 0; + if (operation === 'contended-ready-io') { + const rename = fs.renameSync; + fs.renameSync = (...args) => { + if (String(args[0]).endsWith('.ready.tmp') && failedRenames++ === 0) { + const error = new Error('writer pid file temporarily held by a reader'); + error.code = 'EPERM'; + throw error; + } + return rename(...args); + }; + } + if (operation === 'retire') writer.releaseWriterLock(root); + else writer.markWriterReady(root); + const exited = new Promise(resolve => holder.once('exit', resolve)); + holder.send('release'); + await exited; + if (operation.startsWith('contended-ready')) { + if (operation === 'contended-ready-retire') writer.releaseWriterLock(root); + else if (operation === 'contended-ready-renew') { + writer.releaseWriterLock(root); + writer.tryAcquireWriterLock(root, 'fallback'); + } else if (operation === 'contended-ready-transfer') { + writer.swapWriterLock(root, process.pid, record(333)); + } + // Drive queued readiness work without sleeping while the guard is held. + const deadline = Date.now() + 2000; + while (Date.now() < deadline && !writer.readWriterLock(root)?.ready) { + await new Promise(resolve => setTimeout(resolve, 20)); + } + process.stdout.write(JSON.stringify({ info: writer.readWriterLock(root), pid: process.pid, failedRenames })); + return; + } + const deadline = Date.now() + 2000; + while (writer.readWriterLock(root) && Date.now() < deadline) await new Promise(resolve => setTimeout(resolve, 20)); + const released = writer.readWriterLock(root) === null; + const contender = spawnSync(process.execPath, [__filename, process.argv[2], root, 'reacquire'], { encoding: 'utf8', timeout: 10000 }); + if (contender.status !== 0) throw new Error(contender.stderr); + process.stdout.write(JSON.stringify({ released, ...JSON.parse(contender.stdout) })); + } finally { + if (holder.exitCode === null) holder.kill('SIGKILL'); + } + })().catch(error => { console.error(error); process.exitCode = 1; }); +} else if (operation === 'crash') { + const path = require('node:path'); + const kernel = require(path.join(path.dirname(process.argv[2]), '../extraction/kernel/loader.js')).requireKernel(); + const held = kernel.tryWriterMutationLock(`${writer.getWriterPidPath(root)}.mutation.lock`); + if (!held) process.exit(2); + process.stdout.write(JSON.stringify({ held: true })); + process.exit(0); // Deliberately bypass release/finalizers. +} else if (operation === 'reacquire') { + const result = writer.tryAcquireWriterLock(root, 'direct'); + process.stdout.write(JSON.stringify({ acquired: result.kind === 'acquired' })); +} else if (operation === 'competitor') { + const from = Number(process.argv[5]); + const claimed = writer.swapWriterLock(root, from, record(process.pid, 'handover')); + const promoted = claimed && writer.swapWriterLock(root, process.pid, record(333)); + process.stdout.write(JSON.stringify({ claimed, promoted })); +} else { + const file = writer.getWriterPidPath(root); + const from = operation === 'acquire' ? 999999999 : process.pid; + fs.writeFileSync(file, JSON.stringify(record(from))); + const boundary = operation === 'release' || operation === 'acquire' ? 'unlinkSync' : 'renameSync'; + const original = fs[boundary]; + let paused = false; + let competitor; + let acquired; + fs[boundary] = (...args) => { + const target = boundary === 'renameSync' ? args[1] : args[0]; + if (target === file && !paused) { + paused = true; + if (operation === 'successor') { + writer.releaseWriterLock(root); + acquired = writer.tryAcquireWriterLock(root, 'fallback').kind; + } else { + const child = spawnSync(process.execPath, [__filename, process.argv[2], root, 'competitor', String(from)], { encoding: 'utf8', timeout: 10000 }); + if (child.status !== 0) throw new Error(child.stderr || `actor exited ${child.status}`); + competitor = JSON.parse(child.stdout); + } + } + return original(...args); + }; + let result; + if (operation === 'swap' || operation === 'successor') result = writer.swapWriterLock(root, from, record(operation === 'successor' ? 333 : 222)); + else if (operation === 'ready') writer.markWriterReady(root); + else if (operation === 'release') writer.releaseWriterLock(root); + else result = writer.tryAcquireWriterLock(root, 'fallback').kind; + process.stdout.write(JSON.stringify({ paused, competitor, acquired, result, info: writer.readWriterLock(root), pid: process.pid })); +} diff --git a/__tests__/grammar-load-failure.test.ts b/__tests__/grammar-load-failure.test.ts new file mode 100644 index 0000000000..fba49c671f --- /dev/null +++ b/__tests__/grammar-load-failure.test.ts @@ -0,0 +1,194 @@ +/** + * A grammar that fails to load is an environment failure, not a parse result + * (#2335). + * + * A long-running daemon loads tree-sitter grammars lazily from its install + * directory. When an upgrade deletes that directory underneath it, every later + * grammar load fails (ENOENT) and the extractor answers `parser_error` with no + * nodes. That used to be STORED: the file's good symbols were replaced by a + * zero-node row carrying the new content hash, so `sync` never revisited it + * and `status` stayed green. Now nothing is stored for such a result — the + * file keeps its previous index data and the next sync or index retries it — + * and rows an older engine already wrote that way are re-indexed by `sync`. + * + * The failure is simulated at the native parse-tree boundary. Disabling + * bespoke walkers still parses natively and exercises the generic extractor. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { CodeGraph } from '../src'; +import type { Language } from '../src/types'; + +const { failing, throwing } = vi.hoisted(() => ({ failing: new Set(), throwing: new Set() })); + +vi.mock('../src/extraction/parse-tree', async (importOriginal) => { + const actual = await importOriginal(); + const { KernelUnavailableError } = await import('../src/extraction/kernel/loader'); + return { + ...actual, + parseSourceTreeSync: (source: string, language: Language, budgetMs?: number) => { + if (throwing.has(language)) throw new KernelUnavailableError([]); + return failing.has(language) ? null : actual.parseSourceTreeSync(source, language, budgetMs); + }, + }; +}); + +const MATH_TS = + 'export function add(a: number, b: number): number {\n' + + ' return a + b;\n' + + '}\n' + + 'export class Calculator {\n' + + ' total = 0;\n' + + ' plus(n: number): this { this.total = add(this.total, n); return this; }\n' + + '}\n'; + +const VIEW_TS = + "import { Calculator } from './math';\n" + + 'export function total(n: number): number {\n' + + ' return new Calculator().plus(n).total;\n' + + '}\n'; + +const SUB_TS = '\nexport function sub(a: number, b: number): number {\n return a - b;\n}\n'; + +function names(cg: CodeGraph, file: string): string[] { + return cg.getNodesInFile(file).map((n) => n.name); +} + +describe('grammar load failures are never stored (#2335)', () => { + let dir: string; + let cg: CodeGraph; + let kernelEnv: string | undefined; + + beforeEach(() => { + kernelEnv = process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL = '0'; + failing.clear(); + throwing.clear(); + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-grammar-load-failure-')); + fs.mkdirSync(path.join(dir, 'src')); + fs.writeFileSync(path.join(dir, 'src', 'math.ts'), MATH_TS); + fs.writeFileSync(path.join(dir, 'src', 'view.ts'), VIEW_TS); + }); + + afterEach(() => { + failing.clear(); + throwing.clear(); + cg?.destroy(); + if (kernelEnv === undefined) delete process.env.CODEGRAPH_KERNEL; + else process.env.CODEGRAPH_KERNEL = kernelEnv; + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it.each(['null', 'throw'])('sync preserves symbols and retries a native parser failure (%s)', async (mode) => { + cg = await CodeGraph.init(dir); + await cg.indexAll(); + const before = cg.getFile('src/math.ts')!; + expect(before.nodeCount).toBeGreaterThan(1); + expect(names(cg, 'src/math.ts')).toEqual(expect.arrayContaining(['add', 'Calculator'])); + + // The upgrade removed the grammar; the watcher then re-indexes an edit. + (mode === 'throw' ? throwing : failing).add('typescript'); + fs.appendFileSync(path.join(dir, 'src', 'math.ts'), SUB_TS); + const failed = await cg.sync(); + + expect(failed.failedFilePaths).toContain('src/math.ts'); + const kept = cg.getFile('src/math.ts')!; + expect(kept.nodeCount).toBe(before.nodeCount); + expect(kept.contentHash).toBe(before.contentHash); + expect(kept.errors ?? []).toEqual([]); + expect(names(cg, 'src/math.ts')).toEqual(expect.arrayContaining(['add', 'Calculator'])); + + // Grammar available again (a fresh process from the new install): the + // next sync sees the file as still changed and indexes the edit. + failing.clear(); + throwing.clear(); + const retried = await cg.sync(); + expect(retried.filesModified).toBe(1); + expect(names(cg, 'src/math.ts')).toEqual(expect.arrayContaining(['add', 'Calculator', 'sub'])); + expect(cg.getFile('src/math.ts')!.nodeCount).toBeGreaterThan(before.nodeCount); + }); + + it('a thrown native-loader failure preserves tag-based CFML symbols', async () => { + const file = 'src/Example.cfc'; + fs.writeFileSync(path.join(dir, file), '\n'); + cg = await CodeGraph.init(dir); + await cg.indexAll(); + const before = cg.getFile(file)!; + expect(names(cg, file)).toContain('add'); + + throwing.add('cfml'); + fs.appendFileSync(path.join(dir, file), '\n'); + const failed = await cg.sync(); + expect(failed.failedFilePaths).toContain(file); + expect(cg.getFile(file)!.contentHash).toBe(before.contentHash); + expect(names(cg, file)).toContain('add'); + + throwing.clear(); + await cg.sync(); + expect(cg.getFile(file)!.contentHash).not.toBe(before.contentHash); + expect(names(cg, file)).toContain('add'); + }); + + it('a full index stores nothing for files whose grammar failed to load, and sync adds them later', async () => { + failing.add('typescript'); + cg = await CodeGraph.init(dir); + const result = await cg.indexAll(); + + expect(result.filesErrored).toBe(2); + expect(result.errors.filter((e) => e.code === 'parser_error')).toHaveLength(2); + expect(cg.getFile('src/math.ts')).toBeNull(); + expect(cg.getFile('src/view.ts')).toBeNull(); + + failing.clear(); + const synced = await cg.sync(); + expect(synced.filesAdded).toBe(2); + expect(names(cg, 'src/math.ts')).toEqual(expect.arrayContaining(['add', 'Calculator'])); + expect(names(cg, 'src/view.ts')).toContain('total'); + }); + + it('sync re-indexes rows an older engine stored while the grammar was missing', async () => { + cg = await CodeGraph.init(dir); + await cg.indexAll(); + const before = cg.getFile('src/math.ts')!; + + // What v1.6.2 wrote for a file it re-indexed with no grammar: no nodes, + // the parser error, and the CURRENT content hash — so a hash-based + // reconcile considers it up to date forever. + const db = (cg as unknown as { db: { getDb(): { prepare(sql: string): { run(...args: unknown[]): unknown } } } }).db.getDb(); + db.prepare('DELETE FROM nodes WHERE file_path = ?').run('src/math.ts'); + db.prepare('UPDATE files SET node_count = 0, errors = ? WHERE path = ?').run( + JSON.stringify([{ message: 'Failed to get parser for language: typescript', filePath: 'src/math.ts', severity: 'error', code: 'parser_error' }]), + 'src/math.ts' + ); + expect(cg.getFile('src/math.ts')!.nodeCount).toBe(0); + + await cg.sync(); + + const healed = cg.getFile('src/math.ts')!; + expect(healed.nodeCount).toBe(before.nodeCount); + expect(healed.errors ?? []).toEqual([]); + expect(names(cg, 'src/math.ts')).toEqual(expect.arrayContaining(['add', 'Calculator'])); + }); + + it('a component whose script grammar failed keeps its previous symbols too', async () => { + fs.writeFileSync( + path.join(dir, 'src', 'Counter.svelte'), + '\n\n' + ); + cg = await CodeGraph.init(dir); + await cg.indexAll(); + const before = cg.getFile('src/Counter.svelte')!; + expect(names(cg, 'src/Counter.svelte')).toContain('increment'); + + failing.add('typescript'); + fs.appendFileSync(path.join(dir, 'src', 'Counter.svelte'), '

edited

\n'); + await cg.sync(); + + // The component node alone would have been stored before: the script's + // symbols gone, with a nonzero node count hiding it. + expect(cg.getFile('src/Counter.svelte')!.nodeCount).toBe(before.nodeCount); + expect(names(cg, 'src/Counter.svelte')).toContain('increment'); + }); +}); diff --git a/__tests__/status-index-health.test.ts b/__tests__/status-index-health.test.ts new file mode 100644 index 0000000000..b94562c8eb --- /dev/null +++ b/__tests__/status-index-health.test.ts @@ -0,0 +1,159 @@ +/** + * `codegraph status` must not call an index "up to date" while indexed files + * are missing their symbols (#2336, #2335). + * + * "Up to date" used to mean only "every file's content hash matches the + * index". Two kinds of file pass that check with nothing usable in the graph: + * + * - a file the parser could not read — `export type * from` is valid + * TypeScript 5.0 the bundled grammar has no rule for, so the file is stored + * with a parse error and no symbols (#2336); + * - a row stored without its symbols — by an engine whose grammar failed to + * load (#2335), or the #1541 wipe — which `codegraph sync` re-indexes. + * + * Both are counted in `status` (human and `--json`), and `files --json` carries + * each file's recorded errors. Exercised against the built CLI so the output + * and the JSON field names are what users and scripts actually see. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function spawn(cwd: string, args: string[]) { + return spawnSync(process.execPath, [BIN, ...args], { + cwd, + encoding: 'utf-8', + timeout: 30_000, + env: { + ...process.env, + CODEGRAPH_NO_DAEMON: '1', + CODEGRAPH_TELEMETRY: '0', + CODEGRAPH_NO_UPDATE_CHECK: '1', + NO_COLOR: '1', + }, + }); +} + +function run(cwd: string, args: string[]): { status: number | null; out: string } { + const result = spawn(cwd, args); + return { status: result.status, out: (result.stdout ?? '') + (result.stderr ?? '') }; +} + +function runJson(cwd: string, args: string[]): any { + const result = spawn(cwd, args); + expect(result.status, (result.stdout ?? '') + (result.stderr ?? '')).toBe(0); + return JSON.parse(result.stdout); +} + +describe('status reports files indexed without their symbols (#2336, #2335)', () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-status-health-')); + fs.mkdirSync(path.join(dir, 'src', 'constants'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'src', 'types'), { recursive: true }); + fs.writeFileSync( + path.join(dir, 'src', 'constants', 'index.ts'), + 'export const MAX_ITEMS = 10;\nexport function clamp(n: number): number { return Math.min(n, MAX_ITEMS); }\n' + ); + fs.writeFileSync( + path.join(dir, 'src', 'types', 'database.ts'), + 'export interface Row { id: number; name: string }\nexport type RowId = Row["id"];\n' + ); + }); + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + async function index(): Promise { + const cg = CodeGraph.initSync(dir); + await cg.indexAll(); + cg.destroy(); + } + + it('a clean index is up to date, with zero counts and empty error lists', async () => { + await index(); + + const human = run(dir, ['status']); + expect(human.status, human.out).toBe(0); + expect(human.out).toContain('Index is up to date'); + + const json = runJson(dir, ['status', '--json']); + expect(json.index.filesNeedingReindex).toBe(0); + expect(json.index.filesWithParseErrors).toBe(0); + + const files = runJson(dir, ['files', '--json']); + expect(files).toHaveLength(2); + for (const f of files) expect(f.errors).toEqual([]); + }); + + it('a file the parser could not read keeps status from reporting "up to date"', async () => { + fs.writeFileSync( + path.join(dir, 'src', 'index.ts'), + 'export * from "./constants/index.js";\nexport type * from "./types/database.js";\n' + ); + await index(); + + const human = run(dir, ['status']); + expect(human.status, human.out).toBe(0); + expect(human.out).not.toContain('Index is up to date'); + expect(human.out).toMatch(/1 file could not be parsed/); + expect(human.out).toContain('src/index.ts'); + + const json = runJson(dir, ['status', '--json']); + expect(json.index.filesWithParseErrors).toBe(1); + expect(json.index.filesNeedingReindex).toBe(0); + + const files = runJson(dir, ['files', '--json']); + const broken = files.find((f: any) => f.path === 'src/index.ts'); + expect(broken.errors).toHaveLength(1); + expect(broken.errors[0]).toMatchObject({ severity: 'warning', code: 'parse_error' }); + expect(broken.errors[0].message).toContain('parse produced no symbols'); + expect(files.find((f: any) => f.path === 'src/constants/index.ts').errors).toEqual([]); + }); + + it('rows stored without their symbols ask for a sync, and the sync repairs them', async () => { + await index(); + + // A row v1.6.2 wrote with its grammar missing (#2335), and a #1541 wipe: + // both carry the current content hash, so the hash check calls them current. + const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(path.join(dir, '.codegraph', 'codegraph.db')); + db.prepare('DELETE FROM nodes WHERE file_path IN (?, ?)').run('src/constants/index.ts', 'src/types/database.ts'); + db.prepare('UPDATE files SET node_count = 0, errors = ? WHERE path = ?').run( + JSON.stringify([{ message: 'Failed to get parser for language: typescript', severity: 'error', code: 'parser_error' }]), + 'src/constants/index.ts' + ); + db.prepare('UPDATE files SET node_count = 0 WHERE path = ?').run('src/types/database.ts'); + db.close(); + + const human = run(dir, ['status']); + expect(human.status, human.out).toBe(0); + expect(human.out).not.toContain('Index is up to date'); + expect(human.out).toMatch(/2 files are missing their symbols/); + expect(human.out).toContain('codegraph sync'); + + const json = runJson(dir, ['status', '--json']); + expect(json.index.filesNeedingReindex).toBe(2); + expect(json.index.filesWithParseErrors).toBe(0); + expect(json.pendingChanges).toEqual({ added: 0, modified: 0, removed: 0 }); + + const synced = run(dir, ['sync']); + expect(synced.status, synced.out).toBe(0); + + const after = runJson(dir, ['status', '--json']); + expect(after.index.filesNeedingReindex).toBe(0); + expect(run(dir, ['status']).out).toContain('Index is up to date'); + const files = runJson(dir, ['files', '--json']); + for (const f of files) { + expect(f.nodeCount).toBeGreaterThan(1); + expect(f.errors).toEqual([]); + } + }); +}); diff --git a/__tests__/vbnet-member-bodies.test.ts b/__tests__/vbnet-member-bodies.test.ts new file mode 100644 index 0000000000..8c280a5b55 --- /dev/null +++ b/__tests__/vbnet-member-bodies.test.ts @@ -0,0 +1,209 @@ +/** + * VB.NET member bodies the extractor used to skip: + * + * - A `Structure` lists each member as its own `body` field, so reading the + * field returned only the first member: SCrawler's `UserMedia` indexed its + * nested `States` enum (declared first) and nothing else, and a structure + * that opened with a field indexed no members at all. + * - A property's `Get` / `Set` blocks, its `= initializer` and its + * `As New T` were never walked, nor a `Custom Event`'s accessors. + * - A field's `= initializer` and `As New T` were never walked. + * + * The calls, instantiations and reads in those places belong to the member + * that declares them. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import type { Node } from '../src/types'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-vb-bodies-')); + const files: Record = { + 'Helper.vb': `Public Class Helper + Public Sub New(ByVal n As Integer) + End Sub + Public Shared Function Compute(ByVal n As Integer) As Integer + Return n + End Function + Public Shared Sub Store(ByVal n As Integer) + End Sub + Public Shared Function Seed() As Integer + Return 1 + End Function + Public Shared Function Format(ByVal o As Object) As String + Return "" + End Function + Public Shared Sub Log(ByVal s As String) + End Sub + Public Shared Sub Attach(ByVal h As EventHandler) + End Sub +End Class +`, + 'Widget.vb': `Public Class Widget +End Class +`, + 'UserMedia.vb': `Public Structure UserMedia + Public State As States + Private Count As Integer + Public Enum States + Unknown + Downloaded + End Enum + Public ReadOnly Property Name As String + Get + Return Helper.Format(State) + End Get + End Property + Public Sub New(ByVal s As States) + State = s + Helper.Log("created") + End Sub + Public Function IsDone() As Boolean + Return State = States.Downloaded + End Function +End Structure +`, + 'Box.vb': `Public Class Box + Private _x As Integer + Private _items As New Widget + Private _helper As Helper = New Helper(1) + Private Shared ReadOnly Def As Integer = Helper.Compute(3), Other As Integer = Helper.Seed() + Public Property Value As Integer + Get + Return Helper.Compute(_x) + End Get + Set(ByVal v As Integer) + Helper.Store(v) + _x = v + End Set + End Property + Public Property Auto As Integer = Helper.Seed() + Public ReadOnly Property Lst As New Widget + Public Custom Event Changed As EventHandler + AddHandler(ByVal value As EventHandler) + Helper.Attach(value) + End AddHandler + RemoveHandler(ByVal value As EventHandler) + End RemoveHandler + RaiseEvent(ByVal sender As Object, ByVal e As EventArgs) + End RaiseEvent + End Event +End Class +`, + // The 1.6.2 receiver gate must hold in the newly walked places too: + // `Me.Panel.Controls.Add(…)` and `New System.Drawing.Size(…)` name no + // project member. + 'Collections/DataColorCollection.vb': `Friend Class DataColorCollection + Friend Sub Add(ByVal Item As Object) + End Sub +End Class +`, + 'Editors/UsersInfoForm.vb': `Friend Class UsersInfoForm + Private Enum EComparers + Name + Size + End Enum +End Class +`, + 'MainForm.vb': `Public Class MainForm + Private ReadOnly DefaultSize As System.Drawing.Size = New System.Drawing.Size(184, 25) + Public ReadOnly Property Ready As Boolean + Get + Me.Panel.Controls.Add(Me.Button1) + Return True + End Get + End Property +End Class +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +function member(file: string, qualifiedName: string): Node { + const node = cg.getNodesInFile(file).find((n) => n.qualifiedName === qualifiedName); + expect(node, `${qualifiedName} in ${file}`).toBeDefined(); + return node!; +} + +/** `kind` edges leaving `node`, as target qualified names. */ +function targets(node: Node, kind: string): string[] { + return cg + .getOutgoingEdgesFrom([node.id]) + .filter((e) => e.kind === kind) + .map((e) => cg.getNode(e.target)!.qualifiedName) + .sort(); +} + +describe('VB.NET member bodies', () => { + it('indexes every member of a Structure, not just the first', () => { + const struct = member('UserMedia.vb', 'UserMedia'); + expect(struct.kind).toBe('struct'); + const contained = cg + .getOutgoingEdgesFrom([struct.id]) + .filter((e) => e.kind === 'contains') + .map((e) => cg.getNode(e.target)!) + .map((n) => `${n.kind} ${n.qualifiedName}`) + .sort(); + expect(contained).toEqual([ + 'enum UserMedia::States', + 'field UserMedia::Count', + 'field UserMedia::State', + 'method UserMedia::IsDone', + 'method UserMedia::New', + 'property UserMedia::Name', + ]); + expect(member('UserMedia.vb', 'UserMedia::States::Downloaded').kind).toBe('enum_member'); + expect(targets(member('UserMedia.vb', 'UserMedia::New'), 'calls')).toEqual(['Helper::Log']); + }); + + it("attributes a property's Get and Set blocks to the property", () => { + expect(targets(member('Box.vb', 'Box::Value'), 'calls')).toEqual(['Helper::Compute', 'Helper::Store']); + expect(targets(member('UserMedia.vb', 'UserMedia::Name'), 'calls')).toEqual(['Helper::Format']); + }); + + it("walks a property's initializer and its As New", () => { + expect(targets(member('Box.vb', 'Box::Auto'), 'calls')).toEqual(['Helper::Seed']); + expect(targets(member('Box.vb', 'Box::Lst'), 'instantiates')).toEqual(['Widget']); + }); + + it("walks each field's initializer and its As New, per declarator", () => { + expect(targets(member('Box.vb', 'Box::Def'), 'calls')).toEqual(['Helper::Compute']); + expect(targets(member('Box.vb', 'Box::Other'), 'calls')).toEqual(['Helper::Seed']); + expect(targets(member('Box.vb', 'Box::_items'), 'instantiates')).toEqual(['Widget']); + expect(targets(member('Box.vb', 'Box::_helper'), 'instantiates')).toEqual(['Helper']); + }); + + it("attributes a Custom Event's accessors to the event", () => { + expect(targets(member('Box.vb', 'Box::Changed'), 'calls')).toEqual(['Helper::Attach']); + }); + + it('keeps the class out of what its members do', () => { + expect(targets(member('Box.vb', 'Box'), 'calls')).toEqual([]); + expect(targets(member('Box.vb', 'Box'), 'instantiates')).toEqual([]); + }); + + it('still links nothing through a receiver no name resolves', () => { + const ids = cg.getNodesInFile('MainForm.vb').map((n) => n.id); + const linked = cg + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind !== 'contains') + .map((e) => cg.getNode(e.target)!.qualifiedName); + expect(linked).not.toContain('DataColorCollection::Add'); + expect(linked).not.toContain('UsersInfoForm::EComparers::Size'); + }); +}); diff --git a/__tests__/writer-handover-ordering.test.ts b/__tests__/writer-handover-ordering.test.ts new file mode 100644 index 0000000000..7178888c32 --- /dev/null +++ b/__tests__/writer-handover-ordering.test.ts @@ -0,0 +1,80 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +const MODULE = path.resolve(__dirname, '../dist/mcp/writer-lock.js'); +const ACTOR = path.resolve(__dirname, 'fixtures/writer-lock-ordering.cjs'); + +describe('writer handover ordering', () => { + let root: string; + afterEach(() => { + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + function run(operation: string) { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-writer-order-')); + fs.mkdirSync(path.join(root, '.codegraph')); + const child = spawnSync(process.execPath, [ACTOR, MODULE, root, operation], { encoding: 'utf8', timeout: 15000 }); + expect(child.status, child.stderr).toBe(0); + return JSON.parse(child.stdout); + } + + it.each(['swap', 'ready', 'release', 'acquire'] as const)( + 'refuses a second handover while %s is between its ownership read and mutation', + (operation) => { + const outcome = run(operation); + expect(outcome.paused, `${operation} barrier was not reached`).toBe(true); + expect(outcome.competitor, `${operation} -> competing handover -> ${operation} mutation`).toEqual({ claimed: false, promoted: false }); + if (operation === 'swap') expect(outcome.info).toMatchObject({ pid: 222, mode: 'daemon' }); + else if (operation === 'ready') expect(outcome.info).toMatchObject({ pid: outcome.pid, ready: true }); + else if (operation === 'release') expect(outcome.info).toBeNull(); + else expect(outcome.info).toMatchObject({ pid: outcome.pid, mode: 'fallback' }); + }, + ); + + it('eventually publishes readiness after coordination contention', () => { + const outcome = run('contended-ready'); + expect(outcome.info).toMatchObject({ pid: outcome.pid, ready: true }); + }); + + it('survives a transient readiness rename error and retries publication', () => { + const outcome = run('contended-ready-io'); + expect(outcome.failedRenames).toBeGreaterThan(1); + expect(outcome.info).toMatchObject({ pid: outcome.pid, ready: true }); + }); + + it.each(['retire', 'renew', 'transfer'] as const)( + 'does not publish queued readiness after %s changes ownership', + (operation) => { + const outcome = run(`contended-ready-${operation}`); + if (operation === 'retire') expect(outcome.info).toBeNull(); + else if (operation === 'renew') expect(outcome.info).toMatchObject({ pid: outcome.pid, mode: 'fallback', ready: false }); + else expect(outcome.info).toMatchObject({ pid: 333, ready: false }); + }, + ); + + it('eventually releases a retired writer after coordination contention', () => { + const outcome = run('retire'); + expect(outcome.released).toBe(true); + expect(outcome.acquired).toBe(true); + }); + + it('releases the coordination lock when its process exits without cleanup', () => { + const outcome = run('crash'); + expect(outcome).toEqual({ held: true }); + const child = spawnSync(process.execPath, [ACTOR, MODULE, root, 'reacquire'], { encoding: 'utf8', timeout: 15000 }); + expect(child.status, child.stderr).toBe(0); + expect(JSON.parse(child.stdout)).toEqual({ acquired: true }); + expect(fs.existsSync(path.join(root, '.codegraph/writer.pid.mutation.lock'))).toBe(true); + }); + + it('keeps release and fallback acquisition from invalidating a pending successor', () => { + const outcome = run('successor'); + expect(outcome.result).toBe(true); + expect(outcome.paused).toBe(true); + expect(outcome.acquired, 'successor read -> launcher release -> fallback acquire -> successor rename').toBe('taken'); + expect(outcome.info).toMatchObject({ pid: 333, mode: 'daemon' }); + }); +}); diff --git a/codegraph-kernel/Cargo.toml b/codegraph-kernel/Cargo.toml index da7a9fdb88..e8d7472a10 100644 --- a/codegraph-kernel/Cargo.toml +++ b/codegraph-kernel/Cargo.toml @@ -2,6 +2,7 @@ name = "codegraph-kernel" version = "0.1.0" edition = "2021" +rust-version = "1.89" license = "MIT" publish = false description = "Native extraction kernel for CodeGraph — tree-sitter parse+extract with one JS boundary crossing per file" diff --git a/codegraph-kernel/src/csharp/mod.rs b/codegraph-kernel/src/csharp/mod.rs index 3d40121796..f97b5192e6 100644 --- a/codegraph-kernel/src/csharp/mod.rs +++ b/codegraph-kernel/src/csharp/mod.rs @@ -5,7 +5,7 @@ //! extractor, bug-for-bug, checked by the kernel parity tests and the full-index //! dump-diff gate. The authoritative quirk list is //! docs/design/csharp-kernel-port-checklist.md — including every deliberate -//! emission hole (property/accessor bodies, constructor initializers, +//! emission hole (field/property initializers, constructor initializers, //! delegates/events/operators/indexers, top-level locals) and garbage ref //! (`(repo)` primary-ctor extends, `: byte` enum extends, `nameof` calls) //! this file preserves on purpose. Positions in UTF-16 code units. @@ -341,21 +341,34 @@ impl<'t> Walker<'t> { self.extract_method(node); skip_children = true; } else if kind == "property_declaration" && self.inside_class_like() { - // Property accessor/expression bodies are NEVER walked (calls - // inside are lost by design) — candidates-only scan. - self.extract_property(node); - self.scan_fn_ref_subtree(node, 0); + // Body references belong to the property; initializer candidates + // stay in the surrounding scope and skip the bodies already walked. + let walked: Vec = match self.extract_property(node) { + Some((row, name)) => { + let bodies = property_bodies(node); + if !bodies.is_empty() { + self.stack.push(Scope { row, kind: "property", name }); + for body in &bodies { + self.visit_for_calls_and_structure(*body); + } + self.stack.pop(); + } + bodies.iter().map(|body| body.id()).collect() + } + None => Vec::new(), + }; + self.scan_fn_ref_subtree(node, 0, &walked); skip_children = true; } else if kind == "field_declaration" && self.inside_class_like() { self.extract_field(node); - self.scan_fn_ref_subtree(node, 0); + self.scan_fn_ref_subtree(node, 0, &[]); skip_children = true; } else if kind == "local_declaration_statement" && !self.inside_class_like() { // Top-level statements: extractVariable's generic fallback finds no // direct identifier/variable_declarator children (C# nests them in // variable_declaration) → ZERO nodes, zero refs. Candidates only. self.extract_variable(node); - self.scan_fn_ref_subtree(node, 0); + self.scan_fn_ref_subtree(node, 0, &[]); skip_children = true; } else if kind == "using_directive" { self.extract_import(node); @@ -544,9 +557,8 @@ impl<'t> Walker<'t> { } /// extractProperty (1986) — property_declaration only (dispatch-gated to - /// class-like scopes). Accessor bodies and `=>` value clauses are never - /// walked; type refs DO come from the `type` field. - fn extract_property(&mut self, node: Node<'t>) { + /// class-like scopes). The caller walks bodies with this row on the stack. + fn extract_property(&mut self, node: Node<'t>) -> Option<(u32, String)> { let docstring = preceding_docstring(node, self.src); let visibility = Some(self.visibility_of(node)); let is_static = Some(self.is_static(node)); // ?? false — always concrete @@ -558,10 +570,10 @@ impl<'t> Walker<'t> { named_kids(node) .find(|c| c.kind() == "identifier") }); - let Some(name_node) = name_node else { return }; + let name_node = name_node?; let name = self.text(name_node).to_string(); if name.is_empty() { - return; + return None; } // Generic scan (isTsJsField=false): FIRST namedChild that isn't a @@ -595,11 +607,9 @@ impl<'t> Walker<'t> { &name, node, Extra { docstring, signature: Some(signature), visibility, is_static, ..Extra::default() }, - ); - if let Some(row) = row { - // decorators: none for C#; then the csharp type-ref path. - self.extract_csharp_type_refs(node, row); - } + )?; + self.extract_csharp_type_refs(node, row); + Some((row, name)) } /// extractField (2046) — field_declaration; each declarator becomes a @@ -829,3 +839,25 @@ fn find_anonymous_class_body(node: Node) -> Option { None } + +// Walk accessor and expression bodies; keep the existing initializer handling. +fn property_bodies(node: Node) -> Vec { + let mut bodies = Vec::new(); + if let Some(accessors) = node.child_by_field_name("accessors") { + for i in 0..accessors.named_child_count() { + let Some(accessor) = accessors.named_child(i) else { continue }; + if accessor.kind() != "accessor_declaration" { + continue; + } + if let Some(body) = accessor.child_by_field_name("body") { + bodies.push(body); + } + } + } + if let Some(value) = node.child_by_field_name("value") { + if value.kind() == "arrow_expression_clause" { + bodies.push(value); + } + } + bodies +} diff --git a/codegraph-kernel/src/csharp/refs.rs b/codegraph-kernel/src/csharp/refs.rs index 754bdf646d..07dcfa5862 100644 --- a/codegraph-kernel/src/csharp/refs.rs +++ b/codegraph-kernel/src/csharp/refs.rs @@ -119,9 +119,9 @@ impl<'t> Walker<'t> { } - pub(super) fn scan_fn_ref_subtree(&mut self, node: Node<'t>, depth: u32) { + pub(super) fn scan_fn_ref_subtree(&mut self, node: Node<'t>, depth: u32, walked: &[usize]) { stack_guard!(); - if depth > 12 { + if depth > 12 || walked.contains(&node.id()) { return; } // functionTypes is EMPTY for C#; the literal halt list applies — @@ -137,7 +137,7 @@ impl<'t> Walker<'t> { } self.maybe_capture_fn_refs(node); for c in named_kids(node) { - self.scan_fn_ref_subtree(c, depth + 1); + self.scan_fn_ref_subtree(c, depth + 1, walked); } } diff --git a/codegraph-kernel/src/lib.rs b/codegraph-kernel/src/lib.rs index a076d903b3..0d091be22c 100644 --- a/codegraph-kernel/src/lib.rs +++ b/codegraph-kernel/src/lib.rs @@ -395,6 +395,7 @@ mod ruby; mod rustlang; mod scala; mod contained_source; +mod writer_lock; mod stack; mod swift; mod textutil; diff --git a/codegraph-kernel/src/writer_lock.rs b/codegraph-kernel/src/writer_lock.rs new file mode 100644 index 0000000000..50f9c8dc7d --- /dev/null +++ b/codegraph-kernel/src/writer_lock.rs @@ -0,0 +1,33 @@ +use std::fs::{File, OpenOptions}; + +/// Keep this file in place: replacing its inode would split the lock domain. +#[napi_derive::napi] +pub struct WriterMutationLock { + file: Option, +} + +#[napi_derive::napi] +impl WriterMutationLock { + #[napi] + pub fn release(&mut self) { + self.file.take(); + } +} + +/// OS locks release on close or process exit, so a crashed writer cannot +/// leave a stale coordination lock that another process must delete. +#[napi_derive::napi] +pub fn try_writer_mutation_lock(path: String) -> napi::Result> { + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(path) + .map_err(|e| napi::Error::from_reason(e.to_string()))?; + match file.try_lock() { + Ok(()) => Ok(Some(WriterMutationLock { file: Some(file) })), + Err(std::fs::TryLockError::WouldBlock) => Ok(None), + Err(std::fs::TryLockError::Error(e)) => Err(napi::Error::from_reason(e.to_string())), + } +} diff --git a/docs/design/csharp-kernel-port-checklist.md b/docs/design/csharp-kernel-port-checklist.md index 41894cb067..663889de2a 100644 --- a/docs/design/csharp-kernel-port-checklist.md +++ b/docs/design/csharp-kernel-port-checklist.md @@ -1,6 +1,6 @@ # C# kernel port (R7b) — the bug-for-bug checklist -**Status: PORT COMPLETE (2026-07-20)** — walker `codegraph-kernel/src/csharp.rs`, +**Status: PORT COMPLETE (2026-07-20)** — walker `codegraph-kernel/src/csharp/mod.rs`, all gates passed (parity sweeps 0-diff on serilog 211/216 / Newtonsoft.Json 914/945 / jellyfin 2104/2105 — every deferral matching §arch-6's predictions; full-init dump gates byte-identical ×3; kernel-csharp-parity suite + grammar @@ -80,7 +80,7 @@ grammar bump is needed; grammar prep is a Cargo pin + parity-test entry only. raw-path perf number on an ASP.NET repo and conclude the port is broken. 3. **The framework extractor itself needs NO port** — regex over raw source (§Frameworks below), runs in extractFromSource:6736-6758 after either arm. -4. **One walker module** (suggest `codegraph-kernel/src/csharp.rs`), registered +4. **One walker module** (suggest `codegraph-kernel/src/csharp/mod.rs`), registered in langs.rs; per-file `has_error()` → `defer:` like every walker. **java.rs is the skeleton** (§java.rs mapping below) — same namespace-node concept, same field/constant split, same static-member/value-ref/fn-ref chassis. @@ -234,7 +234,7 @@ all PRESERVE): | `enum_declaration` | enumTypes:1064 → extractEnum:1914 | body `enum_member_declaration_list` required (bodiless → no node); extractInheritance sees `base_list` → **the underlying type `: byte` emits an `extends` ref named `byte`** (quirk, §inheritance); `enum_member_declaration` children → extractEnumMembers:1958 — `name` field path: ONE `enum_member` node per member, positioned at the member node (attributes included in its span), values/attributes ignored; non-member children (preproc_*, comment) → visitNode (no-op) | | `method_declaration` | methodTypes:1027 → extractMethod:1737 | classifyMethodNode absent → always extractMethod. Gate 1747 passes via class-like (a method_declaration outside a type does not occur in non-erroring C# — top-level `void M(){}` parses as local_function_statement, probed); bodyless interface/partial signatures mint nodes with no body walk; **expression-bodied methods have `body: arrow_expression_clause` (a real body FIELD, probed) → walked** | | `constructor_declaration` | methodTypes → extractMethod | name field = the class-name identifier → **method node named like the class**; returnType undefined; **`constructor_initializer` (`: base(args)` / `: this(args)`) is a sibling of the body field → NEVER walked → calls inside initializer args are LOST** (probed); expression-bodied ctor body = arrow_expression_clause → walked | -| `property_declaration` (inside class-like) | propertyTypes:1075 → extractProperty:1986 | property node + scanFnRefSubtree (capture-only) + skipChildren → **accessor bodies (`get { … }`, `get => …`) and the `=> expr` value clause are NEVER walked — calls inside property getters/setters/expression bodies emit NOTHING** (only fn-ref candidates). §property below | +| `property_declaration` (inside class-like) | propertyTypes:1075 → extractProperty:1986 | property node, then **propertyBodies — each accessor's `body` (`get { … }`, `set => …`) and an expression-bodied `=> expr` `value:` — walked by visitFunctionBody with the property pushed** (calls, instantiates, static reads and fn-ref candidates attribute to the property; changed 2026-10-04, previously never walked), then scanFnRefSubtree (capture-only, attributed to the class) over the rest — **the `= initializer` stays unwalked** — skipping the walked bodies, + skipChildren. §property below | | `field_declaration` (inside class-like) | fieldTypes:1084 → extractField:2046 | field/constant nodes per declarator + scanFnRefSubtree + skipChildren → **field initializers emit no calls/instantiates/static-member refs** (fn-ref candidates only). §field below | | `local_declaration_statement` | variableTypes:1098 (only reachable at top level — global statements; body locals go through visitFunctionBody instead) | not class-like → extractVariable:2538 → **generic fallback (2863-2881) finds no direct `identifier`/`variable_declarator` children (the declarator nests inside `variable_declaration`, probed) → ZERO nodes minted**; isClassScopeConstantAssignment (1508) needs node.type `assignment` → never true. skipChildren=true + scanFnRefSubtree → **a top-level `var builder = WebApplication.CreateBuilder(args);` produces NO node, NO calls ref, NO instantiates** — only fn-ref candidates. PRESERVE | | `using_directive` | importTypes:1209 → extractImport:3170 | hook (§config) → import node + ONE generic `imports` ref {fromNodeId: nodeStack top (namespace node if present, else file), referenceName: moduleName, line/col of the directive}; **no per-binding emitter** (the TS/py/rust/php/ruby ladder at 3197-3234 excludes csharp) | @@ -305,15 +305,22 @@ for C#) or bare `name`. QUIRKS (probed, PRESERVE): - An expression-bodied property (`public int Computed => MaxItems + 1;`) has children [modifier, predefined_type, identifier, **arrow_expression_clause (`value:` field)**] → typeNode = predefined_type → signature `"int Computed"`; - the arrow clause is NEVER walked (calls inside lost). -- `{ get; } = new();` initializers: the `value:` implicit_object_creation and - the accessor_list are both skipped/excluded → no refs, no instantiates. + the arrow clause is a property body (walked as the property, below). +- `{ get; } = new();` initializers: the `value:` implicit_object_creation is + NOT a body → no refs, no instantiates (candidates-only scan, attributed to + the class); the accessor_list is excluded from the type scan. Then extractDecoratorsFor (no-op) and **extractTypeAnnotations (2037) → extractCsharpTypeRefs** — the `type` field IS walked for refs (so `public List Items` emits references `List` + `Foo` even though the signature -kept the raw text). Return value feeds no body walk (the classifyMethodNode -initializer-walk path at 1031-1047 is TS-only). +kept the raw text). The returned node is pushed while propertyBodies +(tree-sitter.ts; csharp/mod.rs `property_bodies`) are walked: every +`accessor_declaration`'s `body` field (block or arrow_expression_clause, in +accessor order), then the property's `value:` when it is an +arrow_expression_clause. The fn-ref scan that follows skips those subtrees +by node id, so a candidate is captured once — from the property when it sits +in a body, from the class when it sits in an initializer. (The +classifyMethodNode initializer-walk path at 1031-1047 is TS-only.) ### extractField (2046) — field_declaration @@ -758,7 +765,8 @@ AspNetCore refs / Program.cs / Startup.cs / controller-source scan. + multi-declarator + instance fields (signatures `Type name`); `protected internal` (→ protected); property shapes: predefined-type, bare-identifier type (signature loses type), generic type, expression-bodied - (`=>` calls LOST), `{ get; } = new();`, accessor bodies with calls (LOST); + (`=>` calls → the property), `{ get; } = new();` (initializer LOST), + accessor bodies with calls (→ the property); event_field_declaration + event_declaration with add/remove bodies (no nodes; accessor calls → class); operator + conversion operator + indexer + destructor (no nodes; body calls → class); constructor with diff --git a/site/src/content/docs/reference/api.md b/site/src/content/docs/reference/api.md index 2c638e3053..86a199eb65 100644 --- a/site/src/content/docs/reference/api.md +++ b/site/src/content/docs/reference/api.md @@ -37,6 +37,7 @@ cg.close(); | `CodeGraph.init(path)` / `CodeGraph.open(path)` | Create or open a project index | | `indexAll(opts)` | Full index, with progress callback | | `sync()` | Incremental update | +| `getIndexHealth()` | Sorted project-relative `needsReindex` and `parseErrors` paths | | `searchNodes(query)` | Full-text symbol search | | `getCallers(id)` / `getCallees(id)` | Walk the call graph | | `getImpactRadius(id, depth)` | Transitive impact of a change | diff --git a/site/src/content/docs/reference/cli.md b/site/src/content/docs/reference/cli.md index bee53cc6f3..91b8ec4146 100644 --- a/site/src/content/docs/reference/cli.md +++ b/site/src/content/docs/reference/cli.md @@ -39,6 +39,11 @@ The MCP server (`codegraph serve --mcp`) is launched automatically by your agent ## Query commands +`status` reports files whose symbols need re-indexing and files with recorded parse errors. Its JSON output includes `index.filesNeedingReindex` and `index.filesWithParseErrors`. `files --json` includes each file's extraction `errors` array. + +A transient parser failure preserves a previously indexed file's graph and retries it on the next sync. A recorded syntax error remains visible until the file or parser changes. + + `query`, `callers`, `callees`, and `impact` all accept `--json` for machine-readable output. ```bash diff --git a/site/src/content/docs/reference/languages.md b/site/src/content/docs/reference/languages.md index 13408fc47c..929196a485 100644 --- a/site/src/content/docs/reference/languages.md +++ b/site/src/content/docs/reference/languages.md @@ -5,6 +5,8 @@ description: Every language CodeGraph parses, and the extensions it recognizes. Language support is automatic from the file extension — there's nothing to configure. +C# property accessors and expression-bodied properties contribute calls and references owned by the property. VB.NET member bodies and field initializers also contribute their calls and references. + | Language | Extensions | Status | |---|---|---| | TypeScript | `.ts`, `.tsx` | Full support | @@ -14,6 +16,7 @@ Language support is automatic from the file extension — there's nothing to con | Rust | `.rs` | Full support | | Java | `.java` | Full support | | C# | `.cs` | Full support | +| VB.NET | `.vb` | Full support | | PHP | `.php`, `.inc` | Full support (see Pascal for `.inc` include files) | | Ruby | `.rb` | Full support | | C | `.c`, `.h` | Full support | diff --git a/site/src/content/docs/reference/mcp-server.md b/site/src/content/docs/reference/mcp-server.md index 5dcb5ae719..5cda110fd0 100644 --- a/site/src/content/docs/reference/mcp-server.md +++ b/site/src/content/docs/reference/mcp-server.md @@ -29,6 +29,14 @@ The scan checks its 300 ms budget between files and stops at sixteen matches or When you ask about tests, `codegraph_explore` includes source from the nearest test callers within three caller hops. These links describe static callers, not measured runtime coverage. Pinned JavaScript and TypeScript test files prioritize matching `it`/`test` callbacks. File and output budgets still apply. +## Daemon lifecycle + +Writer handovers, readiness updates and releases share an OS file lock. The persistent `.codegraph/writer.pid.mutation.lock` file stays in place; the OS releases its lock when the process exits. + +The launcher can replace a daemon from an older release when its hello confirms coordinated writer handover. Legacy daemons stay running; new sessions serve reads without auto-sync. Stop the old MCP sessions and daemon, then reconnect with the current install to migrate. A daemon exits when its installation is deleted or its package version changes. Managed builds of the same release still require matching build identities. + +`CODEGRAPH_DAEMON_INSTALL_CHECK_MS` sets the installation-check interval in milliseconds. It defaults to `30000`; `0` disables the check. + ## The other tools Seven more tools exist and stay fully functional, but are **unlisted by default** — everything they return already arrives inline on a `codegraph_explore` response (its blast-radius section, the relationship map, a symbol's body and its callee list): diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 6a9e6ca159..61adfbaf62 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -1115,6 +1115,33 @@ program } }); +/** + * The `status` warnings for indexed files whose symbols are missing although + * their content is current (#2335, #2336): one line per group, saying what is + * wrong, what to do, and naming up to three of the files. + */ +function describeFilesMissingSymbols(health: { needsReindex: string[]; parseErrors: string[] }): string[] { + const dash = getGlyphs().dash; + const sample = (paths: string[]): string => + paths.slice(0, 3).join(', ') + (paths.length > 3 ? ` (+${formatNumber(paths.length - 3)} more)` : ''); + const lines: string[] = []; + const reindex = health.needsReindex.length; + if (reindex > 0) { + lines.push( + `${formatNumber(reindex)} ${reindex === 1 ? 'file is missing its' : 'files are missing their'} symbols ${dash} ` + + `run "codegraph sync" to re-index ${reindex === 1 ? 'it' : 'them'}: ${sample(health.needsReindex)}` + ); + } + const unparsed = health.parseErrors.length; + if (unparsed > 0) { + lines.push( + `${formatNumber(unparsed)} ${unparsed === 1 ? 'file could not be parsed, so its symbols are' : 'files could not be parsed, so their symbols are'} ` + + `missing: ${sample(health.parseErrors)} ${dash} "codegraph files --json" shows the errors` + ); + } + return lines; +} + /** * codegraph status [path] */ @@ -1162,6 +1189,9 @@ program // Zero on a healthy index; non-zero at rest means a resolution pass was // interrupted, so some files' call edges are missing (#1187). const pendingRefs = cg.getPendingReferenceCount(); + // Files whose content is current but whose symbols are missing — the + // content-hash check behind "up to date" cannot see them (#2336). + const health = cg.getIndexHealth(); // JSON output mode if (options.json) { @@ -1202,6 +1232,12 @@ program // interrupted resolution pass left edges missing; the next // sync sweeps them (#1187). pendingRefs, + // Files stored without their symbols (e.g. while their grammar + // could not load, #2335); the next sync re-indexes them. + filesNeedingReindex: health.needsReindex.length, + // Files with a recorded parse error and no symbols from it + // (#2336); unchanged until the file or the parser changes. + filesWithParseErrors: health.parseErrors.length, }, })); cg.destroy(); @@ -1294,7 +1330,10 @@ program console.log(` Removed: ${changes.removed.length} files`); } info('Run "codegraph sync" to update the index'); - } else { + } + const missingSymbols = describeFilesMissingSymbols(health); + for (const line of missingSymbols) warn(line); + if (totalChanges === 0 && missingSymbols.length === 0) { success('Index is up to date'); } console.log(); @@ -1865,6 +1904,14 @@ program language: f.language, nodeCount: f.nodeCount, size: f.size, + // What extraction recorded for the file — a parse error, a skip + // reason — so a health check needn't read the database (#2336). + errors: (f.errors ?? []).map((e) => ({ + severity: e.severity, + code: e.code, + message: e.message, + line: e.line, + })), })); console.log(JSON.stringify(output, null, 2)); cg.destroy(); diff --git a/src/codegraph.ts b/src/codegraph.ts index 29e6dc9805..37b54ec7e5 100644 --- a/src/codegraph.ts +++ b/src/codegraph.ts @@ -28,6 +28,7 @@ import { BuildContextOptions, FindRelevantContextOptions, UnresolvedReference, + IndexHealth, } from './types'; import { DatabaseConnection, getDatabasePath, removeDatabaseFiles } from './db'; import { WalCheckpointValve, resolveWalValveMb } from './db/wal-valve'; @@ -52,6 +53,7 @@ import { logWarn } from './errors'; import type { FileWatcher, WatchOptions, PendingFile } from './sync'; import { EXTRACTION_VERSION } from './extraction/extraction-version'; import { isGeneratedFile } from './extraction/generated-detection'; +import { hasGrammarLoadFailure, isFileLevelOnlyLanguage } from './extraction/grammars'; import { refreshNearDuplicates } from './graph/near-duplicates'; import { getCodeGraphDir } from './directory'; import { deriveProjectNameTokens } from './search/query-utils'; @@ -2349,6 +2351,31 @@ export class CodeGraph { return this.queries.countGeneratedFiles(); } + /** Files needing retry or carrying deterministic parse errors, for status. */ + getIndexHealth(): IndexHealth { + const needsReindex: string[] = []; + const parseErrors: string[] = []; + for (const file of this.queries.getFilesWithoutNodesOrWithErrors()) { + const errors = file.errors ?? []; + if (hasGrammarLoadFailure(errors)) { + // Stored without being parsed — its grammar could not load (#2335). + needsReindex.push(file.path); + } else if (file.nodeCount === 0) { + // Every parse stores at least the file node, so zero nodes means a + // wiped row (#1541) or a recorded failure — except file-level-only + // languages and files over the size limit, which are empty on purpose. + if (isFileLevelOnlyLanguage(file.language)) continue; + if (errors.length === 0) needsReindex.push(file.path); + else if (errors.some((e) => e.severity === 'error' || e.code === 'parse_error')) parseErrors.push(file.path); + } else if (errors.some((e) => e.code === 'parse_error')) { + // Parsed, but the tree had errors and no symbols survived. + parseErrors.push(file.path); + } + } + return { needsReindex, parseErrors }; + } + + // =========================================================================== // Graph Query Methods // =========================================================================== diff --git a/src/db/queries.ts b/src/db/queries.ts index c7b9472f99..335dc6cbb6 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -3424,6 +3424,18 @@ export class QueryBuilder { return rows.map(rowToFileRecord); } + /** + * Files stored with no nodes or with recorded extraction errors — the only + * rows that can be missing their symbols (`CodeGraph.getIndexHealth`). A + * healthy index returns few or none, so this stays cheap on a large one. + */ + getFilesWithoutNodesOrWithErrors(): FileRecord[] { + const rows = this.db + .prepare('SELECT * FROM files WHERE node_count = 0 OR errors IS NOT NULL ORDER BY path') + .all() as FileRow[]; + return rows.map(rowToFileRecord); + } + /** * Most recent index timestamp (ms since epoch) across all tracked files, or * null when nothing is indexed yet. One indexed aggregate, no per-row scan. (#329) diff --git a/src/extraction/cfml-extractor.ts b/src/extraction/cfml-extractor.ts index a41b29f3f6..16d3b5dc24 100644 --- a/src/extraction/cfml-extractor.ts +++ b/src/extraction/cfml-extractor.ts @@ -3,6 +3,7 @@ import { Node, Edge, ExtractionResult, ExtractionError, UnresolvedReference, Lan import { generateNodeId, NodeIdAllocator } from './tree-sitter-helpers'; import { TreeSitterExtractor } from './tree-sitter'; import { parseSourceTreeSync } from './parse-tree'; +import { KernelUnavailableError } from './kernel/loader'; /** Tags whose own children include an expression: ``, ``, ``, ``, and `#…#`. */ const TAG_EXPRESSION_PARENTS: ReadonlySet = new Set([ @@ -101,7 +102,7 @@ export class CfmlExtractor { this.errors.push({ message: `CFML extraction error: ${error instanceof Error ? error.message : String(error)}`, severity: 'error', - code: 'parse_error', + code: error instanceof KernelUnavailableError ? 'parser_error' : 'parse_error', }); } @@ -155,7 +156,7 @@ export class CfmlExtractor { this.errors.push({ message: 'cfml grammar not loaded', severity: 'error', - code: 'unsupported_language', + code: 'parser_error', }); return; } diff --git a/src/extraction/extraction-version.ts b/src/extraction/extraction-version.ts index 0edf9fad57..9fedfeef2b 100644 --- a/src/extraction/extraction-version.ts +++ b/src/extraction/extraction-version.ts @@ -21,4 +21,4 @@ * turns the re-index hint into noise — keep it honest (see CLAUDE.md, "Honesty * in the product is load-bearing"). */ -export const EXTRACTION_VERSION = 38; +export const EXTRACTION_VERSION = 39; diff --git a/src/extraction/grammars.ts b/src/extraction/grammars.ts index e9d47b2b99..b21301fffd 100644 --- a/src/extraction/grammars.ts +++ b/src/extraction/grammars.ts @@ -7,7 +7,7 @@ * of docs/design/kernel-only-extraction-plan.md). */ -import { Language } from '../types'; +import type { ExtractionError, Language } from '../types'; export type GrammarLanguage = Exclude; @@ -563,6 +563,21 @@ export function getSupportedLanguages(): Language[] { return [...GRAMMAR_LANGUAGES, 'svelte', 'vue', 'astro', 'liquid', 'markdown']; } +/** + * Whether these extraction errors say the file was never parsed because its + * grammar was not available (`parser_error`, recorded when + * the native parser cannot return a tree). + * + * That is a fact about the running process, not about the file: a daemon whose + * install was upgraded or deleted underneath it fails every lazy grammar load + * (#2335). Such a result must never replace a file's index data, and a row an + * older engine stored that way must be re-indexed — not treated as current + * because its content hash still matches. + */ +export function hasGrammarLoadFailure(errors: readonly ExtractionError[] | undefined): boolean { + return !!errors && errors.some((e) => e.code === 'parser_error'); +} + /** * Get language display name */ diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 5a9242a950..8ad29b5d04 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -25,7 +25,7 @@ import { DEFAULT_PARSE_POOL_CAP, FILES_PER_PARSE_WORKER, ParseWorkerPool, isRetr import { StoreWriter, StoreBundle, finalizeStoreBundle, attachBindings } from './store-writer'; import { materializeKernelResult } from './kernel'; import { detectGeneratedFile } from './generated-detection'; -import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLanguage, initGrammars, loadGrammarsForLanguages, isMpegTransportStream, hasMpegTsExtension, MPEG_TS_SNIFF_BYTES } from './grammars'; +import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLanguage, initGrammars, loadGrammarsForLanguages, isMpegTransportStream, hasMpegTsExtension, MPEG_TS_SNIFF_BYTES, hasGrammarLoadFailure } from './grammars'; import { loadExtensionOverrides, loadIncludeIgnoredPatterns, loadExcludePatterns, loadIncludePatterns, PROJECT_CONFIG_FILENAME } from '../project-config'; import { isCodeGraphDataDir } from '../directory'; import { logDebug, logWarn } from '../errors'; @@ -2320,11 +2320,17 @@ export class ExtractionOrchestrator { const nodeCount = result.kernelCounts?.nodes ?? result.nodes.length; const edgeCount = result.kernelCounts?.edges ?? result.edges.length; + // A file whose grammar failed to load was never parsed (#2335). + const grammarUnavailable = hasGrammarLoadFailure(result.errors); + // Store: on the writer thread when active (fresh DB — bundles applied // in the same file order this chain dispatches them), else on the main // thread (SQLite connections are per-thread). const language = detectLanguage(filePath, content, overrides); - if (storeWriter) { + if (grammarUnavailable) { + // Store nothing: a row from an earlier run keeps its data, and with no + // row (or an older hash) the next sync or index retries the file. + } else if (storeWriter) { if (result.kernelBuffers) { // Buffers go to the writer as-is; the worker decodes + finalizes. // The main thread's only per-file work stays O(1) + the content hash. @@ -2352,7 +2358,9 @@ export class ExtractionOrchestrator { errors.push(...result.errors); } - if (nodeCount > 0) { + if (grammarUnavailable) { + filesErrored++; + } else if (nodeCount > 0) { filesIndexed++; totalNodes += nodeCount; totalEdges += edgeCount; @@ -2759,7 +2767,9 @@ export class ExtractionOrchestrator { errors.push(...result.errors); } - if (result.nodes.length > 0) { + if (hasGrammarLoadFailure(result.errors)) { + filesErrored++; // nothing was stored (#2335) + } else if (result.nodes.length > 0) { filesIndexed++; totalNodes += result.nodes.length; totalEdges += result.edges.length; @@ -2967,6 +2977,12 @@ export class ExtractionOrchestrator { onYield?: MaybeYield, frameworkEnriched = false ): Promise { + // The file was never parsed: its grammar failed to load (#2335). Storing + // this would replace the file's symbols with an empty row under the new + // content hash, which no hash-based sync revisits. Keep whatever the index + // has; the stale (or missing) row makes the next sync retry the file. + if (hasGrammarLoadFailure(result.errors)) return; + // A kernel result can arrive as an undecoded buffer transport (empty // node/edge arrays, tables riding in kernelBuffers). Decode it before // storing — persisting the transport as-is records the file as having no @@ -2994,7 +3010,9 @@ export class ExtractionOrchestrator { const existingIsMarker = existingFile.nodeCount === 0 && (existingFile.errors?.length ?? 0) > 0; const incomingHasContent = result.nodes.length > 0; - if (!existingIsMarker || !incomingHasContent) { + // A failed parser never indexed these bytes; even an empty real parse replaces it. + const existingNeverParsed = hasGrammarLoadFailure(existingFile.errors); + if (!existingNeverParsed && (!existingIsMarker || !incomingHasContent)) { this.queries.replaceLiteralsForFile(filePath, result.nodes); return; // No changes } @@ -3460,13 +3478,18 @@ export class ExtractionOrchestrator { } const fullPath = path.join(this.rootDir, filePath); const tracked = trackedMap.get(filePath); + // A row an older engine stored while the file's grammar could not load + // (#2335) holds no parse of these bytes: re-index it even though its + // size, mtime and hash all match. Rows with a real parse error are + // deterministic and are not retried. + const neverParsed = tracked !== undefined && hasGrammarLoadFailure(tracked.errors); // Cheap pre-filter: an already-indexed file whose size AND mtime both match // the DB is unchanged — skip it without reading or hashing. (A content // change that preserves both exactly is the blind spot every mtime-based // incremental tool accepts; `index --force` is the escape hatch. Git bumps // mtime on every file it writes during checkout/merge, so pulls are caught.) - if (tracked) { + if (tracked && !neverParsed) { try { const stat = fs.statSync(fullPath); if (stat.size === tracked.size && Math.floor(stat.mtimeMs) === Math.floor(tracked.modifiedAt)) { @@ -3502,7 +3525,7 @@ export class ExtractionOrchestrator { filesToIndex.push(filePath); changedFilePaths.push(filePath); filesAdded++; - } else if (tracked.contentHash !== contentHash) { + } else if (tracked.contentHash !== contentHash || neverParsed) { onFileChange?.(filePath, content); filesToIndex.push(filePath); changedFilePaths.push(filePath); @@ -3580,7 +3603,7 @@ export class ExtractionOrchestrator { try { const result = await this.indexFile(filePath); if (result.errors.some(e => e.severity === 'error')) failedFilePaths.push(filePath); - nodesUpdated += result.nodes.length; + if (!hasGrammarLoadFailure(result.errors)) nodesUpdated += result.nodes.length; } finally { this.conventionInvalidatedFiles.delete(filePath); } diff --git a/src/extraction/kernel/loader.ts b/src/extraction/kernel/loader.ts index c0f54917d2..9b22a72bfe 100644 --- a/src/extraction/kernel/loader.ts +++ b/src/extraction/kernel/loader.ts @@ -290,6 +290,8 @@ export interface ContainedSourceReaderLike { } export interface KernelModule { + /** Required for writer mutations; older kernels remain usable for reads. */ + tryWriterMutationLock?(path: string): { release(): void } | null; /** Descriptor-verified source reads. Older released kernels omit this API; * scans then leave absence unproven. */ ContainedSourceReader?: new (root: string) => ContainedSourceReaderLike; diff --git a/src/extraction/languages/vbnet.ts b/src/extraction/languages/vbnet.ts index b6a92414e5..91fa4606d8 100644 --- a/src/extraction/languages/vbnet.ts +++ b/src/extraction/languages/vbnet.ts @@ -116,8 +116,13 @@ export const vbnetExtractor: LanguageExtractor = { // findable declaration (WinForms/WPF code is built around them). if (node.type === 'event_declaration' || node.type === 'custom_event_declaration') { const nameNode = node.childForFieldName('name'); - if (nameNode) { - ctx.createNode('field', getNodeText(nameNode, ctx.source), node); + const event = nameNode ? ctx.createNode('field', getNodeText(nameNode, ctx.source), node) : null; + // A Custom Event's AddHandler / RemoveHandler / RaiseEvent blocks run + // code, which is the event's. + if (event && node.type === 'custom_event_declaration') { + ctx.pushScope(event.id); + ctx.visitFunctionBody(node, event.id); + ctx.popScope(); } return true; } diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 5214928d2f..faae8298e9 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -18,6 +18,7 @@ import { } from '../types'; import { detectLanguage, isLanguageSupported, isFileLevelOnlyLanguage } from './grammars'; import { parseSourceTreeSync } from './parse-tree'; +import { KernelUnavailableError } from './kernel/loader'; import { NodeIdAllocator, getNodeText, getChildByField, getPrecedingDocstring } from './tree-sitter-helpers'; import { FN_REF_SPECS, captureFnRefCandidates, type FnRefSpec, type FnRefCandidate } from './function-ref'; import { isGeneratedFile } from './generated-detection'; @@ -587,6 +588,7 @@ export class TreeSitterExtractor { */ extract(): ExtractionResult { const startTime = Date.now(); + let parserUnavailable = false; if (!isLanguageSupported(this.language)) { return { @@ -621,6 +623,7 @@ export class TreeSitterExtractor { // serialized tree through the NativeNode facade. this.tree = parseSourceTreeSync(this.source, this.language); if (!this.tree) { + parserUnavailable = true; throw new Error(`Failed to get parser for language: ${this.language}`); } if (['javascript', 'typescript', 'jsx', 'tsx'].includes(this.language)) { @@ -694,7 +697,7 @@ export class TreeSitterExtractor { message: `Parse error: ${msg}`, filePath: this.filePath, severity: 'error', - code: 'parse_error', + code: parserUnavailable || error instanceof KernelUnavailableError ? 'parser_error' : 'parse_error', }); } finally { // Drop the tree with the file; a kernel tree is plain Buffers, so @@ -741,8 +744,10 @@ export class TreeSitterExtractor { * nested function definitions: their bodies are walked — and their * candidates attributed — by extractFunction's own body walk. */ - private scanFnRefSubtree(node: SyntaxNode, depth: number): void { + private scanFnRefSubtree(node: SyntaxNode, depth: number, walked?: ReadonlySet): void { if (!this.fnRefSpec || depth > 12) return; + // Subtrees the body walker has already been through. + if (walked?.has(node.id)) return; const nodeType = node.type; if (depth > 0 && ( this.extractor?.functionTypes.includes(nodeType) || @@ -757,7 +762,7 @@ export class TreeSitterExtractor { this.maybeCaptureFnRefs(node, nodeType); for (let i = 0; i < node.namedChildCount; i++) { const child = node.namedChild(i); - if (child) this.scanFnRefSubtree(child, depth + 1); + if (child) this.scanFnRefSubtree(child, depth + 1, walked); } } @@ -1292,11 +1297,19 @@ export class TreeSitterExtractor { } // Check for class properties (e.g. C# property_declaration) else if (this.extractor.propertyTypes?.includes(nodeType) && this.isInsideClassLikeNode()) { - this.extractProperty(node); - // Property initializers aren't walked — scan for function-as-value - // candidates (#756): Scala `val table = Seq(targetCb)` in an object, - // Kotlin `val cb = ::handler` class properties. - this.scanFnRefSubtree(node, 0); + const propNode = this.extractProperty(node); + // The code a property runs is the property's: its calls, + // instantiations and reads attribute to it, as a method's do. + const bodies = propNode ? this.propertyBodies(node) : []; + if (propNode && bodies.length > 0) { + this.nodeStack.push(propNode.id); + for (const body of bodies) this.visitFunctionBody(body, propNode.id); + this.nodeStack.pop(); + } + // Whatever the body walk didn't cover (a C# `= initializer`, any other + // language's whole declaration) is scanned for function-as-value + // candidates (#756); the bodies captured their own. + this.scanFnRefSubtree(node, 0, new Set(bodies.map((b) => b.id))); skipChildren = true; } // Check for class fields (e.g. Java field_declaration, C# field_declaration) @@ -2226,7 +2239,12 @@ export class TreeSitterExtractor { // (#1093) because the two defaults differ — a bodiless CLASS is kept // unless a language opts into skipping, a bodiless STRUCT is skipped // unless a language opts into keeping. - const body = getChildByField(node, this.extractor.bodyField); + // + // resolveBody first, as for classes and enums: a VB.NET Structure tags + // every member as its own `body` field, so the field alone yields only + // the first member. + const body = this.extractor.resolveBody?.(node, this.extractor.bodyField) + ?? getChildByField(node, this.extractor.bodyField); if (!body && node.type !== 'record_declaration' && !this.extractor.allowBodilessStruct) return; @@ -2418,6 +2436,28 @@ export class TreeSitterExtractor { return propNode; } + /** + * The parts of a property declaration that run code, for the body walker. + * VB.NET writes its `Get` / `Set` blocks, `= initializer` and `As New T` + * as children of the declaration itself, so the declaration is walked + * whole, the way its methods are (resolveBody). C# runs code in each + * accessor's body (`get { … }`, `set => …`) and in an expression-bodied + * property's `=> …`; its `= initializer`, like a field's, stays unwalked. + * Mirrored in the kernel (csharp/mod.rs property_bodies). + */ + private propertyBodies(node: SyntaxNode): SyntaxNode[] { + if (this.language === 'vbnet') return [node]; + if (this.language !== 'csharp') return []; + const bodies: SyntaxNode[] = []; + for (const accessor of getChildByField(node, 'accessors')?.namedChildren ?? []) { + const body = accessor.type === 'accessor_declaration' ? getChildByField(accessor, 'body') : null; + if (body) bodies.push(body); + } + const value = getChildByField(node, 'value'); + if (value?.type === 'arrow_expression_clause') bodies.push(value); + return bodies; + } + /** * Extract a class field declaration (e.g. Java field_declaration, C# field_declaration). * Extracts each declarator as a 'field' kind node inside the owning class. @@ -2526,10 +2566,11 @@ export class TreeSitterExtractor { // candidates, so a lambda / method reference / anonymous class in // `private final Runnable r = () -> target();` contributed NO call // edge at all and `target` looked callerless. Keyed on the `value` - // FIELD, which only Java's `variable_declarator` carries — C#, - // VB.NET and PHP spell their initializer differently and are - // deliberately untouched here. - const valueNode = getChildByField(decl, 'value'); + // FIELD, which only Java's `variable_declarator` carries. VB.NET + // writes `= expr` (the declarator's `initializer`) or `As New T(…)` + // (inside its as_clause), so its whole declarator is walked. C# + // and PHP spell their initializer differently and are untouched. + const valueNode = this.language === 'vbnet' ? decl : getChildByField(decl, 'value'); if (valueNode) { this.nodeStack.push(fieldNode.id); this.visitFunctionBody(valueNode, fieldNode.id); diff --git a/src/mcp/daemon-paths.ts b/src/mcp/daemon-paths.ts index 8f78d5063e..494364a4fe 100644 --- a/src/mcp/daemon-paths.ts +++ b/src/mcp/daemon-paths.ts @@ -123,7 +123,7 @@ export function canProbeDaemonIdentity(info: DaemonLockInfo): boolean { * its socket. A bare PID liveness probe is insufficient because OSes reuse PIDs * after an OOM/SIGKILL (#1553). */ -export function probeDaemonIdentity(info: DaemonLockInfo, timeoutMs = 1_000): Promise { +export function probeDaemonIdentity(info: DaemonLockInfo, timeoutMs = 1_000, requireWriterProtocol = false): Promise { if (!canProbeDaemonIdentity(info)) return Promise.resolve(false); return new Promise((resolve) => { let socket: net.Socket; @@ -156,7 +156,8 @@ export function probeDaemonIdentity(info: DaemonLockInfo, timeoutMs = 1_000): Pr finish( hello.protocol === 1 && hello.pid === info.pid && - (info.version === 'unknown' || hello.codegraph === info.version) + (info.version === 'unknown' || hello.codegraph === info.version) && + (!requireWriterProtocol || hello.writerProtocol === 1) ); } catch { finish(false); diff --git a/src/mcp/daemon-registry.ts b/src/mcp/daemon-registry.ts index aabb79139b..e9f3ba681f 100644 --- a/src/mcp/daemon-registry.ts +++ b/src/mcp/daemon-registry.ts @@ -31,7 +31,14 @@ import { probeDaemonIdentity, type DaemonLockInfo, } from './daemon-paths'; -import { readWriterLock, releaseWriterLock, tryAcquireWriterLock } from './writer-lock'; +import { + readWriterLock, + releaseWriterLock, + swapWriterLock, + tryAcquireWriterLock, + type WriterLockInfo, +} from './writer-lock'; +import { isOlderRelease } from './version'; import { WORKER_START_SETTLE_MS } from '../worker-teardown'; export interface DaemonRecord { @@ -155,7 +162,6 @@ function cleanupDaemonArtifacts( root: string, expectedLockContents: string | null, ): boolean { - const pidPath = getDaemonPidPath(root); // A daemon owns writer.pid before binding or relocating its socket. Claiming // the writer slot therefore freezes every legitimate daemon artifact writer // while we compare the inspected lock snapshot and clean it up. @@ -164,33 +170,39 @@ function cleanupDaemonArtifacts( if (claim.kind === 'taken') return false; try { - if (expectedLockContents === null) { - if (fs.existsSync(pidPath)) return false; - } else { - try { - if (fs.readFileSync(pidPath, 'utf8') !== expectedLockContents) return false; - } catch { - return false; - } - } - // POSIX sockets are real files; Windows named pipes vanish with the process. - // Sweep every candidate before releasing daemon.pid, so no successor can - // acquire the lock and bind a socket that this cleanup then removes. - if (process.platform !== 'win32') { - for (const candidate of getDaemonSocketCandidates(root)) { - try { fs.unlinkSync(candidate); } catch { /* gone */ } - } - } - deregisterDaemon(root); - try { fs.unlinkSync(pidPath); } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false; - } - return true; + return removeDaemonArtifacts(root, expectedLockContents); } finally { releaseWriterLock(root); } } +/** The removal behind {@link cleanupDaemonArtifacts}, for a caller already holding the writer slot. */ +function removeDaemonArtifacts(root: string, expectedLockContents: string | null): boolean { + const pidPath = getDaemonPidPath(root); + if (expectedLockContents === null) { + if (fs.existsSync(pidPath)) return false; + } else { + try { + if (fs.readFileSync(pidPath, 'utf8') !== expectedLockContents) return false; + } catch { + return false; + } + } + // POSIX sockets are real files; Windows named pipes vanish with the process. + // Sweep every candidate before releasing daemon.pid, so no successor can + // acquire the lock and bind a socket that this cleanup then removes. + if (process.platform !== 'win32') { + for (const candidate of getDaemonSocketCandidates(root)) { + try { fs.unlinkSync(candidate); } catch { /* gone */ } + } + } + deregisterDaemon(root); + try { fs.unlinkSync(pidPath); } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return false; + } + return true; +} + /** Remove daemon artifacts only when no matching daemon answers the socket hello. */ export async function clearStaleDaemonArtifacts(root: string): Promise { const pidPath = getDaemonPidPath(root); @@ -239,7 +251,9 @@ export interface StopResult { root: string; pid: number | null; /** 'term' graceful, 'kill' force, 'still-running' refused, 'not-running' stale, 'no-daemon' absent, 'unverified' preserved. */ - outcome: 'term' | 'kill' | 'still-running' | 'not-running' | 'no-daemon' | 'unverified'; + outcome: 'term' | 'kill' | 'still-running' | 'not-running' | 'no-daemon' | 'unverified' | 'legacy-writer'; + /** The daemon's version as its lock recorded it, when the stop got that far. */ + version?: string; } /** @@ -292,13 +306,27 @@ export async function stopDaemonAt( const removed = cleanupDaemonArtifacts(root, lockContents); return { root, pid, outcome: removed ? 'not-running' : 'unverified' }; } + return stopVerifiedDaemon(root, identity, lockContents, options.shutdownGraceMs); +} +/** + * The signalling half of a stop, for a daemon whose socket hello just proved + * `identity` against the lock read as `lockContents`: SIGTERM, wait, SIGKILL + * only one that still answers, then sweep its artifacts. + */ +async function stopVerifiedDaemon( + root: string, + identity: DaemonLockInfo, + lockContents: string | null, + shutdownGraceMs = DAEMON_SHUTDOWN_GRACE_MS, +): Promise { + const { pid, version } = identity; // Identity probing awaits I/O: never act on a superseded ownership record. const sameLock = (): boolean => { try { return fs.readFileSync(getDaemonPidPath(root), 'utf8') === lockContents; } catch { return lockContents === null; } }; - if (!sameLock()) return { root, pid, outcome: 'unverified' }; + if (!sameLock()) return { root, pid, outcome: 'unverified', version }; // POSIX: SIGTERM runs the daemon's graceful shutdown. Windows: TerminateProcess // (no graceful path), so we always sweep artifacts ourselves below. @@ -309,7 +337,7 @@ export async function stopDaemonAt( if (sameLock() && await probeDaemonIdentity(identity) && sameLock()) { try { process.kill(pid, 'SIGKILL'); } catch { /* raced to exit */ } if (!(await waitForDeath(pid, 2000))) { - return { root, pid, outcome: 'still-running' }; + return { root, pid, outcome: 'still-running', version }; } outcome = 'kill'; } else { @@ -321,14 +349,106 @@ export async function stopDaemonAt( // Its identity can't be re-proven without the socket, so signal nothing // more; just wait, bounded, for the PID to go. One that outlives the // wait is reported, as before. - if (!(await waitForDeath(pid, options.shutdownGraceMs ?? DAEMON_SHUTDOWN_GRACE_MS))) { - return { root, pid, outcome: 'still-running' }; + if (!(await waitForDeath(pid, shutdownGraceMs))) { + return { root, pid, outcome: 'still-running', version }; } } } // Compares the lock with the one we signalled, so a successor's is kept. cleanupDaemonArtifacts(root, lockContents); - return { root, pid, outcome }; + return { root, pid, outcome, version }; +} + +/** + * Stop the daemon serving `root` when it runs an older CodeGraph release than + * `version`, so the caller can start one from its own install (#2335). A + * daemon keeps running the code it started with: one that outlived an upgrade + * goes on loading grammars from files the upgrade removed, while it holds the + * project's writer lock and file watcher. Only a daemon whose lock records an + * older release ({@link isOlderRelease}) and whose socket hello confirms that + * lock is signalled, exactly as {@link stopDaemonAt} would; one of the same, a + * newer or an unknown version is never touched, so two installs cannot take + * turns stopping each other's daemon. + * + * The project's writer slot never falls free on the way: before the signal, + * this process takes it over from the old daemon (mode `handover`), and the + * caller hands it on to the daemon it starts, which takes it over in turn + * ({@link swapWriterLock}). The old daemon's sessions serve themselves + * in-process the moment it goes, and one that found the slot free would claim + * it as their writer — with the code of an install the upgrade removed — and + * keep the new daemon from starting. When the old daemon is not stopped, the + * slot goes back to it. + * + * Resolves null when the lock names no older daemon (there is none, or another + * launcher already replaced it) or the slot could not be taken (another + * launcher holds it to replace the same daemon). Otherwise says what became of + * the daemon: `term` or `kill` when it was stopped, and this process now holds + * the slot for its successor (release it with {@link releaseWriterLock} once + * that one has taken over or failed); `not-running` when it had already exited + * (its successor clears the stale lock); `unverified` or `still-running` when + * it is still there. + */ +export async function stopOlderDaemon( + root: string, + version: string, + options: { shutdownGraceMs?: number } = {}, +): Promise { + let lockContents: string; + try { + lockContents = fs.readFileSync(getDaemonPidPath(root), 'utf8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return null; + return { root, pid: null, outcome: 'unverified' }; + } + const identity = decodeLockInfo(lockContents); + if (!identity) return { root, pid: null, outcome: 'unverified' }; + if (!isOlderRelease(identity.version, version)) return null; + const { pid } = identity; + if (!isProcessAlive(pid)) return { root, pid, outcome: 'not-running', version: identity.version }; + // The hello must name this pid and version (#1553): a reused pid is no daemon. + if (!canProbeDaemonIdentity(identity) || !await probeDaemonIdentity(identity)) { + return { root, pid, outcome: 'unverified', version: identity.version }; + } + // A legacy daemon can still publish readiness or release after our claim; + // its unfenced mutations cannot be serialized by the new OS lock. + if (!await probeDaemonIdentity(identity, undefined, true)) { + return { root, pid, outcome: 'legacy-writer', version: identity.version }; + } + const slot = takeWriterSlotFrom(root, pid); + if (!slot) return null; + const result = await stopVerifiedDaemon(root, identity, lockContents, options.shutdownGraceMs); + if (result.outcome === 'term' || result.outcome === 'kill') { + // The stop's own sweep needs the slot this process now holds: clear what + // the old daemon left (all of it on Windows, where the stop is + // TerminateProcess), so its successor starts on a clean lock. + removeDaemonArtifacts(root, lockContents); + } else { + slot.giveBack(); + } + return result; +} + +/** + * Take the project's writer slot from the daemon `pid` this process is about + * to stop (see {@link stopOlderDaemon}), whether the record is that daemon's, + * stale, or absent. Returns how to give it back to that daemon, or null when + * the slot could not be taken: another live process holds it (another + * launcher replacing the same daemon), or the swap lost a race. + */ +function takeWriterSlotFrom(root: string, pid: number): { giveBack(): void } | null { + const previous = readWriterLock(root); + if (previous && previous.pid !== pid && previous.pid !== process.pid && isProcessAlive(previous.pid)) return null; + const claim: WriterLockInfo = { pid: process.pid, mode: 'handover', startedAt: Date.now(), ready: false }; + const held = previous + ? swapWriterLock(root, previous.pid, claim) + : tryAcquireWriterLock(root, 'handover').kind === 'acquired'; + if (!held) return null; + return { + giveBack: () => { + if (previous?.pid === pid) swapWriterLock(root, process.pid, previous); + else releaseWriterLock(root); + }, + }; } /** Stop every registered, live daemon. */ diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index 6f23386114..6618887907 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -54,12 +54,14 @@ import { getDaemonSocketCandidates, getDaemonSocketPath, } from './daemon-paths'; -import { CodeGraphPackageVersion } from './version'; +import { CodeGraphPackageJsonPath, CodeGraphPackageReleaseVersion, CodeGraphPackageVersion } from './version'; import { releaseWriterLock, + swapWriterLock, tryAcquireWriterLock, assertNoRebuild, writerLockHeldMessage, + type WriterLockInfo, } from './writer-lock'; import { registerDaemon, deregisterDaemon } from './daemon-registry'; @@ -123,6 +125,38 @@ const DEFAULT_CLIENT_SWEEP_MS = 30_000; * without it. `CODEGRAPH_DAEMON_CLIENT_HELLO_TIMEOUT_MS` overrides it. */ const DEFAULT_CLIENT_HELLO_TIMEOUT_MS = 3_000; +/** How often the daemon checks that the install it runs from is still in place (#2335). */ +const DEFAULT_INSTALL_CHECK_MS = 30_000; + +/** + * Why the install this daemon was started from no longer matches it, or null + * while it still does. An upgrade replaces or deletes the package directory + * under a running daemon (npm reinstalling the package, the launcher pruning + * an old bundle), and everything the daemon loads lazily from there afterwards + * fails — grammars among them, so every file its watcher re-indexed came out + * empty (#2335). Such a daemon should make way for one started from the + * current install. Any other read failure, or a package.json caught mid-write, + * answers null: the next check looks again. Exported for testing. + */ +export function installChangedReason(packageJsonPath: string, runningVersion: string): string | null { + let raw: string; + try { + raw = fs.readFileSync(packageJsonPath, 'utf8'); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + return code === 'ENOENT' || code === 'ENOTDIR' ? `Install removed (${packageJsonPath} is gone)` : null; + } + let version: unknown; + try { + version = (JSON.parse(raw) as { version?: unknown } | null)?.version; + } catch { + return null; + } + return typeof version === 'string' && version !== runningVersion + ? `Install replaced by v${version} (this daemon is v${runningVersion})` + : null; +} + /** Bytes/parse-window for an oversized hello line — bounded against a malicious peer. */ const MAX_HELLO_LINE_BYTES = 4096; @@ -138,6 +172,7 @@ export interface DaemonHello { pid: number; // daemon pid (informational; for `ps` debugging) socketPath: string; // echoed back so the proxy can log it protocol: 1; // bump if the hello shape changes + writerProtocol?: 1; // ownership mutations use the OS coordination lock } /** @@ -186,19 +221,23 @@ export class Daemon { private lastActivityAt = Date.now(); private maxIdleTimer: NodeJS.Timeout | null = null; private clientSweepTimer: NodeJS.Timeout | null = null; + private installCheckTimer: NodeJS.Timeout | null = null; private engine: MCPEngine; private stopping = false; private socketPath: string; private pidPath: string; + /** The launcher holding the writer slot for this daemon, when it replaced an older one (#2335). */ + private handoverFrom: number | null; constructor( private projectRoot: string, - opts: { idleTimeoutMs?: number; maxIdleMs?: number } = {}, + opts: { idleTimeoutMs?: number; maxIdleMs?: number; handoverFrom?: number | null } = {}, ) { this.socketPath = getDaemonSocketPath(projectRoot); this.pidPath = getDaemonPidPath(projectRoot); this.idleTimeoutMs = opts.idleTimeoutMs ?? resolveIdleTimeoutMs(); this.maxIdleMs = opts.maxIdleMs ?? resolveMaxIdleMs(); + this.handoverFrom = opts.handoverFrom ?? null; // Daemon mode serves many concurrent clients on one event loop, so off-load // read-tool dispatch to a worker pool — otherwise concurrent explores // serialize and starve the MCP transport (clients time out). Direct mode @@ -217,7 +256,17 @@ export class Daemon { // #1740: claim the project writer lock before opening/watching so a // concurrent direct-mode serve --mcp cannot start a second watcher. assertNoRebuild(this.projectRoot); - const writer = tryAcquireWriterLock(this.projectRoot, 'daemon'); + let writer = tryAcquireWriterLock(this.projectRoot, 'daemon'); + // The launcher that stopped an older daemon has held the slot for us since + // (#2335); take it over without letting it fall free. + if (writer.kind === 'taken' && this.handoverFrom !== null && + writer.existing?.pid === this.handoverFrom && writer.existing.mode === 'handover') { + const info: WriterLockInfo = { pid: process.pid, mode: 'daemon', startedAt: Date.now(), ready: false }; + if (swapWriterLock(this.projectRoot, this.handoverFrom, info)) { + writer = { kind: 'acquired', pidPath: writer.pidPath, info }; + process.stderr.write(`[CodeGraph daemon] Took over the writer lock from launcher pid ${this.handoverFrom}, which stopped an older daemon.\n`); + } + } if (writer.kind === 'taken') { const msg = writerLockHeldMessage(writer.existing, writer.pidPath); process.stderr.write(`[CodeGraph daemon] ${msg}\n`); @@ -360,6 +409,10 @@ export class Daemon { clearInterval(this.clientSweepTimer); this.clientSweepTimer = null; } + if (this.installCheckTimer) { + clearInterval(this.installCheckTimer); + this.installCheckTimer = null; + } process.stderr.write(`[CodeGraph daemon] Shutting down (${reason}; clients=${this.clients.size}).\n`); for (const session of [...this.clients]) { try { session.stop(); } catch { /* best-effort */ } @@ -401,6 +454,7 @@ export class Daemon { pid: process.pid, socketPath: this.socketPath, protocol: 1, + writerProtocol: 1, }; socket.write(JSON.stringify(hello) + '\n'); @@ -491,6 +545,31 @@ export class Daemon { this.clientSweepTimer = setInterval(() => this.reapDeadClients(isProcessAlive), sweepMs); this.clientSweepTimer.unref?.(); } + // An install whose version could not be read at startup has nothing to + // compare against; leave that daemon alone. + const installMs = resolveInstallCheckMs(); + if (installMs > 0 && CodeGraphPackageVersion !== '0.0.0-unknown') { + this.installCheckTimer = setInterval(() => { this.checkInstall(); }, installMs); + this.installCheckTimer.unref?.(); + } + } + + /** + * Exit once the install this daemon runs from has been upgraded or removed + * (#2335; see {@link installChangedReason}). Its clients' proxies fall back + * and reconnect, and the next launch starts a daemon from the current + * install. Returns whether the daemon is exiting. The check is injected for + * tests; the timer passes the real one. + */ + checkInstall( + changed: () => string | null = () => installChangedReason(CodeGraphPackageJsonPath, CodeGraphPackageReleaseVersion), + ): boolean { + if (this.stopping) return false; + const reason = changed(); + if (!reason) return false; + process.stderr.write(`[CodeGraph daemon] ${reason}; exiting so the next session starts a daemon from the current install.\n`); + void this.stop('install changed'); + return true; } /** @@ -873,6 +952,14 @@ function resolveClientHelloTimeoutMs(): number { return Math.floor(parsed); } +function resolveInstallCheckMs(): number { + const raw = process.env.CODEGRAPH_DAEMON_INSTALL_CHECK_MS; + if (raw === undefined || raw === '') return DEFAULT_INSTALL_CHECK_MS; + const parsed = Number(raw); + if (!Number.isFinite(parsed) || parsed < 0) return DEFAULT_INSTALL_CHECK_MS; + return Math.floor(parsed); // 0 disables the check +} + /** * Parse one client-hello line. Returns the peer pids if `line` is a well-formed * client-hello (carries the `codegraph_client` marker), or null otherwise — in diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 054093197a..1f0653c9e7 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -44,11 +44,13 @@ import { MCPSession } from './session'; import { Daemon, clearStaleDaemonLock, + installChangedReason, isProcessAlive, tryAcquireDaemonLock, } from './daemon'; -import { clearStaleDaemonArtifacts } from './daemon-registry'; +import { clearStaleDaemonArtifacts, stopOlderDaemon } from './daemon-registry'; import { connectWithHello, runLocalHandshakeProxy } from './proxy'; +import { CodeGraphPackageJsonPath, CodeGraphPackageReleaseVersion, CodeGraphPackageVersion } from './version'; import { readWriterLock, getWriterPidPath, @@ -89,6 +91,13 @@ const DIRECT_QUERY_POOL_MAX = 2; */ const DAEMON_INTERNAL_ENV = 'CODEGRAPH_DAEMON_INTERNAL'; +/** + * Env var naming the launcher that holds the project's writer slot for the + * daemon it spawns, after stopping an older one (#2335). That daemon takes the + * slot over instead of finding it held. + */ +const DAEMON_HANDOVER_ENV = 'CODEGRAPH_DAEMON_HANDOVER'; + /** * Retries for the detached daemon arbitrating the O_EXCL lock against a racing * sibling. Tiny — the lock resolves on the first round in practice; the retries @@ -97,6 +106,13 @@ const DAEMON_INTERNAL_ENV = 'CODEGRAPH_DAEMON_INTERNAL'; const TAKEOVER_MAX_RETRIES = 5; const TAKEOVER_RETRY_DELAY_MS = 100; +/** + * How long a daemon the writer slot is held for (#2335) waits out a racing + * candidate that took the daemon lock first. That candidate cannot get the + * slot, so it leaves; this bounds a wait that never ends. + */ +const HANDOVER_CANDIDATE_WAIT_MS = 5_000; + /** * A fallback that serves reads without a watcher or a writer lock (#1963). * Say so on stderr: otherwise a session that quietly stopped syncing looks @@ -144,6 +160,14 @@ function makeFallbackEngine(root: string): MCPEngine { : `live daemon pid ${existing!.pid} holds the project lock.`; return readOnlyFallback(holder); } + // This session's own install was upgraded or removed underneath it (#2335): + // its code no longer loads whole, so as the project's writer it would only + // keep a daemon from the current install from starting. Its daemon exits + // for the same reason, which is how it got here. + if (CodeGraphPackageVersion !== '0.0.0-unknown' && + installChangedReason(CodeGraphPackageJsonPath, CodeGraphPackageReleaseVersion) !== null) { + return readOnlyFallback('this session\'s CodeGraph install was upgraded or removed; restart the session to use the current one'); + } return new MCPEngine({ writerLockRoot: root, queryPool: true, queryPoolDefaultMax: DIRECT_QUERY_POOL_MAX }); } @@ -252,9 +276,10 @@ function resolveDaemonRoot(explicitPath: string | null): string | null { * `process.execArgv` (carries the runtime flags, so the daemon never re-execs) * and `process.argv[1]` (this script). The spawned process self-arbitrates the * O_EXCL lock, so racing launchers may each spawn one — losers exit and every - * launcher proxies through the single winner. + * launcher proxies through the single winner. `handover` tells the daemon this + * launcher holds the writer slot for it (see {@link DAEMON_HANDOVER_ENV}). */ -function spawnDetachedDaemon(root: string): void { +function spawnDetachedDaemon(root: string, handover = false): void { const scriptPath = process.argv[1]; if (!scriptPath) { // No resolvable CLI entry point to re-invoke — let the caller fall back to @@ -276,6 +301,8 @@ function spawnDetachedDaemon(root: string): void { // where a long-dead session's host pid would trigger spurious shutdowns. const env: NodeJS.ProcessEnv = { ...process.env, [DAEMON_INTERNAL_ENV]: '1' }; delete env[HOST_PPID_ENV]; + if (handover) env[DAEMON_HANDOVER_ENV] = String(process.pid); + else delete env[DAEMON_HANDOVER_ENV]; const child = spawn( process.execPath, [...process.execArgv, scriptPath, 'serve', '--mcp', '--path', root], @@ -286,6 +313,12 @@ function spawnDetachedDaemon(root: string): void { env, }, ); + // An upgrade can delete the install this launcher runs from — its daemon + // then exits (#2335) and a respawn finds no executable. spawn reports that + // as an asynchronous 'error' event, which would crash this process if + // nobody listened. No daemon binds either way; the caller's poll gives up + // and the session is served in-process. + child.on('error', () => { /* no daemon — see above */ }); child.unref(); } finally { // The child holds its own dup of the log fd now; the launcher doesn't need it. @@ -295,6 +328,78 @@ function spawnDetachedDaemon(root: string): void { } } +/** + * How long a launcher replacing an older daemon waits for one that is partway + * through its shutdown, past the stop's own 3s wait. Tool calls are held + * meanwhile, so it is short: a daemon that takes longer leaves the session + * served in-process until the proxy's next retry (#2277) finds it gone. + */ +const OLDER_DAEMON_SHUTDOWN_GRACE_MS = 5_000; + +/** + * Stop the daemon serving `root` from an older install so the caller can start + * one from this install (#2335). A daemon keeps the code it started with: one + * that outlived an upgrade went on loading grammars from the removed install + * and wrote every file it re-indexed as empty, and as the project's writer it + * kept every newer session from syncing. {@link stopOlderDaemon} finds it by + * the project lock, verifies it, and never stops one of the same, a newer or an + * unknown version. `answered` says an older daemon answered this launcher's + * own probe. Resolves `stopped` (this launcher now holds the project's writer + * slot for the daemon it starts next), `none` (no older daemon holds the + * project), or `kept` (one does, unverified or still shutting down). + */ +async function replaceOlderDaemon(root: string, answered: boolean): Promise<'stopped' | 'none' | 'kept'> { + const result = await stopOlderDaemon(root, CodeGraphPackageVersion, { + shutdownGraceMs: OLDER_DAEMON_SHUTDOWN_GRACE_MS, + }); + // No older daemon in the lock (none, or already replaced), or it had exited. + if (!result || result.outcome === 'not-running') return 'none'; + const daemon = result.version + ? `the CodeGraph ${result.version} daemon (pid ${result.pid})` + : 'an older CodeGraph daemon'; + if (result.outcome === 'term' || result.outcome === 'kill') { + process.stderr.write( + `[CodeGraph MCP] Stopped ${daemon} serving this project; starting one from this install (${CodeGraphPackageVersion}).\n` + ); + return 'stopped'; + } + if (result.outcome === 'legacy-writer') { + process.stderr.write(`[CodeGraph MCP] Left ${daemon} running because it lacks coordinated writer handover; serving reads without auto-sync. Stop its old MCP sessions and daemon, then reconnect with this install.\n`); + } else if (result.outcome === 'still-running') { + process.stderr.write(`[CodeGraph MCP] Asked ${daemon} serving this project to stop, but it has not exited yet.\n`); + } else if (answered) { + process.stderr.write( + `[CodeGraph MCP] Found ${daemon} serving this project but could not verify it, so left it running; serving this session in-process.\n` + ); + } + return 'kept'; +} + +/** + * Wait out the daemon candidate `pid` holding the daemon lock while launcher + * `handoverFrom` holds the writer slot for this daemon (#2335): the candidate + * cannot get the slot, so it releases the lock and exits. Resolves true once + * `pid` no longer holds the lock; false at once when no slot is held for this + * daemon, and false when that stops being so or the wait runs out. + */ +async function outwaitCandidate(root: string, handoverFrom: number | null, pid: number): Promise { + const heldForUs = (): boolean => { + const writer = readWriterLock(root); + return handoverFrom !== null && writer?.pid === handoverFrom && writer.mode === 'handover'; + }; + if (!heldForUs()) return false; + process.stderr.write(`[CodeGraph daemon] Waiting for daemon candidate pid ${pid} to give up the lock; the writer lock is held for this daemon.\n`); + const deadline = Date.now() + HANDOVER_CANDIDATE_WAIT_MS; + while (Date.now() < deadline) { + if (!heldForUs()) return false; + let holder: number | undefined; + try { holder = decodeLockInfo(fs.readFileSync(getDaemonPidPath(root), 'utf8'))?.pid; } catch { holder = undefined; } + if (holder !== pid || !isProcessAlive(pid)) return true; + await sleep(25); + } + return false; +} + /** * MCP Server for CodeGraph * @@ -540,11 +645,15 @@ export class MCPServer { // kills/restarts can be placed in time (#1431 — the log was undatable). timestampStderrLines(); const root = resolveDaemonRoot(this.projectPath) ?? this.projectPath ?? process.cwd(); + // Read once and dropped, so nothing this daemon spawns inherits it. + const handoverPid = Number(process.env[DAEMON_HANDOVER_ENV]); + const handoverFrom = Number.isInteger(handoverPid) && handoverPid > 0 ? handoverPid : null; + delete process.env[DAEMON_HANDOVER_ENV]; for (let attempt = 0; attempt < TAKEOVER_MAX_RETRIES; attempt++) { const lock = tryAcquireDaemonLock(root); if (lock.kind === 'acquired') { - const daemon = new Daemon(root); + const daemon = new Daemon(root, { handoverFrom }); await daemon.start(); this.daemon = daemon; this.mode = 'daemon'; @@ -574,6 +683,9 @@ export class MCPServer { stillStarting || await probeDaemonIdentity(existing) ) { + // A daemon the writer slot is held for outwaits a candidate still + // starting in its way, which cannot get the slot and leaves (#2335). + if (stillStarting && await outwaitCandidate(root, handoverFrom, existing.pid)) continue; process.stderr.write( `[CodeGraph daemon] Another daemon (pid ${existing.pid}) already holds the lock; exiting.\n` ); @@ -605,7 +717,8 @@ export class MCPServer { /** * Proxy mode (the common case). Serve the MCP handshake LOCALLY for instant * tool registration, forwarding tool calls to the shared daemon — which is - * connected in the background (probed, then spawned + polled if absent) so the + * connected in the background (probed, then spawned + polled if absent; one of + * an older release is stopped first, #2335) so the * handshake never waits ~600ms on it. Runs until the host disconnects; the * proxy falls back to an in-process engine if the daemon never binds, so this * never wedges a session. @@ -622,9 +735,8 @@ export class MCPServer { for (const candidate of candidates) { const s = await connectWithHello(candidate); // A wrong-version daemon IS up — definitive; propagate so the caller - // serves in-process instead of spawning + polling for 6s. Don't keep - // probing fallbacks past it. - if (s === 'version-mismatch') return s; + // replaces it (an older release) or serves in-process instead of + // spawning + polling for 6s. Don't keep probing fallbacks past it. if (s) return s; } return null; @@ -633,16 +745,31 @@ export class MCPServer { // Fast path: a daemon may already be listening (on either candidate). const probe = await connectAnyCandidate(); if (probe === 'version-mismatch') return null; // definitive — serve in-process, don't poll for 6s - if (probe) return probe; - // None reachable — spawn one (detached) and poll for its bind. - spawnDetachedDaemon(root); - for (let attempt = 0; attempt < DAEMON_CONNECT_MAX_RETRIES; attempt++) { - await sleep(DAEMON_CONNECT_RETRY_DELAY_MS); - const s = await connectAnyCandidate(); - if (s === 'version-mismatch') return null; - if (s) return s; + if (probe && probe !== 'older-version') return probe; + // A daemon of an older release answered — or none did, while the project + // lock may still name one on a socket we don't probe (on Windows, daemons + // before #2278 named their pipe after the root as typed). Either way it + // makes way for one from this install (#2335). + const answered = probe === 'older-version'; + const older = await replaceOlderDaemon(root, answered); + if (older === 'kept' && answered) return null; + // None reachable — spawn one (detached) and poll for its bind. After a + // replacement this launcher holds the writer slot for it meanwhile. + try { + spawnDetachedDaemon(root, older === 'stopped'); + for (let attempt = 0; attempt < DAEMON_CONNECT_MAX_RETRIES; attempt++) { + await sleep(DAEMON_CONNECT_RETRY_DELAY_MS); + const s = await connectAnyCandidate(); + // Another version answering now, older or not, waits for the next + // retry: one replacement per attempt, so this can never loop. + if (s === 'older-version' || s === 'version-mismatch') return null; + if (s) return s; + } + return null; // never bound — the proxy serves this session in-process + } finally { + // The new daemon has taken the slot over, or never came; a no-op unless still ours. + if (older === 'stopped') releaseWriterLock(root); } - return null; // never bound — the proxy serves this session in-process }; await runLocalHandshakeProxy({ getDaemonSocket, makeEngine: () => makeFallbackEngine(root), root }); } diff --git a/src/mcp/project-lifecycle.ts b/src/mcp/project-lifecycle.ts index e7cd74ae11..e9600eb406 100644 --- a/src/mcp/project-lifecycle.ts +++ b/src/mcp/project-lifecycle.ts @@ -118,7 +118,8 @@ async function activate(root: string, project: Project): Promise { if (writer.existing?.mode === 'daemon') { for (const candidate of getDaemonSocketCandidates(root)) { const socket = await connectWithHello(candidate); - if (!socket || socket === 'version-mismatch') continue; + // A daemon of another version, older or not, is left to the project's own launchers. + if (!socket || typeof socket === 'string') continue; if (project.refs === 0) { socket.destroy(); return; } project.socket = socket; socket.once('close', () => { diff --git a/src/mcp/proxy.ts b/src/mcp/proxy.ts index 0a52c512e6..14d384263e 100644 --- a/src/mcp/proxy.ts +++ b/src/mcp/proxy.ts @@ -26,7 +26,7 @@ import { EARLY_PPID } from './early-ppid'; import { supervisionLostReason } from './ppid-watchdog'; import { armStartupHandshakeTimeout } from './startup-handshake'; import { treatStdinFailureAsShutdown } from './stdin-teardown'; -import { CodeGraphPackageVersion } from './version'; +import { CodeGraphPackageVersion, isOlderRelease } from './version'; import { SERVER_INFO, PROTOCOL_VERSION, initializeInstructions } from './session'; import { SERVER_INSTRUCTIONS } from './server-instructions'; import { getStaticTools } from './tools'; @@ -148,14 +148,17 @@ export async function runProxy( /** * Connect to a daemon at `socketPath` and verify its hello (exact version match). - * Returns the live socket (hello already consumed) or null if unreachable / stale - * / version-mismatched. Unlike {@link runProxy} it does NOT pipe — the caller - * owns the socket. Used by the local-handshake proxy's background connect. + * Returns the live socket (hello already consumed), null if unreachable / stale, + * or — for a daemon of another version — `'older-version'` when it runs an older + * release ({@link isOlderRelease}; the launcher replaces it, #2335) and + * `'version-mismatch'` for any other version, which is left alone. Unlike + * {@link runProxy} it does NOT pipe — the caller owns the socket. Used by the + * local-handshake proxy's background connect. */ export async function connectWithHello( socketPath: string, expectedVersion: string = CodeGraphPackageVersion, -): Promise { +): Promise { if (process.platform !== 'win32' && !fs.existsSync(socketPath)) return null; const socket = net.createConnection(socketPath); socket.setEncoding('utf8'); @@ -176,12 +179,14 @@ export async function connectWithHello( } if (hello.codegraph !== expectedVersion) { // A daemon IS up but it's the wrong version — definitive, not a "not yet". - // Don't poll; the caller serves in-process so we never run stale-vs-new. + socket.destroy(); + // An older release is the launcher's to replace (#2335); it says what it did. + if (isOlderRelease(hello.codegraph, expectedVersion)) return 'older-version'; + // Any other: don't poll; the caller serves in-process so we never run stale-vs-new. process.stderr.write( `[CodeGraph MCP] Found a daemon on ${socketPath} but version (${hello.codegraph}) ` + `differs from ours (${expectedVersion}); serving this session in-process.\n` ); - socket.destroy(); return 'version-mismatch'; } logAttachedDaemon(socketPath, hello); @@ -212,8 +217,9 @@ type JsonRpc = Record; /** Dependencies the local-handshake proxy needs, injected by MCPServer (which * owns the daemon-spawn machinery and the engine factory). */ export interface LocalHandshakeDeps { - /** Probe → spawn → retry → hello-verify; resolves a connected daemon socket, - * or null when the daemon path is genuinely unavailable (→ in-process fallback). */ + /** Probe (replacing a daemon of an older release, #2335) → spawn → retry → + * hello-verify; resolves a connected daemon socket, or null when the daemon + * path is genuinely unavailable (→ in-process fallback). */ getDaemonSocket(): Promise; /** Lazily create an in-process engine — used only while the daemon is * unreachable, preserving the "a broken daemon never wedges a session" diff --git a/src/mcp/version.ts b/src/mcp/version.ts index 5b5e539318..c906a84cd1 100644 --- a/src/mcp/version.ts +++ b/src/mcp/version.ts @@ -12,8 +12,8 @@ * revision so two builds of the same package release cannot share a stale * daemon. * - * The one deliberate exception is `codegraph upgrade`, which compares against - * npm registry versions and so must use the bare published version. + * `codegraph upgrade` and installation-change checks compare package releases + * and use the bare published version. * * Resolution strategy: read the bundled `package.json` two levels up from * this file — same relative position whether we're loaded from `src/mcp/` or @@ -26,24 +26,58 @@ import * as fs from "fs"; import * as path from "path"; +/** The `package.json` of the install this process runs from. */ +export const CodeGraphPackageJsonPath = path.join(__dirname, '..', '..', 'package.json'); + +function readReleaseVersion(): string { + try { + const parsed = JSON.parse(fs.readFileSync(CodeGraphPackageJsonPath, 'utf8')); + if (typeof parsed?.version === 'string' && parsed.version.length > 0) return parsed.version; + } catch { + // Fall through to the identity sentinel. + } + return '0.0.0-unknown'; +} + +/** Cached package release for detecting an installation replaced in place. */ +export const CodeGraphPackageReleaseVersion = readReleaseVersion(); + function readPackageVersion(): string { + const release = CodeGraphPackageReleaseVersion; + if (release === '0.0.0-unknown') return release; try { - const pkgPath = path.join(__dirname, "..", "..", "package.json"); - const raw = fs.readFileSync(pkgPath, "utf8"); - const parsed = JSON.parse(raw); - if (typeof parsed?.version === "string" && parsed.version.length > 0) { - const buildPath = path.join(__dirname, "..", "..", "dist", "build-revision.json"); - if (fs.existsSync(buildPath)) { - const build = JSON.parse(fs.readFileSync(buildPath, "utf8")); - if (!/^[a-f0-9]{40}$/.test(build?.revision ?? "")) return "0.0.0-unknown"; - return `${parsed.version.split("+")[0]}+${build.revision}`; - } - return parsed.version; + const buildPath = path.join(__dirname, '..', '..', 'dist', 'build-revision.json'); + if (fs.existsSync(buildPath)) { + const build = JSON.parse(fs.readFileSync(buildPath, 'utf8')); + if (!/^[a-f0-9]{40}$/.test(build?.revision ?? '')) return '0.0.0-unknown'; + return `${release.split('+')[0]}+${build.revision}`; } + return release; } catch { - // Fall through to sentinel. + return '0.0.0-unknown'; } - return "0.0.0-unknown"; } export const CodeGraphPackageVersion = readPackageVersion(); + +/** + * Whether `version` is a CodeGraph release older than `than` — the test a + * launcher applies before it replaces a running daemon (#2335). Plain + * `MAJOR.MINOR.PATCH` and managed revisions compare: a prerelease, an arbitrary suffix or the + * "0.0.0-unknown" sentinel is never older. A daemon is replaced only by a + * strictly newer release, so two installed versions can never take turns + * stopping each other's daemon. + */ +export function isOlderRelease(version: string, than: string): boolean { + const a = parseRelease(version); + const b = parseRelease(than); + if (!a || !b) return false; + if (a[0] !== b[0]) return a[0] < b[0]; + if (a[1] !== b[1]) return a[1] < b[1]; + return a[2] < b[2]; +} + +function parseRelease(version: string): [number, number, number] | null { + const m = /^(\d+)\.(\d+)\.(\d+)(?:\+[a-f0-9]{40})?$/.exec(version); + return m ? [Number(m[1]), Number(m[2]), Number(m[3])] : null; +} diff --git a/src/mcp/writer-lock.ts b/src/mcp/writer-lock.ts index 8fea5b248c..0e15880301 100644 --- a/src/mcp/writer-lock.ts +++ b/src/mcp/writer-lock.ts @@ -16,6 +16,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { getCodeGraphDir } from '../directory'; +import { requireKernel } from '../extraction/kernel/loader'; /** Signal-0 liveness (EPERM ⇒ alive). Local copy to avoid a daemon↔writer cycle. */ function isProcessAlive(pid: number): boolean { try { @@ -58,6 +59,35 @@ export type WriterAcquireResult = | { kind: 'acquired'; pidPath: string; info: WriterLockInfo } | { kind: 'taken'; existing: WriterLockInfo | null; pidPath: string }; +const pendingReleases = new Map(); +const pendingReadiness = new Map(); + +function cancelPendingReadiness(pidPath: string): void { + const timer = pendingReadiness.get(pidPath); + if (timer) clearTimeout(timer); + pendingReadiness.delete(pidPath); +} + +function cancelPendingRelease(pidPath: string): void { + const timer = pendingReleases.get(pidPath); + if (timer) clearTimeout(timer); + pendingReleases.delete(pidPath); +} + +/** Serialize every ownership comparison and mutation, including stale cleanup. + * Keep the guard file permanently; the OS releases its lock on process exit. */ +function lockWriterMutation(pidPath: string, waitMs = 1000): { release(): void } | null { + const acquire = requireKernel().tryWriterMutationLock; + if (!acquire) throw new Error('Native kernel lacks writer coordination; rebuild or reinstall CodeGraph.'); + const deadline = Date.now() + waitMs; + for (;;) { + const lock = acquire(`${pidPath}.mutation.lock`); + if (lock) return lock; + if (Date.now() >= deadline) return null; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5); + } +} + function encode(info: WriterLockInfo): string { return JSON.stringify(info) + '\n'; } @@ -89,100 +119,216 @@ export function tryAcquireWriterLock( const pidPath = getWriterPidPath(projectRoot, lockName); fs.mkdirSync(path.dirname(pidPath), { recursive: true }); - const info: WriterLockInfo = { - pid: process.pid, - mode, - startedAt: Date.now(), - ready: false, - }; + const mutation = lockWriterMutation(pidPath); + if (!mutation) return { kind: 'taken', existing: readWriterLock(projectRoot, lockName), pidPath }; + try { + const info: WriterLockInfo = { + pid: process.pid, + mode, + startedAt: Date.now(), + ready: false, + }; - const attempt = (): WriterAcquireResult => { - const tmp = `${pidPath}.${process.pid}.tmp`; - let acquired = false; - try { - fs.writeFileSync(tmp, encode(info), { mode: 0o600 }); + const attempt = (): WriterAcquireResult => { + const tmp = `${pidPath}.${process.pid}.tmp`; + let acquired = false; try { - fs.linkSync(tmp, pidPath); - acquired = true; - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'EEXIST') { - // taken - } else { - // No hard links — O_EXCL create. - try { - const fd = fs.openSync(pidPath, 'wx', 0o600); + fs.writeFileSync(tmp, encode(info), { mode: 0o600 }); + try { + fs.linkSync(tmp, pidPath); + acquired = true; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') { + // taken + } else { + // No hard links — O_EXCL create. try { - fs.writeSync(fd, encode(info)); - acquired = true; - } finally { - fs.closeSync(fd); + const fd = fs.openSync(pidPath, 'wx', 0o600); + try { + fs.writeSync(fd, encode(info)); + acquired = true; + } finally { + fs.closeSync(fd); + } + } catch (e2: unknown) { + if ((e2 as NodeJS.ErrnoException).code !== 'EEXIST') throw e2; } - } catch (e2: unknown) { - if ((e2 as NodeJS.ErrnoException).code !== 'EEXIST') throw e2; } } + } finally { + try { fs.unlinkSync(tmp); } catch { /* ignore */ } } - } finally { - try { fs.unlinkSync(tmp); } catch { /* ignore */ } - } - if (acquired) return { kind: 'acquired', pidPath, info }; - - let existing: WriterLockInfo | null = null; - try { - existing = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); - } catch { /* unreadable */ } - return { kind: 'taken', existing, pidPath }; - }; + if (acquired) { + cancelPendingReadiness(pidPath); + cancelPendingRelease(pidPath); + return { kind: 'acquired', pidPath, info }; + } - let result = attempt(); - if (result.kind === 'taken' && result.existing && result.existing.pid === process.pid) { - // Same process already holds it (daemon acquired before engine watch). - return { kind: 'acquired', pidPath: result.pidPath, info: result.existing }; - } - if (result.kind === 'taken') { - const existing = result.existing; - if (!existing || existing.pid <= 0 || !isProcessAlive(existing.pid)) { - // Stale — clear (pid-verified) and retry once. + let existing: WriterLockInfo | null = null; try { - const raw = fs.readFileSync(pidPath, 'utf8'); - const cur = decodeWriterLockInfo(raw); - if (!cur || cur.pid === existing?.pid) { - if (!cur || cur.pid <= 0 || !isProcessAlive(cur.pid)) { - fs.unlinkSync(pidPath); + existing = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + } catch { /* unreadable */ } + return { kind: 'taken', existing, pidPath }; + }; + + let result = attempt(); + if (result.kind === 'taken' && result.existing && result.existing.pid === process.pid) { + // Same process already holds it (daemon acquired before engine watch). + cancelPendingRelease(pidPath); + return { kind: 'acquired', pidPath: result.pidPath, info: result.existing }; + } + if (result.kind === 'taken') { + const existing = result.existing; + if (!existing || existing.pid <= 0 || !isProcessAlive(existing.pid)) { + // Stale — clear (pid-verified) and retry once. + try { + const raw = fs.readFileSync(pidPath, 'utf8'); + const cur = decodeWriterLockInfo(raw); + if (!cur || cur.pid === existing?.pid) { + if (!cur || cur.pid <= 0 || !isProcessAlive(cur.pid)) { + fs.unlinkSync(pidPath); + } } - } - } catch { /* ENOENT ok */ } - result = attempt(); + } catch { /* ENOENT ok */ } + result = attempt(); + } } + return result; + } finally { + mutation.release(); } - return result; } -/** Publish catch-up readiness without exposing a partially-written pidfile. */ +/** Publish catch-up readiness without exposing a partially-written pidfile. + * Contention retains a retry until publication or an ownership change. */ export function markWriterReady(projectRoot: string): void { const pidPath = getWriterPidPath(projectRoot); - const info = readWriterLock(projectRoot); - if (!info || info.pid !== process.pid) return; - const tmp = `${pidPath}.${process.pid}.ready.tmp`; + const retry = (): void => { + if (pendingReadiness.has(pidPath)) return; + const timer = setTimeout(() => { + pendingReadiness.delete(pidPath); + try { + publish(0); + } catch (error) { + // Permanent failures stay local to publication, like the lifecycle's + // synchronous catch boundary; an unguarded timer would exit the server. + process.stderr.write(`[CodeGraph MCP] Writer readiness publication failed: ${error instanceof Error ? error.message : String(error)}\n`); + } + }, 100); + timer.unref(); + pendingReadiness.set(pidPath, timer); + }; + const publish = (waitMs: number): void => { + let mutation: { release(): void } | null = null; + try { + if (!fs.existsSync(pidPath)) { + cancelPendingReadiness(pidPath); + return; + } + mutation = lockWriterMutation(pidPath, waitMs); + if (!mutation) { + retry(); + return; + } + cancelPendingReadiness(pidPath); + const info = readWriterLock(projectRoot); + if (!info || info.pid !== process.pid) return; + const tmp = `${pidPath}.${process.pid}.ready.tmp`; + try { + fs.writeFileSync(tmp, encode({ ...info, ready: true }), { mode: 0o600 }); + fs.renameSync(tmp, pidPath); + } finally { + try { fs.unlinkSync(tmp); } catch { /* best-effort */ } + } + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (['EPERM', 'EACCES', 'EBUSY', 'EINTR', 'EAGAIN'].includes(code ?? '')) retry(); + else throw error; + } finally { + mutation?.release(); + } + }; + publish(1000); +} + +/** Waits between retries of a writer-record swap refused by Windows (another process has the file open). */ +const SWAP_RETRY_DELAYS_MS = [25, 50, 100, 200]; + +/** + * Replace the writer record of `fromPid` with `next`, and only that record, so + * the slot passes between two processes without ever falling free (#2335). A + * stopped daemon's sessions serve themselves in-process the moment it goes, and + * one that found the slot free or stale would claim it as their own writer. + * The OS coordination lock keeps comparison and rename in one critical + * section. Callers check whether the record was replaced. Never throws. + */ +export function swapWriterLock(projectRoot: string, fromPid: number, next: WriterLockInfo): boolean { + const pidPath = getWriterPidPath(projectRoot); + const tmp = `${pidPath}.${process.pid}.swap.tmp`; + let mutation: { release(): void } | null = null; try { - fs.writeFileSync(tmp, encode({ ...info, ready: true }), { mode: 0o600 }); - fs.renameSync(tmp, pidPath); + mutation = lockWriterMutation(pidPath); + if (!mutation) return false; + if (readWriterLock(projectRoot)?.pid !== fromPid) return false; + fs.writeFileSync(tmp, encode(next), { mode: 0o600 }); + for (let attempt = 0; ; attempt++) { + try { + fs.renameSync(tmp, pidPath); + cancelPendingReadiness(pidPath); + if (next.pid === process.pid) cancelPendingRelease(pidPath); + return true; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + const delay = SWAP_RETRY_DELAYS_MS[attempt]; + if (process.platform !== 'win32' || !['EPERM', 'EACCES', 'EBUSY'].includes(code ?? '') || delay === undefined) { + return false; + } + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, delay); + if (readWriterLock(projectRoot)?.pid !== fromPid) return false; + } + } + } catch { + return false; } finally { - try { fs.unlinkSync(tmp); } catch { /* best-effort */ } + if (mutation) { + try { fs.unlinkSync(tmp); } catch { /* renamed, or never written */ } + } + mutation?.release(); } } -/** Release if we still own the lock (pid match). */ +/** Release if we still own the lock. Contention keeps a retry obligation; + * callers may retire the engine while this process remains alive. */ export function releaseWriterLock(projectRoot: string, lockName: 'writer.pid' | 'rebuild.pid' = 'writer.pid'): void { const pidPath = getWriterPidPath(projectRoot, lockName); - try { - if (!fs.existsSync(pidPath)) return; - const info = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); - if (info && info.pid === process.pid) { - fs.unlinkSync(pidPath); + cancelPendingReadiness(pidPath); + const release = (waitMs: number): void => { + let mutation: { release(): void } | null = null; + try { + if (!fs.existsSync(path.dirname(pidPath))) return; + mutation = lockWriterMutation(pidPath, waitMs); + if (!mutation) { + // One unref'd retry per record. Reacquisition by this process cancels it, + // so an old retirement cannot release a newly started watcher. + if (!pendingReleases.has(pidPath)) { + const timer = setTimeout(() => { + pendingReleases.delete(pidPath); + release(0); + }, 100); + timer.unref(); + pendingReleases.set(pidPath, timer); + } + return; + } + cancelPendingRelease(pidPath); + const info = readWriterLock(projectRoot, lockName); + if (info?.pid === process.pid) fs.unlinkSync(pidPath); + } catch { /* best-effort for unavailable files or kernel */ } finally { + mutation?.release(); } - } catch { /* best-effort */ } + }; + release(1000); } /** Read current lock without acquiring. */ diff --git a/src/types.ts b/src/types.ts index 23ea775d3d..ce2e345e18 100644 --- a/src/types.ts +++ b/src/types.ts @@ -274,6 +274,24 @@ export interface FileRecord { generated?: boolean; } +/** + * Indexed files whose symbols are missing even though their content hash is + * current — what a hash comparison alone calls "up to date" (#2336). Paths are + * project-relative and sorted. + */ +export interface IndexHealth { + /** + * Files `sync` re-indexes: stored while their grammar could not load (#2335) + * or stored with no nodes and no recorded reason (#1541). + */ + needsReindex: string[]; + /** + * Files with a recorded parse error and no symbols from it. Deterministic: + * they stay this way until the file (or the parser) changes. + */ + parseErrors: string[]; +} + // ============================================================================= // Extraction Types // =============================================================================