Conversation
NickCirv
added a commit
that referenced
this pull request
Apr 20, 2026
Windows CI on PR #15 failed all 22 bash-postool assertions because expected values were hard-coded as POSIX paths (`/proj/src/foo.ts`) while the implementation calls `path.resolve()` which returns platform-native paths — backslashes on Windows, forward slashes on POSIX systems. Fix: build expected values through `path.resolve` the same way the implementation does. The test now captures the platform-native cwd once and derives expected absolute paths from it, so the same source asserts correctly on macOS, Linux, and Windows. The implementation itself is unchanged — `pathResolve` already does the right thing per platform. This was purely a cross-platform test hygiene issue. Also swapped the hard-coded `/tmp/foo.ts` absolute-path test for a platform-resolved equivalent so Windows doesn't choke on missing drive prefix. Local verify: 25/25 bash-postool tests pass on macOS Node 25. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the brainstorming session outcome for the three-release plan: - v2.1 "Reliability + Zero-Friction Install" — close the bleeding, merge contributor PRs (#6, #13), fix #11, ship engram update / engram doctor / engram setup, close #14 via Bash PostTool parser. - v2.2 "Spine" — integrate Serena as an engram provider via a new reusable MCP-client subsystem. Engram becomes the orchestrator, not a fighter of semantic-search tools. - v3.0 "Landmines" — mistakes-as-moat expansion + R2 repositioning ("the context tool that remembers what broke"). Keep the name, rebrand the tagline. Strategic choices recorded: - Trilogy (alpha) over mega-release (beta) or split (gamma) - R2 (keep name, rebrand tagline) over R1 (keep everything) or R3 (rename, prohibitive cost) - Update UX: option A — passive notify + manual install, zero telemetry, ENGRAM_NO_UPDATE_CHECK + \$CI opt-out Grounded in measured research: - npm downloads 1.3K/week, 10/day organic baseline - r/LocalLLaMA post ratioed (0.44) due to name collision with 4 other "Engram" projects launched Mar-Apr 2026 - Serena just hit stable with published evals — credible complement - 2 active external contributors writing substantive PRs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the v2.1 "seamless install" track — users on the 1.3K/week organic install curve never hear about hotfixes today. This bridges the gap without introducing any telemetry or auto-install surprise. New subsystem at src/update/: - check.ts — anonymous GET to registry.npmjs.org/engramx/latest. Cached at ~/.engram/last-update-check for 7 days so we do not hammer the registry on every CLI invocation. Strict semver compare handles pre-release correctly (2.1.0 > 2.1.0-beta.1). 1.5s hard timeout on the fetch so a flaky network never hangs a CLI invocation. - install.ts — detects the package manager that owns the engram binary via install-path substring markers (pnpm/yarn/bun/npm fallback). Shells out to npm/pnpm/yarn/bun with execFileSync — no shell interpretation, no injection surface. Package name is a compile-time constant. --dry-run prints the exact command. --manager overrides detection for edge cases. - notify.ts — passive hint on any engram subcommand. Writes to stderr so stdout stays clean for pipes and the intercept protocol. At most one line per process. Silent when no cache exists (no cold-start surprise), silent under \$CI or ENGRAM_NO_UPDATE_CHECK=1. Privacy invariants verified: - Zero telemetry. No machine ID, no install count, no repo path sent. - One GET per 7 days max, and only when user runs an engram command. - Hint always goes to stderr, never stdout (protocol safety for intercept). - $CI and ENGRAM_NO_UPDATE_CHECK=1 both gate the entire subsystem. Tests: 38 new tests across 3 files. All offline — opt-out path is exercised to avoid live registry calls in CI. Closes the "users never hear about hotfixes" half of the v2.1 spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wraps the existing component-status.ts probes (HTTP, LSP, AST, IDE adapters) plus four new checks (graph.db presence, Sentinel hook installation, engram version freshness, IDE count) into a human report with remediation hints. Motivated by issue #11 — users have engram LSP enabled but the provider silently reports unavailable. Doctor surfaces this on demand and prints the exact fix. Every future silent-failure bug gets a new check added here, making triage a single CLI invocation. Features: - engram doctor → human report with icons and summary - engram doctor --verbose → include remediation hints for warn/fail - engram doctor --json / --export → redacted JSON for bug reports, projectRoot intentionally omitted (can contain usernames) - Exit code reflects severity: 0 ok, 1 warn, 2 fail → CI-friendly Checks: - version (engram vX.Y.Z, flagged if cached newer version available) - graph (.engram/graph.db present + size) - hook (Sentinel hook in any .claude/settings*.json) - http / lsp / ast providers (via component-status) - ides (adapter count) Severity aggregation = worst-case wins. All probes file-existence only, no network calls — safe for CI. Tests: 6 new tests covering report construction, severity aggregation, verbose remediation output, JSON export redaction, and graph.db detection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops install-to-first-value from 4 commands to 1. Addresses the "seamless install" track of v2.1 — the second half that matters as much as reliability fixes for the 10/day organic install curve. Three pieces: 1. engram setup — first-run wizard Runs init → install-hook → detects IDE adapters → verifies via doctor. Each step idempotent, skips cleanly if already done. Interactive prompts by default; --yes for non-interactive, --dry-run prints intent without acting. Exit code reflects doctor severity. 2. engram init --with-hook Shorthand for init + install-hook in one invocation. The #1 thing every user does after init was install-hook; now it is one step. 3. First-run hint On any engram subcommand invoked in a repo lacking .engram/graph.db, print one line to stderr: "💡 First time in this repo? Run engram setup for a zero-friction install." Throttled via ~/.engram/first-run-shown — fires once per machine, not per repo. Skipped in \$CI and under JSON-stdout-producing commands (intercept, cursor-intercept, hud-label). New file src/setup/detect.ts detects Claude Code, Cursor, Windsurf, Continue.dev, Aider from config-file presence. Used by both the wizard and engram doctor. CLI wiring also adds the passive update-notify call alongside the first-run hint, both gated by the same FIRST_RUN_SILENT_CMDS set so neither pollutes the intercept protocol. Tests: 6 new detection tests. Wizard itself is exercised end-to-end via the doctor + init integration test surface already in place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a strict parser for file-mutating Bash commands — rm, mv, cp,
git rm, git mv, and single-redirect commands. Returns a list of
FileOp { action: "reindex" | "prune", path: absolute } that the
PostToolUse dispatch path will hand to syncFile() from watcher.ts.
This is HALF of issue #14. The other half is the dispatch wire-up
(route PostToolUse:Bash through this parser and invoke syncFile for
each op) which depends on PR #13's --auto-reindex flag landing first.
The parser ships standalone so it is review-ready and test-complete
before #13 merges.
Parser philosophy (mirrors handlers/bash.ts PreToolUse):
STRICT. Any ambiguity → passthrough. False negatives cost tokens;
false positives corrupt the graph. Optimize for the latter.
Supported shapes (intercepted):
rm [-rf] <path> [<path>...] → prune each
mv <src> <dst> → prune src, reindex dst
cp <src> <dst> → reindex dst
git rm [-r] <path> → prune
git mv <src> <dst> → prune src, reindex dst
<simple-cmd> > <dst> → reindex dst (single redirect)
<simple-cmd> >> <dst> → reindex dst (single redirect)
Intentionally NOT supported (pass through):
- globs (rm *.ts)
- pipes (find . | xargs rm)
- subshells / command substitution (\$(…), backticks, <(…), >(…))
- touch (empty file, nothing to index)
- directory-level ops (needs directory-prefix prune primitive,
tracked for v2.2)
Tests: 29 new tests across rm/mv/cp/git-rm/git-mv variants,
redirections, pass-through edge cases, and the high-level
handleBashPostTool wrapper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…h-postool ## Closes #11 — AST/LSP reported unavailable despite enabled Credit: Tommaso Tessarolo (@ttessarolo) — precise forensics + suggested fix. ### checkAst: flattened-bundle path resolution When tsup/esbuild flattens the CLI into `engramx/dist/chunk-*.js`, import.meta.url resolves `here = engramx/dist`. The previous candidates: - `here + '../grammars'` → engramx/grammars (✗ does not exist) - `here + '../../dist/grammars'` → parent/dist/grammars (✗ does not exist) Fix: add `join(here, "grammars")` as the FIRST candidate. This resolves to `engramx/dist/grammars/` where grammars actually ship. Dev-time layout (src/intercept/) still works via the third candidate. ### checkLsp: socket candidates out of sync The check only looked for `tsserver.sock` and `typescript-language-server.sock`, but `lsp-connection.ts::candidateSockets()` also probes: - tsserver-<uid>.sock - lsp-server.sock - pyright-<uid>.sock - rust-analyzer.sock Also: the `.engram/lsp-available` flag was documented as "written by the lsp provider on successful connection" but no code path writes it. Kept the marker as an explicit user opt-in (back-compat) but synced the socket list with lsp-connection.ts so HUD availability matches actual provider availability. ### Regression test tests/intercept/component-status-11.test.ts — asserts refreshComponentStatus returns a boolean for every scenario + honors the .engram/lsp-available marker. ## Wires bash-postool into PostToolUse (issue #14 full wire-up) With @gabiudrescu's PR #12 already merged, syncFile() is available. We can wire the v2.1 Bash parser into the PostToolUse observer path without waiting for PR #13's install-hook --auto-reindex flag. - post-tool.ts imports handleBashPostTool + syncFile. - On PostToolUse with tool=Bash, parse the command; for each FileOp, call syncFile(absPath, projectRoot) fire-and-forget. - Gated by ENGRAM_AUTO_REINDEX=1 (opt-in) until PR #13 lands and we can migrate to the install-hook flag. - Observer semantics preserved: never blocks the PostToolUse response, errors swallowed, never surfaces bugs to Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- README: lead Quickstart with 'engram setup' one-command flow. Keep the individual commands as a sub-example for users who prefer the explicit path. Add a diagnostics+update block so first-time users know the surface. - CHANGELOG: document the full v2.1 'Reliability + Zero-Friction Install' track under [Unreleased] — update / doctor / setup, init --with-hook flag, first-run hint, Bash PostTool parser + wire-up, plus the #11 AST path + LSP socket fixes. No code changes — ships with the v2.1 feature set already on this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Windows CI on PR #15 failed all 22 bash-postool assertions because expected values were hard-coded as POSIX paths (`/proj/src/foo.ts`) while the implementation calls `path.resolve()` which returns platform-native paths — backslashes on Windows, forward slashes on POSIX systems. Fix: build expected values through `path.resolve` the same way the implementation does. The test now captures the platform-native cwd once and derives expected absolute paths from it, so the same source asserts correctly on macOS, Linux, and Windows. The implementation itself is unchanged — `pathResolve` already does the right thing per platform. This was purely a cross-platform test hygiene issue. Also swapped the hard-coded `/tmp/foo.ts` absolute-path test for a platform-resolved equivalent so Windows doesn't choke on missing drive prefix. Local verify: 25/25 bash-postool tests pass on macOS Node 25. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Bug 1 — doctor.checkHook always reported "not found" in production
Root cause: three functions in src/doctor/report.ts, two in
src/setup/detect.ts, and one in src/setup/wizard.ts used bare
require() calls at runtime — but tsup bundles to ESM where require
is undefined. The ReferenceError was caught by each function's own
catch block and silently treated as "check failed," so:
- doctor reported "Sentinel hook not found" even after a successful
install-hook (verified live against a fresh tmp project)
- detect.ts's Claude Code detection reported configured=false even
when the hook was wired
- setup --user scope would hit "homedir is not a function"
Why tests passed: vitest's test runtime provides CommonJS interop so
require() works in test environment. The bundled CLI doesn't.
v2.0.1 shipped a similar "CI-passed, prod-broken" pattern for a
different reason; this is the same class of bug.
Fix: move every require() to a proper top-level ESM import.
- report.ts: import readFileSync + cachePath/isNewer at top
- detect.ts: import readFileSync at top
- wizard.ts: import readFileSync, writeFileSync, mkdirSync, dirname,
homedir at top
Verified live:
$ engram setup -y -p /tmp/fresh-project
✓ Sentinel hook installed
✓ hook Sentinel hook active (via /tmp/fresh-project/.claude/settings.local.json)
## Bug 2 — empty "Next steps for detected IDEs:" block
When only Claude Code was detected-but-unconfigured (and there's no
suggest entry for Claude Code because install-hook handles it in
step 2), the wizard printed "Next steps for detected IDEs:" with
no subsequent lines.
Fix: collect suggestions first. If the list is empty, either:
- skip with "Claude Code hook declined" if Claude Code is the
culprit (user declined hook install in step 2)
- done with "no additional adapters needed" otherwise
The header line only prints when there's at least one actionable
suggestion.
## Audit result
While auditing for this bug I grepped every new file for `require(`
and found 7 offenders across 3 files. All fixed in one commit.
No other ESM-interop issues found.
Verified: 746/746 tests still pass, lint clean, build clean, live
E2E test of setup → doctor → Bash auto-reindex all behave
correctly. Bash PostToolUse with ENGRAM_AUTO_REINDEX=1 successfully
prunes the graph when the underlying file is rm'd.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f5e8123 to
0d9470c
Compare
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
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
First release in the v2.1 / v2.2 / v3.0 elevation trilogy (design spec
in
docs/superpowers/specs/2026-04-20-engram-elevation-trilogy-design.md).This PR lands the full v2.1 scope: reliability fixes + the zero-friction
install track. 746/746 tests pass locally on macOS Node 25. CI will
verify Ubuntu + Windows × Node 20+22.
What ships
Seamless install (the track that addresses the 1,300 weekly downloads / 10/day organic baseline)
engram update— passive notify on everyengram *invocationwhen a newer version is cached, plus manual self-upgrade via the
detected package manager (npm / pnpm / yarn / bun via install-path
markers).
--checkdry-probes,--forcebypasses the 7-daythrottle cache,
--dry-runprints the command without executing,--manager <mgr>overrides detection. Zero telemetry: the onlynetwork call is an anonymous GET to
registry.npmjs.org/engramx/latest.ENGRAM_NO_UPDATE_CHECK=1and$CIdisable the entire subsystem.Addresses the "users never hear about hotfixes" problem — v2.0.2's
security hotfix reached maybe 10% of the install base.
engram doctor— component health report wrapping existingprobes (HTTP, LSP, AST, IDE adapters) plus four new checks (engram
version freshness,
.engram/graph.dbpresence, Sentinel hookinstallation, IDE adapter count).
--verboseshows remediationhints;
--json/--exportproduces redacted JSON for bug-reportattachment (
projectRootintentionally omitted). Exit code reflectsseverity — CI-friendly.
engram setup— zero-friction first-run wizard. One command for"go from cloned repo to working engram in under 30 seconds." Runs
init → install-hook → IDE detection → doctor verify. Each step
idempotent.
--yesnon-interactive,--dry-runprints intent.Drops install-to-first-value from 4 commands to 1.
engram init --with-hook— shorthand for init + install-hook inone step. The will this work for studies? #1 thing every user does after
initwasinstall-hook; now it's one command.First-run hint — one line on stderr the first time a user invokes
any engram command in a repo without
.engram/graph.db. Throttledglobally (not per-repo) via
~/.engram/first-run-shown. Silent in$CIand under JSON-stdout commands.Reliability fixes
Closes AST and LSP providers report unavailable=true despite enabled=true (path resolution bug in AST grammar detection) #11 — AST grammar detection in flattened bundles. When
tsup/esbuild flattens chunks to
engramx/dist/chunk-*.js, theprevious candidate paths all missed the actual grammar dir. Fix:
join(here, "grammars")added as first candidate. Dev-time layoutstill works via the third candidate. Credit: @ttessarolo for exact
forensics and suggested fix.
Partial AST and LSP providers report unavailable=true despite enabled=true (path resolution bug in AST grammar detection) #11 — LSP socket coverage.
checkLsponly looked for twosocket names while
lsp-connection.ts::candidateSockets()probessix. Synced the list; HUD availability now matches actual provider
availability. Kept
.engram/lsp-availableas explicit user opt-infor back-compat.
Closes Feature: auto-reindex on Bash file ops (rm/mv/cp/git-rm/redirection) #14 Bash half — PostToolUse parser for file-mutating Bash
commands. Strict parser handles
rm,mv,cp,git rm,git mv,single-redirect
<cmd> > <dst>. Globs, pipes, subshells,command-substitution all pass through untouched. Wired into
handlers/post-tool.tsto firesyncFile()per FileOp.Complementary to PR feat(cli): engram reindex <file> + optional --auto-reindex hook (#8) #13 — feat(cli): engram reindex <file> + optional --auto-reindex hook (#8) #13 handles Edit/Write/MultiEdit via
a dedicated
reindex-hooksubcommand; this branch handles Bash viathe existing
interceptdispatcher. Both paths coexist cleanly.Test plan
npm run lint— clean (tsc --noEmit)npm run build— clean, 6 grammars bundled intodist/grammars/npx vitest run— 746/746 pass (670 baseline + 76 new acrossupdate/doctor/setup/bash-postool/issue-11 regression)
engram init+engram doctoron a fresh/tmpproject — AST now reports
✓ reachablefrom the flattened-bundleglobal-install layout (the exact scenario that was broken in
v2.0.2 before the AST and LSP providers report unavailable=true despite enabled=true (path resolution bug in AST grammar detection) #11 fix).
v2.0.1 was a hotfix for exactly this unverified-CI failure mode;
we do not skip it.
Relationship to PR #13
Truly complementary, no conflicts expected (different insertion points
in
cli.ts, separate files for the new functionality). If PR #13merges first, this branch rebases cleanly — the bash-postool wiring
in
handlers/post-tool.tsis in a file PR #13 doesn't touch, and ourCHANGELOG conflict is a 2-line resolve.
Recommendation: merge PR #13 first (contributor credit to
@gabiudrescu via GitHub merge UI), then rebase this PR on top.
Not in this release
@gabiudrescu. Will land in v2.1.1 or as part of v2.2.
v2.1 publishes.
spec; full spec written after v2.2 ships.
🤖 Generated with Claude Code