From fcf941d272ca566537a45b108dd147f2ad713c7d Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Tue, 21 Jul 2026 21:07:08 +0000 Subject: [PATCH 1/2] fix(graph-deps): harden ensureGraphDeps for unattended self-heal Prepares ensureGraphDeps to run unattended from session hooks: - Ownership-safe install lock: mkdir lockdir with an owner token; release only removes our own lock, stale reclaim (30 min) re-acquires instead of clobbering a live owner, and the lock mtime is refreshed before npm and before the heal so a long install is not reclaimed mid-flight. - The ready marker is deleted BEFORE any repair mutation and re-stamped only after BOTH npm install and the native heal succeed, keyed by spec set + platform + arch + Node ABI - a crash mid-repair leaves no marker. - Heal is strict: a missing ensure-tree-sitter script is a failure, and the script runs with HIVEMIND_STRICT_POSTINSTALL=1 so a failed bindings load exits non-zero instead of stamping a broken install as ready. - Offline backoff: a failed attempt is recorded (.graph-deps.attempt) and retried at most every 6h, so network-less machines do not spawn an npm install on every session; the satisfied fast path never touches it and spawns no subprocess at all. --- src/cli/graph-deps.ts | 310 ++++++++++++++++++++++++++++---- tests/shared/graph-deps.test.ts | 287 +++++++++++++++++++++++++++-- 2 files changed, 544 insertions(+), 53 deletions(-) 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/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"); + }); + }); }); From 58efef0bae4d9e3425149ab0a1d055dbbf8b3d52 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Tue, 21 Jul 2026 21:07:14 +0000 Subject: [PATCH 2/2] feat(graph-deps): self-heal existing installs from session start The graph auto-build hook resolves tree-sitter from the shared ~/.hivemind/embed-deps, but ensureGraphDeps only ran from 'graph init' and 'embeddings install' - installs that predate #323 never run either, so their graph-on-stop hook degrades to a skip forever (verified on a real machine: 0 tree-sitter packages, every session end logging "build threw: Cannot find package 'tree-sitter'"). Spawn a detached graph-deps worker from the claude-code and codex session-start-setup paths, before the credentials gate (provisioning is local): the hook returns immediately (the 120s async allowance never bounds the npm install), the install lock serializes concurrent sessions, and the codex setup worker now spawns unconditionally with credentialed remote setup still gated inside the worker. --- esbuild.config.mjs | 8 ++++ src/hooks/codex/session-start-setup.ts | 12 +++++ src/hooks/codex/session-start.ts | 13 +++-- src/hooks/graph-deps-worker.ts | 39 +++++++++++++++ src/hooks/session-start-setup.ts | 12 +++++ .../session-start-setup-branches.test.ts | 3 ++ .../session-start-setup-hook.test.ts | 47 +++++++++++++++++++ tests/codex/codex-integration.test.ts | 7 ++- tests/codex/codex-session-start-hook.test.ts | 28 +++++++++-- .../codex-session-start-setup-hook.test.ts | 45 ++++++++++++++++++ 10 files changed, 206 insertions(+), 8 deletions(-) create mode 100644 src/hooks/graph-deps-worker.ts 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/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" });