diff --git a/esbuild.config.mjs b/esbuild.config.mjs index a90080c3..9a6a3241 100644 --- a/esbuild.config.mjs +++ b/esbuild.config.mjs @@ -101,6 +101,10 @@ const ccHooks = [ // pulls the freshest cloud snapshot for HEAD if newer than local. // See src/hooks/graph-pull-worker.ts. { entry: "dist/src/hooks/graph-pull-worker.js", out: "graph-pull-worker" }, + // Detached provisioning worker for the code-graph tree-sitter parsers. + // Spawned by session-start-setup so a cold npm install + native compile + // can outlive the hook's ~120s async timeout. See src/hooks/graph-deps-worker.ts. + { entry: "dist/src/hooks/graph-deps-worker.js", out: "graph-deps-worker" }, ]; const ccShell = [ @@ -167,6 +171,10 @@ const codexHooks = [ // recently-used org skill (judging runs on the codex CLI). Same shared module CC uses. { entry: "dist/src/skillify/skillopt-worker.js", out: "skillopt-worker" }, { entry: "dist/src/hooks/graph-pull-worker.js", out: "graph-pull-worker" }, + // Detached provisioning worker for the code-graph tree-sitter parsers — + // codex parity with CC. Spawned by session-start-setup so a cold npm install + // + native compile can outlive the hook. See src/hooks/graph-deps-worker.ts. + { entry: "dist/src/hooks/graph-deps-worker.js", out: "graph-deps-worker" }, // G3: code-graph auto-build parity for Codex (same shared hook as CC/Cursor). // graph-on-stop is built separately via buildGraphOnStop() (code-split). ]; diff --git a/src/cli/graph-deps.ts b/src/cli/graph-deps.ts index c5d065ca..08138aad 100644 --- a/src/cli/graph-deps.ts +++ b/src/cli/graph-deps.ts @@ -1,5 +1,6 @@ import { execFileSync } from "node:child_process"; -import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, rmSync, statSync, utimesSync, writeFileSync } from "node:fs"; +import { randomBytes } from "node:crypto"; import { join } from "node:path"; import { ensureDir, log, pkgRoot, warn, writeJson } from "./util.js"; import { SHARED_DIR, SHARED_NODE_MODULES } from "./embeddings.js"; @@ -46,6 +47,48 @@ export function treeSitterSpecs(pkgJsonPath: string = join(pkgRoot(), "package.j .map((n) => `${n}@${opt[n]}`); } +/** + * The "ready" key stamped into the marker after BOTH the npm install AND the + * native heal succeed. Keyed by the exact spec set PLUS the runtime identity + * that a compiled native addon is bound to — platform, arch, and the Node + * module ABI (`process.versions.modules`). A change in any of these (a version + * bump, moving the home dir across machines, or upgrading Node to a new ABI) + * flips the key and forces a full reprovision, so a stale `.node` built for a + * different ABI never lingers as a silent load failure. + * + * NOTE: `specs` are the REQUESTED ranges (`tree-sitter@^0.21.1`), not the + * resolved versions npm actually laid down. A range bump in package.json flips + * the key (good), but re-resolving the SAME range to a newer patch inside the + * range does NOT — that's an accepted trade-off: the heal re-validates the + * bindings load on every reprovision, so a functionally-broken resolve still + * fails the heal and leaves no marker. + */ +export function graphDepsReadyKey(specs: string[]): string { + return [ + specs.join("\n"), + `platform=${process.platform}`, + `arch=${process.arch}`, + `abi=${process.versions.modules}`, + ].join("\n"); +} + +/** + * Max age of the install lockdir before a contender reclaims it as stale. + * 30 minutes: a from-source tree-sitter compile on a slow arm64 box can take + * many minutes, and we refresh the lockdir mtime around each long phase (see + * `refreshLock`), so only a genuinely dead installer ages past this window. + */ +const LOCK_STALE_MS = 30 * 60 * 1000; + +/** + * How long a recorded provisioning FAILURE suppresses the next attempt. Without + * this, an offline box (npm can't reach the registry) would re-attempt — and + * re-fail after a network timeout — on every single session start: a retry + * storm. 6 hours lets a transient outage self-heal by the next working day + * while never touching this file on the healthy fast path. + */ +const ATTEMPT_BACKOFF_MS = 6 * 60 * 60 * 1000; + /** * Injection seam for ensureGraphDeps. Tests replace the two external boundaries * (`runNpm` = the npm install, `runHeal` = the native-build heal) and point @@ -64,18 +107,28 @@ export interface GraphDepsDeps { /** * Install the tree-sitter parsers into the shared embed-deps dir. Idempotent - * (a specs-hash marker + a per-package presence check skip the re-download, - * and catch version bumps / partial installs) and best-effort: any failure is - * logged and swallowed so it never aborts the caller — the graph stays - * disabled but nothing else breaks, and the graph-on-stop hook degrades - * gracefully rather than crashing. + * and best-effort: any failure is logged and swallowed so it never aborts the + * caller — the graph stays disabled but nothing else breaks, and the + * graph-on-stop hook degrades gracefully rather than crashing. + * + * Called from EVERY session start (before the credentials early-return), so it + * must be a genuine no-op on the common already-provisioned path: when the + * ready-marker matches the current specs + platform + arch + Node ABI AND + * every parser dir is present, it returns without spawning npm OR the heal. + * + * Concurrency: a mkdir-based lockdir in the shared dir serializes contenders. + * A session that can't acquire the lock (another install is in flight) skips + * silently — the next session retries. A lock older than LOCK_STALE_MS is + * reclaimed so a crashed installer never wedges provisioning forever. + * + * The marker is a true "ready" marker: it is written ONLY after BOTH the npm + * install AND scripts/ensure-tree-sitter.mjs (the native heal) succeed. A + * partial/interrupted install therefore leaves no marker and re-runs next time. * * Two-phase native provisioning: `npm install --ignore-scripts` first, so a * platform without a prebuild (arm64 / Node 24) doesn't fail the download, - * THEN scripts/ensure-tree-sitter.mjs always runs — it validates the bindings - * load and compiles from source where no prebuild exists (a fast no-op on - * healthy prebuilt installs). Running the heal unconditionally repairs a - * previously interrupted or ABI-mismatched install. + * THEN the heal validates the bindings load and compiles from source where no + * prebuild exists. * * Decoupled from the ~600 MB embeddings download: this installs ONLY the * parsers (tens of MB), so the code graph can work without semantic search. @@ -91,40 +144,206 @@ export function ensureGraphDeps(deps: GraphDepsDeps = {}): void { warnFn = warn, } = deps; try { + // Same disable switch the graph-on-stop auto-build hook honors + // (HIVEMIND_GRAPH_ON_STOP=0): if the graph feature is off there's nothing + // to provision for, so skip the install entirely. + if (process.env.HIVEMIND_GRAPH_ON_STOP === "0") { + logFn(` Graph provisioning skipped (HIVEMIND_GRAPH_ON_STOP=0)`); + return; + } if (specs.length === 0) { warnFn(` Graph no tree-sitter optionalDependencies found in package.json — skipping`); return; } + const marker = join(sharedDir, ".graph-deps"); + const attemptFile = join(sharedDir, ".graph-deps.attempt"); + const wantKey = graphDepsReadyKey(specs); + // Cheap no-op fast path: BEFORE taking the lock, touching npm, or reading + // the attempt file, if the ready-marker already matches (same specs + + // platform + arch + ABI) AND every parser is present, there's nothing to + // do. This is the path every steady-state session hits, so it must spawn + // nothing AND never touch the attempt-backoff file — a healthy install + // stays completely untouched. + if (readMarker(marker) === wantKey && isGraphDepsInstalled(sharedNodeModules, specs)) { + logFn(` Graph tree-sitter parsers already present at ${sharedDir}`); + return; + } + + // Offline retry-storm guard: if the LAST attempt failed recently, skip + // this one. Checked AFTER the fast-path (healthy installs never see it) + // but BEFORE the lock (a wedged network shouldn't even contend). A + // successful provision clears this file; the fast path above means a + // once-provisioned box never re-reads it. + if (recentlyFailed(attemptFile)) { + logFn(` Graph skipping provision — a recent attempt failed (backing off ${ATTEMPT_BACKOFF_MS / 3_600_000}h)`); + return; + } + ensureDir(sharedDir); - // Create a package.json if none exists yet (user never ran embeddings - // install). Don't clobber an existing one — it may already declare - // transformers; the parsers are installed alongside. - const pkgPath = join(sharedDir, "package.json"); - if (!existsSync(pkgPath)) { - writeJson(pkgPath, { name: "hivemind-embed-deps", version: "1.0.0", private: true, dependencies: {} }); + // Global atomic install lock. Acquired BEFORE re-checking markers/packages + // so only one contender provisions at a time. A failure to acquire means + // another install is in flight (or we couldn't create the dir) — skip + // silently; the next session retries. + const lockDir = join(sharedDir, ".graph-deps.lock"); + const token = lockToken(); + if (!acquireLock(lockDir, token)) { + logFn(` Graph another install holds the lock — skipping (will retry next session)`); + return; } - // Skip the (re)download only when the exact spec set was installed before - // AND every package is still present. The marker also catches version - // bumps: a changed spec set forces a reinstall. - const marker = join(sharedDir, ".graph-deps"); - const wantKey = specs.join("\n"); - const haveKey = existsSync(marker) ? readFileSync(marker, "utf8") : ""; - if (haveKey !== wantKey || !isGraphDepsInstalled(sharedNodeModules, specs)) { + try { + // Re-check under the lock: a contender we raced may have finished the + // install while we waited, making our work redundant. + if (readMarker(marker) === wantKey && isGraphDepsInstalled(sharedNodeModules, specs)) { + logFn(` Graph tree-sitter parsers already present at ${sharedDir}`); + return; + } + // Invalidate the ready marker BEFORE any mutation. From here until the + // final re-stamp the install is "in progress" and MUST NOT be trusted: + // if npm or the heal crashes mid-flight, the absent marker forces the + // next session to retry rather than fast-path over a half-broken tree. + try { rmSync(marker, { force: true }); } catch { /* absent is fine */ } + // Create a package.json if none exists yet (user never ran embeddings + // install). Don't clobber an existing one — it may already declare + // transformers; the parsers are installed alongside. + const pkgPath = join(sharedDir, "package.json"); + if (!existsSync(pkgPath)) { + writeJson(pkgPath, { name: "hivemind-embed-deps", version: "1.0.0", private: true, dependencies: {} }); + } logFn(` Graph installing tree-sitter parsers into ${sharedDir} (code-graph; ~tens of MB)`); + // Refresh the lock mtime immediately before npm so a genuinely long + // install isn't reclaimed as stale mid-flight by a racing session. + refreshLock(lockDir); runNpm(specs, sharedDir); + // Heal AFTER the install: validates bindings load + compiles from source + // where no prebuild exists (arm64 / Node 24). Repairs an interrupted or + // ABI-mismatched one. Refresh the lock again before it — a from-source + // arm64 compile is the single longest phase. + refreshLock(lockDir); + runHeal(sharedDir); + // Only NOW — after BOTH steps succeeded — stamp the ready marker and + // clear any recorded failure. If either threw, we never reach here, the + // marker stays absent, and the catch below records the failure. writeFileSync(marker, wantKey); - } else { - logFn(` Graph tree-sitter parsers already present at ${sharedDir}`); + clearAttempt(attemptFile); + } finally { + releaseLock(lockDir, token); } - // ALWAYS heal: validates bindings load + compiles from source where no - // prebuild exists (arm64 / Node 24). Fast no-op on healthy prebuilt - // installs; repairs an interrupted or ABI-mismatched one. - runHeal(sharedDir); } catch (err) { + // Record the failure so the offline retry-storm guard backs off next time. + // Best-effort: if we can't even write the attempt file (permissions), the + // provision still degrades gracefully — we just lose the backoff. + recordFailure(join(sharedDir, ".graph-deps.attempt")); warnFn(` Graph tree-sitter provisioning failed (${err instanceof Error ? err.message : String(err)}); code graph stays disabled — everything else works`); } } +/** Read the ready marker, or "" when absent/unreadable. */ +function readMarker(marker: string): string { + try { return existsSync(marker) ? readFileSync(marker, "utf8") : ""; } catch { return ""; } +} + +/** A per-process owner token stamped into the lockdir: pid + randomness. */ +function lockToken(): string { + return `${process.pid}.${randomBytes(8).toString("hex")}`; +} + +/** Path of the owner-token file written inside the lockdir on acquire. */ +function ownerFile(lockDir: string): string { + return join(lockDir, "owner"); +} + +/** + * Acquire the install lock via an atomic `mkdir` (fails EEXIST if held), then + * write our owner token inside it so `releaseLock` can prove ownership. If the + * existing lockdir is older than LOCK_STALE_MS it's a crashed installer's + * leftover — reclaim it (rm the stale dir, then attempt a fresh mkdir). The + * reclaim mkdir can itself lose to another concurrent reclaimer (both saw the + * same stale dir, one removed + recreated it first) — that's fine: the loser's + * mkdir throws EEXIST and we return false, so it simply skips this session. + * Returns false when the lock is held by a live contender or the mkdir fails. + */ +function acquireLock(lockDir: string, token: string): boolean { + const claim = (): boolean => { + mkdirSync(lockDir); + writeFileSync(ownerFile(lockDir), token); + return true; + }; + try { + return claim(); + } catch { + // Held (or unmakeable). Reclaim only if demonstrably stale. + try { + const age = Date.now() - statSync(lockDir).mtimeMs; + if (age < LOCK_STALE_MS) return false; + rmSync(lockDir, { recursive: true, force: true }); + return claim(); // may lose to a concurrent reclaimer → EEXIST → false + } catch { + return false; + } + } +} + +/** + * Refresh the lockdir mtime so a long-but-live install isn't reclaimed as stale + * mid-flight. Touches BOTH the dir and its owner file (some filesystems only + * surface mtime changes on the file). Best-effort — a failure here just risks + * an over-eager reclaim, which the ownership-safe release then absorbs. + */ +function refreshLock(lockDir: string): void { + const now = new Date(); + try { utimesSync(lockDir, now, now); } catch { /* best-effort */ } + try { utimesSync(ownerFile(lockDir), now, now); } catch { /* best-effort */ } +} + +/** + * Ownership-safe lock release: remove the lockdir ONLY when its owner token + * matches ours. If a stale-reclaim handed the lock to another process while we + * were still running (we over-ran LOCK_STALE_MS), the token no longer matches + * and we DON'T delete — deleting would rip the lock out from under the new + * owner mid-install. A token we can't read (dir already gone) is treated as + * not-ours and left alone. + */ +function releaseLock(lockDir: string, token: string): void { + try { + const owner = existsSync(ownerFile(lockDir)) ? readFileSync(ownerFile(lockDir), "utf8") : ""; + if (owner !== token) return; // reclaimed by someone else — not ours to remove + rmSync(lockDir, { recursive: true, force: true }); + } catch { /* stale-reclaim handles a leftover */ } +} + +/** + * True when a prior provisioning attempt failed within ATTEMPT_BACKOFF_MS. + * A malformed / unreadable attempt file counts as "not recently failed" so a + * corrupt file can never wedge provisioning forever. + */ +function recentlyFailed(attemptFile: string): boolean { + try { + if (!existsSync(attemptFile)) return false; + const { lastAttemptAt } = JSON.parse(readFileSync(attemptFile, "utf8")) as { lastAttemptAt?: number }; + if (typeof lastAttemptAt !== "number") return false; + return Date.now() - lastAttemptAt < ATTEMPT_BACKOFF_MS; + } catch { + return false; + } +} + +/** Record a failed attempt (timestamp + running failure count). Best-effort. */ +function recordFailure(attemptFile: string): void { + try { + let failures = 0; + try { + const prev = JSON.parse(readFileSync(attemptFile, "utf8")) as { failures?: number }; + if (typeof prev.failures === "number") failures = prev.failures; + } catch { /* no prior file / unreadable → start at 0 */ } + writeFileSync(attemptFile, JSON.stringify({ lastAttemptAt: Date.now(), failures: failures + 1 })); + } catch { /* best-effort — losing the backoff record is non-fatal */ } +} + +/** Clear the recorded-failure file after a successful provision. Best-effort. */ +function clearAttempt(attemptFile: string): void { + try { rmSync(attemptFile, { force: true }); } catch { /* absent is fine */ } +} + /** * Default npm boundary. `--ignore-scripts` fetches the packages without the * native build that fails on platforms lacking a prebuild (the heal does the @@ -143,10 +362,33 @@ function defaultRunNpm(specs: string[], cwd: string): void { }); } -/** Default heal boundary: run scripts/ensure-tree-sitter.mjs if present. */ -function defaultRunHeal(cwd: string): void { - const heal = join(pkgRoot(), "scripts", "ensure-tree-sitter.mjs"); - if (existsSync(heal)) { - execFileSync(process.execPath, [heal], { cwd, stdio: "inherit" }); +/** + * Default heal boundary: run scripts/ensure-tree-sitter.mjs. + * + * Two correctness properties over a naive "run it if present": + * + * 1. STRICT mode (`HIVEMIND_STRICT_POSTINSTALL=1`) turns the script's final + * bindings-load check into a non-zero exit. By DEFAULT that script exits 0 + * even when the from-source rebuild's load check still fails (so end-user + * `npm install` never hard-breaks) — but here a silent heal "success" over + * an unloadable addon is exactly the false-success we must avoid: it would + * stamp the ready marker and the graph would then fail at parse time every + * session. Forcing strict makes a load failure throw, so no marker is + * written and the next session retries. + * + * 2. A MISSING heal script is a FAILURE, not a silent success. The heal is the + * only thing that validates the native bindings actually load on this + * platform; skipping it and stamping the marker would ship a possibly-broken + * install. Throwing here leaves no marker so provisioning retries once the + * script is present. + */ +export function defaultRunHeal(cwd: string, healScript: string = join(pkgRoot(), "scripts", "ensure-tree-sitter.mjs")): void { + if (!existsSync(healScript)) { + throw new Error(`heal script missing at ${healScript} — cannot validate native bindings`); } + execFileSync(process.execPath, [healScript], { + cwd, + stdio: "inherit", + env: { ...process.env, HIVEMIND_STRICT_POSTINSTALL: "1" }, + }); } diff --git a/src/hooks/codex/session-start-setup.ts b/src/hooks/codex/session-start-setup.ts index 2490b809..72fb6a0a 100644 --- a/src/hooks/codex/session-start-setup.ts +++ b/src/hooks/codex/session-start-setup.ts @@ -19,6 +19,7 @@ import { log as _log } from "../../utils/debug.js"; import { makeWikiLogger } from "../../utils/wiki-log.js"; import { autoUpdate } from "../shared/autoupdate.js"; import { getInstalledVersion } from "../../utils/version-check.js"; +import { spawnDetachedNodeWorker } from "../../utils/spawn-detached.js"; const log = (msg: string) => _log("codex-session-setup", msg); const { log: wikiLog } = makeWikiLogger(join(homedir(), ".codex", "hooks")); @@ -48,6 +49,17 @@ async function main(): Promise { if (process.env.HIVEMIND_WIKI_WORKER === "1") return; const input = await readStdin(); + + // Provision the code-graph tree-sitter parsers into the shared embed-deps + // dir so the graph-on-stop hook can auto-build the graph. Spawned as a + // DETACHED worker — NOT run inline — because a cold provision runs npm + + // a from-source native compile that can exceed this hook's ~120s async + // timeout; the worker outlives the hook and finishes in the background. + // Fired BEFORE the credentials early-return: provisioning is purely local + // and must not depend on login. Best-effort — the spawn helper swallows any + // failure, and ensureGraphDeps inside the worker serializes via its own lock. + spawnDetachedNodeWorker(join(__bundleDir, "graph-deps-worker.js")); + const creds = loadCredentials(); if (!creds?.token) { log("no credentials"); return; } diff --git a/src/hooks/codex/session-start.ts b/src/hooks/codex/session-start.ts index 594bc90a..5cbdbe1e 100644 --- a/src/hooks/codex/session-start.ts +++ b/src/hooks/codex/session-start.ts @@ -58,9 +58,16 @@ async function main(): Promise { creds = await healDriftedOrgToken(creds, log); } - // Spawn async setup (table creation, placeholder, version check) as detached process. - // Codex doesn't support async hooks, so we use the same pattern as the wiki worker. - if (creds?.token) { + // Spawn async setup (graph-deps provisioning, table creation, placeholder, + // version check) as a detached process. Codex doesn't support async hooks, + // so we use the same pattern as the wiki worker. + // + // Spawned UNCONDITIONALLY — not gated on creds. The setup worker runs + // ensureGraphDeps() (purely local code-graph provisioning) BEFORE its own + // credentials early-return, so it must fire even when logged out. The + // remote/credentialed work (autoupdate, table + placeholder) stays gated + // INSIDE the worker on its `if (!creds?.token) return`. + { const setupScript = join(__bundleDir, "session-start-setup.js"); const child = spawn("node", [setupScript], { detached: true, diff --git a/src/hooks/graph-deps-worker.ts b/src/hooks/graph-deps-worker.ts new file mode 100644 index 00000000..3ac6624a --- /dev/null +++ b/src/hooks/graph-deps-worker.ts @@ -0,0 +1,39 @@ +#!/usr/bin/env node + +/** + * Detached background worker that provisions the code-graph tree-sitter + * parsers into the shared embed-deps dir (see src/cli/graph-deps.ts). + * + * Why a dedicated detached worker (NOT an inline call in the SessionStart + * setup hook): a cold provision runs `npm install` plus a from-source native + * compile, which on a slow / arm64 box can take MINUTES. The SessionStart + * setup hooks run under the harness's ~120s async timeout — a synchronous + * inline install would blow that cap and get the hook killed mid-install, + * leaving a half-written tree. Spawning this worker detached + unref'd lets + * the hook return immediately while the install runs to completion in the + * background; the mkdir lockdir inside ensureGraphDeps serializes concurrent + * workers, and the ready marker makes the next session's fast path a no-op. + * + * Best-effort + self-contained: ensureGraphDeps swallows every failure + * internally (logs to the debug channel, records a backoff attempt) and never + * throws to us. The worker writes nothing to stdout/stderr — it's detached and + * any output would go nowhere useful. + * + * The CLI paths (`hivemind graph init`, `installEmbeddings`) still call + * ensureGraphDeps() inline: those run in the foreground where blocking is fine + * and expected. + */ + +import { ensureGraphDeps } from "../cli/graph-deps.js"; +import { log as _log } from "../utils/debug.js"; + +const log = (msg: string) => _log("graph-deps-worker", msg); + +try { + ensureGraphDeps({ logFn: log, warnFn: log }); +} catch (e: any) { + // ensureGraphDeps is best-effort and shouldn't throw, but never let a + // stray error escape to a non-zero exit — the worker is fire-and-forget. + log(`fatal: ${e?.message ?? e}`); +} +process.exit(0); diff --git a/src/hooks/session-start-setup.ts b/src/hooks/session-start-setup.ts index a71828f5..192b1081 100644 --- a/src/hooks/session-start-setup.ts +++ b/src/hooks/session-start-setup.ts @@ -18,6 +18,7 @@ import { makeWikiLogger } from "../utils/wiki-log.js"; import { EmbedClient } from "../embeddings/client.js"; import { embeddingsDisabled, embeddingsStatus } from "../embeddings/disable.js"; import { autoUpdate } from "./shared/autoupdate.js"; +import { spawnDetachedNodeWorker } from "../utils/spawn-detached.js"; const log = (msg: string) => _log("session-setup", msg); const __bundleDir = dirname(fileURLToPath(import.meta.url)); @@ -32,6 +33,17 @@ async function main(): Promise { if (process.env.HIVEMIND_WIKI_WORKER === "1") return; const input = await readStdin(); + + // Provision the code-graph tree-sitter parsers into the shared embed-deps + // dir so the graph-on-stop hook can auto-build the graph. Spawned as a + // DETACHED worker — NOT run inline — because a cold provision runs npm + + // a from-source native compile that can exceed this hook's ~120s async + // timeout; the worker outlives the hook and finishes in the background. + // Fired BEFORE the credentials early-return: provisioning is purely local + // and must not depend on login. Best-effort — the spawn helper swallows any + // failure, and ensureGraphDeps inside the worker serializes via its own lock. + spawnDetachedNodeWorker(join(__bundleDir, "graph-deps-worker.js")); + const creds = loadCredentials(); if (!creds?.token) { log("no credentials"); return; } diff --git a/tests/claude-code/session-start-setup-branches.test.ts b/tests/claude-code/session-start-setup-branches.test.ts index 643168e7..22bb7e1f 100644 --- a/tests/claude-code/session-start-setup-branches.test.ts +++ b/tests/claude-code/session-start-setup-branches.test.ts @@ -54,6 +54,9 @@ vi.mock("../../src/embeddings/client.js", () => ({ async warmup() { return embedWarmupMock(); } }, })); +// The hook spawns a detached graph-deps worker; stub the spawn boundary so +// this branch-coverage test never launches a real process. +vi.mock("../../src/utils/spawn-detached.js", () => ({ spawnDetachedNodeWorker: vi.fn() })); async function runHook(): Promise { delete process.env.HIVEMIND_WIKI_WORKER; diff --git a/tests/claude-code/session-start-setup-hook.test.ts b/tests/claude-code/session-start-setup-hook.test.ts index 15235afd..9ec70e29 100644 --- a/tests/claude-code/session-start-setup-hook.test.ts +++ b/tests/claude-code/session-start-setup-hook.test.ts @@ -18,6 +18,7 @@ const ensureTableMock = vi.fn(); const ensureSessionsTableMock = vi.fn(); const autoUpdateMock = vi.fn(); const embedWarmupMock = vi.fn(); +const spawnDetachedMock = vi.fn(); vi.mock("../../src/utils/stdin.js", () => ({ readStdin: (...a: any[]) => stdinMock(...a) })); vi.mock("../../src/commands/auth.js", () => ({ @@ -38,6 +39,13 @@ vi.mock("../../src/deeplake-api.js", () => ({ vi.mock("../../src/hooks/shared/autoupdate.js", () => ({ autoUpdate: (...a: any[]) => autoUpdateMock(...a), })); +// The hook no longer provisions graph-deps inline: a cold npm install + +// native compile can exceed the ~120s hook timeout, so it spawns a DETACHED +// worker via spawnDetachedNodeWorker. Mock that boundary and assert the hook +// spawns graph-deps-worker.js (and BEFORE the credentials gate). +vi.mock("../../src/utils/spawn-detached.js", () => ({ + spawnDetachedNodeWorker: (...a: any[]) => spawnDetachedMock(...a), +})); vi.mock("../../src/embeddings/client.js", () => ({ EmbedClient: class { async warmup() { return embedWarmupMock(); } @@ -108,6 +116,7 @@ beforeEach(() => { ensureSessionsTableMock.mockReset().mockResolvedValue(undefined); autoUpdateMock.mockReset().mockResolvedValue(undefined); embedWarmupMock.mockReset().mockResolvedValue(true); + spawnDetachedMock.mockReset(); fetchMock.mockReset().mockResolvedValue({ ok: true, json: async () => ({ version: "0.0.1" }), @@ -147,6 +156,44 @@ describe("session-start-setup hook — guards", () => { }); }); +describe("session-start-setup hook — graph-deps provisioning (detached worker)", () => { + const spawnedGraphDeps = () => + spawnDetachedMock.mock.calls.find(([p]) => typeof p === "string" && p.endsWith("graph-deps-worker.js")); + + it("spawns the graph-deps worker detached once on the happy path (NOT inline)", async () => { + await runHook(); + expect(spawnedGraphDeps()).toBeDefined(); + }); + + it("spawns graph-deps even when there are NO credentials (local, login-independent)", async () => { + loadCredsMock.mockReturnValue(null); + await runHook(); + // The worker spawn must fire BEFORE the credentials early-return. + expect(spawnedGraphDeps()).toBeDefined(); + expect(debugLogMock).toHaveBeenCalledWith("no credentials"); + }); + + it("spawns the graph-deps worker BEFORE loadCredentials (ordering)", async () => { + let graphAt = -1; + let credsAt = -1; + let counter = 0; + spawnDetachedMock.mockImplementation((p: string) => { if (p.endsWith("graph-deps-worker.js")) graphAt = counter++; }); + loadCredsMock.mockImplementation(() => { credsAt = counter++; return { token: "tok", userName: "alice" }; }); + await runHook(); + expect(graphAt).toBeGreaterThanOrEqual(0); + expect(credsAt).toBeGreaterThanOrEqual(0); + expect(graphAt).toBeLessThan(credsAt); + }); + + it("the detached spawn does not block the rest of the hook (table setup still runs)", async () => { + // spawnDetachedNodeWorker is fire-and-forget: the hook proceeds to its + // credentialed table-setup work without awaiting the worker. + await runHook(); + expect(spawnedGraphDeps()).toBeDefined(); + expect(ensureTableMock).toHaveBeenCalled(); + }); +}); + describe("session-start-setup hook — userName backfill", () => { it("backfills userName via node:os when missing and saves creds", async () => { loadCredsMock.mockReturnValue({ token: "tok", orgId: "o", orgName: "acme" }); diff --git a/tests/codex/codex-integration.test.ts b/tests/codex/codex-integration.test.ts index 59edfabb..d8b5be28 100644 --- a/tests/codex/codex-integration.test.ts +++ b/tests/codex/codex-integration.test.ts @@ -334,12 +334,15 @@ describe("codex integration: session-start-setup", () => { }); it("exits cleanly with no credentials (HIVEMIND_TOKEN='')", () => { + // HIVEMIND_GRAPH_ON_STOP=0 makes the detached graph-deps worker (spawned + // by this hook) early-return instead of running a real npm install against + // the dev machine's ~/.hivemind/embed-deps during the integration run. const raw = runHook("session-start-setup.js", { session_id: "test-session-setup-002", cwd: "/tmp/test-project", hook_event_name: "SessionStart", model: "gpt-5.2", - }); + }, { HIVEMIND_GRAPH_ON_STOP: "0" }); expect(raw).toBe(""); }); @@ -349,7 +352,7 @@ describe("codex integration: session-start-setup", () => { cwd: "/tmp/test-project", hook_event_name: "SessionStart", model: "gpt-5.2", - }); + }, { HIVEMIND_GRAPH_ON_STOP: "0" }); expect(raw).toBe(""); }); }); diff --git a/tests/codex/codex-session-start-hook.test.ts b/tests/codex/codex-session-start-hook.test.ts index c9807f8d..95d22a52 100644 --- a/tests/codex/codex-session-start-hook.test.ts +++ b/tests/codex/codex-session-start-hook.test.ts @@ -101,7 +101,17 @@ describe("codex session-start hook — guards", () => { it("emits not-logged-in context when creds are missing (no token)", async () => { loadCredsMock.mockReturnValue(null); const out = await runHook(); - expect(spawnMock).not.toHaveBeenCalled(); + // The setup worker now spawns UNCONDITIONALLY (it provisions graph-deps + // before its own creds gate). But the graph-pull-worker stays creds-gated, + // so exactly one spawn (session-start-setup.js) fires here. + const setupCall = spawnMock.mock.calls.find( + ([_cmd, args]) => Array.isArray(args) && args[0]?.includes?.("session-start-setup.js"), + ); + expect(setupCall).toBeDefined(); + const pullCall = spawnMock.mock.calls.find( + ([_cmd, args]) => Array.isArray(args) && args[0]?.includes?.("graph-pull-worker"), + ); + expect(pullCall).toBeUndefined(); // Codex hook now emits JSON, not plain text. Parse + assert on // additionalContext (single-line status). See AGENT_CHANNELS.md → Codex // for why we kept this minimal. @@ -181,10 +191,22 @@ describe("codex session-start hook — spawn async setup", () => { expect(debugLogMock).toHaveBeenCalledWith("spawned async setup process"); }); - it("does not spawn when creds are missing", async () => { + it("spawns the setup worker even when creds are missing (graph-deps is login-independent)", async () => { loadCredsMock.mockReturnValue({ token: "" }); + const fake = makeFakeChild(); + spawnMock.mockReturnValue(fake); await runHook(); - expect(spawnMock).not.toHaveBeenCalled(); + // session-start-setup.js MUST spawn (it provisions graph-deps before its + // own creds gate). The graph-pull-worker MUST NOT (still creds-gated). + const setupCall = spawnMock.mock.calls.find( + ([_cmd, args]) => Array.isArray(args) && args[0]?.includes?.("session-start-setup.js"), + ); + expect(setupCall).toBeDefined(); + expect(debugLogMock).toHaveBeenCalledWith("spawned async setup process"); + const pullCall = spawnMock.mock.calls.find( + ([_cmd, args]) => Array.isArray(args) && args[0]?.includes?.("graph-pull-worker"), + ); + expect(pullCall).toBeUndefined(); }); it("logs 'triggered (background)' on the auto-mine path when creds are missing and worker actually fires", async () => { diff --git a/tests/codex/codex-session-start-setup-hook.test.ts b/tests/codex/codex-session-start-setup-hook.test.ts index d29108cc..3472a2f1 100644 --- a/tests/codex/codex-session-start-setup-hook.test.ts +++ b/tests/codex/codex-session-start-setup-hook.test.ts @@ -21,6 +21,7 @@ const ensureTableMock = vi.fn(); const ensureSessionsTableMock = vi.fn(); const queryMock = vi.fn(); const autoUpdateMock = vi.fn(); +const spawnDetachedMock = vi.fn(); vi.mock("../../src/utils/stdin.js", () => ({ readStdin: (...a: any[]) => stdinMock(...a) })); vi.mock("../../src/commands/auth.js", () => ({ @@ -45,6 +46,13 @@ vi.mock("../../src/deeplake-api.js", () => ({ vi.mock("../../src/hooks/shared/autoupdate.js", () => ({ autoUpdate: (...a: any[]) => autoUpdateMock(...a), })); +// The hook no longer provisions graph-deps inline: a cold npm install + +// native compile can exceed the ~120s hook timeout, so it spawns a DETACHED +// worker via spawnDetachedNodeWorker. Mock that boundary and assert the hook +// spawns graph-deps-worker.js (and BEFORE the credentials gate). +vi.mock("../../src/utils/spawn-detached.js", () => ({ + spawnDetachedNodeWorker: (...a: any[]) => spawnDetachedMock(...a), +})); async function runHook(env: Record = {}): Promise { delete process.env.HIVEMIND_WIKI_WORKER; @@ -80,6 +88,7 @@ beforeEach(() => { ensureSessionsTableMock.mockReset().mockResolvedValue(undefined); queryMock.mockReset().mockResolvedValue([]); // placeholder SELECT → empty, INSERT will follow autoUpdateMock.mockReset().mockResolvedValue(undefined); + spawnDetachedMock.mockReset(); }); afterEach(() => { @@ -100,6 +109,42 @@ describe("codex session-start-setup hook — guards", () => { }); }); +describe("codex session-start-setup hook — graph-deps provisioning (detached worker)", () => { + const spawnedGraphDeps = () => + spawnDetachedMock.mock.calls.find(([p]) => typeof p === "string" && p.endsWith("graph-deps-worker.js")); + + it("spawns the graph-deps worker detached once on the happy path (NOT inline)", async () => { + await runHook(); + expect(spawnedGraphDeps()).toBeDefined(); + }); + + it("spawns graph-deps even when there are NO credentials (local, login-independent)", async () => { + loadCredsMock.mockReturnValue(null); + await runHook(); + // The worker spawn must fire BEFORE the credentials early-return. + expect(spawnedGraphDeps()).toBeDefined(); + expect(debugLogMock).toHaveBeenCalledWith("no credentials"); + }); + + it("spawns the graph-deps worker BEFORE loadCredentials (ordering)", async () => { + let graphAt = -1; + let credsAt = -1; + let counter = 0; + spawnDetachedMock.mockImplementation((p: string) => { if (p.endsWith("graph-deps-worker.js")) graphAt = counter++; }); + loadCredsMock.mockImplementation(() => { credsAt = counter++; return { token: "tok", userName: "alice" }; }); + await runHook(); + expect(graphAt).toBeGreaterThanOrEqual(0); + expect(credsAt).toBeGreaterThanOrEqual(0); + expect(graphAt).toBeLessThan(credsAt); + }); + + it("the detached spawn does not block the rest of the hook (table setup still runs)", async () => { + await runHook(); + expect(spawnedGraphDeps()).toBeDefined(); + expect(ensureTableMock).toHaveBeenCalled(); + }); +}); + describe("codex session-start-setup hook — userName backfill", () => { it("backfills userName when missing and saves creds", async () => { loadCredsMock.mockReturnValue({ token: "tok", orgId: "o", orgName: "acme" }); diff --git a/tests/shared/graph-deps.test.ts b/tests/shared/graph-deps.test.ts index cf42f3c6..55b619b7 100644 --- a/tests/shared/graph-deps.test.ts +++ b/tests/shared/graph-deps.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { mkdtempSync, mkdirSync, rmSync, writeFileSync, readFileSync, existsSync } from "node:fs"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync, readFileSync, existsSync, statSync, utimesSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -13,7 +13,7 @@ vi.mock("node:child_process", async (orig) => ({ })); import { execFileSync } from "node:child_process"; -import { treeSitterSpecs, isGraphDepsInstalled, ensureGraphDeps } from "../../src/cli/graph-deps.js"; +import { treeSitterSpecs, isGraphDepsInstalled, ensureGraphDeps, graphDepsReadyKey, defaultRunHeal } from "../../src/cli/graph-deps.js"; /** * The code-graph auto-build hook resolves tree-sitter from the shared @@ -113,6 +113,24 @@ describe("graph-deps helpers", () => { }; } + it("HIVEMIND_GRAPH_ON_STOP=0 → provisioning skipped (no npm, no heal, no lock)", () => { + const prev = process.env.HIVEMIND_GRAPH_ON_STOP; + process.env.HIVEMIND_GRAPH_ON_STOP = "0"; + try { + const runNpm = vi.fn(); + const runHeal = vi.fn(); + const logFn = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm, runHeal, logFn })); + expect(runNpm).not.toHaveBeenCalled(); + expect(runHeal).not.toHaveBeenCalled(); + expect(existsSync(join(dir, ".graph-deps.lock"))).toBe(false); + expect(logFn).toHaveBeenCalledWith(expect.stringContaining("HIVEMIND_GRAPH_ON_STOP=0")); + } finally { + if (prev === undefined) delete process.env.HIVEMIND_GRAPH_ON_STOP; + else process.env.HIVEMIND_GRAPH_ON_STOP = prev; + } + }); + it("empty specs → warns and skips (no npm, no heal)", () => { const runNpm = vi.fn(); const runHeal = vi.fn(); @@ -123,26 +141,44 @@ describe("graph-deps helpers", () => { expect(warnFn).toHaveBeenCalledWith(expect.stringContaining("no tree-sitter optionalDependencies")); }); - it("fresh → creates package.json, installs, writes the marker, then heals", () => { - const runNpm = vi.fn((s: string[]) => fakeInstall(s)); - const runHeal = vi.fn(); + it("fresh → creates package.json, installs, heals, THEN writes the ready marker", () => { + const order: string[] = []; + const runNpm = vi.fn((s: string[]) => { order.push("npm"); fakeInstall(s); }); + const runHeal = vi.fn(() => { order.push("heal"); }); ensureGraphDeps(baseDeps({ runNpm, runHeal })); expect(runNpm).toHaveBeenCalledTimes(1); expect(runNpm).toHaveBeenCalledWith(SPECS, dir); expect(existsSync(join(dir, "package.json"))).toBe(true); - expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(SPECS.join("\n")); - // Heal ALWAYS runs, even right after a fresh install. + // Marker is the platform/arch/ABI-keyed ready key, not a bare spec join. + expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(graphDepsReadyKey(SPECS)); + // Heal runs, and the install happens BEFORE the heal. expect(runHeal).toHaveBeenCalledTimes(1); + expect(order).toEqual(["npm", "heal"]); }); - it("already present + marker matches → skips the install but STILL heals", () => { + it("satisfied path (ready marker matches + parsers present) → spawns NOTHING", () => { fakeInstall(SPECS); - writeFileSync(join(dir, ".graph-deps"), SPECS.join("\n")); + writeFileSync(join(dir, ".graph-deps"), graphDepsReadyKey(SPECS)); const runNpm = vi.fn(); const runHeal = vi.fn(); + // Also assert the real default boundaries never spawn: no runNpm/runHeal + // injected AND execFileSync (the process seam) must stay untouched. + vi.mocked(execFileSync).mockClear(); ensureGraphDeps(baseDeps({ runNpm, runHeal })); expect(runNpm).not.toHaveBeenCalled(); - expect(runHeal).toHaveBeenCalledTimes(1); + expect(runHeal).not.toHaveBeenCalled(); + // No lockdir was even created (we returned on the pre-lock fast path). + expect(existsSync(join(dir, ".graph-deps.lock"))).toBe(false); + }); + + it("default satisfied path makes zero child_process calls", () => { + fakeInstall(SPECS); + writeFileSync(join(dir, ".graph-deps"), graphDepsReadyKey(SPECS)); + vi.mocked(execFileSync).mockClear(); + // No runNpm/runHeal injected → the real defaults would spawn via + // execFileSync if reached. On the satisfied path they must NOT be. + ensureGraphDeps({ sharedDir: dir, sharedNodeModules: nm, specs: SPECS, logFn: () => {}, warnFn: () => {} }); + expect(vi.mocked(execFileSync)).not.toHaveBeenCalled(); }); it("stale marker (spec set changed) → reinstalls even if the dirs exist", () => { @@ -151,29 +187,137 @@ describe("graph-deps helpers", () => { const runNpm = vi.fn((s: string[]) => fakeInstall(s)); ensureGraphDeps(baseDeps({ runNpm, runHeal: () => {} })); expect(runNpm).toHaveBeenCalledTimes(1); - expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(SPECS.join("\n")); + expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(graphDepsReadyKey(SPECS)); + }); + + it("ABI mismatch (marker for a different Node ABI) → reinstalls", () => { + fakeInstall(SPECS); + // A ready key built for a stale ABI: same specs/platform/arch but a + // different module-ABI line. Simulates a Node major upgrade. + const staleAbiKey = graphDepsReadyKey(SPECS).replace(/abi=\d+/, "abi=0"); + expect(staleAbiKey).not.toBe(graphDepsReadyKey(SPECS)); + writeFileSync(join(dir, ".graph-deps"), staleAbiKey); + const runNpm = vi.fn((s: string[]) => fakeInstall(s)); + const runHeal = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm, runHeal })); + expect(runNpm).toHaveBeenCalledTimes(1); + expect(runHeal).toHaveBeenCalledTimes(1); + expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(graphDepsReadyKey(SPECS)); }); it("partial install (marker matches but a parser dir is missing) → reinstalls", () => { mkdirSync(join(nm, "tree-sitter"), { recursive: true }); // only one of two - writeFileSync(join(dir, ".graph-deps"), SPECS.join("\n")); - const runNpm = vi.fn(); + writeFileSync(join(dir, ".graph-deps"), graphDepsReadyKey(SPECS)); + const runNpm = vi.fn((s: string[]) => fakeInstall(s)); ensureGraphDeps(baseDeps({ runNpm, runHeal: () => {} })); expect(runNpm).toHaveBeenCalledTimes(1); }); - it("npm failure is swallowed (best-effort) — never throws, no marker written", () => { + it("lock contention: a second caller skips while the lock is held", () => { + // Pre-create the lockdir to simulate a live in-flight installer. + mkdirSync(join(dir, ".graph-deps.lock"), { recursive: true }); + const runNpm = vi.fn(); + const runHeal = vi.fn(); + const logFn = vi.fn(); + // Provisioning is needed (no marker) but the lock is held, so this + // contender must skip without installing. + ensureGraphDeps(baseDeps({ runNpm, runHeal, logFn })); + expect(runNpm).not.toHaveBeenCalled(); + expect(runHeal).not.toHaveBeenCalled(); + expect(existsSync(join(dir, ".graph-deps"))).toBe(false); + expect(logFn).toHaveBeenCalledWith(expect.stringContaining("another install holds the lock")); + }); + + it("stale lock (older than the reclaim window) → reclaimed and install proceeds", () => { + const lockDir = join(dir, ".graph-deps.lock"); + mkdirSync(lockDir, { recursive: true }); + // Backdate the lockdir mtime well past the 30-min stale threshold. + const old = new Date(Date.now() - 45 * 60 * 1000); + utimesSync(lockDir, old, old); + const runNpm = vi.fn((s: string[]) => fakeInstall(s)); + const runHeal = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm, runHeal })); + expect(runNpm).toHaveBeenCalledTimes(1); + expect(runHeal).toHaveBeenCalledTimes(1); + expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(graphDepsReadyKey(SPECS)); + // Lock released after a successful provision. + expect(existsSync(lockDir)).toBe(false); + }); + + it("held lock just UNDER the 30-min window is NOT reclaimed (raised threshold)", () => { + const lockDir = join(dir, ".graph-deps.lock"); + mkdirSync(lockDir, { recursive: true }); + // 20 min old: past the OLD 10-min threshold but under the new 30-min one, + // so a genuinely long install is left alone rather than reclaimed. + const t = new Date(Date.now() - 20 * 60 * 1000); + utimesSync(lockDir, t, t); + const runNpm = vi.fn(); + const runHeal = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm, runHeal })); + expect(runNpm).not.toHaveBeenCalled(); + expect(runHeal).not.toHaveBeenCalled(); + expect(existsSync(join(dir, ".graph-deps"))).toBe(false); + }); + + it("release is a no-op when the lockdir was reclaimed (owner token mismatch)", () => { + // Simulate a stale-reclaim by another process WHILE we hold the lock: + // during our runNpm, overwrite the owner token so it no longer matches + // ours. releaseLock must then NOT delete the lockdir out from under the + // new owner. We assert the lockdir + the foreign owner survive. + const lockDir = join(dir, ".graph-deps.lock"); + const runNpm = vi.fn((s: string[]) => { + fakeInstall(s); + // Another reclaimer took the lock and stamped its own token. + writeFileSync(join(lockDir, "owner"), "someone-else-99"); + }); + const runHeal = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm, runHeal })); + // Our provision still completed (marker written), but the lockdir is left + // intact because its owner is no longer us. + expect(readFileSync(join(dir, ".graph-deps"), "utf8")).toBe(graphDepsReadyKey(SPECS)); + expect(existsSync(lockDir)).toBe(true); + expect(readFileSync(join(lockDir, "owner"), "utf8")).toBe("someone-else-99"); + }); + + it("refreshes the lock mtime before heal so a long install isn't reclaimed mid-flight", () => { + // Inside runNpm we backdate the lockdir to simulate a mid-flight long + // install. The refresh BEFORE heal must bump it forward again — we read + // the mtime inside runHeal (after that refresh ran) and assert it's fresh. + const lockDir = join(dir, ".graph-deps.lock"); + let mtimeAfterBackdate = 0; + let mtimeInsideHeal = 0; + const runNpm = vi.fn((s: string[]) => { + const past = new Date(Date.now() - 60 * 60 * 1000); + utimesSync(lockDir, past, past); + mtimeAfterBackdate = statSync(lockDir).mtimeMs; + fakeInstall(s); + }); + const runHeal = vi.fn(() => { mtimeInsideHeal = statSync(lockDir).mtimeMs; }); + ensureGraphDeps(baseDeps({ runNpm, runHeal })); + // The pre-heal refresh moved the mtime forward from the backdated value. + expect(mtimeInsideHeal).toBeGreaterThan(mtimeAfterBackdate); + // And it's genuinely fresh (within the stale window), not still an hour old. + expect(Date.now() - mtimeInsideHeal).toBeLessThan(30 * 60 * 1000); + }); + + it("npm failure is swallowed (best-effort) — never throws, no marker written, lock released", () => { const runNpm = vi.fn(() => { throw new Error("npm exploded"); }); const warnFn = vi.fn(); expect(() => ensureGraphDeps(baseDeps({ runNpm, runHeal: () => {}, warnFn }))).not.toThrow(); expect(existsSync(join(dir, ".graph-deps"))).toBe(false); + // The finally-block release must run even on failure. + expect(existsSync(join(dir, ".graph-deps.lock"))).toBe(false); expect(warnFn).toHaveBeenCalledWith(expect.stringContaining("tree-sitter provisioning failed")); }); - it("heal failure is swallowed too — never throws", () => { + it("heal failure is swallowed too — never throws AND no marker (heal must succeed first)", () => { const runHeal = vi.fn(() => { throw new Error("heal exploded"); }); const warnFn = vi.fn(); expect(() => ensureGraphDeps(baseDeps({ runNpm: (s: string[]) => fakeInstall(s), runHeal, warnFn }))).not.toThrow(); + // Marker is written ONLY after BOTH npm and heal succeed — heal blew up, + // so no marker: next session re-provisions. + expect(existsSync(join(dir, ".graph-deps"))).toBe(false); + expect(existsSync(join(dir, ".graph-deps.lock"))).toBe(false); expect(warnFn).toHaveBeenCalledWith(expect.stringContaining("tree-sitter provisioning failed")); }); @@ -186,9 +330,87 @@ describe("graph-deps helpers", () => { // defaultRunNpm issued `npm install ...`. expect(calls.some((c) => c[0] === "npm" && Array.isArray(c[1]) && c[1][0] === "install")).toBe(true); // defaultRunHeal spawned the ensure-tree-sitter heal via node (the repo - // ships scripts/ensure-tree-sitter.mjs, so the existsSync gate passes). - expect(calls.some((c) => c[0] === process.execPath - && Array.isArray(c[1]) && String(c[1][0]).endsWith("ensure-tree-sitter.mjs"))).toBe(true); + // ships scripts/ensure-tree-sitter.mjs, so the existsSync gate passes), + // AND forced strict mode so a bindings-load failure exits non-zero + // (otherwise the heal false-succeeds over an unloadable addon). + const healCall = calls.find((c) => c[0] === process.execPath + && Array.isArray(c[1]) && String(c[1][0]).endsWith("ensure-tree-sitter.mjs")); + expect(healCall).toBeDefined(); + const healOpts = healCall![2] as { env?: Record } | undefined; + expect(healOpts?.env?.HIVEMIND_STRICT_POSTINSTALL).toBe("1"); + }); + + it("deletes a MATCHING ready marker before repair, so a mid-repair crash leaves no marker", () => { + // Marker matches the current key BUT a parser dir is missing → the + // under-lock path runs the repair. It must DELETE the marker before + // mutating, so a crash during npm/heal can't leave a stale "ready" marker + // fast-pathing over a broken tree. + mkdirSync(join(nm, "tree-sitter"), { recursive: true }); // only one of two present + writeFileSync(join(dir, ".graph-deps"), graphDepsReadyKey(SPECS)); + let markerDuringNpm: string | null = null; + const runNpm = vi.fn(() => { + markerDuringNpm = existsSync(join(dir, ".graph-deps")) + ? readFileSync(join(dir, ".graph-deps"), "utf8") + : null; + throw new Error("npm crashed mid-install"); + }); + ensureGraphDeps(baseDeps({ runNpm, runHeal: () => {}, warnFn: () => {} })); + // The marker was already gone by the time npm ran (deleted before mutation). + expect(markerDuringNpm).toBeNull(); + // And it stays absent after the crash → next session retries. + expect(existsSync(join(dir, ".graph-deps"))).toBe(false); + }); + + it("records a failure attempt on failure, then backs off the NEXT call (no npm re-run)", () => { + const attemptFile = join(dir, ".graph-deps.attempt"); + // First call fails → records the attempt. + const runNpm1 = vi.fn(() => { throw new Error("offline"); }); + ensureGraphDeps(baseDeps({ runNpm: runNpm1, runHeal: () => {}, warnFn: () => {} })); + expect(runNpm1).toHaveBeenCalledTimes(1); + expect(existsSync(attemptFile)).toBe(true); + const rec = JSON.parse(readFileSync(attemptFile, "utf8")); + expect(typeof rec.lastAttemptAt).toBe("number"); + expect(rec.failures).toBe(1); + + // Second call within the backoff window → must SKIP without touching npm. + const runNpm2 = vi.fn(() => { throw new Error("should not run"); }); + const logFn = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm: runNpm2, runHeal: () => {}, logFn, warnFn: () => {} })); + expect(runNpm2).not.toHaveBeenCalled(); + expect(logFn).toHaveBeenCalledWith(expect.stringContaining("a recent attempt failed")); + }); + + it("a stale failure attempt (older than the backoff) does NOT block a retry", () => { + const attemptFile = join(dir, ".graph-deps.attempt"); + // Failure recorded 7h ago — past the 6h backoff. + writeFileSync(attemptFile, JSON.stringify({ lastAttemptAt: Date.now() - 7 * 60 * 60 * 1000, failures: 3 })); + const runNpm = vi.fn((s: string[]) => fakeInstall(s)); + ensureGraphDeps(baseDeps({ runNpm, runHeal: () => {} })); + expect(runNpm).toHaveBeenCalledTimes(1); + }); + + it("a successful provision CLEARS the failure attempt file", () => { + const attemptFile = join(dir, ".graph-deps.attempt"); + // A stale (past-backoff) failure exists so the retry proceeds. + writeFileSync(attemptFile, JSON.stringify({ lastAttemptAt: Date.now() - 7 * 60 * 60 * 1000, failures: 2 })); + ensureGraphDeps(baseDeps({ runNpm: (s: string[]) => fakeInstall(s), runHeal: () => {} })); + expect(existsSync(join(dir, ".graph-deps"))).toBe(true); // marker written + expect(existsSync(attemptFile)).toBe(false); // attempt cleared + }); + + it("the healthy fast path never touches the attempt file", () => { + const attemptFile = join(dir, ".graph-deps.attempt"); + // A recent failure is on disk, but the install is already ready → the + // fast path returns BEFORE the backoff check, so it must not be consulted + // or cleared. (Proves the fast-path/marker check stays first.) + writeFileSync(attemptFile, JSON.stringify({ lastAttemptAt: Date.now(), failures: 9 })); + fakeInstall(SPECS); + writeFileSync(join(dir, ".graph-deps"), graphDepsReadyKey(SPECS)); + const runNpm = vi.fn(); + ensureGraphDeps(baseDeps({ runNpm, runHeal: () => {} })); + expect(runNpm).not.toHaveBeenCalled(); + // Attempt file left exactly as-is (not read, not cleared). + expect(JSON.parse(readFileSync(attemptFile, "utf8")).failures).toBe(9); }); it("does not clobber an existing package.json", () => { @@ -197,9 +419,36 @@ describe("graph-deps helpers", () => { const pkgPath = join(dir, "package.json"); writeFileSync(pkgPath, JSON.stringify({ dependencies: { "@huggingface/transformers": "^3.0.0" } })); fakeInstall(SPECS); - writeFileSync(join(dir, ".graph-deps"), SPECS.join("\n")); + writeFileSync(join(dir, ".graph-deps"), graphDepsReadyKey(SPECS)); ensureGraphDeps(baseDeps({ runNpm: vi.fn(), runHeal: () => {} })); expect(JSON.parse(readFileSync(pkgPath, "utf8")).dependencies["@huggingface/transformers"]).toBe("^3.0.0"); }); }); + + describe("defaultRunHeal", () => { + it("throws when the heal script is missing (missing script = failure, not silent success)", () => { + const missing = join(dir, "no-such-heal.mjs"); + expect(() => defaultRunHeal(dir, missing)).toThrow(/heal script missing/); + // execFileSync must NOT have been reached — the throw happens on the + // existsSync gate, before any spawn. + vi.mocked(execFileSync).mockClear(); + expect(() => defaultRunHeal(dir, missing)).toThrow(); + expect(vi.mocked(execFileSync)).not.toHaveBeenCalled(); + }); + + it("runs the heal with HIVEMIND_STRICT_POSTINSTALL=1 when the script exists", () => { + // Use this test file itself as a stand-in "existing script" so the + // existsSync gate passes; execFileSync is mocked so nothing really runs. + const present = join(dir, "heal.mjs"); + writeFileSync(present, "// noop"); + vi.mocked(execFileSync).mockClear(); + defaultRunHeal(dir, present); + const calls = vi.mocked(execFileSync).mock.calls; + expect(calls).toHaveLength(1); + expect(calls[0][0]).toBe(process.execPath); + expect(calls[0][1]).toEqual([present]); + const opts = calls[0][2] as { env?: Record }; + expect(opts.env?.HIVEMIND_STRICT_POSTINSTALL).toBe("1"); + }); + }); });