diff --git a/__tests__/arkts-resolution.test.ts b/__tests__/arkts-resolution.test.ts index b095a4433..02c28761a 100644 --- a/__tests__/arkts-resolution.test.ts +++ b/__tests__/arkts-resolution.test.ts @@ -25,7 +25,13 @@ beforeAll(async () => { describe('ArkTS attribute-chain resolution precision', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. POSIX unlinks an + // open file happily, which is why this only ever fails on Windows. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -65,7 +71,7 @@ describe('ArkTS attribute-chain resolution precision', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const fns = cg.getNodesByKind('function'); @@ -99,7 +105,13 @@ describe('ArkTS attribute-chain resolution precision', () => { describe('ArkTS ohpm workspace import resolution', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. POSIX unlinks an + // open file happily, which is why this only ever fails on Windows. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -146,7 +158,7 @@ describe('ArkTS ohpm workspace import resolution', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const classes = cg.getNodesByKind('class'); @@ -168,7 +180,13 @@ describe('ArkTS ohpm workspace import resolution', () => { describe('ArkUI state → build() re-render bridge (assignment-gated)', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. POSIX unlinks an + // open file happily, which is why this only ever fails on Windows. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -201,7 +219,7 @@ describe('ArkUI state → build() re-render bridge (assignment-gated)', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const methods = cg.getNodesByKind('method'); @@ -229,7 +247,13 @@ describe('ArkUI state → build() re-render bridge (assignment-gated)', () => { describe('ArkUI @ohos.events.emitter bridge', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. POSIX unlinks an + // open file happily, which is why this only ever fails on Windows. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -266,7 +290,7 @@ describe('ArkUI @ohos.events.emitter bridge', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const methods = cg.getNodesByKind('method'); @@ -299,7 +323,7 @@ describe('ArkUI @ohos.events.emitter bridge', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const fns = cg.getNodesByKind('function'); @@ -314,7 +338,13 @@ describe('ArkUI @ohos.events.emitter bridge', () => { describe('ArkUI router bridge (pushUrl literal → @Entry struct)', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. POSIX unlinks an + // open file happily, which is why this only ever fails on Windows. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -343,7 +373,7 @@ describe('ArkUI router bridge (pushUrl literal → @Entry struct)', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const methods = cg.getNodesByKind('method'); @@ -362,7 +392,13 @@ describe('ArkUI router bridge (pushUrl literal → @Entry struct)', () => { describe('ohpm main entry (custom barrel + .ts consumer)', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. POSIX unlinks an + // open file happily, which is why this only ever fails on Windows. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -408,7 +444,7 @@ describe('ohpm main entry (custom barrel + .ts consumer)', () => { '}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const classes = cg.getNodesByKind('class'); diff --git a/__tests__/cli-ui-command.test.ts b/__tests__/cli-ui-command.test.ts index 7d1f36e5a..d45031e21 100644 --- a/__tests__/cli-ui-command.test.ts +++ b/__tests__/cli-ui-command.test.ts @@ -12,7 +12,7 @@ * observable fact rather than a promise. */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { execFileSync, spawn, type ChildProcess } from 'child_process'; import * as fs from 'fs'; import * as http from 'http'; @@ -221,11 +221,23 @@ describe('codegraph ui — serving', () => { const markerFile = (): string => path.join(markerDir, 'opened.txt'); - /** The opener is async (detached); give it a moment before concluding. */ + /** + * The opener is async (detached); give it a moment before concluding. + * + * Waits for CONTENT, not merely for the file to appear. Shell redirection + * creates the target before the command writes into it — `cmd.exe` opens the + * `>` target as it parses the line, ahead of `echo` — so an existence check + * can return a file that is real but still empty. Callers that expect no + * launch still get null: a file that never gains content times out the same + * as one that never appears. + */ async function waitForMarker(timeoutMs: number): Promise { const deadline = Date.now() + timeoutMs; for (;;) { - if (fs.existsSync(markerFile())) return fs.readFileSync(markerFile(), 'utf-8'); + if (fs.existsSync(markerFile())) { + const body = fs.readFileSync(markerFile(), 'utf-8'); + if (body.trim() !== '') return body; + } if (Date.now() > deadline) return null; await new Promise((r) => setTimeout(r, 50)); } @@ -278,11 +290,11 @@ describe('codegraph ui — serving', () => { }, 60_000); it('CODEGRAPH_BROWSER=none suppresses the launch like --no-open', async () => { - fs.rmSync(markerFile(), { force: true }); const viewer = await startViewer(['--port', '0', projectDir], { CODEGRAPH_BROWSER: 'none' }); try { expect((await get(viewer.port, '/')).status).toBe(200); - expect(await waitForMarker(1_000)).toBeNull(); + await vi.waitFor(() => expect(viewer.output()).toContain('Open that URL in a browser')); + expect(viewer.output()).not.toContain('Opening your browser'); } finally { await stopViewer(viewer.child); } diff --git a/__tests__/frameworks-integration.test.ts b/__tests__/frameworks-integration.test.ts index 3df4f2d88..351da7423 100644 --- a/__tests__/frameworks-integration.test.ts +++ b/__tests__/frameworks-integration.test.ts @@ -731,7 +731,13 @@ describe('Java end-to-end — field-injected bean trace (issue #389)', () => { describe('JVM FQN imports — end-to-end', () => { let tmpDir: string | undefined; + let cg: CodeGraph | undefined; afterEach(() => { + // Windows refuses to delete a file that still has an open handle, so the + // database has to be closed before the temp tree goes. Closing here rather + // than at the end of each test also survives a mid-test assertion failure. + cg?.close(); + cg = undefined; if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); tmpDir = undefined; }); @@ -749,7 +755,7 @@ describe('JVM FQN imports — end-to-end', () => { 'package com.example.app\n\nimport com.example.Bar\n\nclass App {\n fun run() { Bar().greet() }\n}\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const bar = cg.getNodesByKind('class').find((n) => n.qualifiedName === 'com.example::Bar'); @@ -765,8 +771,6 @@ describe('JVM FQN imports — end-to-end', () => { .getIncomingEdges(bar!.id) .find((e) => e.kind === 'imports'); expect(reachesBar, 'an imports edge should resolve to Bar via FQN').toBeDefined(); - - cg.close(); }); it('resolves a Kotlin top-level function import', async () => { @@ -780,7 +784,7 @@ describe('JVM FQN imports — end-to-end', () => { 'package com.example.app\n\nimport com.example.util\n\nfun main() { util() }\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const util = cg.getNodesByKind('function').find((n) => n.qualifiedName === 'com.example::util'); @@ -801,7 +805,7 @@ describe('JVM FQN imports — end-to-end', () => { 'package com.example.app\n\nimport com.example.JavaBar\n\nfun main() { JavaBar().greet() }\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const javaBar = cg.getNodesByKind('class').find((n) => n.qualifiedName === 'com.example::JavaBar'); @@ -833,7 +837,7 @@ describe('JVM FQN imports — end-to-end', () => { 'package app\n\nimport com.example.beta.Bar\n\nfun b() { Bar().who() }\n' ); - const cg = CodeGraph.initSync(tmpDir); + cg = CodeGraph.initSync(tmpDir); await cg.indexAll(); const alphaBar = cg.getNodesByKind('class').find((n) => n.qualifiedName === 'com.example.alpha::Bar'); diff --git a/__tests__/mcp-daemon.test.ts b/__tests__/mcp-daemon.test.ts index c73ac564c..59c4a2ce2 100644 --- a/__tests__/mcp-daemon.test.ts +++ b/__tests__/mcp-daemon.test.ts @@ -38,6 +38,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { CodeGraph } from '../src'; +import { rmTempDir } from './rm-temp'; import { getDaemonSocketPath } from '../src/mcp/daemon-paths'; import { CodeGraphPackageVersion } from '../src/mcp/version'; @@ -162,6 +163,61 @@ function countListeningLines(root: string): number { return readDaemonLog(root).split('\n').filter((l) => l.includes('[CodeGraph daemon] Listening on')).length; } +/** + * Every daemon that reached "Listening", by pid, from the log it writes itself. + * + * The lockfile names one daemon; the log names all of them. Under the + * concurrent-launcher race a second candidate can bind before the first + * candidate's lock is visible to it, and only one of the two ends up in the + * lockfile — so reaping by lockfile alone leaves a live daemon holding the + * database until its idle timeout, long past any teardown. + */ +function listeningPids(root: string): number[] { + const pids = new Set(); + for (const m of readDaemonLog(root).matchAll(/Listening on .*?\(pid (\d+)/g)) { + pids.add(Number(m[1])); + } + return [...pids]; +} + +/** + * Kill every daemon this root ever started, including one that has not started + * yet when the reap begins. + * + * Daemons are detached: there is no handle to close and no exit to await, so + * they have to be found by pid and killed. A single pass is not enough. A + * candidate that is mid-spawn when teardown runs binds a moment later and only + * then writes its "Listening" line, so a one-shot read of the lockfile and log + * cannot see it — and a daemon missed here holds the database open for its full + * idle timeout, which is far longer than any removal is willing to retry. + * + * So this keeps looking until the root has been quiet for several consecutive + * passes with nothing alive, which covers the spawn window rather than assuming + * it has closed. Guards our own pid: the version-mismatch test plants + * `pid: process.pid` in the lockfile, and we must never SIGKILL the worker. + * + * Returns whether the root actually went quiet. The two exits mean opposite + * things — quiet reached is a clean reap, `maxPasses` burned is a daemon that + * kept respawning or refused SIGKILL — and a caller that cannot tell them apart + * reads the second as the first, so the reap can report success on the exact + * run where it did nothing. + */ +async function reapDaemons(root: string, quietPasses = 6, maxPasses = 200): Promise { + let quiet = 0; + for (let pass = 0; pass < maxPasses && quiet < quietPasses; pass++) { + const lockPid = readLockPid(root); + const pids = [...new Set([...(lockPid ? [lockPid] : []), ...listeningPids(root)])] + .filter((pid) => pid !== process.pid); + const alive = pids.filter(isAlive); + for (const pid of alive) { + try { process.kill(pid, 'SIGKILL'); } catch { /* already gone */ } + } + quiet = alive.length === 0 ? quiet + 1 : 0; + await new Promise((r) => setTimeout(r, 25)); + } + return quiet >= quietPasses; +} + function killTree(...procs: ChildProcessWithoutNullStreams[]): void { for (const p of procs) { if (!p.killed) { try { p.kill('SIGKILL'); } catch { /* gone */ } } @@ -185,18 +241,22 @@ describe('Shared MCP daemon (issue #411)', () => { }); afterEach(async () => { + // Registered before the kill, so no exit can land between the two. + const exits = servers.map((s) => + s.child.exitCode === null && s.child.signalCode === null + ? new Promise((resolve) => s.child.once('exit', () => resolve())) + : Promise.resolve() + ); killTree(...servers.map((s) => s.child)); - // The daemon is detached (not a tracked child) — reap it explicitly via the - // pid it recorded, so a test can't leak a background daemon. Guard against - // our own pid: the version-mismatch test plants `pid: process.pid` in the - // lockfile, and we must never SIGKILL the vitest worker. - const daemonPid = readLockPid(realRoot); - if (daemonPid && daemonPid !== process.pid && isAlive(daemonPid)) { - try { process.kill(daemonPid, 'SIGKILL'); } catch { /* race */ } - } - await new Promise((r) => setTimeout(r, 50)); + await Promise.all(exits); + const reaped = await reapDaemons(realRoot); servers.length = 0; - fs.rmSync(tempDir, { recursive: true, force: true }); + await rmTempDir(tempDir); + // Asserted after the removal so a failed reap still cleans up what it can, + // and surfaces as itself rather than as the EPERM it would cause next. + if (!reaped) { + throw new Error(`reapDaemons exhausted its pass budget on ${realRoot} — a daemon is still alive`); + } }); it('two invocations share ONE detached daemon; both attach as proxies', async () => { diff --git a/__tests__/mcp-initialize.test.ts b/__tests__/mcp-initialize.test.ts index 0a320773d..0545e3835 100644 --- a/__tests__/mcp-initialize.test.ts +++ b/__tests__/mcp-initialize.test.ts @@ -16,6 +16,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; +import { rmTempDir } from './rm-temp'; const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); @@ -107,12 +108,19 @@ describe('MCP initialize handshake (issue #172)', () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-init-')); }); - afterEach(() => { - if (child && !child.killed) { - child.kill('SIGKILL'); + afterEach(async () => { + // kill() only asks; the child still owns its cwd and the project database + // when it returns, and Windows will not delete either out from under a live + // process. POSIX removes them regardless, which is why CI never sees this. + if (child) { + if (child.exitCode === null && child.signalCode === null) { + const exited = new Promise((resolve) => child!.once('exit', () => resolve())); + child.kill('SIGKILL'); + await exited; + } child = null; } - fs.rmSync(tempDir, { recursive: true, force: true }); + await rmTempDir(tempDir); }); it('responds to initialize quickly when no .codegraph exists in cwd', async () => { diff --git a/__tests__/mcp-roots.test.ts b/__tests__/mcp-roots.test.ts index 8e1d4520d..85964935e 100644 --- a/__tests__/mcp-roots.test.ts +++ b/__tests__/mcp-roots.test.ts @@ -21,6 +21,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; +import { rmTempDir } from './rm-temp'; const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); @@ -84,13 +85,21 @@ describe('MCP project resolution via roots/list (issue #196)', () => { projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-proj-')); }); - afterEach(() => { - if (child && !child.killed) { - child.kill('SIGKILL'); + afterEach(async () => { + // kill() only asks; the child is still holding its cwd and the project's + // database when it returns. Windows will not delete either while a live + // process owns them, so the exit has to be awaited before the temp trees + // go — on POSIX the removal succeeds regardless, which is why CI is green. + if (child) { + if (child.exitCode === null && child.signalCode === null) { + const exited = new Promise((resolve) => child!.once('exit', () => resolve())); + child.kill('SIGKILL'); + await exited; + } child = null; } - fs.rmSync(cwdDir, { recursive: true, force: true }); - fs.rmSync(projectDir, { recursive: true, force: true }); + await rmTempDir(cwdDir); + await rmTempDir(projectDir); }); it('resolves the project from the client roots/list when no rootUri is sent', async () => { diff --git a/__tests__/mcp-subproject-adoption.test.ts b/__tests__/mcp-subproject-adoption.test.ts index 39abac038..fa959816f 100644 --- a/__tests__/mcp-subproject-adoption.test.ts +++ b/__tests__/mcp-subproject-adoption.test.ts @@ -19,6 +19,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; +import { rmTempDir } from './rm-temp'; const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); @@ -113,12 +114,19 @@ describe('MCP workspace sub-project adoption (#1606) + no-default diagnostics (# ws = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-ws-')); }); - afterEach(() => { - if (child && !child.killed) { - child.kill('SIGKILL'); + afterEach(async () => { + // kill() only asks; the child still owns its cwd and the project database + // when it returns, and Windows will not delete either out from under a live + // process. POSIX removes them regardless, which is why CI never sees this. + if (child) { + if (child.exitCode === null && child.signalCode === null) { + const exited = new Promise((resolve) => child!.once('exit', () => resolve())); + child.kill('SIGKILL'); + await exited; + } child = null; } - fs.rmSync(ws, { recursive: true, force: true }); + await rmTempDir(ws); }); it('adopts the single indexed sub-project below a workspace root as the default project', async () => { diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index decaadee5..303400228 100644 --- a/__tests__/resolution.test.ts +++ b/__tests__/resolution.test.ts @@ -3570,6 +3570,7 @@ int run() { // feature can't silently regress to a no-op in the indexing flow. it('connects #include to the real header file via include-dir scan (end-to-end)', async () => { const tempProject = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-e2e-')); + let db: ReturnType | undefined; try { fs.mkdirSync(path.join(tempProject, 'include'), { recursive: true }); fs.mkdirSync(path.join(tempProject, 'src'), { recursive: true }); @@ -3592,7 +3593,7 @@ int run() { // The `#include "utils.h"` edge should target the real // `include/utils.h` file node — not a floating `import` node // living inside main.cpp. - const db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); + db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); const rows = db.getDb().prepare(` select dst.kind as dstKind, dst.file_path as dstPath from edges e @@ -3612,6 +3613,13 @@ int run() { ); expect(stdlibFile).toBeUndefined(); } finally { + // Windows will not delete a file that still has an open handle. Both + // holders have to go: the graph's own connection, and the direct one + // this test opens to read edges back. Closing here rather than after + // the query keeps that true when a query or an assertion throws. + // close() is idempotent, so the outer afterEach's destroy() stays safe. + db?.close(); + cg?.close(); fs.rmSync(tempProject, { recursive: true, force: true }); } }); @@ -3685,6 +3693,7 @@ class Both : public Base, public Plain {}; // templated + plain in one cla it('resolves require_once to a file→file imports edge (#660)', async () => { const tempProject = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-php-e2e-')); + let db: ReturnType | undefined; try { fs.mkdirSync(path.join(tempProject, 'src'), { recursive: true }); fs.writeFileSync( @@ -3701,7 +3710,7 @@ class Both : public Base, public Plain {}; // templated + plain in one cla // reporter's repro: page.php's `require_once("lib.php")` must resolve // to the real src/lib.php file node — a file→file `imports` edge, so // callers(lib.php) now includes page.php. - const db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); + db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); const rows = db.getDb().prepare(` select dst.kind as dstKind, dst.file_path as dstPath from edges e @@ -3716,12 +3725,15 @@ class Both : public Base, public Plain {}; // templated + plain in one cla ); expect(resolved, 'page.php → src/lib.php imports edge missing').toBeDefined(); } finally { + db?.close(); + cg?.close(); fs.rmSync(tempProject, { recursive: true, force: true }); } }); it('resolves a subdirectory include path to the correct file (#660)', async () => { const tempProject = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-php-subdir-')); + let db: ReturnType | undefined; try { fs.mkdirSync(path.join(tempProject, 'inc'), { recursive: true }); fs.writeFileSync( @@ -3735,7 +3747,7 @@ class Both : public Base, public Plain {}; // templated + plain in one cla cg = await CodeGraph.init(tempProject, { index: true }); - const db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); + db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); const rows = db.getDb().prepare(` select dst.kind as dstKind, dst.file_path as dstPath from edges e @@ -3750,12 +3762,15 @@ class Both : public Base, public Plain {}; // templated + plain in one cla 'index.php → inc/db.php imports edge missing' ).toBeDefined(); } finally { + db?.close(); + cg?.close(); fs.rmSync(tempProject, { recursive: true, force: true }); } }); it('does not mis-connect an unresolvable include to a same-named file elsewhere (#660)', async () => { const tempProject = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-php-misresolve-')); + let db: ReturnType | undefined; try { // app/page.php's `require "inc/db.php"` resolves relative to app/, where // inc/db.php does NOT exist. A same-named lib/inc/db.php exists elsewhere @@ -3774,7 +3789,7 @@ class Both : public Base, public Plain {}; // templated + plain in one cla cg = await CodeGraph.init(tempProject, { index: true }); - const db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); + db = DatabaseConnection.open(path.join(tempProject, '.codegraph', 'codegraph.db')); const rows = db.getDb().prepare(` select dst.kind as dstKind, dst.file_path as dstPath from edges e @@ -3789,6 +3804,8 @@ class Both : public Base, public Plain {}; // templated + plain in one cla 'app/page.php must NOT mis-connect to unrelated lib/inc/db.php' ).toBeUndefined(); } finally { + db?.close(); + cg?.close(); fs.rmSync(tempProject, { recursive: true, force: true }); } }); diff --git a/__tests__/rm-temp.ts b/__tests__/rm-temp.ts new file mode 100644 index 000000000..4a5992856 --- /dev/null +++ b/__tests__/rm-temp.ts @@ -0,0 +1,28 @@ +/** + * Removing a temp tree that a detached daemon may still hold. + * + * The MCP suites spawn `codegraph serve --mcp`, which can start a DETACHED + * daemon. A tracked child can be awaited on its `exit` event, and the suites do + * that, but the daemon is not a child of this process: there is no handle to + * close and no event to wait on, only the OS releasing its files once the + * process is gone. Windows does that a beat late and fails the removal with + * EPERM/EBUSY meanwhile, where POSIX unlinks regardless — which is why this is + * invisible on CI and reproducible on a Windows contributor's machine. + * + * Retrying is the honest tool for that residue specifically. Everything with a + * real holder — a database connection, a tracked child — is closed or awaited + * at its own site rather than papered over here. + */ +import * as fs from 'node:fs'; + +export async function rmTempDir(dir: string, attempts = 40, delayMs = 50): Promise { + for (let i = 0; ; i++) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (err) { + if (i >= attempts) throw err; + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } +} diff --git a/__tests__/ui-server.test.ts b/__tests__/ui-server.test.ts index df25858a8..525b54aaf 100644 --- a/__tests__/ui-server.test.ts +++ b/__tests__/ui-server.test.ts @@ -12,13 +12,15 @@ * name in undici, and forging it is the whole point of half these cases. */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import * as childProcess from 'child_process'; import * as http from 'http'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { browserOpenCommand, + openBrowser, cacheControlFor, contentTypeFor, isAllowedHost, @@ -31,6 +33,11 @@ import { type UiServerHandle, } from '../src/ui-server'; +vi.mock('child_process', async (importOriginal) => ({ + ...await importOriginal(), + spawn: vi.fn(), +})); + interface Response { status: number; headers: http.IncomingHttpHeaders; @@ -539,6 +546,22 @@ describe('security helpers', () => { }); describe('browserOpenCommand', () => { + it('does not spawn a browser when CODEGRAPH_BROWSER=none', () => { + const spawn = vi.spyOn(childProcess, 'spawn').mockImplementation(() => { + throw new Error('Unexpected browser launch'); + }); + vi.stubEnv('CODEGRAPH_BROWSER', 'none'); + try { + for (const platform of ['win32', 'darwin', 'linux'] as const) { + expect(openBrowser('http://127.0.0.1:4747', platform)).toBe(false); + } + expect(spawn).not.toHaveBeenCalled(); + } finally { + spawn.mockRestore(); + vi.unstubAllEnvs(); + } + }); + it('uses the platform opener', () => { expect(browserOpenCommand('http://x', 'darwin')).toEqual({ command: 'open', args: ['http://x'] }); expect(browserOpenCommand('http://x', 'linux')).toEqual({ command: 'xdg-open', args: ['http://x'] }); diff --git a/__tests__/watcher.test.ts b/__tests__/watcher.test.ts index 942fd5bcd..3271f74f5 100644 --- a/__tests__/watcher.test.ts +++ b/__tests__/watcher.test.ts @@ -30,6 +30,7 @@ import { type WatchOptions, } from '../src/sync/watcher'; import CodeGraph from '../src/index'; +import { rmTempDir } from './rm-temp'; type SyncFn = (paths?: string[]) => Promise<{ filesChanged: number; durationMs: number }>; @@ -70,11 +71,16 @@ describe('FileWatcher', () => { fs.writeFileSync(path.join(srcDir, 'index.ts'), 'export const x = 1;'); }); - afterEach(() => { + afterEach(async () => { __setFsWatchForTests(null); // reset the injected fs.watch seam vi.restoreAllMocks(); if (fs.existsSync(testDir)) { - fs.rmSync(testDir, { recursive: true, force: true }); + // The end-to-end test drives a real fs.watch, whose directory handle the + // OS releases a beat after unwatch() returns. Windows fails the removal + // with EPERM in that window; POSIX unlinks regardless, which is why only + // Windows sees it. Every holder this suite owns is already closed at its + // own site — this waits out the release, it does not stand in for it. + await rmTempDir(testDir); } });