From 45a680df07df2297dd7997a9376501351088eee9 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 06:16:25 +0000 Subject: [PATCH 1/8] fix(mcp): one daemon per project however its root is cased on Windows (#2278) (#2285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): canonicalize the daemon rendezvous root on Windows A project spelled d:\work\codegraph or D:\work\codegraph is one directory, but the daemon socket/pipe name and the registry record name hashed a raw path.resolve, which preserves whichever casing the caller passed. A proxy that computed the other casing probed a socket nobody bound, spawned a redundant daemon, and that daemon exited on the lock the running one held, so the session silently served the graph in-process: no shared watcher, no auto-sync, one extra copy of the index per open window. Both names now come from one shared canonicalProjectRoot (native realpath, lowercased on win32), so every spelling meets the same daemon and codegraph list / stop --all see one record per project. (cherry picked from commit 181178801827351d713e1d552886c0482e6db67b) * fix(mcp): share one in-process project however its root is cased (#2278) The engine's project lifecycle and `codegraph stop`'s registry fallback keyed a project by its realpath, which keeps the caller's casing, so two spellings of one root opened two graphs and two watchers in one process. Both now use the same identity key as the daemon socket. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 0a3239e603d5796b9f2cf636b4c09635239e32dd) * docs(changelog): file the Windows rendezvous fix under Unreleased Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: 养心堂主 Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/daemon-rendezvous-case.test.ts | 123 +++++++++++++++++++++++ src/directory.ts | 34 +++++++ src/mcp/daemon-paths.ts | 12 ++- src/mcp/daemon-registry.ts | 11 +- src/mcp/project-lifecycle.ts | 13 ++- 6 files changed, 184 insertions(+), 10 deletions(-) create mode 100644 __tests__/daemon-rendezvous-case.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 01a871b167..62d005a12f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Python, an import split over several lines in parentheses, `from x import (A, B)` written one name per line, is now read in full. Before, its names were invisible to resolution, so Django REST Framework's `serializers.CharField()` and netbox's model mixins linked to the wrong class or to nothing. Thanks @JosefAschauer. - Indexing a large C or C++ project no longer runs out of memory partway through "Resolving refs". Headers whose include guard is written as `#define X_H 1`, as in OpenSceneGraph and osgEarth, or that are included under a build flag codegraph cannot know, used to be re-read on every include path, which grows exponentially with the depth of the include tree. Each is now read once per include state, and a hard limit stops the check on any include tree that is still too large. Thanks @danusha2345, and @coolhitmanleon for the report. (#2127) - In tag-based CFML, calls written in tags are now linked: in ``, ``/``, ``, `` and `#…#` expressions. Before, only `` and `` code was read, so callers and impact found almost nothing in tag-based components. Functions wrapped in tags like `` or `` are now indexed too, and a `` is indexed as an interface that `implements` links to. Re-index CFML projects after upgrading. Thanks @HarryMuc for the report. (#2091) +- On Windows, a project opened with a different drive-letter or folder-name case, like `d:\work\app` and `D:\Work\App`, now joins the CodeGraph background server that is already running for it. Before, each spelling started a server of its own that exited at once, so the session quietly served the graph by itself with no shared file watching or auto-sync, and `codegraph list` showed the project twice. Thanks @greatjackzhou. (#2278) ## [1.6.1] - 2026-09-29 diff --git a/__tests__/daemon-rendezvous-case.test.ts b/__tests__/daemon-rendezvous-case.test.ts new file mode 100644 index 0000000000..6c8a1a1a3e --- /dev/null +++ b/__tests__/daemon-rendezvous-case.test.ts @@ -0,0 +1,123 @@ +/** + * One project must map to ONE daemon rendezvous key, however its root is + * spelled — the regression behind "the MCP server starts but never shares the + * daemon". + * + * The daemon's named pipe (Windows) / tmpdir socket (POSIX) is + * `…codegraph-.slice(0,16)`, and the lockfile is shared + * while the SOCKET NAME is derived independently by each process. So the moment + * two processes hash the same directory differently they stop meeting: the + * proxy's probe finds nothing, it spawns a redundant daemon, and that daemon + * exits on the lock the first one holds + * (`Another daemon (pid N) already holds the lock; exiting.`) — the session then + * serves in-process, without the shared watcher or auto-sync. + * + * That is reachable on Windows because the root arrives two ways: a cwd-derived + * root is the on-disk casing (`D:\work\codegraph`), while a client-supplied + * `rootUri`/`workspaceFolders` path arrives as `file:///d%3A/…` → `d:\…`, and + * `path.resolve` preserves whichever it got (NTFS is case-insensitive, so both + * name one directory). Pinned here at the level that actually broke: the key, + * not the filename. + * + * The real-filesystem assertions are Windows-gated — on POSIX the two spellings + * are genuinely different directories, so only the Windows case has a + * case-insensitive filesystem to converge on. + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { canonicalProjectRoot } from '../src/directory'; +import { getDaemonSocketPath } from '../src/mcp/daemon-paths'; +import { acquireProject } from '../src/mcp/project-lifecycle'; +import type CodeGraph from '../src/index'; + +const tmpDirs: string[] = []; +function makeDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rendezvous-')); + tmpDirs.push(dir); + return dir; +} +afterEach(() => { + while (tmpDirs.length) { + try { + fs.rmSync(tmpDirs.pop()!, { recursive: true, force: true }); + } catch { + /* best-effort */ + } + } +}); + +/** The same drive letter, forced to one case. No-op on a path without a drive. */ +function withDriveCase(p: string, upper: boolean): string { + return p.replace(/^[a-z]:/i, (drive) => (upper ? drive.toUpperCase() : drive.toLowerCase())); +} + +describe('daemon rendezvous key', () => { + it('collapses redundant path segments to one socket name', () => { + const root = makeDir(); + const indirect = path.join(root, 'a', '..', 'b', '.'); + const direct = path.join(root, 'b'); + // `.`, `..` and a trailing separator all name the same directory, so both + // spellings must resolve to the same socket — otherwise `--path` with a + // stray separator is enough to miss a running daemon. + expect(getDaemonSocketPath(indirect)).toBe(getDaemonSocketPath(direct)); + }); + + it.runIf(process.platform === 'win32')( + 'maps both drive-letter cases of one directory onto one pipe', + () => { + const root = makeDir(); + const upper = withDriveCase(root, true); + const lower = withDriveCase(root, false); + + expect(canonicalProjectRoot(upper)).toBe(canonicalProjectRoot(lower)); + expect(getDaemonSocketPath(upper)).toBe(getDaemonSocketPath(lower)); + // …and the converged key is the on-disk casing, lowercased, so it does not + // depend on which of the two the caller happened to hold. + expect(canonicalProjectRoot(upper)).toBe(fs.realpathSync.native(root).toLowerCase()); + }, + ); + + it.runIf(process.platform === 'win32')( + 'converges case variants even when the root cannot be realpath’d', + () => { + // The `.codegraph/` root normally exists, so this is the fallback arm — + // but a divergence there would fail identically, so pin it. + const missing = path.join(os.tmpdir(), 'cg-rendezvous-absent', 'nested'); + expect(canonicalProjectRoot(missing)).toBe(canonicalProjectRoot(missing.toUpperCase())); + }, + ); +}); + +/** True when the temp filesystem ignores case (default macOS APFS, NTFS). */ +function caseInsensitiveTmp(): boolean { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-CaseProbe-')); + try { + return fs.existsSync(dir.replace('cg-CaseProbe-', 'cg-caseprobe-')); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +describe('in-process project sharing (#2278)', () => { + it.runIf(caseInsensitiveTmp())('opens one graph for two casings of one root', async () => { + const root = path.join(makeDir(), 'Repo'); + fs.mkdirSync(root); + let opened = 0; + const open = () => { + opened++; + return { close() {}, isIndexing: () => false } as unknown as CodeGraph; + }; + const a = acquireProject(root, open, {}); + const b = acquireProject(path.join(path.dirname(root), 'REPO'), open, {}); + try { + expect(opened).toBe(1); + expect(b.cg).toBe(a.cg); + } finally { + await a.release(); + await b.release(); + } + }); +}); diff --git a/src/directory.ts b/src/directory.ts index d99860f04e..c458457e4e 100644 --- a/src/directory.ts +++ b/src/directory.ts @@ -303,6 +303,40 @@ export function statInode(p: string): string | null { } } +/** + * Canonicalize a project root for IDENTITY: hashing it into a rendezvous key + * (the daemon's named pipe / tmpdir socket, a registry record). Not for display + * — the caller-visible root keeps its own spelling. + * + * The contract is that two spellings of ONE directory yield ONE string, because + * a client that derives a different key silently fails to meet the daemon that + * is already running: it probes a socket nobody bound, spawns a redundant + * daemon, and that daemon dies on the lock the first one holds, so the session + * degrades to a single-process engine. `path.resolve` alone does NOT satisfy + * that contract — NTFS is case-insensitive, so `d:\work\codegraph` and + * `D:\work\codegraph` name one directory but two strings, and non-native + * `fs.realpathSync` keeps whichever casing the caller passed (only `.native` + * asks the filesystem for the on-disk name — the same reason + * {@link isSameIndexRoot} uses it). Both spellings really do occur: a + * cwd-derived root is the on-disk case, while a client-supplied + * `rootUri`/`workspaceFolders` path arrives as `file:///d%3A/…`. + * + * The Windows lowercase is belt-and-braces on top of the native realpath — NTFS + * can't hold two directories differing only by case, and it also absorbs a + * `\\?\`-prefixed native result diverging from a plain one. + */ +export function canonicalProjectRoot(projectRoot: string): string { + const resolved = path.resolve(projectRoot); + let canonical = resolved; + try { + canonical = fs.realpathSync.native(resolved); + } catch { + // ENOENT/EACCES/ELOOP — the root is normally there (`.codegraph/` lives in + // it), so this is a fallback rather than a path we expect to take. + } + return process.platform === 'win32' ? canonical.toLowerCase() : canonical; +} + /** * Whether two resolved index roots are one index spelled two ways — a symlinked * checkout, or a case-variant on a case-insensitive mount (macOS, NTFS, WSL diff --git a/src/mcp/daemon-paths.ts b/src/mcp/daemon-paths.ts index 34751814f3..8f78d5063e 100644 --- a/src/mcp/daemon-paths.ts +++ b/src/mcp/daemon-paths.ts @@ -32,14 +32,20 @@ import * as crypto from 'crypto'; import * as net from 'net'; import * as os from 'os'; import * as path from 'path'; -import { getCodeGraphDir } from '../directory'; +import { canonicalProjectRoot, getCodeGraphDir } from '../directory'; /** Soft upper bound for in-project socket paths. */ const POSIX_SOCKET_PATH_LIMIT = 100; -/** Short stable identifier for a project root — used in tmpdir/pipe names. */ +/** + * Short stable identifier for a project root — used in tmpdir/pipe names. + * + * Hashed over {@link canonicalProjectRoot}, never a raw `path.resolve`: the key + * is a rendezvous, so every spelling of one directory must land on one name or + * a proxy probes a pipe the running daemon never bound (see that function). + */ function projectHash(projectRoot: string): string { - return crypto.createHash('sha256').update(path.resolve(projectRoot)).digest('hex').slice(0, 16); + return crypto.createHash('sha256').update(canonicalProjectRoot(projectRoot)).digest('hex').slice(0, 16); } /** diff --git a/src/mcp/daemon-registry.ts b/src/mcp/daemon-registry.ts index b4a01c307f..c6c4043b60 100644 --- a/src/mcp/daemon-registry.ts +++ b/src/mcp/daemon-registry.ts @@ -22,6 +22,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import * as crypto from 'crypto'; +import { canonicalProjectRoot } from '../directory'; import { getDaemonPidPath, getDaemonSocketCandidates, @@ -50,8 +51,14 @@ export function getRegistryDir(): string { return path.join(os.homedir(), '.codegraph', 'daemons'); } +/** + * One record per project, so it is keyed the same way the daemon socket is: + * over {@link canonicalProjectRoot}, not a raw `path.resolve` — otherwise the + * same project spelled with another drive-letter case files two records, and + * `list` over-lists while `stop --all` misses one. + */ function recordPath(root: string): string { - const hash = crypto.createHash('sha256').update(path.resolve(root)).digest('hex').slice(0, 16); + const hash = crypto.createHash('sha256').update(canonicalProjectRoot(root)).digest('hex').slice(0, 16); return path.join(getRegistryDir(), `${hash}.json`); } @@ -242,7 +249,7 @@ export async function stopDaemonAt(root: string, options: { preserveUnverified?: } if (pid == null) { const rec = listDaemons({ prune: false }).find( - (r) => path.resolve(r.root) === path.resolve(root) + (r) => canonicalProjectRoot(r.root) === canonicalProjectRoot(root) ); pid = rec?.pid ?? null; if (rec) identity = rec; diff --git a/src/mcp/project-lifecycle.ts b/src/mcp/project-lifecycle.ts index ba405f57d1..b7a52c1a5b 100644 --- a/src/mcp/project-lifecycle.ts +++ b/src/mcp/project-lifecycle.ts @@ -2,13 +2,15 @@ import { realpathSync } from 'fs'; import type { Socket } from 'net'; import type CodeGraph from '../index'; -import { isInitialized } from '../directory'; +import { canonicalProjectRoot, isInitialized } from '../directory'; import { LockUnavailableError, watchDisabledReason } from '../sync'; import { getDaemonSocketCandidates } from './daemon-paths'; import { connectWithHello } from './proxy'; import { markWriterReady, readWriterLock, releaseWriterLock, tryAcquireWriterLock } from './writer-lock'; interface Project { + /** Identity key: one entry however the root is spelled (#2278). */ + key: string; cg: CodeGraph; refs: number; owner: boolean; @@ -35,13 +37,14 @@ export function acquireProject( options: Parameters[0], ): ProjectLease { root = realpathSync(root); - let project = projects.get(root); + const key = canonicalProjectRoot(root); + let project = projects.get(key); if (!project) { const cg = open(); - project = { cg, refs: 0, owner: false, caughtUp: false, retirement: null, gate: null, socket: null, options, + project = { key, cg, refs: 0, owner: false, caughtUp: false, retirement: null, gate: null, socket: null, options, timer: setInterval(() => { void ready(root, project!); }, 1000) }; project.timer.unref(); - projects.set(root, project); + projects.set(key, project); } const entry = project; entry.refs++; @@ -80,7 +83,7 @@ function retire(root: string, project: Project): Promise { setTimeout(finish, 25); return; } - projects.delete(root); + projects.delete(project.key); project.cg.close(); if (project.owner) releaseWriterLock(root); resolve(); From e7389b22fb18381eae9639feb6e09ae2af5d128d Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 06:18:02 +0000 Subject: [PATCH 2/8] fix(mcp): a session serving in-process hands the project back to the shared daemon (#2277) (#2293) * fix(mcp): a session serving in-process hands the project back to the shared daemon (#2277) When a proxy lost its daemon (or never reached one) it served the rest of the session through an in-process engine. That engine takes the project's writer lock in fallback mode and nothing ever released it or tried the daemon again, so every later daemon start for the project failed on the held lock and every other session ran read-only without auto-sync until that one session ended. Sessions that fell back read-only stayed read-only for life too. The degraded proxy now retries the shared daemon with backoff (5 s, doubling to a 5 min cap; CODEGRAPH_DAEMON_RETRY_MS / _MAX_MS, 0 disables) through the existing probe/spawn/poll path. A writing engine is stopped first, after the calls it is serving have answered, so its lock is free before a daemon is spawned and two writers never overlap; calls arriving meanwhile are buffered and go to the daemon, or are served in-process again if none comes up. A read-only engine keeps serving until a daemon answers and is closed once its calls finish. The client's initialize is replayed to the new daemon with its reply suppressed, as on first attach. Co-Authored-By: Claude Opus 5.5 * docs(changelog): a degraded session hands the project back (#2277) Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/mcp-daemon.test.ts | 138 ++++++++++++++++++++++++++++++ src/mcp/engine.ts | 5 ++ src/mcp/proxy.ts | 159 +++++++++++++++++++++++++++++------ 4 files changed, 279 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62d005a12f..cf27df4914 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -172,6 +172,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Indexing a large C or C++ project no longer runs out of memory partway through "Resolving refs". Headers whose include guard is written as `#define X_H 1`, as in OpenSceneGraph and osgEarth, or that are included under a build flag codegraph cannot know, used to be re-read on every include path, which grows exponentially with the depth of the include tree. Each is now read once per include state, and a hard limit stops the check on any include tree that is still too large. Thanks @danusha2345, and @coolhitmanleon for the report. (#2127) - In tag-based CFML, calls written in tags are now linked: in ``, ``/``, ``, `` and `#…#` expressions. Before, only `` and `` code was read, so callers and impact found almost nothing in tag-based components. Functions wrapped in tags like `` or `` are now indexed too, and a `` is indexed as an interface that `implements` links to. Re-index CFML projects after upgrading. Thanks @HarryMuc for the report. (#2091) - On Windows, a project opened with a different drive-letter or folder-name case, like `d:\work\app` and `D:\Work\App`, now joins the CodeGraph background server that is already running for it. Before, each spelling started a server of its own that exited at once, so the session quietly served the graph by itself with no shared file watching or auto-sync, and `codegraph list` showed the project twice. Thanks @greatjackzhou. (#2278) +- An agent session that lost its connection to CodeGraph's shared background server, or couldn't reach it at startup, no longer stops that server from running for the rest of the session. It reconnects on its own once the server can start, so other sessions on the same project get auto-sync back instead of running read-only. Thanks @ijbranch for the report. (#2277) ## [1.6.1] - 2026-09-29 diff --git a/__tests__/mcp-daemon.test.ts b/__tests__/mcp-daemon.test.ts index 71cc7b71ff..5b8b305002 100644 --- a/__tests__/mcp-daemon.test.ts +++ b/__tests__/mcp-daemon.test.ts @@ -817,4 +817,142 @@ describe('Shared MCP daemon (issue #411)', () => { expect(resp.result !== undefined || resp.error !== undefined).toBe(true); expect(isAlive(server.child.pid!)).toBe(true); }, 45000); + + it('a proxy serving in-process hands the writer lock back to a fresh daemon (#2277)', async () => { + // After its daemon dies, the proxy's in-process engine takes writer.pid in + // fallback mode. Every later daemon start used to fail on that lock for the + // rest of the session, so every other session on the project ran read-only + // without auto-sync. The degraded proxy now retries the daemon: it stops + // its engine (releasing the lock), lets a daemon start, and proxies again. + const env = { + CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', + CODEGRAPH_PPID_POLL_MS: '5000', + // The default, spelled out: long enough to observe the lockout first. + CODEGRAPH_DAEMON_RETRY_MS: '5000', + }; + const a = spawnServer(tempDir, env); + servers.push(a); + const proxyPid = a.child.pid!; + sendInitialize(a.child, `file://${tempDir}`, 1); + await waitFor(() => findResponse(a.stdout, 1), 20000, 25, 'initialize response'); + await waitFor(() => a.stderr.some((l) => l.includes('Attached to shared daemon')), 8000, 25, 'first daemon attach'); + sendMessage(a.child, { jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + await waitFor(() => findResponse(a.stdout, 2), 30000, 25, 'warm tools/call via daemon'); + const firstDaemon = readLockPid(realRoot)!; + + process.kill(firstDaemon, 'SIGTERM'); + expect(await waitProcessExit(firstDaemon, 8000)).toBe(true); + await waitFor(() => a.stderr.some((l) => l.includes('serving this session in-process')), 8000, 25, 'in-process failover'); + sendMessage(a.child, { jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + await waitFor(() => findResponse(a.stdout, 3), 15000, 25, 'in-process tools/call'); + + // The lockout: the in-process engine owns the project's writer slot. + expect(readWriterInfo(realRoot)).toMatchObject({ pid: proxyPid, mode: 'fallback' }); + + // Another session starting now cannot get a daemon while that lock is held. + const b = spawnServer(tempDir, env); + servers.push(b); + sendInitialize(b.child, `file://${tempDir}`, 1); + await waitFor(() => findResponse(b.stdout, 1), 20000, 25, 'second session initialize'); + + // Keep calling through the handover: every request gets exactly one reply. + let nextId = 4; + const send = (): void => { + sendMessage(a.child, { jsonrpc: '2.0', id: nextId++, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + }; + const ticker = setInterval(send, 250); + let daemonWriter: { pid: number; mode: string } | null = null; + try { + daemonWriter = await waitFor(() => { + const w = readWriterInfo(realRoot); + return w && w.mode === 'daemon' && w.pid !== proxyPid && isAlive(w.pid) ? w : null; + }, 30000, 25, 'a daemon to own the writer lock'); + await waitFor( + () => a.stderr.some((l) => l.includes(`Attached to shared daemon`) && l.includes(`(pid ${daemonWriter!.pid},`)), + 15000, 25, 'the proxy to reattach to the new daemon', + ); + } finally { + clearInterval(ticker); + } + send(); + const lastId = nextId - 1; + await waitFor(() => findResponse(a.stdout, lastId), 15000, 25, 'a tools/call through the new daemon'); + for (let id = 2; id <= lastId; id++) { + const replies = a.stdout.filter((line) => { + try { const m = JSON.parse(line); return m.id === id && ('result' in m || 'error' in m); } catch { return false; } + }); + expect(replies, `replies to request ${id}`).toHaveLength(1); + expect(JSON.parse(replies[0]).error, `request ${id}`).toBeUndefined(); + } + expect(readLockPid(realRoot)).toBe(daemonWriter!.pid); + + // The session that started during the lockout ends up on the shared daemon too. + await waitFor( + () => b.stderr.some((l) => l.includes('Attached to shared daemon') && l.includes(`(pid ${daemonWriter!.pid},`)), + 30000, 25, 'the second session to attach', + ); + sendMessage(b.child, { jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + const bReply = await waitFor(() => findResponse(b.stdout, 2), 15000, 25, 'second session tools/call'); + expect(bReply.error).toBeUndefined(); + }, 90000); + + it('a read-only in-process session moves to the shared daemon once one can start (#2277)', async () => { + // A session that fell back read-only (another daemon held the project) + // used to stay that way for life. Once the blocker is gone it now starts + // and attaches to a daemon of its own version. + const sockPath = getDaemonSocketPath(realRoot); + const pidPath = path.join(realRoot, '.codegraph', 'daemon.pid'); + fs.writeFileSync(pidPath, JSON.stringify({ pid: process.pid, version: '0.0.0-mismatch', socketPath: sockPath, startedAt: Date.now() })); + const miniServer = net.createServer((sock) => { + sock.write(JSON.stringify({ codegraph: '0.0.0-mismatch', pid: process.pid, socketPath: sockPath, protocol: 1 }) + '\n'); + }); + await new Promise((resolve) => miniServer.listen(sockPath, () => resolve())); + let miniServerOpen = true; + try { + const server = spawnServer(tempDir, { + CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS: '30000', + CODEGRAPH_DAEMON_RETRY_MS: '300', + CODEGRAPH_DAEMON_RETRY_MAX_MS: '1000', + }); + servers.push(server); + sendInitialize(server.child, `file://${tempDir}`, 1); + await waitFor(() => server.stderr.some((l) => l.includes('serving this session in-process')), 10000, 25, 'in-process fallback'); + sendMessage(server.child, { jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + const local = await waitFor(() => findResponse(server.stdout, 2), 10000, 25, 'read-only tools/call'); + expect(local.error).toBeUndefined(); + expect(server.stderr.some((l) => l.includes('Serving reads in-process without auto-sync'))).toBe(true); + // Retries while the other version still answers start no daemon. + await waitFor( + () => server.stderr.filter((l) => l.includes('differs from ours')).length >= 3, + 10000, 25, 'repeated retries against the other version', + ); + expect(countListeningLines(realRoot)).toBe(0); + + // The other-version daemon goes away. + await new Promise((resolve) => miniServer.close(() => resolve())); + miniServerOpen = false; + fs.rmSync(pidPath, { force: true }); + + const attached = await waitFor( + () => server.stderr.find((l) => l.includes('Attached to shared daemon')), + 30000, 25, 'the session to attach to a daemon', + ); + const daemonPid = readLockPid(realRoot)!; + expect(attached).toContain(`(pid ${daemonPid},`); + expect(readWriterInfo(realRoot)).toMatchObject({ pid: daemonPid, mode: 'daemon' }); + sendMessage(server.child, { jsonrpc: '2.0', id: 3, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + const viaDaemon = await waitFor(() => findResponse(server.stdout, 3), 15000, 25, 'tools/call through the daemon'); + expect(viaDaemon.error).toBeUndefined(); + expect(JSON.stringify(viaDaemon.result)).toContain('CodeGraph Status'); + } finally { + if (miniServerOpen) await new Promise((resolve) => miniServer.close(() => resolve())); + } + }, 60000); }); + +function readWriterInfo(root: string): { pid: number; mode: string } | null { + try { + const info = JSON.parse(fs.readFileSync(path.join(root, '.codegraph', 'writer.pid'), 'utf8')); + return typeof info.pid === 'number' && typeof info.mode === 'string' ? info : null; + } catch { return null; } +} diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index 2898f203b1..5409c36d48 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -167,6 +167,11 @@ export class MCPEngine { this.toolHandler.setDefaultProjectHint(projectPath); } + /** Whether this engine only reads: no watcher, no sync, no writer slot. */ + isReadOnly(): boolean { + return this.opts.readOnly; + } + /** Project root that the engine resolved on first init (null if none). */ getProjectPath(): string | null { return this.projectPath; diff --git a/src/mcp/proxy.ts b/src/mcp/proxy.ts index 17c5a59cb1..a983c2ebdc 100644 --- a/src/mcp/proxy.ts +++ b/src/mcp/proxy.ts @@ -39,6 +39,15 @@ import type { MCPEngine } from './engine'; /** Default poll cadence for the PPID watchdog (same as the direct server). */ const DEFAULT_PPID_POLL_MS = 5000; +/** + * How long a proxy serving in-process waits before trying the shared daemon + * again, doubling after each miss up to the cap (#2277). Tunable through + * `CODEGRAPH_DAEMON_RETRY_MS` (`0` turns retrying off) and + * `CODEGRAPH_DAEMON_RETRY_MAX_MS`. + */ +const DEFAULT_DAEMON_RETRY_MS = 5_000; +const DEFAULT_DAEMON_RETRY_MAX_MS = 300_000; + /** * Env var that opts INTO the "attached to shared daemon" log line. Off by * default: the line is benign INFO, but MCP hosts render any server stderr at @@ -196,8 +205,9 @@ 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). */ getDaemonSocket(): Promise; - /** Lazily create an in-process engine — used ONLY if the daemon never comes up, - * preserving the "a broken daemon never wedges a session" guarantee. */ + /** Lazily create an in-process engine — used only while the daemon is + * unreachable, preserving the "a broken daemon never wedges a session" + * guarantee. Called again whenever the previous engine was retired (#2277). */ makeEngine(): MCPEngine; /** Project root for the fallback engine's lazy init. */ root: string; @@ -215,6 +225,12 @@ export interface LocalHandshakeDeps { * the local one). If the daemon never comes up (version mismatch / spawn fail), * a lazily-created in-process engine serves the calls — so the handshake speedup * never costs the old fall-back-to-direct robustness. + * + * Serving in-process is not for the rest of the session (#2277): that engine + * may own the project's writer lock, and no daemon can start while it does, so + * every other session on the project would be stuck without auto-sync. The + * proxy retries the daemon with backoff, handing the writer lock back first, + * and proxies again once one answers. */ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise { // The proxy is long-lived and can serve fallback tool calls in-process. Match @@ -224,6 +240,7 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< let daemonStatus: 'connecting' | 'ready' | 'failed' = 'connecting'; let daemonSocket: net.Socket | null = null; let clientInitId: unknown = undefined; // suppress the daemon's reply to the forwarded initialize + let clientInitLine: string | undefined; // replayed to a daemon that comes up mid-session (#2277) // Telemetry attribution for the in-process fallback only — calls routed to // the daemon are counted by the daemon's own session (which receives the // forwarded initialize, clientInfo included), never double-counted here. @@ -231,7 +248,14 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< const pending: string[] = []; // client lines buffered until the daemon resolves let engine: MCPEngine | null = null; let engineReady: Promise | null = null; + // Calls being served in-process; the engine is only ever stopped after they finish. + const localCalls = new Set>(); let shuttingDown = false; + const retryBaseMs = parseDelayMs(process.env.CODEGRAPH_DAEMON_RETRY_MS, DEFAULT_DAEMON_RETRY_MS); + const retryMaxMs = Math.max(retryBaseMs, parseDelayMs(process.env.CODEGRAPH_DAEMON_RETRY_MAX_MS, DEFAULT_DAEMON_RETRY_MAX_MS)); + let retryDelayMs = retryBaseMs; + let retryTimer: NodeJS.Timeout | null = null; + let attachedAt = 0; // Requests forwarded to the daemon and not yet answered, keyed by JSON-RPC id. // If the daemon dies mid-session (#662 — e.g. an MCP host SIGTERM's it when a // new session starts), these would otherwise hang forever; we re-serve them @@ -256,24 +280,47 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< const shutdown = (): void => { if (shuttingDown) return; shuttingDown = true; try { livenessWatchdog?.stop(); } catch { /* ignore */ } + if (retryTimer) clearTimeout(retryTimer); try { daemonSocket?.destroy(); } catch { /* ignore */ } try { engine?.stop(); } catch { /* ignore */ } process.exit(0); }; - const ensureEngine = (): Promise => { - if (!engine) engine = deps.makeEngine(); - if (!engineReady) engineReady = engine.ensureInitialized(deps.root).catch(() => { /* degraded */ }); - return engineReady; + // Resolves the engine a call was started on, so a call in flight while the + // engine is being retired still finishes on it. + const ensureEngine = async (): Promise => { + if (!engine) { + engine = deps.makeEngine(); + engineReady = engine.ensureInitialized(deps.root).catch(() => { /* degraded */ }); + } + const current = engine; + await engineReady; + return current; + }; + // Stop the in-process engine once the calls it is serving have answered. + // Stopping it releases the writer lock if it held one. + const retireEngine = async (): Promise => { + const retired = engine; + engine = null; + engineReady = null; + await Promise.allSettled([...localCalls]); + try { await retired?.stop(); } catch { /* best-effort */ } }; // Daemon-unavailable fallback: serve a client message in-process. - const handleLocally = async (line: string): Promise => { + const handleLocally = (line: string): Promise => { + const call = serveLocally(line); + localCalls.add(call); + const done = (): void => { localCalls.delete(call); }; + call.then(done, done); + return call; + }; + const serveLocally = async (line: string): Promise => { let msg: JsonRpc; try { msg = JSON.parse(line) as JsonRpc; } catch { return; } const id = msg.id; if (msg.method === 'tools/call' && id !== undefined) { try { - await ensureEngine(); + const local = await ensureEngine(); const params = (msg.params || {}) as { name: string; arguments?: Record }; - const result = await engine!.getToolHandler().execute(params.name, params.arguments || {}, exploreSession); + const result = await local.getToolHandler().execute(params.name, params.arguments || {}, exploreSession); writeClient({ jsonrpc: '2.0', id, result }); getTelemetry().recordUsage('mcp_tool', params.name, !result.isError, telemetryClient); } catch (err) { @@ -314,6 +361,7 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< let msg: JsonRpc; try { msg = JSON.parse(line) as JsonRpc; } catch { routeToDaemon(line); continue; } if (msg.method === 'initialize') { clientInitId = msg.id; + clientInitLine = line; const initParams = (msg.params ?? {}) as { clientInfo?: { name?: unknown; version?: unknown } }; if (initParams.clientInfo) { telemetryClient = { @@ -364,15 +412,58 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< }); // ---- daemon connection (background) ---- - let socket: net.Socket | null = null; - try { socket = await deps.getDaemonSocket(); } catch { socket = null; } + const connectDaemon = async (): Promise => { + let socket: net.Socket | null = null; + try { socket = await deps.getDaemonSocket(); } catch { socket = null; } + // `socket.destroyed`: the connect-window error guard above can absorb an + // 'error' that already destroyed the socket before we got here (#974) — treat + // a dead socket as "no daemon" so we cleanly fall back to the in-process engine. + if (socket && (socket.destroyed || shuttingDown)) { + try { socket.destroy(); } catch { /* ignore */ } + socket = null; + } + return socket; + }; + // Every buffered call binds to the engine now, so a later retirement waits for all of them. + const serveBufferedLocally = (): void => { + for (const line of pending.splice(0)) void handleLocally(line); + }; + const scheduleDaemonRetry = (): void => { + if (shuttingDown || retryTimer || retryBaseMs <= 0) return; + const delay = retryDelayMs; + retryDelayMs = Math.min(retryDelayMs * 2, retryMaxMs); + retryTimer = setTimeout(() => { retryTimer = null; void retryDaemon(); }, delay); + retryTimer.unref?.(); + }; + const retryDaemon = async (): Promise => { + if (shuttingDown || daemonStatus !== 'failed') return; + // While a writing engine owns writer.pid no daemon can start (#2277). Hand + // the lock back first, once the calls it is serving have answered, and + // buffer new calls until a daemon answers or we resume in-process. One + // writer at a time: the engine is fully stopped before a daemon is spawned. + const handover = engine !== null && !engine.isReadOnly(); + if (handover) { + daemonStatus = 'connecting'; + await retireEngine(); + } + const socket = await connectDaemon(); + if (shuttingDown) return; + if (socket) { + process.stderr.write('[CodeGraph MCP] Shared daemon reachable; proxying this session to it instead of serving in-process.\n'); + attachDaemon(socket, true); + // A read-only engine holds no lock: let its calls finish, then close it. + if (!handover) void retireEngine(); + return; + } + daemonStatus = 'failed'; + serveBufferedLocally(); + scheduleDaemonRetry(); + }; - // `!socket.destroyed`: the connect-window error guard above can absorb an - // 'error' that already destroyed the socket before we got here (#974) — treat - // a dead socket as "no daemon" so we cleanly fall back to the in-process engine. - if (socket && !socket.destroyed && !shuttingDown) { + const attachDaemon = (socket: net.Socket, replayInitialize: boolean): void => { daemonSocket = socket; daemonStatus = 'ready'; + attachedAt = Date.now(); let sockBuf = ''; socket.setEncoding('utf8'); socket.on('data', (chunk: string) => { @@ -397,33 +488,48 @@ export async function runLocalHandshakeProxy(deps: LocalHandshakeDeps): Promise< // The daemon going away does NOT end the session (#662). An MCP host can // SIGTERM the shared daemon when another session starts; if we exited here, // this host would silently lose CodeGraph and any in-flight request would - // hang. Instead, fall back to the in-process engine for the rest of the - // session and re-serve whatever the dead daemon never answered. + // hang. Instead, fall back to the in-process engine until a daemon answers + // again, and re-serve whatever the dead daemon never answered. const onDaemonLost = (): void => { - if (shuttingDown || daemonStatus !== 'ready') return; // host teardown, or already handled + if (shuttingDown || daemonSocket !== socket) return; // host teardown, or already handled daemonStatus = 'failed'; - try { daemonSocket?.destroy(); } catch { /* ignore */ } daemonSocket = null; + try { socket.destroy(); } catch { /* ignore */ } process.stderr.write( `[CodeGraph MCP] Shared daemon connection lost; serving this session in-process (degraded), re-serving ${inflight.size} in-flight request(s).\n` ); const orphaned = [...inflight.values()]; inflight.clear(); for (const line of orphaned) void handleLocally(line); + // A link that stayed up past the longest wait starts the backoff over; one + // that keeps dropping right after it attaches keeps backing off. + if (Date.now() - attachedAt >= retryMaxMs) retryDelayMs = retryBaseMs; + scheduleDaemonRetry(); }; socket.on('close', onDaemonLost); socket.on('error', onDaemonLost); + // A daemon that came up mid-session has never seen this client: prime its + // session with the client's own initialize, whose reply is suppressed + // above like the first one. + if (replayInitialize && clientInitLine !== undefined) { + try { socket.write(clientInitLine + '\n'); } catch { /* close path */ } + } for (const line of pending) { trackInflight(line); if (process.env.CODEGRAPH_MCP_DEBUG) process.stderr.write(`[mcp-debug] proxy-flush ${line.slice(0, 80)}\n`); try { socket.write(line + '\n'); } catch { /* ignore */ } } pending.length = 0; + }; + + const socket = await connectDaemon(); + if (socket) { + attachDaemon(socket, false); } else if (!shuttingDown) { daemonStatus = 'failed'; process.stderr.write('[CodeGraph MCP] Shared daemon unavailable; serving this session in-process (degraded).\n'); - const buffered = pending.splice(0); - for (const line of buffered) await handleLocally(line); + serveBufferedLocally(); + scheduleDaemonRetry(); } await new Promise(() => { /* stdin keeps the loop alive; exit via shutdown() */ }); @@ -588,10 +694,15 @@ function startPpidWatchdog(socket: net.Socket): void { } function parsePollMs(raw: string | undefined): number { - if (raw === undefined || raw === '') return DEFAULT_PPID_POLL_MS; + return parseDelayMs(raw, DEFAULT_PPID_POLL_MS); +} + +/** A non-negative millisecond env value; unset or malformed gives `fallback`. */ +function parseDelayMs(raw: string | undefined, fallback: number): number { + if (raw === undefined || raw === '') return fallback; const parsed = Number(raw); - if (!Number.isFinite(parsed)) return DEFAULT_PPID_POLL_MS; - if (parsed < 0) return DEFAULT_PPID_POLL_MS; + if (!Number.isFinite(parsed)) return fallback; + if (parsed < 0) return fallback; return Math.floor(parsed); } From 7bbc1fb7f6bc4627a6e7930f7229b34879a1cf57 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 06:18:46 +0000 Subject: [PATCH 3/8] fix(tests): the test suite no longer writes to the developer's real home dir (#2275) (#2295) Running the suite on a machine with Claude Code configured could wire the CodeGraph prompt hook into the developer's own ~/.claude/settings.json: every upgrade test whose fake upgrade succeeded ran the real post-upgrade prompt-hook self-heal, which detected the real global Claude profile and wrote it. Daemon suites likewise left records in the real ~/.codegraph/daemons. On Windows a HOME-only redirect would not have helped, since os.homedir() reads USERPROFILE there. - The prompt-hook writer is now injected through UpgradeDeps (wirePromptHook); the CLI passes the real defaultWirePromptHook and the upgrade tests a recorder. New tests pin that a successful upgrade calls the recorder and that the real writer never touches a configured profile. - A vitest setupFiles entry gives every engine test file a throwaway home: HOME/USERPROFILE (plus HOMEDRIVE/HOMEPATH/APPDATA/LOCALAPPDATA on Windows), XDG_CONFIG_HOME, and GIT_CONFIG_GLOBAL (seeded with a dummy identity) point into it, and CLAUDE_CONFIG_DIR/CODEX_HOME/COPILOT_HOME/ HERMES_HOME are cleared. Spawned CLI/MCP children inherit it. A sandbox test asserts os.homedir(), the Claude profile, the daemon registry, a child process and git's global config all resolve inside it. Co-authored-by: Claude Opus 5.5 --- AGENTS.md | 2 + __tests__/setup-home-sandbox.ts | 66 +++++++++++++++++ __tests__/test-home-sandbox.test.ts | 65 +++++++++++++++++ __tests__/upgrade.test.ts | 109 +++++++++++++++++++++++++++- src/bin/codegraph.ts | 1 + src/upgrade/index.ts | 26 ++++++- vitest.config.mts | 3 + vitest.workspace.mts | 6 +- 8 files changed, 270 insertions(+), 8 deletions(-) create mode 100644 __tests__/setup-home-sandbox.ts create mode 100644 __tests__/test-home-sandbox.test.ts diff --git a/AGENTS.md b/AGENTS.md index 2abe06c4ad..0f5e9a882e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -181,6 +181,8 @@ Tests live in `__tests__/` and mirror the module they cover. Notable ones beyond Tests create temp dirs with `fs.mkdtempSync` and clean up in `afterEach`. They write real files and exercise real SQLite — there is no DB mocking. +Every engine test file runs in a throwaway home dir (`__tests__/setup-home-sandbox.ts`, a `setupFiles` entry): `HOME`/`USERPROFILE` (+ Windows vars), `XDG_CONFIG_HOME` and `GIT_CONFIG_GLOBAL` point into it, and `CLAUDE_CONFIG_DIR`/`CODEX_HOME`/… are cleared; spawned children inherit it. It's a backstop — still inject writes to global state (e.g. `UpgradeDeps.wirePromptHook`, #2275). + ### Windows-gated tests Behavior that differs by platform (path resolution, drive letters, `SENSITIVE_PATHS`, `%APPDATA%` config dirs, CRLF) must be gated, not assumed. Use `it.runIf(process.platform === 'win32')(...)` for Windows-only assertions and `it.runIf(process.platform !== 'win32')(...)` for POSIX-only ones — e.g. `/etc` is sensitive on POSIX but resolves to `C:\etc` (non-existent) on Windows, so an ungated `/etc` assertion fails on Windows. Validate the Windows side for real (see below); don't merge a Windows-gated test you haven't seen run. diff --git a/__tests__/setup-home-sandbox.ts b/__tests__/setup-home-sandbox.ts new file mode 100644 index 0000000000..5acb473944 --- /dev/null +++ b/__tests__/setup-home-sandbox.ts @@ -0,0 +1,66 @@ +/** + * Give every engine test file a throwaway home directory (#2275). + * + * The suite exercises code that writes to the user's GLOBAL state: the Claude + * prompt hook in `~/.claude/settings.json`, the daemon registry under + * `~/.codegraph/daemons`, every agent target's global config. A test that + * forgot to redirect the home dir wrote to the developer's real one — a + * successful fake `codegraph upgrade` wired the prompt hook into their own + * Claude profile, and the daemon suites left thousands of registry records + * behind. CI never noticed: its runners have no Claude profile to edit. + * + * So the home dir is redirected here, before any test module loads, rather + * than file by file. Everything that resolves a home dir is covered: + * `os.homedir()` reads HOME on POSIX but USERPROFILE on Windows; git on + * Windows falls back to HOMEDRIVE+HOMEPATH; the per-agent overrides + * (CLAUDE_CONFIG_DIR, CODEX_HOME, ...) win over either, so they are cleared; + * and git's global config is pinned to a seeded file, so a test's `git + * commit` neither reads the developer's identity, signing or hooks nor + * depends on them. Spawned CLI / MCP processes inherit all of it. + * + * Tests that redirect the home dir themselves (spying on `os.homedir`, or + * setting HOME and restoring it) keep working: they restore to this sandbox. + */ +import { afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +// Resolved, so a comparison against a realpath'd cwd (macOS `/var` → +// `/private/var`) sees the same home everything else does. +const home = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-home-'))); + +process.env.HOME = home; +process.env.USERPROFILE = home; +if (process.platform === 'win32') { + const drive = /^([A-Za-z]:)(\\.*)$/.exec(home); + if (drive) { + process.env.HOMEDRIVE = drive[1]; + process.env.HOMEPATH = drive[2]; + } + process.env.APPDATA = path.join(home, 'AppData', 'Roaming'); + process.env.LOCALAPPDATA = path.join(home, 'AppData', 'Local'); +} else { + // Only ever set on Windows in real life; on POSIX a stray one would point + // the opencode/Copilot legacy-path logic somewhere real. + delete process.env.APPDATA; + delete process.env.LOCALAPPDATA; +} +process.env.XDG_CONFIG_HOME = path.join(home, '.config'); +for (const override of ['CLAUDE_CONFIG_DIR', 'CODEX_HOME', 'COPILOT_HOME', 'HERMES_HOME']) { + delete process.env[override]; +} + +// Both spellings: GIT_CONFIG_GLOBAL for git >= 2.32, and `$HOME/.gitconfig` +// (the same file) for older ones. +const gitConfig = path.join(home, '.gitconfig'); +fs.writeFileSync(gitConfig, '[user]\n\tname = CodeGraph Test\n\temail = test@codegraph.invalid\n'); +process.env.GIT_CONFIG_GLOBAL = gitConfig; + +afterAll(() => { + try { + fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 }); + } catch { + /* a straggling child still holding a file there; it's in the temp dir */ + } +}); diff --git a/__tests__/test-home-sandbox.test.ts b/__tests__/test-home-sandbox.test.ts new file mode 100644 index 0000000000..f5ebb7a718 --- /dev/null +++ b/__tests__/test-home-sandbox.test.ts @@ -0,0 +1,65 @@ +/** + * The suite's home-dir sandbox (`setup-home-sandbox.ts`, #2275) is in force: + * nothing a test runs — in-process or spawned — resolves the developer's real + * home, Claude profile, daemon registry or git identity. + */ +import { describe, it, expect } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { claudeTarget } from '../src/installer/targets/claude'; +import { getRegistryDir } from '../src/mcp/daemon-registry'; + +const tmpRoot = fs.realpathSync(os.tmpdir()); +const under = (p: string, dir: string): boolean => path.resolve(p).startsWith(dir + path.sep); +/** The sandbox home — asserted to be one, so the checks below can't pass on a real home. */ +function sandboxHome(): string { + const home = os.homedir(); + expect(under(home, tmpRoot), home).toBe(true); + return home; +} + +describe('test home sandbox', () => { + it('os.homedir() is a throwaway dir under the temp root, not the real home', () => { + const home = sandboxHome(); + expect(path.basename(home)).toMatch(/^codegraph-test-home-/); + // The account's profile dir, which ignores HOME / USERPROFILE. + expect(home).not.toBe(os.userInfo().homedir); + }); + + it('the global Claude profile and the daemon registry resolve inside it', () => { + const home = sandboxHome(); + expect(process.env.CLAUDE_CONFIG_DIR).toBeUndefined(); + expect(under(claudeTarget.detect('global').configPath, home)).toBe(true); + expect(under(getRegistryDir(), home)).toBe(true); + }); + + it('a spawned child sees the same home', () => { + const childHome = execFileSync(process.execPath, ['-p', 'require("os").homedir()'], { + encoding: 'utf-8', + windowsHide: true, + }).trim(); + expect(childHome).toBe(sandboxHome()); + }); + + it("git's global config is the seeded sandbox file", () => { + const gitConfig = process.env.GIT_CONFIG_GLOBAL ?? ''; + expect(under(gitConfig, sandboxHome())).toBe(true); + const name = execFileSync('git', ['config', '--global', '--get', 'user.name'], { + encoding: 'utf-8', + windowsHide: true, + }).trim(); + expect(name).toBe('CodeGraph Test'); + }); + + // os.homedir() reads USERPROFILE there, and git falls back to + // HOMEDRIVE+HOMEPATH — a HOME-only sandbox would not hold. + it.runIf(process.platform === 'win32')('covers the Windows home variables', () => { + const home = sandboxHome(); + expect(process.env.USERPROFILE).toBe(home); + expect(`${process.env.HOMEDRIVE}${process.env.HOMEPATH}`).toBe(home); + expect(under(process.env.APPDATA ?? '', home)).toBe(true); + expect(under(process.env.LOCALAPPDATA ?? '', home)).toBe(true); + }); +}); diff --git a/__tests__/upgrade.test.ts b/__tests__/upgrade.test.ts index 244957af4e..e1b05e4d40 100644 --- a/__tests__/upgrade.test.ts +++ b/__tests__/upgrade.test.ts @@ -14,6 +14,7 @@ import { reindexAdvisory, runUpgrade, verifyResolvedVersion, + defaultWirePromptHook, buildWindowsUpgradeScript, NPM_PACKAGE, type InstallMethod, @@ -230,13 +231,15 @@ interface Calls { captures: Array<{ cmd: string; args: string[] }>; logs: string[]; errors: string[]; + /** How many times the upgrade asked deps to wire the Claude prompt hook. */ + promptHookWires: number; } function makeDeps( overrides: Partial & { method: InstallMethod; currentVersion: string }, runExit = 0 ): { deps: UpgradeDeps; calls: Calls } { - const calls: Calls = { runs: [], captures: [], logs: [], errors: [] }; + const calls: Calls = { runs: [], captures: [], logs: [], errors: [], promptHookWires: 0 }; const deps: UpgradeDeps = { currentVersion: overrides.currentVersion, method: overrides.method, @@ -252,6 +255,12 @@ function makeDeps( return overrides.capture ? overrides.capture(cmd, args) : null; }, hasCommand: overrides.hasCommand ?? ((c) => c === 'curl'), + // A recorder, never the real writer: that one edits the GLOBAL Claude + // profile, which here would be the developer's own (#2275). + wirePromptHook: async () => { + calls.promptHookWires += 1; + return overrides.wirePromptHook ? overrides.wirePromptHook() : false; + }, log: (m) => calls.logs.push(m), warn: (m) => calls.logs.push(m), error: (m) => calls.errors.push(m), @@ -473,6 +482,104 @@ describe('runUpgrade beta signup offer', () => { }); }); +// --------------------------------------------------------------------------- +// Post-upgrade prompt-hook self-heal — only ever through deps (#2275). The +// real writer edits the GLOBAL Claude profile; while the upgrade called it +// directly, every successful fake upgrade in this file could wire the hook +// into the developer's own ~/.claude/settings.json. +// --------------------------------------------------------------------------- + +describe('post-upgrade prompt-hook self-heal', () => { + // A configured global Claude profile the REAL writer would act on, so an + // upgrade that bypasses deps shows up as a settings.json written here. + const KEYS = ['CLAUDE_CONFIG_DIR', 'CODEGRAPH_NO_PROMPT_HOOK', 'CODEGRAPH_PROMPT_HOOK'] as const; + const saved: Partial> = {}; + let profile: string; + + function configureProfile(): void { + fs.writeFileSync( + path.join(profile, '.claude.json'), + JSON.stringify({ mcpServers: { codegraph: { command: 'codegraph', args: ['serve', '--mcp'] } } }), + ); + } + + beforeEach(() => { + for (const k of KEYS) saved[k] = process.env[k]; + delete process.env.CODEGRAPH_NO_PROMPT_HOOK; + delete process.env.CODEGRAPH_PROMPT_HOOK; + profile = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-upgrade-claude-')); + process.env.CLAUDE_CONFIG_DIR = profile; + }); + afterEach(() => { + for (const k of KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + fs.rmSync(profile, { recursive: true, force: true }); + }); + + it('a successful upgrade wires the hook through deps, never the real writer', async () => { + configureProfile(); + const { deps, calls } = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' }); + expect(await runUpgrade({}, deps)).toBe(0); + expect(fs.existsSync(path.join(profile, 'settings.json')), 'the real writer ran').toBe(false); + expect(calls.promptHookWires).toBe(1); + }); + + it('notes the hook only when the writer changed something', async () => { + const quiet = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' }); + expect(await runUpgrade({}, quiet.deps)).toBe(0); + expect(quiet.calls.logs.join('\n')).not.toMatch(/front-load hook/); + + const wired = makeDeps({ + method: { kind: 'npm', scope: 'global' }, + currentVersion: '0.9.8', + wirePromptHook: async () => true, + }); + expect(await runUpgrade({}, wired.deps)).toBe(0); + expect(wired.calls.logs.join('\n')).toMatch(/Enabled the CodeGraph front-load hook/); + }); + + it('the kill-switch skips the writer entirely', async () => { + process.env.CODEGRAPH_NO_PROMPT_HOOK = '1'; + const { deps, calls } = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' }); + expect(await runUpgrade({}, deps)).toBe(0); + expect(calls.promptHookWires).toBe(0); + }); + + it('does not wire on --check, when up to date, or when the upgrade fails', async () => { + const check = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' }); + expect(await runUpgrade({ check: true }, check.deps)).toBe(0); + const current = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.9' }); + expect(await runUpgrade({}, current.deps)).toBe(0); + const failed = makeDeps({ method: { kind: 'npm', scope: 'global' }, currentVersion: '0.9.8' }, 1); + expect(await runUpgrade({}, failed.deps)).toBe(1); + for (const { calls } of [check, current, failed]) expect(calls.promptHookWires).toBe(0); + }); + + it('a throwing writer never fails the upgrade', async () => { + const { deps } = makeDeps({ + method: { kind: 'npm', scope: 'global' }, + currentVersion: '0.9.8', + wirePromptHook: async () => { throw new Error('EACCES'); }, + }); + expect(await runUpgrade({}, deps)).toBe(0); + }); + + // The production writer itself, pointed at the temp profile above. + it('the real writer wires a configured profile once, and leaves an unconfigured one alone', async () => { + const settings = path.join(profile, 'settings.json'); + expect(await defaultWirePromptHook()).toBe(false); + expect(fs.existsSync(settings)).toBe(false); + + configureProfile(); + expect(await defaultWirePromptHook()).toBe(true); + const written = JSON.parse(fs.readFileSync(settings, 'utf-8')); + expect(JSON.stringify(written.hooks.UserPromptSubmit)).toMatch(/prompt-hook/); + expect(await defaultWirePromptHook()).toBe(false); // idempotent + }); +}); + // --------------------------------------------------------------------------- // Post-upgrade self-heal of installed agent surfaces // --------------------------------------------------------------------------- diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 64078dfab6..e5f8eacfc6 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -2859,6 +2859,7 @@ program run: up.defaultRun, capture: up.defaultCapture, hasCommand: up.hasCommand, + wirePromptHook: up.defaultWirePromptHook, log: (m: string) => console.log(m), warn: (m: string) => warn(m), error: (m: string) => error(m), diff --git a/src/upgrade/index.ts b/src/upgrade/index.ts index 625e45b831..76354378a8 100644 --- a/src/upgrade/index.ts +++ b/src/upgrade/index.ts @@ -297,6 +297,15 @@ export interface UpgradeDeps { warn: (msg: string) => void; error: (msg: string) => void; platform: NodeJS.Platform; + /** + * Wire Claude Code's front-load prompt hook into the GLOBAL Claude profile + * when that profile already has CodeGraph configured; resolves true when it + * changed the settings file. That file is the user's real + * `~/.claude/settings.json`, so the writer is injected like every other side + * effect here — the CLI passes {@link defaultWirePromptHook}, unit tests a + * recorder (#2275: tests that reached the real one rewrote the developer's). + */ + wirePromptHook: () => Promise; /** * Offer the one-time CodeGraph Pro beta opt-in after a successful update * (see installer/beta-signup — self-gating: TTY only, and silent forever @@ -528,10 +537,7 @@ function selfHealInstalledSurfaces(deps: UpgradeDeps): void { */ async function selfHealPromptHook(deps: UpgradeDeps): Promise { if (process.env.CODEGRAPH_NO_PROMPT_HOOK === '1' || process.env.CODEGRAPH_PROMPT_HOOK === '0') return; - const { claudeTarget, writePromptHookEntry } = await import('../installer/targets/claude'); - if (!claudeTarget.detect('global').alreadyConfigured) return; - const res = writePromptHookEntry('global'); - if (res.action === 'created' || res.action === 'updated') { + if (await deps.wirePromptHook()) { deps.log( c.dim('Enabled the CodeGraph front-load hook for Claude Code (structural prompts). Disable any time: CODEGRAPH_NO_PROMPT_HOOK=1'), ); @@ -714,3 +720,15 @@ export function defaultCapture(cmd: string, args: string[]): { code: number; std if (r.error) return null; return { code: r.status ?? -1, stdout: r.stdout ?? '' }; } + +/** + * The production `UpgradeDeps.wirePromptHook`: writes the hook only when the + * global Claude profile already carries CodeGraph's MCP entry, and leaves the + * file byte-for-byte alone once the hook is there. + */ +export async function defaultWirePromptHook(): Promise { + const { claudeTarget, writePromptHookEntry } = await import('../installer/targets/claude'); + if (!claudeTarget.detect('global').alreadyConfigured) return false; + const res = writePromptHookEntry('global'); + return res.action === 'created' || res.action === 'updated'; +} diff --git a/vitest.config.mts b/vitest.config.mts index ae02ba84c6..96fb2573a6 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -14,6 +14,9 @@ export default defineConfig({ include: ['__tests__/**/*.test.ts'], // Suites that spawn the built CLI need a current dist/ (#1879). globalSetup: ['./__tests__/global-setup-dist.ts'], + // A throwaway home dir (and git global config) per test file, so nothing + // the suite runs can write to the developer's real one (#2275). + setupFiles: ['./__tests__/setup-home-sandbox.ts'], /** * Several MCP integration tests (mcp-daemon, mcp-initialize, mcp-ppid-watchdog, * mcp-roots) spawn `dist/bin/codegraph.js serve --mcp` with `process.execPath` diff --git a/vitest.workspace.mts b/vitest.workspace.mts index c37fae14ba..9abdc721c7 100644 --- a/vitest.workspace.mts +++ b/vitest.workspace.mts @@ -20,9 +20,9 @@ import { defineWorkspace } from 'vitest/config'; * browser builds of `web-tree-sitter` and friends, and the failures that * causes look nothing like their cause. * - * The engine project `extends` the shared base, so the env vars and Node guard - * in `vitest.config.mts` still apply to every engine test. The ui project does - * not — see the note on it. + * The engine project `extends` the shared base, so the env vars, Node guard and + * home-dir sandbox in `vitest.config.mts` still apply to every engine test. The + * ui project does not — see the note on it. */ export default defineWorkspace([ { From 273e5cdc4c2ce60a5e99b134c2d05681a8de6cd7 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 06:19:41 +0000 Subject: [PATCH 4/8] fix(upgrade): a Windows upgrade no longer breaks the install while CodeGraph is running (#2185) (#2294) * fix(upgrade): a Windows upgrade no longer breaks the install while CodeGraph is running (#2185) On Windows, `codegraph upgrade` renamed only node.exe aside and then Copy-Item'd the new bundle over current\. Any open agent session runs a CodeGraph MCP server from current\, which keeps the native kernel loaded, so the copy failed on lib\kernel\codegraph-kernel.node halfway through, leaving no node.exe and a mix of versions that no codegraph command could repair. Re-running install.ps1 failed the same way, because it Remove-Items current\. Both now share one PowerShell function (a test pins the two copies equal). The bundle is unpacked next to current\ (same volume), then swapped in file by file: every file being replaced or dropped is first renamed aside (Windows allows renaming a running exe or a loaded DLL, just not overwriting or deleting it), then the staged file is moved in. Any failure or interruption undoes every step. Renamed-aside files, including the node.exe.old-* left by earlier upgrades, are deleted on the next successful run once nothing holds them. The upgrade's exit code now tells whether the install was left unchanged or needs a reinstall, and the message says which. Co-Authored-By: Claude Opus 5.5 * docs(changelog): Windows upgrade with CodeGraph running (#2185) Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/upgrade.test.ts | 154 ++++++++++++++++++++++++++++++++-- install.ps1 | 105 ++++++++++++++++++++--- src/upgrade/index.ts | 172 +++++++++++++++++++++++++++++++------- 4 files changed, 382 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf27df4914..348a804c87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In tag-based CFML, calls written in tags are now linked: in ``, ``/``, ``, `` and `#…#` expressions. Before, only `` and `` code was read, so callers and impact found almost nothing in tag-based components. Functions wrapped in tags like `` or `` are now indexed too, and a `` is indexed as an interface that `implements` links to. Re-index CFML projects after upgrading. Thanks @HarryMuc for the report. (#2091) - On Windows, a project opened with a different drive-letter or folder-name case, like `d:\work\app` and `D:\Work\App`, now joins the CodeGraph background server that is already running for it. Before, each spelling started a server of its own that exited at once, so the session quietly served the graph by itself with no shared file watching or auto-sync, and `codegraph list` showed the project twice. Thanks @greatjackzhou. (#2278) - An agent session that lost its connection to CodeGraph's shared background server, or couldn't reach it at startup, no longer stops that server from running for the rest of the session. It reconnects on its own once the server can start, so other sessions on the same project get auto-sync back instead of running read-only. Thanks @ijbranch for the report. (#2277) +- On Windows, `codegraph upgrade` no longer breaks the install while an agent session is using CodeGraph. It used to stop partway and leave CodeGraph unable to start; it now swaps the new files in with CodeGraph running and puts the previous version back if anything fails, and re-running the PowerShell installer works the same way. If an earlier upgrade already broke your install, re-run `irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex` to repair it. Thanks @tippmar-nr for the report. (#2185) ## [1.6.1] - 2026-09-29 diff --git a/__tests__/upgrade.test.ts b/__tests__/upgrade.test.ts index e1b05e4d40..ad46a17ad4 100644 --- a/__tests__/upgrade.test.ts +++ b/__tests__/upgrade.test.ts @@ -16,6 +16,8 @@ import { verifyResolvedVersion, defaultWirePromptHook, buildWindowsUpgradeScript, + WINDOWS_SWAP_FUNCTION, + WINDOWS_UPGRADE_DAMAGED, NPM_PACKAGE, type InstallMethod, type UpgradeDeps, @@ -211,17 +213,124 @@ describe('version helpers', () => { expect(a).toContain('codegraph index -f'); }); - it('buildWindowsUpgradeScript targets the right asset per arch and renames-not-deletes the exe', () => { + it('buildWindowsUpgradeScript targets the right asset per arch', () => { const arm = buildWindowsUpgradeScript('C:\\cg\\current', 'v1.2.3', 'arm64'); expect(arm).toContain('releases/download/v1.2.3/codegraph-win32-arm64.zip'); expect(arm).toContain("$dest='C:\\cg\\current'"); - expect(arm).toContain('Rename-Item'); // never Remove-Item on the locked exe - expect(arm).not.toMatch(/Remove-Item[^;]*\$dest'?\s*;/); // doesn't delete current\ + expect(arm).toContain("Join-Path $stage 'codegraph-win32-arm64'"); const x64 = buildWindowsUpgradeScript('C:\\cg\\current', 'v1.2.3', 'x64'); expect(x64).toContain('codegraph-win32-x64.zip'); }); }); +// --------------------------------------------------------------------------- +// Windows file swap (#2185) — a running CodeGraph process (an agent session's +// MCP server) keeps node.exe and the native kernel locked: they can be renamed +// but not overwritten or deleted. The upgrade used to rename only node.exe and +// then Copy-Item over the rest, so the locked kernel failed the copy halfway +// and left the install with no node.exe. PowerShell can't run on the macOS +// test host, so these pin the script's shape; the behavior is validated on a +// real Windows machine. +// --------------------------------------------------------------------------- + +describe('windows bundle swap script (#2185)', () => { + const INSTALL_PS1 = path.join(__dirname, '..', 'install.ps1'); + const installPs1 = () => fs.readFileSync(INSTALL_PS1, 'utf-8').replace(/\r\n/g, '\n'); + const script = () => buildWindowsUpgradeScript('C:\\Users\\me\\AppData\\Local\\codegraph\\current', 'v1.6.2', 'x64'); + /** The swap function's own statements, comments dropped. */ + const swapCode = () => WINDOWS_SWAP_FUNCTION.split('\n').filter((l) => !l.trim().startsWith('#')).join('\n'); + + it('install.ps1 carries the same swap function, verbatim', () => { + const m = /# >>> Install-CodeGraphFiles[^\n]*\n([\s\S]*?)# <<< Install-CodeGraphFiles/.exec(installPs1()); + expect(m, 'install.ps1 lost its Install-CodeGraphFiles markers').not.toBeNull(); + expect(m![1]).toBe(WINDOWS_SWAP_FUNCTION); + }); + + it('never copies over or deletes current\\ in place', () => { + for (const text of [script(), installPs1()]) { + expect(text).not.toMatch(/Copy-Item/); + expect(text).not.toMatch(/Remove-Item[^\n]*\$dest\b/); + expect(text).not.toMatch(/Expand-Archive[^\n]*\$dest\b/); + } + }); + + it('unpacks next to current\\ (same volume), not into %TEMP%, then swaps', () => { + const s = script(); + expect(s).toContain(`$stage=Join-Path (Split-Path -Parent $dest) ('.staging-'`); + expect(s).toMatch(/Expand-Archive -Path \$zip -DestinationPath \$stage/); + expect(s.indexOf('Expand-Archive')).toBeLessThan(s.indexOf('Install-CodeGraphFiles $(')); + // install.ps1 stages next to current\ too. + expect(installPs1()).toContain(`$stage = Join-Path $installDir ('.staging-'`); + expect(installPs1()).toMatch(/Install-CodeGraphFiles \$\(.*\) \$dest$/m); + }); + + it('renames every replaced file aside before moving the staged file in', () => { + const code = swapCode(); + // Replaced files and files the new version drops are both renamed aside… + expect(code.match(/Move-Logged \$at "\$at\.old-\$token"/g)).toHaveLength(2); + // …and the aside-rename of a target comes before the staged file moves in. + const aside = code.indexOf('if ([IO.File]::Exists($at)) { Move-Logged $at "$at.old-$token" }'); + const moveIn = code.indexOf('Move-Logged ($stageDir + $rel) $at'); + expect(aside).toBeGreaterThan(0); + expect(moveIn).toBeGreaterThan(aside); + // No in-place overwrite or delete of a live file during the swap. + expect(code).not.toMatch(/\[IO\.File\]::(Copy|Replace)\(/); + expect(code).not.toMatch(/Move-Item|Rename-Item/); + }); + + it('rolls back on failure and on interruption, and says whether the install still works', () => { + const code = swapCode(); + expect(code).toMatch(/catch \{[\s\S]*\$lost = Undo-Logged[\s\S]*\} finally \{\s*if \(-not \$done\) \{ \[void\]\(Undo-Logged\) \}/); + // Undo walks the log backwards, moving each file back to where it was. + expect(code).toContain('for ($n = $undo.Count - 1; $n -ge 0; $n--)'); + expect(code).toContain('[IO.File]::Move($u[1], $u[0])'); + expect(code).toContain('Nothing was changed: the existing install still works.'); + expect(code).toContain("$e.Data['codegraphDamaged'] = [bool]$lost"); + // The upgrade script maps that flag to the exit code runUpgrade reads. + expect(script()).toContain(`$code=if($_.Exception.Data['codegraphDamaged']){${WINDOWS_UPGRADE_DAMAGED}}else{1}`); + expect(script().trimEnd().endsWith('exit $code')).toBe(true); + }); + + it('refuses a download that is not a bundle before touching current\\', () => { + const code = swapCode(); + expect(code).toContain("foreach ($need in 'node.exe', 'bin\\codegraph.cmd')"); + expect(code.indexOf("foreach ($need in")).toBeLessThan(code.indexOf('try {')); + }); + + it('cleans up renamed-aside leftovers, including ones from earlier upgrades', () => { + const m = /\$asideName = '([^']+)'/.exec(WINDOWS_SWAP_FUNCTION); + expect(m).not.toBeNull(); + const aside = new RegExp(m![1]!, 'i'); // PowerShell -match is case-insensitive + // The pre-fix upgrade left node.exe.old-<32-hex guid>; this one uses 8 hex. + expect(aside.test('node.exe.old-0123456789abcdef0123456789ABCDEF')).toBe(true); + expect(aside.test('codegraph-kernel.node.old-deadbeef')).toBe(true); + for (const shipped of ['node.exe', 'codegraph-kernel.node', 'old-deadbeef.js', 'x.old-1234567', 'a.old-deadbeef.js']) { + expect(aside.test(shipped), shipped).toBe(false); + } + // No file a bundle actually ships looks like a leftover. + const walk = (dir: string): string[] => + fs.readdirSync(dir, { withFileTypes: true }).flatMap((e) => (e.isDirectory() ? walk(path.join(dir, e.name)) : [e.name])); + const distDir = path.join(__dirname, '..', 'dist'); + if (fs.existsSync(distDir)) expect(walk(distDir).filter((n) => aside.test(n))).toEqual([]); + // Deleted only after a successful swap, and a still-locked one is skipped. + const code = swapCode(); + expect(code.indexOf('[IO.File]::Delete($f.FullName)')).toBeGreaterThan(code.indexOf('$done = $true')); + expect(code).toContain('try { [IO.File]::Delete($f.FullName) } catch {}'); + }); + + it('quotes the install path for PowerShell', () => { + const s = buildWindowsUpgradeScript("C:\\Users\\o'brien\\codegraph\\current", 'v1.6.2', 'x64'); + expect(s).toContain("$dest='C:\\Users\\o''brien\\codegraph\\current'"); + }); + + it('fits a Windows command line even for a long install path', () => { + const root = `C:\\${'very-long-directory-name\\'.repeat(8)}codegraph\\current`; + const encoded = Buffer.from(buildWindowsUpgradeScript(root, 'v10.20.30', 'arm64'), 'utf16le').toString('base64'); + // CreateProcess caps the whole command line at 32,767 characters. + expect(encoded.length).toBeLessThan(24_000); + }); +}); + // --------------------------------------------------------------------------- // runUpgrade orchestration — mocked side-effects // --------------------------------------------------------------------------- @@ -335,11 +444,10 @@ describe('runUpgrade', () => { expect(calls.runs).toHaveLength(1); expect(calls.runs[0].cmd).toBe('powershell.exe'); const decoded = decodeEncodedCommand(calls.runs[0].args); - // Downloads the right asset, renames the locked exe aside, copies over current\. + // Downloads the right asset and swaps it in with the shared rename-aside function. expect(decoded).toContain('releases/download/v0.9.9/codegraph-win32-'); - expect(decoded).toContain('Rename-Item'); - expect(decoded).toContain('node.exe.old-'); - expect(decoded).toContain('Copy-Item'); + expect(decoded).toContain(WINDOWS_SWAP_FUNCTION); + expect(decoded).toMatch(/^\s*Install-CodeGraphFiles .* \$dest$/m); }); it('windows bundle: a non-zero installer exit is a failure', async () => { @@ -353,7 +461,37 @@ describe('runUpgrade', () => { ); const code = await runUpgrade({}, deps); expect(code).toBe(1); - expect(calls.errors.join('\n')).toMatch(/exited with code/i); + // Exit 1 is the script's "failed, install unchanged" code (#2185). + expect(calls.errors.join('\n')).toMatch(/did not complete; your existing install was left as it was/i); + }); + + it('windows bundle: an incomplete rollback points at the reinstall command', async () => { + const { deps, calls } = makeDeps( + { + method: { kind: 'bundle', os: 'windows', bundleRoot: 'C:/x/codegraph/current', installDir: 'C:/x/codegraph' }, + currentVersion: '0.9.8', + platform: 'win32', + }, + WINDOWS_UPGRADE_DAMAGED + ); + const code = await runUpgrade({}, deps); + expect(code).toBe(1); + expect(calls.errors.join('\n')).toMatch(/could not be put back/i); + expect(calls.logs.join('\n')).toContain('install.ps1 | iex'); + expect(calls.runs).toHaveLength(1); // no post-upgrade refresh/probe after a failure + }); + + it('windows bundle: any other exit code is reported as-is', async () => { + const { deps, calls } = makeDeps( + { + method: { kind: 'bundle', os: 'windows', bundleRoot: 'C:/x/codegraph/current', installDir: 'C:/x/codegraph' }, + currentVersion: '0.9.8', + platform: 'win32', + }, + -1 + ); + expect(await runUpgrade({}, deps)).toBe(1); + expect(calls.errors.join('\n')).toMatch(/exited with code -1/i); }); it('npm global: shells out to npm install -g @pkg@latest', async () => { diff --git a/install.ps1 b/install.ps1 index 7c0b523caf..a079205b46 100644 --- a/install.ps1 +++ b/install.ps1 @@ -5,7 +5,8 @@ # # irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex # -# Upgrade with `codegraph upgrade` (or just re-run this). To uninstall: remove +# Upgrade with `codegraph upgrade` (or just re-run this -- safe even while agent +# sessions are running CodeGraph). To uninstall: remove # $env:LOCALAPPDATA\codegraph and drop its \current\bin entry from your user PATH. # # Environment: @@ -13,6 +14,81 @@ # CODEGRAPH_INSTALL_DIR install location (default: %LOCALAPPDATA%\codegraph) $ErrorActionPreference = 'Stop' + +# >>> Install-CodeGraphFiles -- keep identical to WINDOWS_SWAP_FUNCTION in src/upgrade/index.ts +function Install-CodeGraphFiles([string]$Stage, [string]$Dest) { + # Move an unpacked bundle into $Dest. Windows can't overwrite or delete a + # running node.exe or a loaded .node addon, but it can rename one, so every + # file being replaced (or dropped by the new version) is first renamed aside + # to .old-. Any failure puts every file back, so the install is + # never left half-replaced or without its node.exe. + $ErrorActionPreference = 'Stop' + $stageDir = (Resolve-Path -LiteralPath $Stage).ProviderPath.TrimEnd('\') + foreach ($need in 'node.exe', 'bin\codegraph.cmd') { + if (-not (Test-Path -LiteralPath (Join-Path $stageDir $need))) { throw "The CodeGraph download is incomplete (no $need); nothing was changed." } + } + $token = [guid]::NewGuid().ToString('N').Substring(0, 8) + $asideName = '\.old-[0-9a-f]{8,32}$' + $files = @{}; $dirs = @{} + foreach ($i in @(Get-ChildItem -LiteralPath $stageDir -Recurse -Force)) { + $rel = $i.FullName.Substring($stageDir.Length) + if ($i.PSIsContainer) { $dirs[$rel] = $true } else { $files[$rel] = $true } + } + $undo = New-Object System.Collections.ArrayList + function Move-Logged([string]$From, [string]$To) { [IO.File]::Move($From, $To); [void]$undo.Add(@($From, $To)) } + function Undo-Logged { + $lost = 0 + for ($n = $undo.Count - 1; $n -ge 0; $n--) { + $u = $undo[$n] + try { if ($u.Count -eq 2) { [IO.File]::Move($u[1], $u[0]) } else { [IO.Directory]::Delete($u[0]) } } catch { if ($u.Count -eq 2) { $lost++ } } + } + $undo.Clear() + $lost + } + $done = $false; $at = $Dest + try { + if (-not (Test-Path -LiteralPath $Dest)) { [void][IO.Directory]::CreateDirectory($Dest); [void]$undo.Add(@($Dest)) } + $destDir = (Resolve-Path -LiteralPath $Dest).ProviderPath.TrimEnd('\') + foreach ($f in @(Get-ChildItem -LiteralPath $destDir -Recurse -Force -File)) { + if (-not $files.ContainsKey($f.FullName.Substring($destDir.Length)) -and $f.Name -notmatch $asideName) { + $at = $f.FullName; Move-Logged $at "$at.old-$token" + } + } + foreach ($rel in @($dirs.Keys | Sort-Object Length)) { + $at = $destDir + $rel + if (-not [IO.Directory]::Exists($at)) { [void][IO.Directory]::CreateDirectory($at); [void]$undo.Add(@($at)) } + } + foreach ($rel in @($files.Keys)) { + $at = $destDir + $rel + if ([IO.File]::Exists($at)) { Move-Logged $at "$at.old-$token" } + Move-Logged ($stageDir + $rel) $at + } + $done = $true + } catch { + $x = $_.Exception; while ($x.InnerException) { $x = $x.InnerException } + $lost = Undo-Logged + $msg = "Could not replace $at ($($x.Message))." + if ($lost) { + $msg += " $lost file(s) could not be put back, so the install may not start. Close your agent sessions and any running codegraph commands, then reinstall: irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex" + } else { + $msg += " Nothing was changed: the existing install still works. If another program has CodeGraph's files open, close your agent sessions (they run the CodeGraph MCP server) and any running codegraph commands, then try again." + } + $e = New-Object System.Exception($msg); $e.Data['codegraphDamaged'] = [bool]$lost; throw $e + } finally { + # Interrupted (Ctrl+C) without reaching catch: still put everything back. + if (-not $done) { [void](Undo-Logged) } + } + # Delete what this run and earlier ones renamed aside. A file a running + # process still holds can't be deleted yet; the next install retries it. + foreach ($f in @(Get-ChildItem -LiteralPath $destDir -Recurse -Force -File -ErrorAction SilentlyContinue)) { + if ($f.Name -match $asideName) { try { [IO.File]::Delete($f.FullName) } catch {} } + } + foreach ($d in @(Get-ChildItem -LiteralPath $destDir -Recurse -Force -Directory -ErrorAction SilentlyContinue | Sort-Object { $_.FullName.Length } -Descending)) { + if (-not $dirs.ContainsKey($d.FullName.Substring($destDir.Length))) { try { [IO.Directory]::Delete($d.FullName) } catch {} } + } +} +# <<< Install-CodeGraphFiles + $repo = 'colbymchenry/codegraph' $installDir = if ($env:CODEGRAPH_INSTALL_DIR) { $env:CODEGRAPH_INSTALL_DIR } else { Join-Path $env:LOCALAPPDATA 'codegraph' } @@ -27,25 +103,28 @@ if (-not $version) { } if (-not $version) { throw "codegraph: could not resolve latest version; set CODEGRAPH_VERSION." } -# 3. Download + extract the bundle into a stable 'current' dir (overwritten on upgrade). +# 3. Download the bundle and unpack it next to the stable 'current' dir (same +# volume, so moving it in is renames, not copies), then move it into 'current'. +# 'current' is never deleted wholesale: a CodeGraph process that is still +# running (an agent session's MCP server) holds node.exe and the native kernel, +# which can't be deleted -- Install-CodeGraphFiles renames them aside instead. $url = "https://github.com/$repo/releases/download/$version/codegraph-$target.zip" Write-Host "Installing CodeGraph $version ($target)..." $tmp = Join-Path $env:TEMP ("cg-" + [guid]::NewGuid().ToString()) New-Item -ItemType Directory -Force -Path $tmp | Out-Null $zip = Join-Path $tmp 'cg.zip' -Invoke-WebRequest -Uri $url -OutFile $zip - $dest = Join-Path $installDir 'current' -if (Test-Path $dest) { Remove-Item -Recurse -Force $dest } -New-Item -ItemType Directory -Force -Path $dest | Out-Null -Expand-Archive -Path $zip -DestinationPath $dest -Force -# Archives contain a top-level codegraph-\ dir; flatten it. -$inner = Join-Path $dest "codegraph-$target" -if (Test-Path $inner) { - Get-ChildItem -Force $inner | Move-Item -Destination $dest -Force - Remove-Item -Recurse -Force $inner +$stage = Join-Path $installDir ('.staging-' + [guid]::NewGuid().ToString('N').Substring(0, 8)) +try { + Invoke-WebRequest -Uri $url -OutFile $zip + New-Item -ItemType Directory -Force -Path $stage | Out-Null + Expand-Archive -Path $zip -DestinationPath $stage -Force + # Archives contain a top-level codegraph-\ dir. + $inner = Join-Path $stage "codegraph-$target" + Install-CodeGraphFiles $(if (Test-Path $inner) { $inner } else { $stage }) $dest +} finally { + Remove-Item -LiteralPath $stage, $tmp -Recurse -Force -ErrorAction SilentlyContinue } -Remove-Item -Recurse -Force $tmp # 4. Put the launcher dir on the user's PATH. $binDir = Join-Path $dest 'bin' diff --git a/src/upgrade/index.ts b/src/upgrade/index.ts index 76354378a8..0e088e9982 100644 --- a/src/upgrade/index.ts +++ b/src/upgrade/index.ts @@ -17,11 +17,12 @@ * vendored `node` binary and a `bin/codegraph` launcher next to its `lib/`, so * we can recognize it from the running file's path without a marker file. * - * Windows wrinkle: a running `node.exe` is locked and can't be deleted, so the - * bundle's `current\` dir can't be overwritten in place by the process doing - * the upgrade. We therefore spawn a DETACHED helper that waits for this - * process to exit (releasing the lock), then runs `install.ps1`. This is the - * conventional Windows self-update dance (rustup/nvm-windows do the same). + * Windows wrinkle: a running `node.exe` and a loaded `.node` addon are locked — + * they can't be overwritten or deleted, by this process or by the MCP servers + * of open agent sessions. They CAN be renamed, so the Windows upgrade unpacks + * the new bundle next to `current\` and swaps it in file by file, renaming each + * replaced file aside and rolling every step back on failure (see + * `WINDOWS_SWAP_FUNCTION`, shared verbatim with `install.ps1`). */ import * as fs from 'fs'; @@ -34,6 +35,7 @@ export const REPO = 'colbymchenry/codegraph'; export const NPM_PACKAGE = '@colbymchenry/codegraph'; const RAW_BASE = `https://raw.githubusercontent.com/${REPO}/main`; export const INSTALL_SH_URL = `${RAW_BASE}/install.sh`; +export const INSTALL_PS1_URL = `${RAW_BASE}/install.ps1`; // --------------------------------------------------------------------------- // Install-method detection (pure — fully unit-testable via injected probes) @@ -578,36 +580,140 @@ function upgradeUnixBundle( return 0; } +/** + * The PowerShell function that moves an unpacked Windows bundle into the + * install's `current\` dir. Shared VERBATIM with `install.ps1` (a test pins the + * two copies equal), so a first install, a re-run of the installer, and + * `codegraph upgrade` all replace files the same way. + * + * Why file-by-file renames (#2185): every open agent session runs a CodeGraph + * MCP server from `current\`, which keeps `node.exe` and the native kernel + * (`lib\kernel\codegraph-kernel.node`) locked. Windows refuses to overwrite or + * delete a running exe or a loaded DLL but does let it be renamed. The old + * upgrade renamed only `node.exe` and then `Copy-Item`ed over the rest, so the + * locked kernel failed the copy halfway — leaving no `node.exe` and a mix of + * versions, which no `codegraph` command could repair. Now each file being + * replaced (or dropped by the new version) is renamed aside to + * `.old-` before the staged file is moved in; any failure undoes + * every step, and the renamed-aside files are deleted on the next successful + * run once nothing holds them. + */ +export const WINDOWS_SWAP_FUNCTION = String.raw`function Install-CodeGraphFiles([string]$Stage, [string]$Dest) { + # Move an unpacked bundle into $Dest. Windows can't overwrite or delete a + # running node.exe or a loaded .node addon, but it can rename one, so every + # file being replaced (or dropped by the new version) is first renamed aside + # to .old-. Any failure puts every file back, so the install is + # never left half-replaced or without its node.exe. + $ErrorActionPreference = 'Stop' + $stageDir = (Resolve-Path -LiteralPath $Stage).ProviderPath.TrimEnd('\') + foreach ($need in 'node.exe', 'bin\codegraph.cmd') { + if (-not (Test-Path -LiteralPath (Join-Path $stageDir $need))) { throw "The CodeGraph download is incomplete (no $need); nothing was changed." } + } + $token = [guid]::NewGuid().ToString('N').Substring(0, 8) + $asideName = '\.old-[0-9a-f]{8,32}$' + $files = @{}; $dirs = @{} + foreach ($i in @(Get-ChildItem -LiteralPath $stageDir -Recurse -Force)) { + $rel = $i.FullName.Substring($stageDir.Length) + if ($i.PSIsContainer) { $dirs[$rel] = $true } else { $files[$rel] = $true } + } + $undo = New-Object System.Collections.ArrayList + function Move-Logged([string]$From, [string]$To) { [IO.File]::Move($From, $To); [void]$undo.Add(@($From, $To)) } + function Undo-Logged { + $lost = 0 + for ($n = $undo.Count - 1; $n -ge 0; $n--) { + $u = $undo[$n] + try { if ($u.Count -eq 2) { [IO.File]::Move($u[1], $u[0]) } else { [IO.Directory]::Delete($u[0]) } } catch { if ($u.Count -eq 2) { $lost++ } } + } + $undo.Clear() + $lost + } + $done = $false; $at = $Dest + try { + if (-not (Test-Path -LiteralPath $Dest)) { [void][IO.Directory]::CreateDirectory($Dest); [void]$undo.Add(@($Dest)) } + $destDir = (Resolve-Path -LiteralPath $Dest).ProviderPath.TrimEnd('\') + foreach ($f in @(Get-ChildItem -LiteralPath $destDir -Recurse -Force -File)) { + if (-not $files.ContainsKey($f.FullName.Substring($destDir.Length)) -and $f.Name -notmatch $asideName) { + $at = $f.FullName; Move-Logged $at "$at.old-$token" + } + } + foreach ($rel in @($dirs.Keys | Sort-Object Length)) { + $at = $destDir + $rel + if (-not [IO.Directory]::Exists($at)) { [void][IO.Directory]::CreateDirectory($at); [void]$undo.Add(@($at)) } + } + foreach ($rel in @($files.Keys)) { + $at = $destDir + $rel + if ([IO.File]::Exists($at)) { Move-Logged $at "$at.old-$token" } + Move-Logged ($stageDir + $rel) $at + } + $done = $true + } catch { + $x = $_.Exception; while ($x.InnerException) { $x = $x.InnerException } + $lost = Undo-Logged + $msg = "Could not replace $at ($($x.Message))." + if ($lost) { + $msg += " $lost file(s) could not be put back, so the install may not start. Close your agent sessions and any running codegraph commands, then reinstall: irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex" + } else { + $msg += " Nothing was changed: the existing install still works. If another program has CodeGraph's files open, close your agent sessions (they run the CodeGraph MCP server) and any running codegraph commands, then try again." + } + $e = New-Object System.Exception($msg); $e.Data['codegraphDamaged'] = [bool]$lost; throw $e + } finally { + # Interrupted (Ctrl+C) without reaching catch: still put everything back. + if (-not $done) { [void](Undo-Logged) } + } + # Delete what this run and earlier ones renamed aside. A file a running + # process still holds can't be deleted yet; the next install retries it. + foreach ($f in @(Get-ChildItem -LiteralPath $destDir -Recurse -Force -File -ErrorAction SilentlyContinue)) { + if ($f.Name -match $asideName) { try { [IO.File]::Delete($f.FullName) } catch {} } + } + foreach ($d in @(Get-ChildItem -LiteralPath $destDir -Recurse -Force -Directory -ErrorAction SilentlyContinue | Sort-Object { $_.FullName.Length } -Descending)) { + if (-not $dirs.ContainsKey($d.FullName.Substring($destDir.Length))) { try { [IO.Directory]::Delete($d.FullName) } catch {} } + } +} +`; + +/** Single-quote a value for PowerShell (a `'` inside is doubled). */ +function psQuote(value: string): string { + return `'${value.replace(/'/g, "''")}'`; +} + +/** Exit code of the upgrade script when files could not all be put back. */ +export const WINDOWS_UPGRADE_DAMAGED = 2; + /** Build the in-place Windows upgrade script (exported for unit-testing). */ export function buildWindowsUpgradeScript(bundleRoot: string, version: string, arch: string): string { const target = `win32-${arch}`; const url = `https://github.com/${REPO}/releases/download/${version}/codegraph-${target}.zip`; - // Windows can't DELETE a running exe but CAN rename it, so we upgrade IN - // PLACE: download → rename the locked node.exe aside → extract the new bundle - // over current\. Synchronous, no detached helper (which dies under SSH/job - // objects and has worse UX). The running process keeps its renamed node.exe - // mapped; the NEXT `codegraph` invocation uses the new one. We can't reuse - // install.ps1 here — it `Remove-Item`s current\, which fails on the locked exe. + // Synchronous, no detached helper (which dies under SSH/job objects and has + // worse UX). The bundle is unpacked into a sibling of current\ — the same + // volume, so the swap is renames, never a half-finished copy — and nothing + // in current\ changes until it is fully unpacked. The running process keeps + // its renamed node.exe mapped; the NEXT `codegraph` invocation uses the new + // one. Exit codes: 0 installed, 1 failed with the install unchanged, + // WINDOWS_UPGRADE_DAMAGED when the rollback could not restore every file. return [ `$ErrorActionPreference='Stop'`, - `$dest='${bundleRoot}'`, - `$url='${url}'`, - `Write-Host "Downloading $url"`, + WINDOWS_SWAP_FUNCTION, + `$dest=${psQuote(bundleRoot)}`, + `$url=${psQuote(url)}`, `$tmp=Join-Path $env:TEMP ('cg-up-'+[guid]::NewGuid().ToString('N'))`, - `New-Item -ItemType Directory -Force -Path $tmp | Out-Null`, - `$zip=Join-Path $tmp 'cg.zip'`, - `Invoke-WebRequest -Uri $url -OutFile $zip`, - `$stage=Join-Path $tmp 'stage'`, - `Expand-Archive -Path $zip -DestinationPath $stage -Force`, - `$inner=Join-Path $stage 'codegraph-${target}'`, - `$src=if(Test-Path $inner){$inner}else{$stage}`, - `$node=Join-Path $dest 'node.exe'`, - `if(Test-Path $node){Rename-Item -Path $node -NewName ('node.exe.old-'+[guid]::NewGuid().ToString('N')) -Force}`, - `Copy-Item -Path (Join-Path $src '*') -Destination $dest -Recurse -Force`, - `Get-ChildItem -Path $dest -Filter 'node.exe.old-*' -ErrorAction SilentlyContinue | ForEach-Object { try { Remove-Item $_.FullName -Force -ErrorAction Stop } catch {} }`, - `Remove-Item -Recurse -Force $tmp -ErrorAction SilentlyContinue`, - `Write-Host "Installed CodeGraph ${version} to $dest"`, - ].join(';'); + `$stage=Join-Path (Split-Path -Parent $dest) ('.staging-'+[guid]::NewGuid().ToString('N').Substring(0,8))`, + `$code=0`, + `try {`, + ` Write-Host "Downloading $url"`, + ` New-Item -ItemType Directory -Force -Path $tmp | Out-Null`, + ` $zip=Join-Path $tmp 'cg.zip'`, + ` Invoke-WebRequest -Uri $url -OutFile $zip`, + ` Expand-Archive -Path $zip -DestinationPath $stage -Force`, + ` $inner=Join-Path $stage 'codegraph-${target}'`, + ` Install-CodeGraphFiles $(if(Test-Path $inner){$inner}else{$stage}) $dest`, + ` Write-Host "Installed CodeGraph ${version} to $dest"`, + `} catch {`, + ` [Console]::Error.WriteLine($_.Exception.Message)`, + ` $code=if($_.Exception.Data['codegraphDamaged']){${WINDOWS_UPGRADE_DAMAGED}}else{1}`, + `}`, + `Remove-Item -LiteralPath $stage,$tmp -Recurse -Force -ErrorAction SilentlyContinue`, + `exit $code`, + ].join('\n'); } function upgradeWindowsBundle( @@ -624,7 +730,15 @@ function upgradeWindowsBundle( deps.log(c.dim(`Downloading and installing ${latest}…`)); const code = deps.run('powershell.exe', ['-NoProfile', '-ExecutionPolicy', 'Bypass', '-EncodedCommand', encoded]); if (code !== 0) { - deps.error(`Installer exited with code ${code}.`); + // The script has already printed what went wrong and what to do. + if (code === WINDOWS_UPGRADE_DAMAGED) { + deps.error('The upgrade failed and some files could not be put back.'); + deps.log(c.dim(`Close your agent sessions and any running codegraph commands, then reinstall: irm ${INSTALL_PS1_URL} | iex`)); + } else if (code === 1) { + deps.error(`The upgrade to ${latest} did not complete; your existing install was left as it was.`); + } else { + deps.error(`Installer exited with code ${code}.`); + } return 1; } deps.log(''); From 8abebda02cbe893a393d60ecc4f231f18d194f4a Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 07:07:34 +0000 Subject: [PATCH 5/8] test(windows): two #1910 tests no longer time out on Windows (#2298) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oversize-file-not-read's drift test wrote a 1.4 MB `.js` made of one statement repeated 110k times. Windows Defender's script scan of that freshly written file takes about 54 s on its first read (the viewer's read in the test), so the test always timed out on Windows and its cleanup then hit EPERM on the still open file. A varied 2.4 MB `.js` and a real 2 MB bundle scan in 35 ms and 144 ms, so this is the fixture, not something users hit; the fixture now has a different statement on every line. The mpeg-ts git-path cases build a git repository and index it, the last one twice. At ~100 ms per git process on Windows that is 5–6 s, past vitest's 5 s default; the describe now allows 30 s. Co-authored-by: Claude Opus 5.5 --- __tests__/mpeg-ts-not-typescript.test.ts | 5 ++++- __tests__/oversize-file-not-read.test.ts | 13 +++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/__tests__/mpeg-ts-not-typescript.test.ts b/__tests__/mpeg-ts-not-typescript.test.ts index 5d44003d2e..dfc23a1950 100644 --- a/__tests__/mpeg-ts-not-typescript.test.ts +++ b/__tests__/mpeg-ts-not-typescript.test.ts @@ -215,7 +215,10 @@ describe('MPEG-TS video named .ts is skipped, real TypeScript is indexed (#1910) }); }); -describe('a video .ts never stays pending (#1910)', () => { +// Each case builds a git repository and indexes it, the last one twice over. On +// Windows every git process costs ~100 ms, which puts these cases at 5–6 s there, +// past vitest's 5 s default. +describe('a video .ts never stays pending (#1910)', { timeout: 30_000 }, () => { const dirs: string[] = []; afterEach(() => { for (const d of dirs.splice(0)) fs.rmSync(d, { recursive: true, force: true }); }); const gitProject = (): string => { diff --git a/__tests__/oversize-file-not-read.test.ts b/__tests__/oversize-file-not-read.test.ts index f32267f73f..fad014c375 100644 --- a/__tests__/oversize-file-not-read.test.ts +++ b/__tests__/oversize-file-not-read.test.ts @@ -89,8 +89,17 @@ describe('an unchanged file over the size limit is not reported as drifted (#191 dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-oversize-drift-')); fs.writeFileSync(path.join(dir, 'app.ts'), 'export function alpha() { return 1; }\n'); // 1.4 MB of ordinary text: over the index limit, under the viewer's 8 MB read cap. - const line = 'const x = 1;\n'; - fs.writeFileSync(path.join(dir, 'big.js'), line.repeat(Math.ceil((1.4 * 1024 * 1024) / line.length))); + // Each line differs: Windows Defender's script scan of a freshly written `.js` + // made of one statement repeated 110k times takes close to a minute on the + // first read, so the viewer's read below blew the test timeout there. Real + // code of the same size is scanned in milliseconds. + const line = 'const last = 0;\n'; + const lines: string[] = []; + for (let size = 0, i = 0; size < 1.4 * 1024 * 1024; i++) { + lines.push(`const x${i} = ${i};\n`); + size += lines[i]!.length; + } + fs.writeFileSync(path.join(dir, 'big.js'), lines.join('')); const cg = await CodeGraph.init(dir, { index: true }); try { const record = cg.getFiles().find((f) => f.path === 'big.js')!; From 91b9bba1336cb6fe33113b0df25c37ae07b078ca Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 08:48:01 +0000 Subject: [PATCH 6/8] fix(extraction): never terminate a parse worker while it is loading grammars (#2301) On a loaded Windows machine `codegraph init` / `index` sometimes died right after the parse phase with 0xC0000005 (STATUS_ACCESS_VIOLATION), no WER report and no output past the last parsed file. The parse pool's destroy() terminated every worker at once, and a short index often ends while a late-spawned worker is still compiling its grammar WASM; terminating a worker with that compile in flight crashes the process. Reproduced on a Windows 11 ARM64 VM without CodeGraph's pipeline: starting three parse workers with preloaded grammar bytes and terminating them mid-load crashed 5 of 720 child processes, terminating them only after grammars-loaded crashed 0 of 360. Under the real load (24 parallel `codegraph init` runs on 4 cores) main crashed after parsing in 8 of 1,200 runs; with this change 0 of 1,080 did. destroy() now waits for a worker that is still loading to report grammars-loaded or exit before terminating it, capped at 15 s so a wedged load can't hold teardown forever. Recycling only ever touches idle workers, and a hung parse is still killed by the hard timeout. Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/parse-pool.test.ts | 79 ++++++++++++++++++++++++++++++++++++ src/extraction/parse-pool.ts | 54 ++++++++++++++++++++++-- 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 348a804c87..f7b195d0d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - On Windows, a project opened with a different drive-letter or folder-name case, like `d:\work\app` and `D:\Work\App`, now joins the CodeGraph background server that is already running for it. Before, each spelling started a server of its own that exited at once, so the session quietly served the graph by itself with no shared file watching or auto-sync, and `codegraph list` showed the project twice. Thanks @greatjackzhou. (#2278) - An agent session that lost its connection to CodeGraph's shared background server, or couldn't reach it at startup, no longer stops that server from running for the rest of the session. It reconnects on its own once the server can start, so other sessions on the same project get auto-sync back instead of running read-only. Thanks @ijbranch for the report. (#2277) - On Windows, `codegraph upgrade` no longer breaks the install while an agent session is using CodeGraph. It used to stop partway and leave CodeGraph unable to start; it now swaps the new files in with CodeGraph running and puts the previous version back if anything fails, and re-running the PowerShell installer works the same way. If an earlier upgrade already broke your install, re-run `irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex` to repair it. Thanks @tippmar-nr for the report. (#2185) +- On Windows, `codegraph init`, `codegraph index` and the background server's syncs no longer sometimes crash right after parsing on a busy machine (exit code 3221225477, an access violation). Shutting down the parsing threads could stop one while it was still loading its language grammars, which took the whole process down; they now finish loading first. ## [1.6.1] - 2026-09-29 diff --git a/__tests__/parse-pool.test.ts b/__tests__/parse-pool.test.ts index 6211481a44..ac51b984b5 100644 --- a/__tests__/parse-pool.test.ts +++ b/__tests__/parse-pool.test.ts @@ -248,3 +248,82 @@ describe('ParseWorkerPool', () => { await expect(pool.requestParse(task('y.ts'))).rejects.toThrow(/destroyed/); }); }); + +/** + * A fake whose grammar load takes `loadMs` (or never finishes, with `null`), and + * which records when it finished loading and when it was terminated. + */ +class SlowLoadWorker implements ParsePoolWorker { + private msgCb?: (m: unknown) => void; + private exitCb?: (code: number) => void; + loadedAt: number | null = null; + terminatedAt: number | null = null; + constructor(private loadMs: number | null) {} + on(event: string, cb: (...args: any[]) => void): void { + if (event === 'message') this.msgCb = cb; + else if (event === 'exit') this.exitCb = cb; + } + postMessage(msg: unknown): void { + if ((msg as { type: string }).type !== 'load-grammars' || this.loadMs === null) return; + setTimeout(() => { + if (this.terminatedAt !== null) return; + this.loadedAt = performance.now(); + this.msgCb?.({ type: 'grammars-loaded' }); + }, this.loadMs); + } + exit(code: number): void { this.exitCb?.(code); } + terminate(): Promise { this.terminatedAt = performance.now(); return Promise.resolve(0); } +} + +describe('teardown never terminates a worker mid-load', () => { + // Terminating a worker whose grammar WASM is still compiling can crash the + // process with an access violation (0xC0000005) on Windows; a pool torn down + // right after a short index often has a late-spawned worker in that state. + const poolOf = (worker: SlowLoadWorker, loadSettleMs?: number) => + new ParseWorkerPool({ languages: ['typescript'] as Language[], size: 1, createWorker: () => worker, loadSettleMs }); + + it('waits for a loading worker to finish before terminating it', async () => { + const w = new SlowLoadWorker(120); + const pool = poolOf(w); + await pool.destroy(); + expect(w.loadedAt).not.toBeNull(); + expect(w.terminatedAt).not.toBeNull(); + expect(w.terminatedAt!).toBeGreaterThanOrEqual(w.loadedAt!); + }); + + it('terminates a worker that is already loaded right away', async () => { + const w = new SlowLoadWorker(0); + const pool = poolOf(w); + await sleep(20); + expect(w.loadedAt).not.toBeNull(); + const t0 = performance.now(); + await pool.destroy(); + expect(w.terminatedAt! - t0).toBeLessThan(50); + }); + + it('does not wait past the cap on a load that never finishes', async () => { + const w = new SlowLoadWorker(null); + const pool = poolOf(w, 80); + const t0 = performance.now(); + await pool.destroy(); + expect(w.terminatedAt).not.toBeNull(); + expect(w.terminatedAt! - t0).toBeGreaterThanOrEqual(70); + expect(w.terminatedAt! - t0).toBeLessThan(1000); + }); + + it('does not wait on a worker that died while loading', async () => { + const w = new SlowLoadWorker(null); + // The pool respawns a crashed worker; the replacement loads normally. The + // default 15s cap stays, so waiting on the dead one would time the test out. + const workers = [w]; + const pool = new ParseWorkerPool({ + languages: ['typescript'] as Language[], size: 1, + createWorker: () => workers.shift() ?? new SlowLoadWorker(0), + }); + w.exit(1); + await sleep(20); + const t0 = performance.now(); + await pool.destroy(); + expect(performance.now() - t0).toBeLessThan(200); + }); +}); diff --git a/src/extraction/parse-pool.ts b/src/extraction/parse-pool.ts index 9b2b100e4e..62967a3dfb 100644 --- a/src/extraction/parse-pool.ts +++ b/src/extraction/parse-pool.ts @@ -75,6 +75,16 @@ const MAX_SCALED_PARSE_TIMEOUT_MS = 20_000; * only a worker that stays silent the whole window is treated as hung. */ const HARD_KILL_MULTIPLIER = 3; +/** + * How long `destroy()` waits for a worker that is still loading its grammars + * before terminating it anyway. Terminating a worker whose WebAssembly + * compiles are still in flight can take the whole process down with an access + * violation (0xC0000005, seen on Windows): a pool torn down right after a short + * index often has a late-spawned worker in exactly that state. A load finishes + * in well under a second normally and a few seconds under heavy load; the cap + * only bounds a load that is wedged. + */ +const GRAMMAR_LOAD_SETTLE_MS = 15_000; /** * Max workers cold-starting at once. A worker's cold start is heavy (module load * + grammar WASM compile); starting the whole pool simultaneously thrashes CPU. @@ -169,6 +179,8 @@ export interface ParseWorkerPoolOptions { parseTimeoutMs?: number; /** Worker factory (tests inject a fake). Defaults to a real `worker_threads` Worker. */ createWorker?: () => ParsePoolWorker; + /** How long destroy() waits on a worker still loading grammars (tests shorten it). Default 15s. */ + loadSettleMs?: number; /** Optional verbose logger (the orchestrator's `[worker] …` logger). */ log?: (msg: string) => void; /** @@ -190,6 +202,10 @@ export class ParseWorkerPool { // Spawned but not yet 'grammars-loaded'. Growth counts these so a single first // parse doesn't spawn the whole pool before the eager worker reports ready. private pending = new Set(); + // Each worker's grammar load, settled when it reports 'grammars-loaded' or + // goes away — what destroy() waits on before terminating it (see + // GRAMMAR_LOAD_SETTLE_MS). + private loads = new Map; settle: () => void }>(); private parseCounts = new Map(); private nextId = 1; private totalCrashes = 0; @@ -200,6 +216,7 @@ export class ParseWorkerPool { private readonly recycleInterval: number; private readonly parseTimeoutMs: number; private readonly createWorker: () => ParsePoolWorker; + private readonly loadSettleMs: number; private readonly log: (msg: string) => void; private readonly grammarBuffers?: Record; @@ -209,6 +226,7 @@ export class ParseWorkerPool { this.maxSize = Math.max(1, Math.min(opts.size, MAX_PARSE_POOL_SIZE)); this.recycleInterval = opts.recycleInterval ?? DEFAULT_RECYCLE_INTERVAL; this.parseTimeoutMs = opts.parseTimeoutMs ?? DEFAULT_PARSE_TIMEOUT_MS; + this.loadSettleMs = opts.loadSettleMs ?? GRAMMAR_LOAD_SETTLE_MS; this.log = opts.log ?? (() => {}); if (opts.createWorker) { this.createWorker = opts.createWorker; @@ -278,9 +296,12 @@ export class ParseWorkerPool { this.workers.add(w); this.pending.add(w); this.parseCounts.set(w, 0); + let settle!: () => void; + const settled = new Promise((resolve) => { settle = resolve; }); + this.loads.set(w, { settled, settle }); w.on('message', (m) => this.onMessage(w, (m ?? {}) as ParseWorkerMessage)); - w.on('error', (e) => this.onWorkerGone(w, `Worker error: ${e?.message ?? 'unknown'}`)); - w.on('exit', (code) => { if (code !== 0) this.onWorkerGone(w, `Worker exited with code ${code}`); }); + w.on('error', (e) => { this.loadSettled(w); this.onWorkerGone(w, `Worker error: ${e?.message ?? 'unknown'}`); }); + w.on('exit', (code) => { this.loadSettled(w); if (code !== 0) this.onWorkerGone(w, `Worker exited with code ${code}`); }); // Load grammars; the worker replies 'grammars-loaded' and only then is idle. // Pre-read WASM bytes (when the orchestrator provided them) make this a // memory load instead of a per-spawn disk read. @@ -289,6 +310,7 @@ export class ParseWorkerPool { private onMessage(w: ParsePoolWorker, m: ParseWorkerMessage): void { if (m.type === 'grammars-loaded') { + this.loadSettled(w); if (!this.workers.has(w)) return; // recycled/destroyed before ready this.pending.delete(w); this.idle.push(w); @@ -351,6 +373,32 @@ export class ParseWorkerPool { if (this.healthy && !this.destroyed) this.spawnOne(); } + /** The worker's grammar load is over: it reported ready, errored or exited. */ + private loadSettled(w: ParsePoolWorker): void { + const load = this.loads.get(w); + if (!load) return; + this.loads.delete(w); + load.settle(); + } + + /** + * Terminate a worker, but not while it is still loading grammars: wait for + * the load to finish (or the worker to go away), up to `loadSettleMs` + * (GRAMMAR_LOAD_SETTLE_MS — see that constant for the crash this avoids). + */ + private async terminateSettled(w: ParsePoolWorker): Promise { + const load = this.loads.get(w); + if (load) { + let cap: NodeJS.Timeout | undefined; + await Promise.race([ + load.settled, + new Promise((resolve) => { cap = setTimeout(resolve, this.loadSettleMs); cap.unref?.(); }), + ]); + clearTimeout(cap); + } + try { await w.terminate(); } catch { /* already gone */ } + } + private removeWorker(w: ParsePoolWorker): void { this.workers.delete(w); this.pending.delete(w); @@ -471,6 +519,6 @@ export class ParseWorkerPool { } this.inflight.clear(); this.queue = []; - await Promise.all(ws.map((w) => Promise.resolve(w.terminate()).catch(() => { /* already gone */ }))); + await Promise.all(ws.map((w) => this.terminateSettled(w))); } } From e0f9c95e2f7e5b8752b5b1bbd1362842c44b1ff3 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 10:20:23 +0000 Subject: [PATCH 7/8] fix(resolution): a busy machine doesn't boot the resolver pool for a small project (#2303) The resolve loop boots the worker pool mid-run when the remaining refs' projected sequential settle (observed per-ref rate x refs left) clears 400 ms. Load inflates the observed rate, so on a loaded 4-core Windows VM a ten-file project (19 refs) projected 506 ms for its last 11 and booted three workers in 122 of 240 runs under 24 parallel inits (0 of 80 at 8). The pool only fans out batches of 1,000 refs, so it resolved nothing; synthesis then waited out its boot (dedupe-merge 6.3 s vs a 6.5 s boot) and its three extra database connections pushed the VM's free commit to 11 MB. Adaptive engagement now also needs 20,000 refs left. tokio-class Rust repos (~55k refs at ~36 us) still engage; a smaller remainder never repays a seconds-long boot. With the floor, the same load booted the pool in 0 of 240 runs. Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/resolver-pool-sizing.test.ts | 16 +++++++++++++++- src/resolution/index.ts | 4 ++-- src/resolution/resolver-pool.ts | 17 +++++++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7b195d0d2..0669709589 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - An agent session that lost its connection to CodeGraph's shared background server, or couldn't reach it at startup, no longer stops that server from running for the rest of the session. It reconnects on its own once the server can start, so other sessions on the same project get auto-sync back instead of running read-only. Thanks @ijbranch for the report. (#2277) - On Windows, `codegraph upgrade` no longer breaks the install while an agent session is using CodeGraph. It used to stop partway and leave CodeGraph unable to start; it now swaps the new files in with CodeGraph running and puts the previous version back if anything fails, and re-running the PowerShell installer works the same way. If an earlier upgrade already broke your install, re-run `irm https://raw.githubusercontent.com/colbymchenry/codegraph/main/install.ps1 | iex` to repair it. Thanks @tippmar-nr for the report. (#2185) - On Windows, `codegraph init`, `codegraph index` and the background server's syncs no longer sometimes crash right after parsing on a busy machine (exit code 3221225477, an access violation). Shutting down the parsing threads could stop one while it was still loading its language grammars, which took the whole process down; they now finish loading first. +- On a busy machine, indexing or syncing a small project no longer starts extra background threads for resolving references that it can't use. Under heavy load the slower pace made a ten-file project look big enough to need them, which added seconds to the run and held extra memory. ## [1.6.1] - 2026-09-29 diff --git a/__tests__/resolver-pool-sizing.test.ts b/__tests__/resolver-pool-sizing.test.ts index cd949a4f0e..e70b7a6911 100644 --- a/__tests__/resolver-pool-sizing.test.ts +++ b/__tests__/resolver-pool-sizing.test.ts @@ -8,7 +8,7 @@ */ import { describe, it, expect } from 'vitest'; import * as os from 'os'; -import { ResolverPool } from '../src/resolution/resolver-pool'; +import { ResolverPool, ADAPTIVE_ENGAGE_MIN_REFS, shouldEngageAdaptively } from '../src/resolution/resolver-pool'; import { cgroupMemoryAvailable, darwinMemoryAvailable, @@ -106,3 +106,17 @@ describe('memory budget helpers', () => { } ); }); + +describe('adaptive pool engagement needs a real amount of work left', () => { + it('does not boot the pool for a small remainder, however slow the observed rate', () => { + // A busy machine: 11 refs left projected at 506 ms (seen on a loaded 4-core VM). + expect(shouldEngageAdaptively(506, 11, 400)).toBe(false); + expect(shouldEngageAdaptively(60_000, ADAPTIVE_ENGAGE_MIN_REFS - 1, 400)).toBe(false); + }); + + it('boots it for a large remainder whose projected settle clears the bar', () => { + // tokio-class: ~55k Rust refs at ~36 µs each. + expect(shouldEngageAdaptively(1_980, 55_000, 400)).toBe(true); + expect(shouldEngageAdaptively(399, 55_000, 400)).toBe(false); + }); +}); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index f4bb91a237..1eaf14b145 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -27,7 +27,7 @@ import { isCppConstructorRef, matchCppConstructor } from './cpp-constructor'; import { gateSwiftTypeTarget, clearSwiftTypeVisibility, swiftExtendedConformances } from './swift-type-visibility'; import { gateTypeParameter, clearTypeParameterMemos } from './type-parameters'; import { resolveViaImport, resolvePhpImportedStaticCall, resolvePhpQualifiedClassRef, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isJsPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; -import { ResolverPool, minRefsForPool } from './resolver-pool'; +import { ResolverPool, minRefsForPool, shouldEngageAdaptively } from './resolver-pool'; import { resolveAliasBinding } from './alias-binding'; import { detectFrameworks } from './frameworks'; import { synthesizeCallbackEdges } from './callback-synthesizer'; @@ -2109,7 +2109,7 @@ export class ReferenceResolver { adaptiveSeqRefs += batch.length; const remaining = total - processed - batch.length; const projectedMs = (adaptiveSeqMs / Math.max(1, adaptiveSeqRefs)) * Math.max(0, remaining); - if (projectedMs >= ADAPTIVE_ENGAGE_SETTLE_MS) { + if (shouldEngageAdaptively(projectedMs, remaining, ADAPTIVE_ENGAGE_SETTLE_MS)) { if (process.env.CODEGRAPH_SYNTH_TIMINGS) { console.error(`[pool-timing] adaptive engage: projected ${Math.round(projectedMs)}ms sequential settle over ${remaining} remaining refs`); } diff --git a/src/resolution/resolver-pool.ts b/src/resolution/resolver-pool.ts index 56b9785a66..96f4340d8d 100644 --- a/src/resolution/resolver-pool.ts +++ b/src/resolution/resolver-pool.ts @@ -58,6 +58,23 @@ export function minRefsForPool(): number { return 150_000; } +/** + * Fewest refs still to settle for the batch loop to boot the pool mid-run + * from its observed per-ref rate. Booting takes seconds (each worker opens the + * database and warms a resolver), only batches of MIN_PARALLEL_BATCH refs fan + * out, and synthesis waits for a booting pool. A rate measured on a busy + * machine inflates every projection, so with no floor a ten-file project booted + * the pool in half its runs under heavy load and paid seconds for nothing. + * Repos the adaptive bar is for (Rust-class per-ref cost) have tens of + * thousands of refs and still engage. + */ +export const ADAPTIVE_ENGAGE_MIN_REFS = 20_000; + +/** Whether the resolve loop should boot the pool mid-run (see ADAPTIVE_ENGAGE_MIN_REFS). */ +export function shouldEngageAdaptively(projectedMs: number, remainingRefs: number, barMs: number): boolean { + return remainingRefs >= ADAPTIVE_ENGAGE_MIN_REFS && projectedMs >= barMs; +} + export class ResolverPool { private workers: PoolWorker[] = []; private nextId = 0; From 9d4e805bec36d62332d7fba0a735329ba7621777 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 2 Oct 2026 10:36:42 +0000 Subject: [PATCH 8/8] test(windows): daemon-pid-reuse cleanup retries a still-held temp dir (#2304) afterEach SIGKILLs the servers and the detached daemon, waits 50 ms and removes the temp dir. A killed process releases its handles asynchronously on Windows, and under full-suite load the removal hit EPERM on the VM (the test itself passed; it passes 3/3 alone). Retry the removal like the other process-spawning suites do. Co-authored-by: Claude Opus 5.5 --- __tests__/daemon-pid-reuse.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/__tests__/daemon-pid-reuse.test.ts b/__tests__/daemon-pid-reuse.test.ts index 48650ccdf5..8bb116a0ce 100644 --- a/__tests__/daemon-pid-reuse.test.ts +++ b/__tests__/daemon-pid-reuse.test.ts @@ -197,7 +197,9 @@ describe('Shared MCP daemon (issue #411)', () => { } await new Promise((r) => setTimeout(r, 50)); servers.length = 0; - fs.rmSync(tempDir, { recursive: true, force: true }); + // A killed process releases its handles asynchronously on Windows; under + // full-suite load 50 ms is not always enough, so retry EPERM/EBUSY. + fs.rmSync(tempDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); }); it('takes over after SIGKILL even when the stale PID has been reused (#1553)', async () => {