From 398d39593025ba7b2092870141b7dec040494b90 Mon Sep 17 00:00:00 2001 From: Emanuele Fenocchi Date: Sat, 18 Jul 2026 05:02:56 +0000 Subject: [PATCH] fix(graph): stop graph-on-stop hook crashing when tree-sitter is absent The graph-on-stop Stop/SessionEnd hook (claude-code, codex, cursor, hermes) statically imported the tree-sitter extractor chain (runBuildCommand -> extract/index -> import Parser from "tree-sitter"). esbuild hoisted the external tree-sitter import to the top of the bundle, so Node resolved it at module load -- before main()'s try/catch -- and on an install where the optional native grammar failed to build (Node 24 / arm64) the hook aborted with ERR_MODULE_NOT_FOUND and exit code 1. Mirrors the CLI fix already in bundle/cli.js (splitting), which only covered the CLI, not the hook. - src/hooks/graph-on-stop.ts: load runBuildCommand lazily via `await import("../commands/graph.js")` behind the gate, inside the existing try/catch. A missing extractor now degrades to a logged skip + exit 0. - esbuild.config.mjs: build graph-on-stop for the four harnesses via buildGraphOnStop() with splitting, so the dynamic import stays a runtime chunk load and tree-sitter lives only in graph-chunks/; clears stale hashed chunks first; removed graph-on-stop from the four shared harness lists. - esbuild.config.mjs: the lazy import() defeats tree-shaking in the non-split openclaw graph-worker build, pulling user-config.ts's HIVEMIND_CONFIG_PATH into openclaw/dist/graph-on-stop.js and tripping ClawHub's env-harvesting critical. Rewrite HIVEMIND_CONFIG_PATH to undefined in the existing openclawGraphWorkerDefine (the call site has a ?? homedir() fallback). - tests: unit reject-path coverage + a bundle-level guard asserting each of the four shipped entries has no static tree-sitter import and loads it only via graph-chunks/. Verified: bug reproduced on a clean current-main build (worktree); the fixed bundle exits 0 (graceful) with tree-sitter absent and still builds the graph when present; ClawHub audit 0 critical; full suite 5541 passed. --- esbuild.config.mjs | 109 ++++++++++++++++-- src/hooks/graph-on-stop.ts | 17 ++- .../shared/graph/graph-on-stop-bundle.test.ts | 69 +++++++++++ tests/shared/graph/graph-on-stop-main.test.ts | 31 +++++ 4 files changed, 215 insertions(+), 11 deletions(-) create mode 100644 tests/shared/graph/graph-on-stop-bundle.test.ts diff --git a/esbuild.config.mjs b/esbuild.config.mjs index e64d261c7..a90080c34 100644 --- a/esbuild.config.mjs +++ b/esbuild.config.mjs @@ -1,5 +1,5 @@ import { build } from "esbuild"; -import { chmodSync, writeFileSync, readFileSync } from "node:fs"; +import { chmodSync, writeFileSync, readFileSync, rmSync } from "node:fs"; const esmPackageJson = '{"type":"module"}\n'; const hivemindVersion = JSON.parse(readFileSync("package.json", "utf-8")).version; @@ -7,6 +7,73 @@ const openclawVersion = JSON.parse(readFileSync("harnesses/openclaw/package.json const openclawSkillBody = readFileSync("harnesses/openclaw/skills/SKILL.md", "utf-8"); const openclawGraphSkillBody = readFileSync("harnesses/openclaw/skills/hivemind-graph/SKILL.md", "utf-8"); +// tree-sitter + language grammars ship native .node prebuilds esbuild can't +// bundle; they're always external and resolved from node_modules at runtime. +const treeSitterExternals = [ + "tree-sitter", + "tree-sitter-typescript", + "tree-sitter-javascript", + "tree-sitter-python", + "tree-sitter-go", + "tree-sitter-rust", + "tree-sitter-java", + "tree-sitter-ruby", + "tree-sitter-c", + "tree-sitter-cpp", +]; + +/** + * Build the graph-on-stop Stop/SessionEnd hook as its OWN code-split bundle + * for a harness (claude-code / codex / cursor / hermes). + * + * Why splitting instead of a plain entry in the shared harness build list: + * the hook's build path (runBuildCommand → extract/index → `import Parser + * from "tree-sitter"`) is a chain of static ESM imports. As a plain bundle, + * esbuild hoists the external `import ... from "tree-sitter"` to the TOP of + * graph-on-stop.js, so Node resolves tree-sitter at MODULE LOAD — before + * main() runs. On an install where the tree-sitter optionalDependency failed + * to build (Node 24 / arm64), that load fails with ERR_MODULE_NOT_FOUND and + * the Stop hook exits 1 (the reported crash). main()'s catch never fires. + * + * With `splitting: true`, graph-on-stop.ts's `await import("../commands/graph.js")` + * stays a runtime import into a separate chunk; the tree-sitter statics live + * only in that chunk, loaded lazily behind the gate. A missing grammar then + * rejects the dynamic import, which main()'s try/catch turns into a logged + * skip + exit 0. This mirrors the CLI fix (bundle/cli.js) already in this file. + * The entry filename stays graph-on-stop.js so hook registrations are + * unchanged; the chunk lands under graph-chunks/. + * + * Stale chunks are cleared first (their names are content-hashed, so a code + * change would otherwise leave the previous chunk behind to ship as dead + * weight). Scoped to graph-chunks/ so the shared bundle outputs are untouched. + */ +async function buildGraphOnStop(outdir) { + rmSync(`${outdir}/graph-chunks`, { recursive: true, force: true }); + await build({ + entryPoints: { "graph-on-stop": "dist/src/hooks/graph-on-stop.js" }, + bundle: true, + platform: "node", + format: "esm", + outdir, + splitting: true, + chunkNames: "graph-chunks/[name]-[hash]", + external: [ + "node:*", + "node-liblzma", + "@mongodb-js/zstd", + "@huggingface/transformers", + "onnxruntime-node", + "onnxruntime-common", + "sharp", + ...treeSitterExternals, + ], + define: { + __HIVEMIND_VERSION__: JSON.stringify(hivemindVersion), + }, + }); + chmodSync(`${outdir}/graph-on-stop.js`, 0o755); +} + // Claude Code plugin const ccHooks = [ { entry: "dist/src/hooks/session-start.js", out: "session-start" }, @@ -26,10 +93,9 @@ const ccHooks = [ { entry: "dist/src/skillify/skillopt-worker.js", out: "skillopt-worker" }, // codebase-graph Phase 1.5: auto-build the graph at SessionEnd, gated // on (a) 10-min rate limit, (b) HEAD changed since last build, (c) ≥1 - // source file diff. See src/hooks/graph-on-stop.ts. - // Filename keeps the "on-stop" suffix for backward-compat with prior - // builds; the hook itself is registered under SessionEnd, not Stop. - { entry: "dist/src/hooks/graph-on-stop.js", out: "graph-on-stop" }, + // source file diff. See src/hooks/graph-on-stop.ts. Built separately via + // buildGraphOnStop() (code-split so tree-sitter loads lazily) — NOT in + // this shared list. Filename keeps the "on-stop" suffix for backward-compat. // codebase-graph Phase 3 v1.1: async auto-pull on SessionStart. // Spawned detached via nohup from each agent's SessionStart hook; // pulls the freshest cloud snapshot for HEAD if newer than local. @@ -102,7 +168,7 @@ const codexHooks = [ { entry: "dist/src/skillify/skillopt-worker.js", out: "skillopt-worker" }, { entry: "dist/src/hooks/graph-pull-worker.js", out: "graph-pull-worker" }, // G3: code-graph auto-build parity for Codex (same shared hook as CC/Cursor). - { entry: "dist/src/hooks/graph-on-stop.js", out: "graph-on-stop" }, + // graph-on-stop is built separately via buildGraphOnStop() (code-split). ]; const codexShell = [ @@ -166,8 +232,8 @@ const cursorHooks = [ { entry: "dist/src/hooks/graph-pull-worker.js", out: "graph-pull-worker" }, // A1 (graph Cursor parity): same auto-build hook as Claude Code, wired // to Cursor's stop + sessionEnd events in install-cursor.ts. Reuses the - // shared src/hooks/graph-on-stop.ts entry (no per-agent logic). - { entry: "dist/src/hooks/graph-on-stop.js", out: "graph-on-stop" }, + // shared src/hooks/graph-on-stop.ts entry (no per-agent logic). Built + // separately via buildGraphOnStop() (code-split). ]; // Hermes Agent shell-hook bundles (matches Claude Code's wire protocol; see @@ -184,7 +250,7 @@ const hermesHooks = [ { entry: "dist/src/skillify/skillopt-worker.js", out: "skillopt-worker" }, { entry: "dist/src/hooks/graph-pull-worker.js", out: "graph-pull-worker" }, // G3: code-graph auto-build parity for Hermes (registered on on_session_end). - { entry: "dist/src/hooks/graph-on-stop.js", out: "graph-on-stop" }, + // graph-on-stop is built separately via buildGraphOnStop() (code-split). ]; const cursorShell = [ @@ -328,6 +394,22 @@ for (const h of piWorker) { writeFileSync("harnesses/pi/bundle/package.json", esmPackageJson); writeFileSync("harnesses/hermes/bundle/package.json", esmPackageJson); +// Code-split graph-on-stop bundles for every harness that registers the hook +// as a Stop/SessionEnd handler. Kept out of the shared build lists above so +// its tree-sitter dependency loads lazily (see buildGraphOnStop). Each +// harness's bundle/package.json ({"type":"module"}) has already been written, +// so the emitted graph-chunks/ resolve as ESM. (OpenClaw ships graph-on-stop +// via its own graph-worker build below, which has separate env-rewrite +// handling, so it is intentionally not included here.) +for (const outdir of [ + "harnesses/claude-code/bundle", + "harnesses/codex/bundle", + "harnesses/cursor/bundle", + "harnesses/hermes/bundle", +]) { + await buildGraphOnStop(outdir); +} + // OpenClaw plugin bundle. The shared CC/Codex source modules reference a // handful of HIVEMIND_* env vars for dev-only overrides. Those env paths are // never taken in the openclaw runtime (the plugin loads config from @@ -579,6 +661,15 @@ const openclawGraphWorkerDefine = { "process.env.HIVEMIND_CODEBASE_TABLE": "undefined", "process.env.HIVEMIND_SESSIONS_TABLE": "undefined", "process.env.HIVEMIND_STATE_DIR": "globalThis.__hivemind_tuning__.HIVEMIND_STATE_DIR", + // Config-path resolver (src/user-config.ts). Pulled into the graph-on-stop + // bundle via graph-on-stop.ts's lazy `import("../commands/graph.js")` → + // config.js → user-config.ts: the dynamic import defeats the tree-shaking + // that previously dropped it under the old static import, so the literal + // `process.env.HIVEMIND_CONFIG_PATH` read now sits alongside fetch() and + // trips ClawHub's env-harvesting critical. openclaw has no reason to + // redirect the config path, so rewrite to undefined — the call site's + // `?? homedir()/.deeplake/config.json` fallback yields the correct path. + "process.env.HIVEMIND_CONFIG_PATH": "undefined", }, }; diff --git a/src/hooks/graph-on-stop.ts b/src/hooks/graph-on-stop.ts index a689eaa1d..bce4ea652 100644 --- a/src/hooks/graph-on-stop.ts +++ b/src/hooks/graph-on-stop.ts @@ -49,7 +49,6 @@ import { createHash } from "node:crypto"; import { appendFileSync, mkdirSync } from "node:fs"; import { join } from "node:path"; -import { runBuildCommand } from "../commands/graph.js"; import { acquireBuildLock, releaseBuildLock } from "../graph/build-lock.js"; import { readLastBuild } from "../graph/last-build.js"; import { repoDir } from "../graph/snapshot.js"; @@ -208,7 +207,6 @@ export interface MainDeps { * crashes the user's session. */ export async function main(deps: MainDeps = {}): Promise { - const runBuildFn = deps.runBuildCommand ?? runBuildCommand; const acquireFn = deps.acquireBuildLock ?? acquireBuildLock; const releaseFn = deps.releaseBuildLock ?? releaseBuildLock; const gateFn = deps.decideGate ?? decideGate; @@ -255,6 +253,21 @@ export async function main(deps: MainDeps = {}): Promise { // not "which underlying event"; both feed the same gate + lock so the // distinction is invisible to consumers. try { + // Lazy-load runBuildCommand so this hook's MODULE LOAD never depends on + // the tree-sitter native extractors. runBuildCommand → extract/index → + // `import Parser from "tree-sitter"` is a chain of static ESM imports; + // pulling it at the top of this file would make Node resolve tree-sitter + // at load time. tree-sitter is an optionalDependency (may be absent on a + // codex/cursor install whose native build failed — Node 24 / arm64), so + // an unresolved package would abort module load with ERR_MODULE_NOT_FOUND + // and exit the Stop hook with code 1 — BEFORE main()'s catch can swallow + // it. Deferring the import to here (behind the gate, inside this try) + // means a missing extractor degrades to a logged skip + exit 0, per this + // hook's contract that "a buggy hook must never break the user's session". + // The esbuild build for this entry uses `splitting` so the dynamic import + // stays a runtime chunk load (mirrors the CLI fix in bundle/cli.js). + const runBuildFn = + deps.runBuildCommand ?? (await import("../commands/graph.js")).runBuildCommand; await runBuildFn(["--trigger", "session-end"]); } catch (err) { logToFile(ctx.cwd, `build threw: ${err instanceof Error ? err.message : String(err)}`); diff --git a/tests/shared/graph/graph-on-stop-bundle.test.ts b/tests/shared/graph/graph-on-stop-bundle.test.ts new file mode 100644 index 000000000..23374526b --- /dev/null +++ b/tests/shared/graph/graph-on-stop-bundle.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect } from "vitest"; +import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; + +/** + * Bundle-level guard for the graph-on-stop tree-sitter regression. + * + * The bug: graph-on-stop's build path statically imports `tree-sitter` (a + * native optionalDependency). When esbuild bundles that chain into the entry, + * it hoists `import ... from "tree-sitter"` to the TOP of the entry file, so + * Node resolves it at MODULE LOAD — before main()'s try/catch runs. On an + * install without the grammar, the Stop hook exits 1 (ERR_MODULE_NOT_FOUND). + * + * The fix builds graph-on-stop as its own `splitting: true` bundle so the + * `await import("../commands/graph.js")` stays a runtime chunk load and the + * tree-sitter statics live only in graph-chunks/. These assertions FAIL if + * splitting is dropped or the tree-sitter chain leaks back into the entry — + * the exact regression the unit test (which injects runBuildCommand) can't see. + * + * Covers the four harnesses that register graph-on-stop as a Stop/SessionEnd + * hook. OpenClaw ships graph-on-stop via a separate graph-worker build with + * its own env-rewrite handling and is out of scope here. + */ +const REPO_ROOT = join(__dirname, "..", "..", ".."); +const HARNESSES = ["claude-code", "codex", "cursor", "hermes"]; + +describe("graph-on-stop shipped bundle (tree-sitter isolation)", () => { + const built = HARNESSES.map((h) => ({ + harness: h, + entry: join(REPO_ROOT, "harnesses", h, "bundle", "graph-on-stop.js"), + chunkDir: join(REPO_ROOT, "harnesses", h, "bundle", "graph-chunks"), + })).filter((b) => existsSync(b.entry)); + + it("has every harness bundle built (none silently missing)", () => { + // Require ALL four harnesses, not just one: a filtered subset would let a + // per-harness packaging regression (or a build that skipped a harness) + // pass unnoticed. Asserting the exact set also guards against the glob + // matching nothing after a path drift. Run `npm run build` first if this + // trips — bundles are gitignored. + expect(built.map((b) => b.harness)).toEqual(HARNESSES); + }); + + for (const b of built) { + describe(b.harness, () => { + const entrySrc = readFileSync(b.entry, "utf8"); + + it("entry does NOT statically import tree-sitter", () => { + // Any `from "tree-sitter..."` in the entry means the native import was + // hoisted to module top → the exact load-time crash we fixed. + expect(entrySrc).not.toMatch(/from\s*["']tree-sitter/); + }); + + it("entry loads the build path via a lazy graph-chunks/ import", () => { + expect(entrySrc).toMatch(/import\(\s*["']\.\/graph-chunks\/graph-[^"']+\.js["']\s*\)/); + }); + + it("tree-sitter lives only in the lazy graph chunk", () => { + expect(existsSync(b.chunkDir)).toBe(true); + const chunks = readdirSync(b.chunkDir).filter((f) => f.endsWith(".js")); + const withTreeSitter = chunks.filter((f) => + /from\s*["']tree-sitter/.test(readFileSync(join(b.chunkDir, f), "utf8")), + ); + // At least one chunk must carry the grammar imports (proves they were + // split out, not dropped), and the entry proved they're not inline. + expect(withTreeSitter.length).toBeGreaterThan(0); + }); + }); + } +}); diff --git a/tests/shared/graph/graph-on-stop-main.test.ts b/tests/shared/graph/graph-on-stop-main.test.ts index 5a2c515ab..3ee4e5ec4 100644 --- a/tests/shared/graph/graph-on-stop-main.test.ts +++ b/tests/shared/graph/graph-on-stop-main.test.ts @@ -113,6 +113,37 @@ describe("graph-on-stop main()", () => { } }); + it("injected runBuildCommand that rejects with ERR_MODULE_NOT_FOUND (tree-sitter missing) → lock released, error logged, main() resolves", async () => { + // Simulates the codex/cursor case where the tree-sitter native extractor + // is not installed: the lazy `import("../commands/graph.js")` rejects. + // main() must swallow it (log + release lock + resolve) so the Stop hook + // exits 0 instead of crashing with a non-zero code. + const acquire = vi.fn(() => ({ acquired: true, reason: "acquired" })); + const release = vi.fn(); + const run = vi.fn(async () => { + const err = new Error("Cannot find package 'tree-sitter'"); + (err as NodeJS.ErrnoException).code = "ERR_MODULE_NOT_FOUND"; + throw err; + }); + await expect( + main({ + decideGate: () => ({ fire: true, reason: "test-fire" }), + acquireBuildLock: acquire, + releaseBuildLock: release, + runBuildCommand: run, + }), + ).resolves.toBeUndefined(); + expect(release).toHaveBeenCalledTimes(1); + // Logging is part of this regression's contract (the operator's only trace + // that the build was skipped), so assert it unconditionally with the + // specific message rather than guarding on the file's existence. + const log = join(baseDir, ".graph-on-stop.log"); + expect(existsSync(log)).toBe(true); + expect(readFileSync(log, "utf8")).toContain( + "build threw: Cannot find package 'tree-sitter'", + ); + }); + it("decideGate throws → early return, no lock attempt, error logged", async () => { const acquire = vi.fn(); const release = vi.fn();