fix(cli): install UX cascade — opt-in LLM import, Node version doctor probe, hook shim flag injection (#445) - #447
Open
LiuShiyuMath wants to merge 3 commits into
Open
fix(cli): install UX cascade — opt-in LLM import, Node version doctor probe, hook shim flag injection (#445)#447LiuShiyuMath wants to merge 3 commits into
LiuShiyuMath wants to merge 3 commits into
Conversation
Issue #445 reproduces 3 cascading bugs on a non-technical user's first install. The root cause of the headline symptom (init silently hangs 8+ minutes, then burns the user's Claude Code subscription quota through 231 serial `claude -p` calls) is that `teamagent init` runs the LLM-based rule structurer by default whenever CLAUDE.md / .cursorrules are present. Fix: flip the default. LLM rule import is now opt-in via `--import-rules`. The existing `--skip-import` flag still works and now overrides `--import-rules` for parity with the historical escape hatch. Programmatic / test callers that inject `opts.llmClient` explicitly still get the import path, so all 78 existing init tests keep passing without modification. CLI surface: teamagent init # default: skip LLM import teamagent init --import-rules # opt-in (was the old default) teamagent init --skip-import # explicit skip (unchanged) Added 4 tests covering: default-skips, opt-in runs, skipImport beats importRules, CLI flag parses. 82/82 init tests pass. Closes none of #445 alone but resolves the headline symptom; bugs #2 (hooks node:sqlite) and #3 (doctor Node version probe) remain.
The install transcript on issue #445 reproduces on macOS Apple Silicon with Homebrew's default Node 23.3.0. `node:sqlite` only becomes a stable built-in module on Node 23.5; on 23.0-23.4 it requires `NODE_OPTIONS='--experimental-sqlite'`. Worse: the installed hooks (~/.teamagent/hooks/bin-*.cjs) are spawned by Claude Code, NOT by the user's shell, so a zshrc export does not propagate to them. The user sees `teamagent doctor` report node-version PASS while every hook fires `ERR_UNKNOWN_BUILTIN_MODULE: node:sqlite` and crashes silently. Fix: split the version check into three tiers - <22 → fail with nvm install 22 hint (unchanged) - 23.0-23.4 → fail with explicit "hooks won't inherit NODE_OPTIONS" detail message and nvm install 24 hint (new) - >=22 except 23.0-23.4 → pass with "node:sqlite stable on >=23.5" Exported `checkNodeVersionAt(rawVersion)` for unit testing without mocking process.version. Added 6 tests covering the boundary tiers.
… band (#445) Issue #445 bug 2: hooks spawned by Claude Code do NOT inherit NODE_OPTIONS from the user's shell rc, so even when the install transcript user set `export NODE_OPTIONS='--experimental-sqlite'` in zshrc, every fired Stop / SessionEnd / PreToolUse hook still crashed with: Error [ERR_UNKNOWN_BUILTIN_MODULE]: No such built-in module: node:sqlite at Object.<anonymous> (~/.teamagent/hooks/bin-stop.cjs) Fix: the existing bash -c shim that wraps every user-level hook entry in settings.json now probes the running Node version via `node -p process.versions.node` and injects `--experimental-sqlite` only on the broken band (Node 22.5-22.x and 23.0-23.4 where `node:sqlite` exists but is experimental). case "$v" in 22.[5-9].*|22.[1-9][0-9].*|23.[0-4].*) exec node --experimental-sqlite "$1" ;; *) exec node "$1" ;; esac Node 22.0-22.4 (no node:sqlite at all) and Node >=23.5 / 24+ (node:sqlite stable) get the plain `exec node` path; the broken band silently gains the flag, hooks stop crashing on Node 23.3 without any user action. Added 5 runtime tests using a fake `node` binary on PATH: - 23.3.0 → flag injected - 22.10.0 → flag injected (covers 22.[1-9][0-9].* glob) - 23.5.0 → no flag - 24.0.0 → no flag - 22.4.0 → no flag (below the broken band) All 67 install-hook + install-user-hook + user-level-hook-shim tests pass.
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.
Summary
Fixes the three cascading install-UX bugs reported in #445. All three were reproducible on a non-technical user's first run on macOS Apple Silicon with Node 23.3.0 (Homebrew default). Each bug now has its own atomic commit with regression tests.
Changes
1.
initLLM rule import is now opt-in (fix(cli): make LLM rule import opt-in)Default
teamagent initno longer spawns ~231 serialclaude -pcalls againstCLAUDE.md/.cursorrules. New--import-rulesflag opts in. The--skip-importescape hatch still works. Programmatic / test callers that injectopts.llmClientkeep the import path, so all 78 existing init tests pass without modification.Kills the headline symptom (init silently hangs 8+ minutes with no progress) and the subscription-burn cascade.
2.
doctorflags Node 23.0–23.4 as broken fornode:sqliteSplit
checkNodeVersioninto three tiers:<22→ fail (unchanged);23.0–23.4→ fail with explicit "hooks don't inheritNODE_OPTIONSfrom shell rc" diagnostic +nvm install 24hint (new);>=22except the broken band → pass. ExportedcheckNodeVersionAt(rawVersion)for unit-testing the boundary tiers.3. Hook shim auto-injects
--experimental-sqliteon the broken Node bandThe
bash -cshim wrapping every user-level hook entry in~/.claude/settings.jsonnow probes the running Node version vianode -p process.versions.nodeand injects--experimental-sqliteonly on Node 22.5–22.x and 23.0–23.4 (wherenode:sqliteexists but is experimental). Node 22.0–22.4 (nonode:sqlite) and >=23.5 / 24+ (stable) get the plainexec nodepath.This is the structural fix: even if the user does NOT update
~/.zshrc, fired hooks on Node 23.3 no longer crash withERR_UNKNOWN_BUILTIN_MODULE: node:sqlite.Test plan
pnpm vitest run packages/cli/src/__tests__/init.test.ts— 82/82 (4 new for [bug] teamagent init 在无 ANTHROPIC_API_KEY 时静默挂死 8+ 分钟 #445)pnpm vitest run packages/cli/src/__tests__/doctor.test.ts— 63/63 (6 new for [bug] teamagent init 在无 ANTHROPIC_API_KEY 时静默挂死 8+ 分钟 #445)pnpm vitest run packages/cli/src/__tests__/user-level-hook-shim.test.ts— 14/14 (6 new for [bug] teamagent init 在无 ANTHROPIC_API_KEY 时静默挂死 8+ 分钟 #445)pnpm vitest run packages/cli/src/__tests__/install-hook.test.ts packages/cli/src/__tests__/install-user-hook.test.ts— 67/67 (regression: existing shim-shape tests still pass with the new case-statement form)pnpm --filter @teamagent/cli typecheck— cleanDiff
7 files changed, 268 insertions(+), 18 deletions(-)