fix(graph): stop graph-on-stop hook crashing when tree-sitter is absent#321
Merged
Conversation
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.
📝 WalkthroughWalkthroughGraph-on-stop is built separately for each harness with tree-sitter isolated into lazy chunks. The hook dynamically loads the graph command after gate and lock checks, while regression tests cover bundle structure and missing-native-dependency handling. ChangesGraph-on-stop tree-sitter isolation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 1 file changed
Generated for commit da981eb. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/shared/graph/graph-on-stop-bundle.test.ts`:
- Around line 47-50: Update the static-import assertion in the “entry does NOT
statically import tree-sitter” test to match both from-clause imports and
side-effect imports such as import "tree-sitter". Apply the same regex
correction to the related assertion around the additional referenced lines,
preserving the existing load-time regression coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d1058e73-979d-4b8b-bc00-754ac662873c
📒 Files selected for processing (4)
esbuild.config.mjssrc/hooks/graph-on-stop.tstests/shared/graph/graph-on-stop-bundle.test.tstests/shared/graph/graph-on-stop-main.test.ts
Comment on lines
+47
to
+50
| 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/); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Detect side-effect static imports too.
The regex misses import "tree-sitter" syntax, allowing the original module-load failure to regress while this test passes.
Proposed fix
- expect(entrySrc).not.toMatch(/from\s*["']tree-sitter/);
+ expect(entrySrc).not.toMatch(
+ /(?:\bfrom\s*|^\s*import\s*)["']tree-sitter[^"']*["']/m,
+ );
...
- /from\s*["']tree-sitter/.test(readFileSync(join(b.chunkDir, f), "utf8")),
+ /(?:\bfrom\s*|^\s*import\s*)["']tree-sitter[^"']*["']/m.test(
+ readFileSync(join(b.chunkDir, f), "utf8"),
+ ),Also applies to: 57-62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/shared/graph/graph-on-stop-bundle.test.ts` around lines 47 - 50, Update
the static-import assertion in the “entry does NOT statically import
tree-sitter” test to match both from-clause imports and side-effect imports such
as import "tree-sitter". Apply the same regex correction to the related
assertion around the additional referenced lines, preserving the existing
load-time regression coverage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
graph-on-stopStop/SessionEnd hook (claude-code, codex, cursor, hermes) crashes with exit code 1 on installs where the optionaltree-sitternative grammar failed to build (Node 24 / arm64):Root cause: the hook's build path is a chain of static ESM imports —
runBuildCommand→extract/index→import Parser from "tree-sitter". esbuild hoists the externaltree-sitterimport to the top of the bundle, so Node resolves it at module load — beforemain()'stry/catchruns.main().catch(() => process.exit(0))never fires. This is still reproducible on currentmain(v0.7.133); PR #295 fixed the same class of bug forbundle/cli.js(splitting) but not for the hook.Fix
src/hooks/graph-on-stop.ts— loadrunBuildCommandlazily viaawait import("../commands/graph.js")behind the gate, inside the existingtry/catch. A missing extractor degrades to a logged skip + exit 0.esbuild.config.mjs— buildgraph-on-stopfor the four harnesses viabuildGraphOnStop()withsplitting: true(mirrors the CLI fix), so the dynamic import stays a runtime chunk load andtree-sitterlives only ingraph-chunks/. Clears stale hashed chunks first; removedgraph-on-stopfrom the four shared harness build lists.esbuild.config.mjs— the lazyimport()defeats tree-shaking in the non-split openclaw graph-worker build, pullinguser-config.ts'sprocess.env.HIVEMIND_CONFIG_PATHintoopenclaw/dist/graph-on-stop.jsand tripping ClawHub'senv-harvestingcritical. RewroteHIVEMIND_CONFIG_PATH→undefinedin the existingopenclawGraphWorkerDefine(call site has a?? homedir()/.deeplake/config.jsonfallback). The openclaw graph-worker build is otherwise untouched.tree-sitterimport and loads it only viagraph-chunks/.Scope
Fixes the four harness bundles that register
graph-on-stopas a Stop/SessionEnd hook (the visible exit-1 surface). OpenClaw'sgraph-on-stop/graph-pull-workerandgraph-pull-workeron the other harnesses keep the same latent tree-sitter gap but run detached, so they can't break the session lifecycle — left as follow-up.Verification
ERR_MODULE_NOT_FOUNDbuild threw: Cannot find package 'tree-sitter'graph-on-stop.jshad 3 top-leveltree-sitterimports.tree-sitterimports; e2e run exits 0.Session Context
Session transcript
Summary by CodeRabbit
Bug Fixes
Tests