ref(cli): resolve ast-grep without an install-time step

[thecodedrift]

The CLI no longer ships the @ast-grep/cli wrapper to consumers, and finding the ast-grep binary no longer depends on anything happening at install time.

Most of this migration was already done: the seven @ast-grep/cli-<platform> packages were already declared in optionalDependencies, and findSgBinary() already resolved them from our own module context and executed by path. The wrapper was a leftover. Its only job is a postinstall that hardlinks the platform binary into itself so its bin: {sg, ast-grep} entries resolve — entries nothing here invokes. It moves to devDependencies, where scripts/fetch-ast-grep-schema.ts still reads its version to pick the schema tag.

That postinstall is not hypothetical dead weight. findSgBinary()'s existing comment records why it was written: the hardlink fails under pnpm dlx's strict dependency isolation and leaves a placeholder text file where the binary should be. It also needs an opt-in entry in the root pnpm.onlyBuiltDependencies to run at all. With it out of the runtime dependency set, the binary is present purely by dependency resolution.

Two behavior changes worth reviewing closely.

Platform packages are now pinned exactly at 0.41.0 rather than ^0.41.0. The wrapper had been enforcing alignment implicitly by pinning its own optionalDependencies to its exact version; without it, two hosts could resolve different ast-grep versions against the same rules and legitimately disagree about findings — a divergence that shows up as inconsistent results, not as an install error. The pin is deliberately held at 0.41.0 rather than taking upstream's 0.45.0, so that if ast-grep behavior shifts, this change isn't a candidate explanation. Bumping is worth doing separately.

findSgBinary() now searches every candidate before giving up — the platform package, node_modules/.bin, then sg and ast-grep on PATH — and throws an error naming what it tried. Previously it returned a bare "sg" on any miss and let spawn's ENOENT be the message, produced by a caller that couldn't say where it had looked. All three call sites already route through it, so they're unchanged; they just get a better answer, or a real error.

Each candidate is also verified by running it (--version must identify as ast-grep) rather than by existsSync. That closes the hole this resolver exists for: the wrapper's failed hardlink leaves a placeholder text file at the binary's path, which an existence check accepts happily. Asking the candidate to identify itself is the only check that tells the real binary from a file merely sitting where it belongs. Resolution is cached per process, since the search now spawns a subprocess per candidate.

Alpine gets better, though the diff reads like it might get worse. Removing the wrapper removes a fallback, so the fair question is what happens where no platform package exists. Upstream publishes no musl build at either 0.41.0 or 0.45.0, and its Linux packages declare libc: ["glibc"], so package managers already skip them on musl.

Verified on node:22-alpine (musl, aarch64) rather than inferred from registry metadata:

	npm install
Wrapper as a hard dependency (today)	exits 1 — "Failed to move @ast-grep/cli binary into place."
Platform packages as optionalDependencies (this PR)	exits 0, installs no @ast-grep/*, falls through to PATH

One task stays open by necessity: verifying the resolved path under a real pnpm dlx, which needs a published build and so lands after release.

This also sets the pattern that Vale binary packaging will follow. For ast-grep it meant deleting a layer; for Vale it will mean publishing platform packages that don't exist yet.

Typecheck, lint, and the full suite pass.

Fixes OSS-23

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 3m 16s —— View job

---

Review complete

• Gather context (read diff, changed files, CLAUDE.md conventions)
• Understand the request (code review of PR ref(cli): resolve ast-grep without an install-time step #69)
• Review packages/cli/src/rules/scan.ts changes (findSgBinary logic)
• Review packages/cli/package.json dependency changes
• Review packages/cli/scripts/fetch-ast-grep-schema.ts
• Review new test file packages/cli/test/sg-binary.test.ts
• Review OpenSpec docs for consistency with implementation
• Post final review

Overall assessment

Solid, well-documented refactor. The dependency change is exactly what the description claims (@ast-grep/cli → devDependencies, platform packages pinned to exact 0.41.0, fetch-ast-grep-schema.ts reading from devDependencies), and findSgBinary()'s new exhaustive search with a descriptive failure is a clear improvement over the old bare-"sg"-and-let-ENOENT-explain-it behavior. All three call sites (scan.ts, verify.ts, narrow.ts:43) already route through findSgBinary()/buildPath() and are unaffected. The pnpm-lock.yaml diff matches the package.json changes exactly, and no other workspace package depends on @ast-grep/cli or sg/ast-grep directly, so this is safely scoped to packages/cli.

Left two minor inline notes on scan.ts:

• The child.on("error", ...) ENOENT handler in runAstGrepScan (not touched by this diff, so no inline comment landed there) is now effectively unreachable — findSgBinary() already validates existsSync and throws its own descriptive error before spawn is called. If it ever does fire (e.g. a TOCTOU race), its message ("Is @ast-grep/cli installed?") points at a wrapper that's no longer in the resolution chain at all. Worth a follow-up to update or remove it.
• A small asymmetry in the new candidate list: the node_modules/.bin entry only checks the sg/sg.exe name, while the PATH entries check both sg and ast-grep names. Low risk in practice since the wrapper's bin field declares both names for the same target, but worth a look for consistency.

One doc-accuracy note (no code impact): the PR description says confirming the Alpine install "is still open," but the archived tasks.md (2b.4/3.5) shows that verification as already done — confirmed on node:22-alpine (musl, aarch64) — along with the buildPath() question (4.1, "kept," rationale recorded). The only item actually still open per tasks.md is 3.2 (verifying the resolved path under a real pnpm dlx against a published build), which the PR body doesn't call out specifically. Might be worth tightening the PR description before merge so reviewers aren't chasing already-resolved questions.

Test coverage is reasonable and the tests read as intentional (the docstring in sg-binary.test.ts explains what's covered and why). The author's own tasks.md already flags that the PATH-fallback and exhausted-search branches aren't unit-tested (would need module mocking) — a reasonable, acknowledged trade-off rather than an oversight.

No security, correctness, or performance issues found. Per instructions, I did not run typecheck/lint/test locally or check CI status — that's handled by CI on the PR itself.

[thecodedrift]

Re: @claude[bot] — "Claude finished @thecodedrift's task in 3m 16s — Review complete…"
#69 (comment)

All three points addressed in d077898.

node_modules/.bin asymmetry — fixed; it now checks both sg and ast-grep, matching the PATH search, with a comment explaining why either name may be linked.

Unreachable ENOENT handler — you were right that its message pointed at a package no longer in the resolution chain. It now reports that the binary vanished between resolution and execution, which is the only way it can still fire.

Stale PR description — correct, and my error: I wrote the body before running the Alpine verification and never updated it. The description now carries the measured result (wrapper as a hard dependency exits 1 on node:22-alpine; platform packages as optionalDependencies exit 0) and names 3.2 as the single remaining item, since it needs a published build.

Your first note also prompted a fix beyond what was reported. Candidates were selected with existsSync, which accepts precisely the artifact this resolver exists to avoid — the failed hardlink leaves a placeholder text file at the binary's path. Both the old code and my search would have returned it. Each candidate is now asked to identify itself via --version, with tests covering a placeholder file, a non-ast-grep executable, and a missing path. 378 → 382 tests.

— AI Coding Agent