From 24dbbcf3e165767cfb9f5a0be0b7b0a00fa99d8c Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 15:00:27 -0700 Subject: [PATCH 1/9] docs(openspec): add direct-sg-platform-deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We depend on @ast-grep/cli for a bin shim we never invoke. All three call sites already go through findSgBinary, which resolves the platform package directly and execs by path — the wrapper contributes only a postinstall hardlink and a .bin fallback. That hardlink has already failed here: findSgBinary's comment exists because pnpm dlx's strict isolation leaves a placeholder text file in place of the binary. The wrapper also needs a dependency build script to run at all, which pnpm 10 blocks by default. Upstream publishes the platform packages independently — os/cpu declared, no scripts, no bin — so depending on them directly deletes a layer rather than adding one, and matches the model add-vale-binary-packages establishes for Vale. The one path that could regress is musl: findSgBinary maps every Linux to -gnu, so the shim may be doing real work on Alpine today. Verifying that is called out rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- .../direct-sg-platform-deps/.openspec.yaml | 2 + .../changes/direct-sg-platform-deps/design.md | 74 +++++++++++++++++++ .../direct-sg-platform-deps/proposal.md | 27 +++++++ .../direct-sg-platform-deps/specs/cli/spec.md | 55 ++++++++++++++ .../changes/direct-sg-platform-deps/tasks.md | 31 ++++++++ 5 files changed, 189 insertions(+) create mode 100644 openspec/changes/direct-sg-platform-deps/.openspec.yaml create mode 100644 openspec/changes/direct-sg-platform-deps/design.md create mode 100644 openspec/changes/direct-sg-platform-deps/proposal.md create mode 100644 openspec/changes/direct-sg-platform-deps/specs/cli/spec.md create mode 100644 openspec/changes/direct-sg-platform-deps/tasks.md diff --git a/openspec/changes/direct-sg-platform-deps/.openspec.yaml b/openspec/changes/direct-sg-platform-deps/.openspec.yaml new file mode 100644 index 00000000..8e7013b8 --- /dev/null +++ b/openspec/changes/direct-sg-platform-deps/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-27 diff --git a/openspec/changes/direct-sg-platform-deps/design.md b/openspec/changes/direct-sg-platform-deps/design.md new file mode 100644 index 00000000..043f70f9 --- /dev/null +++ b/openspec/changes/direct-sg-platform-deps/design.md @@ -0,0 +1,74 @@ +## Context + +`packages/cli` depends on `@ast-grep/cli`. That package ships `sg`, `ast-grep`, and `postinstall.js`, and declares seven per-platform packages as `optionalDependencies` filtered by `os`/`cpu`. Its `postinstall` resolves the matching platform package and hardlinks (falling back to copy) the binary into itself, so its `bin: {sg, ast-grep}` entries resolve. + +We never use those `bin` entries. Every ast-grep invocation in the CLI — `rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43` — calls `findSgBinary()` (`rules/scan.ts:38-61`), which does: + +``` +createRequire(import.meta.url).resolve('@ast-grep/cli-/package.json') +→ dirname → join(binary) → exec by path +``` + +falling back to `"sg"` on `PATH`. The platform package is already the primary resolution; the wrapper contributes only the fallback shim in `node_modules/.bin`, reached via `buildPath()` (`rules/scan.ts:17-22`). + +Verified: `@ast-grep/cli-darwin-arm64` is independently published on npm, declares `os: [darwin]` / `cpu: [arm64]`, carries **no `scripts` and no `bin`**, is MIT, and ships `ast-grep` at mode `-rwxr-xr-x`. Only the host-matching package installs — this repo's store contains that one and no other. + +The wrapper's install step has already failed here. `findSgBinary()`'s comment records the reason it was written: under `pnpm dlx`'s strict dependency isolation the hardlink cannot resolve, **leaving a placeholder text file instead of the real binary**. Separately, pnpm 10 does not run dependency lifecycle scripts without an `onlyBuiltDependencies` allowlist, so the wrapper's `postinstall` is subject to a policy set by whoever installs the CLI. + +## Goals / Non-Goals + +**Goals:** depend on the artifact we actually use; remove any dependence on a dependency lifecycle script; make ast-grep and Vale resolve through one model. + +**Non-Goals:** changing how ast-grep is invoked or what it does; republishing ast-grep binaries ourselves (upstream already publishes exactly the packages we need); changing `findSgBinary()`'s resolution order. + +## Decisions + +### D1 — Depend on the platform packages directly; drop the wrapper + +`packages/cli` declares the `@ast-grep/cli-` packages in `optionalDependencies` and removes `@ast-grep/cli`. + +The wrapper exists to populate `bin` entries for human use. We exec by path, so its entire contribution is a shim we never call and an install-time step that has already proven fragile. Removing it means the binary is present purely by dependency resolution — nothing to approve, nothing a package-manager policy can block, no placeholder-file failure mode. + +This is the same model `add-vale-binary-packages` adopts for Vale. There the work is publishing packages that don't exist; here upstream already publishes them, so the work is deleting a layer. + +- **Alternative — keep the wrapper and rely on `findSgBinary()` to route around it:** rejected; that is today's arrangement, and it means carrying a dependency whose install step can fail in ways we then have to explain. The comment in `scan.ts` is the cost of that choice, already paid once. +- **Alternative — republish ast-grep binaries under our own scope:** rejected; upstream's platform packages are already script-free, `os`/`cpu`-filtered, and independently installable. Mirroring them would add a pipeline and a lag behind upstream for no gain. (Vale needs this only because no equivalent exists.) + +### D2 — Pin every platform package to one exact ast-grep version + +All platform packages are pinned to the same exact version and moved together. A mixed set would mean different hosts running different ast-grep versions against the same rules — a difference that would surface as inconsistent findings rather than as an install error. + +The wrapper previously enforced this implicitly, by pinning its own `optionalDependencies` to its exact version. Declaring them directly moves that obligation to us, so it needs a check rather than a convention. + +### D3 — Resolution order is unchanged; the fallback narrows + +`findSgBinary()` keeps preferring the platform package and falling back to `"sg"`. What changes is only what the fallback can find: previously the wrapper's `node_modules/.bin/sg` shim **or** a host install; now a host install alone. + +This matters exactly where the platform package is missing — an unsupported architecture, or musl (D4). It is a narrowing, so it deserves to be stated rather than discovered. + +`buildPath()` exists to put `node_modules/.bin` on `PATH` for that shim. Whether it still earns its place once the shim is gone is an implementation question; it is harmless either way, and other tooling may rely on it. + +### D4 — musl is an existing gap, neither widened nor closed here + +`findSgBinary()` maps every Linux to `-gnu`, so on Alpine the resolve already misses and falls through to `PATH`. Upstream's own `postinstall` uses `detect-libc` and would have found a musl package. + +So on musl the wrapper's shim may currently be doing real work, and removing it could turn a working setup into a `PATH`-dependent one. This change does not attempt to fix libc detection — that belongs with the shared resolver in `add-vale-rule-engine` — but it must confirm whether upstream publishes musl platform packages and, if so, include them, so the set is at least no worse than today. + +## Risks / Trade-offs + +- **musl/Alpine regression** (D4) → confirm whether upstream publishes musl platform packages; include them if so. Verify Alpine behavior explicitly rather than assuming, since the shim may be load-bearing there today. +- **Version drift across the platform set** (D2) → all pinned exactly and bumped together; add a check so an update cannot land partially applied. +- **Upstream reorganizes its platform packages** → we would be depending on packages upstream treats as an implementation detail of its wrapper, even though they are independently published and stable. Mitigated by exact pins: an upstream change cannot reach us until we bump. Worth noting as an ongoing, low-likelihood obligation. +- **Losing the `.bin` shim narrows the fallback** (D3) → accepted and documented; the primary path never used it. + +## Migration Plan + +Swap the dependency, reinstall, verify the binary resolves on a supported platform, and confirm `check`, `verify`, and the runtime narrow path all still run ast-grep. No on-disk or user-visible change; nothing to roll forward or back beyond the dependency itself. + +Rollback is restoring the `@ast-grep/cli` dependency — `findSgBinary()` works under either arrangement, which is what makes this safe to try. + +## Open Questions + +- Does upstream publish musl platform packages, and at which architectures? +- Should this land before or after `add-vale-binary-packages`? They are independent, but doing this one first proves the model against packages that already exist, before the Vale change commits to publishing new ones. +- Does `buildPath()` still have a consumer once the `.bin/sg` shim is gone? diff --git a/openspec/changes/direct-sg-platform-deps/proposal.md b/openspec/changes/direct-sg-platform-deps/proposal.md new file mode 100644 index 00000000..78fc66e1 --- /dev/null +++ b/openspec/changes/direct-sg-platform-deps/proposal.md @@ -0,0 +1,27 @@ +## Why + +The CLI depends on `@ast-grep/cli`, a wrapper package whose only job is to run a `postinstall` that hardlinks the real binary out of a platform package and into itself, so its `bin: {sg, ast-grep}` entries work. We don't use those `bin` entries — all three call sites resolve the platform package directly via `findSgBinary()` and exec by path. We carry the wrapper, and its install-time step, for a shim we never invoke. + +That step is not free. `findSgBinary()`'s own comment records why it exists: the hardlink **fails under `pnpm dlx`'s strict dependency isolation, leaving a placeholder text file** where the binary should be. The wrapper also requires a dependency build script to run at all, which pnpm 10 blocks by default. + +`add-vale-binary-packages` establishes the pattern for Vale: platform packages as `optionalDependencies`, resolved and executed by path, no lifecycle script anywhere. ast-grep should use the same one — for it, that means removing a layer rather than adding one. + +## What Changes + +- Replace the `@ast-grep/cli` dependency with direct `optionalDependencies` on the `@ast-grep/cli-` packages, which are independently published with `os`/`cpu` declared and carry no scripts and no `bin`. +- Pin every platform package to the same exact ast-grep version, keeping the set in lockstep. +- Keep `findSgBinary()`'s resolution order unchanged — it already prefers the platform package; only the `PATH` fallback's meaning narrows, from "the wrapper's `.bin` shim or a host install" to "a host install". +- Remove the CLI's dependence on any dependency lifecycle script running. + +## Capabilities + +### Modified Capabilities + +- `cli`: The CLI declares ast-grep platform packages as `optionalDependencies` pinned to an exact version instead of depending on the `@ast-grep/cli` wrapper, and its ast-grep binary is resolved without any install-time step. + +## Impact + +- **Modified**: `packages/cli/package.json` (drop `@ast-grep/cli`, add the platform packages), the lockfile, and possibly `buildPath()` in `packages/cli/src/rules/scan.ts` if the `node_modules/.bin` augmentation no longer earns its place. +- **Unchanged**: all three ast-grep call sites (`rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43`) already route through `findSgBinary()`, so the happy path needs no code change. +- **Related**: `add-vale-binary-packages` applies this same model to Vale. This change makes the two engines consistent, and lets one shared resolver serve both (`add-vale-rule-engine` task 5.1). +- **Behavioral risk**: platforms where no `@ast-grep/cli-` package exists — notably musl/Alpine — lose the wrapper's fallback path and rely on `PATH` alone. diff --git a/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md b/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md new file mode 100644 index 00000000..b9c91c29 --- /dev/null +++ b/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md @@ -0,0 +1,55 @@ +## ADDED Requirements + +### Requirement: ast-grep is declared as platform packages, not a wrapper + +`packages/cli` SHALL declare the ast-grep platform packages in `optionalDependencies` and SHALL NOT depend on the `@ast-grep/cli` wrapper package. Every platform package SHALL be pinned to the same exact ast-grep version. + +An unsupported host SHALL install the CLI successfully with no platform package present. + +#### Scenario: Only the host-matching package installs + +- **WHEN** the CLI is installed on a supported platform +- **THEN** only the ast-grep platform package matching the host's `os` and `cpu` is installed, and the binary is resolvable from the CLI's module context + +#### Scenario: The wrapper is absent + +- **WHEN** the CLI's dependencies are inspected +- **THEN** `@ast-grep/cli` is not among them + +#### Scenario: Platform packages stay in lockstep + +- **WHEN** the declared ast-grep platform packages are inspected +- **THEN** every one is pinned to the same exact version, so no two hosts run different ast-grep versions against the same rules + +#### Scenario: Unsupported platform still installs + +- **WHEN** the CLI is installed on a platform with no published ast-grep package +- **THEN** the install succeeds with no platform package present and no install-time error + +### Requirement: The ast-grep binary requires no install-time step + +The ast-grep binary SHALL be usable purely by dependency resolution. Its availability SHALL NOT depend on a dependency lifecycle script having run, and SHALL NOT depend on any file having been copied or linked into another package at install time. + +#### Scenario: Binary is available with lifecycle scripts disabled + +- **WHEN** the CLI is installed with dependency lifecycle scripts disabled +- **THEN** the ast-grep binary is present and executable, and every ast-grep-backed command runs + +#### Scenario: Isolated installs resolve the real binary + +- **WHEN** the CLI is executed from an install with strict dependency isolation, such as `pnpm dlx` +- **THEN** the resolved path is the real executable, never a placeholder file + +### Requirement: Binary resolution prefers the platform package over PATH + +Resolution SHALL attempt the platform package first and fall back to `PATH` only when it cannot be resolved. All ast-grep invocations SHALL use this single resolution path. + +#### Scenario: Platform package wins over a host install + +- **WHEN** the platform package resolves and an unrelated ast-grep is also present on `PATH` +- **THEN** the platform package's binary is executed + +#### Scenario: Fallback covers an unresolvable platform package + +- **WHEN** no platform package resolves for the host +- **THEN** resolution falls back to `PATH`, and the engine reports itself unavailable if nothing is found there diff --git a/openspec/changes/direct-sg-platform-deps/tasks.md b/openspec/changes/direct-sg-platform-deps/tasks.md new file mode 100644 index 00000000..8762aa01 --- /dev/null +++ b/openspec/changes/direct-sg-platform-deps/tasks.md @@ -0,0 +1,31 @@ +## 1. Confirm the upstream surface + +- [ ] 1.1 Enumerate the published `@ast-grep/cli-` packages at the pinned ast-grep version and confirm each declares `os`/`cpu`, carries no `scripts` and no `bin`, and ships the executable with its permission bit +- [ ] 1.2 Determine whether upstream publishes musl variants and at which architectures (upstream's own `postinstall` uses `detect-libc`, implying they exist) +- [ ] 1.3 Confirm nothing in the repo — scripts, CI, or docs — invokes `sg`/`ast-grep` via the wrapper's `bin` entries rather than through `findSgBinary()` + +## 2. Swap the dependency + +- [ ] 2.1 Remove `@ast-grep/cli` from `packages/cli` dependencies +- [ ] 2.2 Add every `@ast-grep/cli-` package to `optionalDependencies`, all pinned to the same exact version, including musl variants if 1.2 found them +- [ ] 2.3 Reinstall and confirm the lockfile resolves only the host-matching package +- [ ] 2.4 Add a check that the platform set stays version-aligned, so an update cannot land partially applied + +## 3. Verify resolution and execution + +- [ ] 3.1 Confirm `findSgBinary()` resolves the platform package unchanged, and that `check`, `rule verify`, and the runtime narrow path (`rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43`) all still execute ast-grep +- [ ] 3.2 Verify under `pnpm dlx` that the resolved path is the real executable, not a placeholder — the failure `findSgBinary()`'s comment was written to route around +- [ ] 3.3 Verify with dependency lifecycle scripts disabled that the binary is present and every ast-grep-backed command runs +- [ ] 3.4 Verify on a platform with no published package that install succeeds and resolution falls back to `PATH` +- [ ] 3.5 Verify Alpine/musl behavior explicitly and record it — the wrapper's `.bin` shim may be load-bearing there today, so this is the one path that could regress + +## 4. Clean up + +- [ ] 4.1 Decide whether `buildPath()` (`rules/scan.ts:17-22`) still has a consumer once the `.bin/sg` shim is gone; remove it or document what still needs it +- [ ] 4.2 Remove any pnpm build-script approval for `@ast-grep/cli` that is no longer needed +- [ ] 4.3 Update comments in `rules/scan.ts` that describe routing around the wrapper's postinstall, which no longer applies once the wrapper is gone + +## 5. Quality gates + +- [ ] 5.1 `pnpm --filter @taskless/cli typecheck && lint && test` clean +- [ ] 5.2 Confirm the published CLI tarball declares the platform packages as optional dependencies at exact versions and does not declare `@ast-grep/cli` From ade7a9510439aa8ed863d8e9b3874024d1481b00 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 16:03:24 -0700 Subject: [PATCH 2/9] docs(openspec): record group 1 findings; devDeps wrapper, exhaustive search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group 1 changed the shape of this change. The seven platform packages are already declared in optionalDependencies — only the wrapper is left over — and musl inverts from a risk to an improvement: no musl package exists at any version, the gnu packages declare libc, and today's postinstall exits 1 on Alpine. The wrapper moves to devDependencies rather than disappearing: fetch-ast-grep-schema.ts reads its version for the schema tag, which is a build-time need. Consumers stop installing it either way, so the pnpm dlx failure mode leaves the shipped product. Resolution now exhausts every known location — platform package, node_modules/.bin, sg and ast-grep on PATH — and throws naming what it tried, instead of returning a bare "sg" for spawn to fail on. If there's no sg, there's no sg; we just have better and worse places to look first. Pin held at 0.41.0 deliberately, not bumped to 0.45.0, so the swap stays independently verifiable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- .../changes/direct-sg-platform-deps/design.md | 64 ++++++++++++++----- .../direct-sg-platform-deps/proposal.md | 19 +++--- .../direct-sg-platform-deps/specs/cli/spec.md | 31 +++++---- .../changes/direct-sg-platform-deps/tasks.md | 27 +++++--- 4 files changed, 96 insertions(+), 45 deletions(-) diff --git a/openspec/changes/direct-sg-platform-deps/design.md b/openspec/changes/direct-sg-platform-deps/design.md index 043f70f9..434c4b31 100644 --- a/openspec/changes/direct-sg-platform-deps/design.md +++ b/openspec/changes/direct-sg-platform-deps/design.md @@ -1,6 +1,8 @@ ## Context -`packages/cli` depends on `@ast-grep/cli`. That package ships `sg`, `ast-grep`, and `postinstall.js`, and declares seven per-platform packages as `optionalDependencies` filtered by `os`/`cpu`. Its `postinstall` resolves the matching platform package and hardlinks (falling back to copy) the binary into itself, so its `bin: {sg, ast-grep}` entries resolve. +**Current state, verified:** `packages/cli` already declares all seven `@ast-grep/cli-` packages in `optionalDependencies` — at caret ranges (`^0.41.0`) — _and_ still depends on `@ast-grep/cli` itself. The migration was started and not finished. + +The wrapper ships `sg`, `ast-grep`, and `postinstall.js`, and declares the same seven packages as its own `optionalDependencies` filtered by `os`/`cpu`. Its `postinstall` resolves the matching platform package and hardlinks (falling back to copy) the binary into itself, so its `bin: {sg, ast-grep}` entries resolve. Running that script required an opt-in: the root `package.json` lists `@ast-grep/cli` in `pnpm.onlyBuiltDependencies`, alongside `esbuild`. We never use those `bin` entries. Every ast-grep invocation in the CLI — `rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43` — calls `findSgBinary()` (`rules/scan.ts:38-61`), which does: @@ -23,40 +25,68 @@ The wrapper's install step has already failed here. `findSgBinary()`'s comment r ## Decisions -### D1 — Depend on the platform packages directly; drop the wrapper +### D1 — Remove the wrapper; the platform packages are already declared + +The platform packages stay as they are; `@ast-grep/cli` is removed. + +The wrapper exists to populate `bin` entries for human use. We exec by path, so its entire contribution is a shim we never call plus an install-time step that has already proven fragile. With the platform packages already declared, removing it leaves the binary present purely by dependency resolution — nothing to approve, nothing a package-manager policy can block, no placeholder-file failure mode, and one less allowlist entry. + +This is the same model `add-vale-binary-packages` adopts for Vale. There the work is publishing packages that do not exist; here upstream already publishes them and we already depend on them, so the work is deleting a layer. -`packages/cli` declares the `@ast-grep/cli-` packages in `optionalDependencies` and removes `@ast-grep/cli`. +One consumer of the wrapper's _declaration_ — not its binary — keeps it alive, but only for us: `scripts/fetch-ast-grep-schema.ts:19` reads `dependencies["@ast-grep/cli"]` to choose which upstream tag to fetch the rule schema from. That is a build-time need, so `@ast-grep/cli` **moves to `devDependencies`** rather than disappearing. -The wrapper exists to populate `bin` entries for human use. We exec by path, so its entire contribution is a shim we never call and an install-time step that has already proven fragile. Removing it means the binary is present purely by dependency resolution — nothing to approve, nothing a package-manager policy can block, no placeholder-file failure mode. +Consequences of that placement, both intended: -This is the same model `add-vale-binary-packages` adopts for Vale. There the work is publishing packages that don't exist; here upstream already publishes them, so the work is deleting a layer. +- Consumers of `@taskless/cli` never install it, so the wrapper's `postinstall` — and the `pnpm dlx` failure mode — leave the shipped product entirely. The fragility is confined to this repository's own installs. +- The root `pnpm.onlyBuiltDependencies` entry for `@ast-grep/cli` therefore stays. It now permits a script that only ever runs for contributors. + +- **Alternative — remove the wrapper outright and read the version from a platform package:** viable and would drop the allowlist entry too, but it spreads the schema script's version source across seven declarations to avoid a devDependency that costs consumers nothing. - **Alternative — keep the wrapper and rely on `findSgBinary()` to route around it:** rejected; that is today's arrangement, and it means carrying a dependency whose install step can fail in ways we then have to explain. The comment in `scan.ts` is the cost of that choice, already paid once. - **Alternative — republish ast-grep binaries under our own scope:** rejected; upstream's platform packages are already script-free, `os`/`cpu`-filtered, and independently installable. Mirroring them would add a pipeline and a lag behind upstream for no gain. (Vale needs this only because no equivalent exists.) ### D2 — Pin every platform package to one exact ast-grep version -All platform packages are pinned to the same exact version and moved together. A mixed set would mean different hosts running different ast-grep versions against the same rules — a difference that would surface as inconsistent findings rather than as an install error. +They are currently declared at caret ranges (`^0.41.0`), which permits different hosts resolving different ast-grep versions against the same rules — a divergence that surfaces as inconsistent findings, not as an install error. Upstream has since published `0.45.0`, so the ranges are live, not theoretical. + +The wrapper enforced alignment implicitly today, by pinning its own `optionalDependencies` to its exact version — one more thing lost when it leaves the runtime dependency set, and the reason exact pins belong in this change rather than a later one. Because that alignment stops being structural, it needs a check rather than a convention. + +**The pin holds at `0.41.0`.** Upstream is at `0.45.0`, and taking it here would be free-riding on a structural change: if ast-grep's behavior shifted, nobody could tell whether this change or the version caused it. Holding keeps the swap independently verifiable — findings before and after must be identical. Bumping to `0.45.0` is worth doing, as its own small change where a behavior difference has exactly one candidate explanation. -The wrapper previously enforced this implicitly, by pinning its own `optionalDependencies` to its exact version. Declaring them directly moves that obligation to us, so it needs a check rather than a convention. +### D3 — Resolution exhausts every known location, then fails clearly -### D3 — Resolution order is unchanged; the fallback narrows +Today `findSgBinary()` returns the literal string `"sg"` when the platform package does not resolve, and the failure surfaces later as a spawn error — from a caller that cannot say where it looked. -`findSgBinary()` keeps preferring the platform package and falling back to `"sg"`. What changes is only what the fallback can find: previously the wrapper's `node_modules/.bin/sg` shim **or** a host install; now a host install alone. +Resolution becomes an ordered search over candidate locations, each checked for an actual executable: -This matters exactly where the platform package is missing — an unsupported architecture, or musl (D4). It is a narrowing, so it deserves to be stated rather than discovered. +1. the host's `@ast-grep/cli-` package (the normal case), +2. `node_modules/.bin`, for a host that still has the wrapper or another provider, +3. `sg`, then `ast-grep`, on `PATH`. -`buildPath()` exists to put `node_modules/.bin` on `PATH` for that shim. Whether it still earns its place once the shim is gone is an implementation question; it is harmless either way, and other tooling may rely on it. +If every candidate misses, resolution **fails with an error naming the locations it tried**. There is no ast-grep, and saying so plainly beats handing a bare `"sg"` to `spawn` and letting `ENOENT` explain it. -### D4 — musl is an existing gap, neither widened nor closed here +The ordering is the point: these are best-guess locations tried in descending confidence, not a chain where a later entry is a lesser version of an earlier one. A bundled platform package is preferred over a host install because it is the version we pinned; a host install is still better than nothing. -`findSgBinary()` maps every Linux to `-gnu`, so on Alpine the resolve already misses and falls through to `PATH`. Upstream's own `postinstall` uses `detect-libc` and would have found a musl package. +This is also the shape `add-vale-rule-engine` needs for Vale — platform package, then `PATH`, then a clear unavailable report — which is what makes one shared helper serve both engines. -So on musl the wrapper's shim may currently be doing real work, and removing it could turn a working setup into a `PATH`-dependent one. This change does not attempt to fix libc detection — that belongs with the shared resolver in `add-vale-rule-engine` — but it must confirm whether upstream publishes musl platform packages and, if so, include them, so the set is at least no worse than today. +`buildPath()` puts `node_modules/.bin` on `PATH` for spawned processes; candidate (2) makes that location explicit in the search rather than implicit in the environment. Whether `buildPath()` still earns its place is an implementation question, and it is harmless either way. + +### D4 — musl improves; there is no musl package to lose + +Verified against the registry, so this replaces the concern that dropping the wrapper might regress Alpine: + +- **No musl package exists.** `@ast-grep/cli` publishes exactly seven platform packages at `0.41.0`, and the same seven at `0.45.0` — darwin `x64`/`arm64`, linux `x64-gnu`/`arm64-gnu`, win32 `x64`/`ia32`/`arm64` msvc. `@ast-grep/cli-linux-x64-musl` and `-linux-arm64-musl` are unpublished. +- **The gnu packages declare `libc: ["glibc"]`**, so a package manager skips them on musl rather than installing a binary that cannot exec. + +Upstream's `postinstall` uses `detect-libc` and therefore computes `@ast-grep/cli-linux-x64-musl` on Alpine — a package that does not exist. It falls back to `target/release`, then `target/debug` (neither present in a published package), then `console.error` and `exit 1`. **Today, wherever dependency scripts actually run, installing on Alpine fails.** + +After this change there is no script, so the install succeeds with no platform package present and `findSgBinary()` falls through to `PATH`. Alpine goes from a failed install to a clean install plus a documented fallback. + +`findSgBinary()` mapping every Linux to `-gnu` is consequently harmless: on glibc it names the package that exists, and on musl `libc` filtering has already ensured nothing is installed to resolve. Fixing libc detection would be correctness for its own sake — worth doing with the shared resolver in `add-vale-rule-engine`, not required here. ## Risks / Trade-offs -- **musl/Alpine regression** (D4) → confirm whether upstream publishes musl platform packages; include them if so. Verify Alpine behavior explicitly rather than assuming, since the shim may be load-bearing there today. +- ~~musl/Alpine regression~~ → **resolved, and it improves** (D4): no musl package exists at any version, the gnu packages declare `libc: ["glibc"]` so they are skipped there, and today's wrapper `postinstall` actively fails the install on Alpine. Still worth verifying once on a real Alpine image rather than reasoning from metadata alone. - **Version drift across the platform set** (D2) → all pinned exactly and bumped together; add a check so an update cannot land partially applied. - **Upstream reorganizes its platform packages** → we would be depending on packages upstream treats as an implementation detail of its wrapper, even though they are independently published and stable. Mitigated by exact pins: an upstream change cannot reach us until we bump. Worth noting as an ongoing, low-likelihood obligation. - **Losing the `.bin` shim narrows the fallback** (D3) → accepted and documented; the primary path never used it. @@ -69,6 +99,8 @@ Rollback is restoring the `@ast-grep/cli` dependency — `findSgBinary()` works ## Open Questions -- Does upstream publish musl platform packages, and at which architectures? - Should this land before or after `add-vale-binary-packages`? They are independent, but doing this one first proves the model against packages that already exist, before the Vale change commits to publishing new ones. - Does `buildPath()` still have a consumer once the `.bin/sg` shim is gone? +- Should the ast-grep version be bumped (`0.41.0` → `0.45.0`) while touching these pins, or held constant to keep this change purely structural? Holding it constant makes the swap independently verifiable. + +**Resolved:** upstream publishes no musl packages at any version, and the gnu packages declare `libc` (D4). diff --git a/openspec/changes/direct-sg-platform-deps/proposal.md b/openspec/changes/direct-sg-platform-deps/proposal.md index 78fc66e1..ff6395f9 100644 --- a/openspec/changes/direct-sg-platform-deps/proposal.md +++ b/openspec/changes/direct-sg-platform-deps/proposal.md @@ -1,17 +1,17 @@ ## Why -The CLI depends on `@ast-grep/cli`, a wrapper package whose only job is to run a `postinstall` that hardlinks the real binary out of a platform package and into itself, so its `bin: {sg, ast-grep}` entries work. We don't use those `bin` entries — all three call sites resolve the platform package directly via `findSgBinary()` and exec by path. We carry the wrapper, and its install-time step, for a shim we never invoke. +The migration to platform packages is already half done. `packages/cli` declares all seven `@ast-grep/cli-` packages in `optionalDependencies`, and `findSgBinary()` resolves them directly and execs by path — every ast-grep call site already goes through it. What remains is the `@ast-grep/cli` wrapper, still declared as a hard dependency beside them. -That step is not free. `findSgBinary()`'s own comment records why it exists: the hardlink **fails under `pnpm dlx`'s strict dependency isolation, leaving a placeholder text file** where the binary should be. The wrapper also requires a dependency build script to run at all, which pnpm 10 blocks by default. +The wrapper's only job is a `postinstall` that hardlinks the binary out of a platform package into itself, so its `bin: {sg, ast-grep}` entries resolve. We never invoke those entries. So we carry a redundant dependency, plus its install-time step — a step that has already failed here: `findSgBinary()`'s comment records that the hardlink **breaks under `pnpm dlx`'s strict dependency isolation, leaving a placeholder text file** where the binary should be. It also required opting in to run at all, which is why the root `package.json` lists `@ast-grep/cli` in `pnpm.onlyBuiltDependencies`. -`add-vale-binary-packages` establishes the pattern for Vale: platform packages as `optionalDependencies`, resolved and executed by path, no lifecycle script anywhere. ast-grep should use the same one — for it, that means removing a layer rather than adding one. +`add-vale-binary-packages` establishes this pattern for Vale. Finishing it for ast-grep means deleting the leftover, not building anything. ## What Changes -- Replace the `@ast-grep/cli` dependency with direct `optionalDependencies` on the `@ast-grep/cli-` packages, which are independently published with `os`/`cpu` declared and carry no scripts and no `bin`. -- Pin every platform package to the same exact ast-grep version, keeping the set in lockstep. -- Keep `findSgBinary()`'s resolution order unchanged — it already prefers the platform package; only the `PATH` fallback's meaning narrows, from "the wrapper's `.bin` shim or a host install" to "a host install". -- Remove the CLI's dependence on any dependency lifecycle script running. +- Move `@ast-grep/cli` from `dependencies` to `devDependencies`. The seven platform packages already in `optionalDependencies` become the only ast-grep dependency consumers install, so the wrapper's `postinstall` leaves the shipped product entirely. +- Tighten those declarations from caret ranges (`^0.41.0`) to exactly `0.41.0`, so the set cannot drift apart across hosts. **Held deliberately at `0.41.0`** rather than bumped to upstream's `0.45.0`, keeping this change purely structural. +- Point `scripts/fetch-ast-grep-schema.ts` at the new location — it reads the schema version from `dependencies["@ast-grep/cli"]` and throws when absent, and is the only genuine consumer of that declaration. +- Make binary resolution **exhaust every known location before failing**: platform package, `node_modules/.bin`, then `sg` and `ast-grep` on `PATH`, and a clear error naming what was tried. Today it returns a bare `"sg"` and lets `spawn` produce the failure. ## Capabilities @@ -21,7 +21,8 @@ That step is not free. `findSgBinary()`'s own comment records why it exists: the ## Impact -- **Modified**: `packages/cli/package.json` (drop `@ast-grep/cli`, add the platform packages), the lockfile, and possibly `buildPath()` in `packages/cli/src/rules/scan.ts` if the `node_modules/.bin` augmentation no longer earns its place. -- **Unchanged**: all three ast-grep call sites (`rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43`) already route through `findSgBinary()`, so the happy path needs no code change. +- **Modified**: `packages/cli/package.json` (wrapper to `devDependencies`, platform packages pinned exactly), `packages/cli/scripts/fetch-ast-grep-schema.ts` (version source), `packages/cli/src/rules/scan.ts` (`findSgBinary()` becomes an exhaustive search), and the lockfile. +- **Unchanged**: all three ast-grep call sites (`rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43`) already route through `findSgBinary()`, so they need no change — they simply get a better answer, or a real error. +- **Kept**: `@ast-grep/cli` stays in the root `pnpm.onlyBuiltDependencies`, now permitting a script that runs only for contributors. - **Related**: `add-vale-binary-packages` applies this same model to Vale. This change makes the two engines consistent, and lets one shared resolver serve both (`add-vale-rule-engine` task 5.1). - **Behavioral risk**: platforms where no `@ast-grep/cli-` package exists — notably musl/Alpine — lose the wrapper's fallback path and rely on `PATH` alone. diff --git a/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md b/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md index b9c91c29..011542c3 100644 --- a/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md +++ b/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md @@ -2,19 +2,21 @@ ### Requirement: ast-grep is declared as platform packages, not a wrapper -`packages/cli` SHALL declare the ast-grep platform packages in `optionalDependencies` and SHALL NOT depend on the `@ast-grep/cli` wrapper package. Every platform package SHALL be pinned to the same exact ast-grep version. +`packages/cli` SHALL declare the ast-grep platform packages in `optionalDependencies` and SHALL NOT declare the `@ast-grep/cli` wrapper package as a runtime dependency. Every platform package SHALL be pinned to the same exact ast-grep version. -An unsupported host SHALL install the CLI successfully with no platform package present. +The wrapper MAY be declared as a `devDependency` where a build-time script needs its version, since a devDependency is not installed for consumers and therefore cannot affect the shipped product. + +`optionalDependencies` specifically: a `devDependency` is not installed for consumers of the CLI, and a hard `dependency` would fail the install on every host the package does not match. `optional` is what allows `os`/`cpu` filtering to install exactly one package and skip the rest without error, so an unsupported host SHALL install the CLI successfully with no platform package present. #### Scenario: Only the host-matching package installs - **WHEN** the CLI is installed on a supported platform - **THEN** only the ast-grep platform package matching the host's `os` and `cpu` is installed, and the binary is resolvable from the CLI's module context -#### Scenario: The wrapper is absent +#### Scenario: The wrapper is absent from what consumers install -- **WHEN** the CLI's dependencies are inspected -- **THEN** `@ast-grep/cli` is not among them +- **WHEN** a published CLI tarball's `dependencies` and `optionalDependencies` are inspected +- **THEN** `@ast-grep/cli` is not among them, so no consumer installs it or runs its install script #### Scenario: Platform packages stay in lockstep @@ -40,16 +42,23 @@ The ast-grep binary SHALL be usable purely by dependency resolution. Its availab - **WHEN** the CLI is executed from an install with strict dependency isolation, such as `pnpm dlx` - **THEN** the resolved path is the real executable, never a placeholder file -### Requirement: Binary resolution prefers the platform package over PATH +### Requirement: Binary resolution searches all known locations before failing + +Resolution SHALL search candidate locations in descending order of confidence — the host's platform package, then `node_modules/.bin`, then `sg` and `ast-grep` on `PATH` — and SHALL return the first that is an executable file. All ast-grep invocations SHALL use this single resolution path. -Resolution SHALL attempt the platform package first and fall back to `PATH` only when it cannot be resolved. All ast-grep invocations SHALL use this single resolution path. +When no candidate yields an executable, resolution SHALL fail with an error naming the locations it tried. It SHALL NOT return a bare command name and leave the failure to surface as a spawn error. #### Scenario: Platform package wins over a host install - **WHEN** the platform package resolves and an unrelated ast-grep is also present on `PATH` -- **THEN** the platform package's binary is executed +- **THEN** the platform package's binary is executed, because it is the version the CLI pinned + +#### Scenario: A host install is used when no platform package resolves + +- **WHEN** no platform package is installed for the host but ast-grep is available on `PATH` +- **THEN** resolution returns that binary and ast-grep-backed commands run normally -#### Scenario: Fallback covers an unresolvable platform package +#### Scenario: Exhausted search fails with a useful message -- **WHEN** no platform package resolves for the host -- **THEN** resolution falls back to `PATH`, and the engine reports itself unavailable if nothing is found there +- **WHEN** no candidate location yields an executable +- **THEN** resolution fails with an error naming the locations that were tried, rather than deferring to a spawn failure diff --git a/openspec/changes/direct-sg-platform-deps/tasks.md b/openspec/changes/direct-sg-platform-deps/tasks.md index 8762aa01..e1910c1a 100644 --- a/openspec/changes/direct-sg-platform-deps/tasks.md +++ b/openspec/changes/direct-sg-platform-deps/tasks.md @@ -1,15 +1,24 @@ ## 1. Confirm the upstream surface -- [ ] 1.1 Enumerate the published `@ast-grep/cli-` packages at the pinned ast-grep version and confirm each declares `os`/`cpu`, carries no `scripts` and no `bin`, and ships the executable with its permission bit -- [ ] 1.2 Determine whether upstream publishes musl variants and at which architectures (upstream's own `postinstall` uses `detect-libc`, implying they exist) -- [ ] 1.3 Confirm nothing in the repo — scripts, CI, or docs — invokes `sg`/`ast-grep` via the wrapper's `bin` entries rather than through `findSgBinary()` +- [x] 1.1 Enumerate the published `@ast-grep/cli-` packages at the pinned ast-grep version and confirm each declares `os`/`cpu`, carries no `scripts` and no `bin`, and ships the executable with its permission bit — **seven at `0.41.0`**: darwin `x64`/`arm64`, linux `x64-gnu`/`arm64-gnu`, win32 `x64`/`ia32`/`arm64` msvc. Verified no `scripts`, no `bin`, MIT, and `ast-grep` at mode `-rwxr-xr-x` +- [x] 1.2 Determine whether upstream publishes musl variants — **it does not**, at `0.41.0` or `0.45.0`; the gnu packages declare `libc: ["glibc"]` so they are skipped on musl (D4) +- [x] 1.3 Confirm nothing invokes `sg`/`ast-grep` via the wrapper's `bin` entries — **nothing does**; all three call sites use `findSgBinary()`. But `scripts/fetch-ast-grep-schema.ts:19` reads `dependencies["@ast-grep/cli"]` for the schema tag and throws when absent, so it consumes the wrapper's _declaration_ and must move (2.3) -## 2. Swap the dependency +## 2. Remove the wrapper -- [ ] 2.1 Remove `@ast-grep/cli` from `packages/cli` dependencies -- [ ] 2.2 Add every `@ast-grep/cli-` package to `optionalDependencies`, all pinned to the same exact version, including musl variants if 1.2 found them -- [ ] 2.3 Reinstall and confirm the lockfile resolves only the host-matching package -- [ ] 2.4 Add a check that the platform set stays version-aligned, so an update cannot land partially applied +- [ ] 2.1 Move `@ast-grep/cli` from `dependencies` to `devDependencies` — consumers stop installing it and its `postinstall` leaves the shipped product; `scripts/fetch-ast-grep-schema.ts` keeps its single version source +- [ ] 2.2 Tighten the seven platform declarations from `^0.41.0` to exactly `0.41.0` — held deliberately, not bumped to upstream's `0.45.0`, so this change stays purely structural (D2) +- [ ] 2.3 Update `scripts/fetch-ast-grep-schema.ts:19` to read from `devDependencies` instead of `dependencies`, and fail with a clear message naming the expected location +- [ ] 2.4 Leave `@ast-grep/cli` in the root `pnpm.onlyBuiltDependencies`; it now permits a script that runs only for contributors. Add a comment recording why it remains +- [ ] 2.5 Reinstall and confirm the lockfile resolves only the host-matching platform package, and that the wrapper is dev-only +- [ ] 2.6 Add a check that the platform set stays version-aligned and exactly pinned, so an update cannot land partially applied + +## 2b. Exhaustive binary resolution + +- [ ] 2b.1 Rewrite `findSgBinary()` to search candidates in order — platform package → `node_modules/.bin` → `sg` on `PATH` → `ast-grep` on `PATH` — returning the first that is an executable file +- [ ] 2b.2 Throw a clear error when every candidate misses, naming the locations tried; stop returning a bare `"sg"` for `spawn` to fail on +- [ ] 2b.3 Keep the shape reusable — `add-vale-rule-engine` extracts this into a helper shared with Vale, so candidate lists should be data, not control flow +- [ ] 2b.4 Tests: platform package wins over a `PATH` install; a `PATH` install is used when no platform package resolves; an exhausted search throws naming the locations ## 3. Verify resolution and execution @@ -17,7 +26,7 @@ - [ ] 3.2 Verify under `pnpm dlx` that the resolved path is the real executable, not a placeholder — the failure `findSgBinary()`'s comment was written to route around - [ ] 3.3 Verify with dependency lifecycle scripts disabled that the binary is present and every ast-grep-backed command runs - [ ] 3.4 Verify on a platform with no published package that install succeeds and resolution falls back to `PATH` -- [ ] 3.5 Verify Alpine/musl behavior explicitly and record it — the wrapper's `.bin` shim may be load-bearing there today, so this is the one path that could regress +- [ ] 3.5 Verify on a real Alpine image that the install now succeeds and resolution falls through to `PATH`. Per D4 this should be an improvement — today's `postinstall` exits 1 there — but confirm it rather than inferring from registry metadata ## 4. Clean up From e4e1a70218e3bb378ab6fada01a85066c929fc62 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 17:04:18 -0700 Subject: [PATCH 3/9] feat(cli): drop the ast-grep wrapper from runtime deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seven platform packages were already declared; only the wrapper was left over. It moves to devDependencies, where fetch-ast-grep-schema still reads its version, so consumers stop installing it and its postinstall — the one that leaves a placeholder text file under pnpm dlx — leaves the shipped product entirely. Platform packages pinned exactly at 0.41.0. The wrapper was enforcing version alignment implicitly by pinning its own optionalDependencies; with it gone from runtime, carets would let hosts resolve different ast-grep versions against the same rules. Held at 0.41.0 rather than bumped to 0.45.0 so this stays structural and independently verifiable. findSgBinary now exhausts every candidate — platform package, node_modules/.bin, sg then ast-grep on PATH — and throws naming what it tried. It previously returned a bare "sg" and let spawn's ENOENT be the error message, from a caller that could not say where it looked. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- packages/cli/package.json | 16 ++-- packages/cli/scripts/fetch-ast-grep-schema.ts | 10 +- packages/cli/src/rules/scan.ts | 92 ++++++++++++++----- pnpm-lock.yaml | 20 ++-- 4 files changed, 91 insertions(+), 47 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index be2b783e..0a1ff15e 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -32,7 +32,6 @@ "node": ">=22.22.0" }, "dependencies": { - "@ast-grep/cli": "^0.41.0", "@clack/prompts": "^1.2.0", "chalk": "^5.6.2", "citty": "^0.1.6", @@ -47,6 +46,7 @@ "zod": "^4.3.6" }, "devDependencies": { + "@ast-grep/cli": "0.41.0", "@types/sprintf-js": "^1.1.4", "openapi-typescript": "^7.13.0", "typescript": "^5.7.2", @@ -58,12 +58,12 @@ "taskless": "./dist/index.js" }, "optionalDependencies": { - "@ast-grep/cli-darwin-arm64": "^0.41.0", - "@ast-grep/cli-darwin-x64": "^0.41.0", - "@ast-grep/cli-linux-arm64-gnu": "^0.41.0", - "@ast-grep/cli-linux-x64-gnu": "^0.41.0", - "@ast-grep/cli-win32-arm64-msvc": "^0.41.0", - "@ast-grep/cli-win32-ia32-msvc": "^0.41.0", - "@ast-grep/cli-win32-x64-msvc": "^0.41.0" + "@ast-grep/cli-darwin-arm64": "0.41.0", + "@ast-grep/cli-darwin-x64": "0.41.0", + "@ast-grep/cli-linux-arm64-gnu": "0.41.0", + "@ast-grep/cli-linux-x64-gnu": "0.41.0", + "@ast-grep/cli-win32-arm64-msvc": "0.41.0", + "@ast-grep/cli-win32-ia32-msvc": "0.41.0", + "@ast-grep/cli-win32-x64-msvc": "0.41.0" } } diff --git a/packages/cli/scripts/fetch-ast-grep-schema.ts b/packages/cli/scripts/fetch-ast-grep-schema.ts index 53d9017d..3aebc01d 100644 --- a/packages/cli/scripts/fetch-ast-grep-schema.ts +++ b/packages/cli/scripts/fetch-ast-grep-schema.ts @@ -12,14 +12,16 @@ const OUTPUT_PATH = resolve( "ast-grep-rule-schema.json" ); -// Read @ast-grep/cli version from package.json +// Read @ast-grep/cli version from package.json. It is a devDependency: the +// shipped CLI depends only on the per-platform binary packages, and this +// wrapper is kept solely as the version source for this script. const packageJson = JSON.parse(readFileSync(CLI_PACKAGE_JSON, "utf8")) as { - dependencies?: Record; + devDependencies?: Record; }; -const rawVersion = packageJson.dependencies?.["@ast-grep/cli"]; +const rawVersion = packageJson.devDependencies?.["@ast-grep/cli"]; if (!rawVersion) { throw new Error( - "@ast-grep/cli not found in packages/cli/package.json dependencies" + "@ast-grep/cli not found in packages/cli/package.json devDependencies" ); } diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index ebd57861..0a2875fd 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -21,43 +21,85 @@ export function buildPath(): string { return `${binDirectory}${separator}${process.env.PATH ?? ""}`; } -/** - * Resolve the ast-grep binary path. - * - * The @ast-grep/cli package relies on a postinstall script that uses - * require.resolve() to find platform-specific binary packages and hardlink - * them into place. Under pnpm dlx, the strict dependency isolation can - * prevent that resolution from working, leaving a placeholder text file - * instead of the real binary. - * - * To work around this, we resolve the platform-specific package ourselves - * (from our own module context where optionalDependencies are accessible) - * and return the full path to the binary. Falls back to "sg" via PATH - * for environments where the normal .bin shim works fine. - */ -export function findSgBinary(): string { +/** The npm package carrying this host's prebuilt ast-grep binary. */ +function platformPackageName(): string { const parts: string[] = [process.platform, process.arch]; if (process.platform === "linux") { parts.push("gnu"); } else if (process.platform === "win32") { parts.push("msvc"); } + return `@ast-grep/cli-${parts.join("-")}`; +} - const platformPackage = `@ast-grep/cli-${parts.join("-")}`; - const binary = process.platform === "win32" ? "ast-grep.exe" : "ast-grep"; - +/** Absolute path to the binary inside the resolved platform package, if any. */ +function platformPackageBinary(binary: string): string | undefined { try { const require = createRequire(import.meta.url); - const packageJsonPath = require.resolve(`${platformPackage}/package.json`); - const binaryPath = resolve(dirname(packageJsonPath), binary); - if (existsSync(binaryPath)) { - return binaryPath; - } + const packageJsonPath = require.resolve( + `${platformPackageName()}/package.json` + ); + return resolve(dirname(packageJsonPath), binary); } catch { - // Platform package not resolvable — fall through to PATH-based lookup + // Not installed for this host (unsupported arch, or musl — upstream + // publishes no musl package and marks the gnu ones `libc: [glibc]`). + return undefined; + } +} + +/** First entry on PATH that holds an executable named `command`. */ +function findOnPath(command: string): string | undefined { + const separator = process.platform === "win32" ? ";" : ":"; + for (const directory of (process.env.PATH ?? "").split(separator)) { + if (directory === "") continue; + const candidate = resolve(directory, command); + if (existsSync(candidate)) return candidate; + } + return undefined; +} + +/** + * Resolve the ast-grep binary, searching every place it could reasonably live + * before giving up. + * + * We never rely on an install-time step to put the binary somewhere: the CLI + * depends on the per-platform packages directly and executes by path. That + * matters because the `@ast-grep/cli` wrapper's postinstall hardlink fails + * under pnpm dlx's strict isolation, leaving a placeholder text file where the + * binary should be. + * + * Candidates are ordered by confidence, not by quality of outcome — the + * platform package first because it is the version we pinned, then a locally + * linked binary, then whatever the host provides. If every candidate misses + * there is no ast-grep, and we say so plainly rather than handing a bare + * command name to spawn and letting ENOENT explain it. + */ +export function findSgBinary(): string { + const binary = process.platform === "win32" ? "ast-grep.exe" : "ast-grep"; + const alternative = process.platform === "win32" ? "sg.exe" : "sg"; + const localBin = resolve( + dirname(fileURLToPath(import.meta.url)), + "..", + "node_modules", + ".bin" + ); + + const candidates: Array<[label: string, path: string | undefined]> = [ + [platformPackageName(), platformPackageBinary(binary)], + ["node_modules/.bin", resolve(localBin, alternative)], + ["PATH", findOnPath(alternative)], + ["PATH", findOnPath(binary)], + ]; + + for (const [, path] of candidates) { + if (path !== undefined && existsSync(path)) return path; } - return "sg"; + const tried = candidates.map(([label]) => label).join(", "); + throw new Error( + `ast-grep binary not found. Looked in: ${tried}. Install a supported ` + + `platform build, or put \`${alternative}\` on your PATH.` + ); } /** Run ast-grep scan and return parsed results */ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9309d158..597c17d3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -71,9 +71,6 @@ importers: packages/cli: dependencies: - '@ast-grep/cli': - specifier: ^0.41.0 - version: 0.41.0 '@clack/prompts': specifier: ^1.2.0 version: 1.2.0 @@ -111,6 +108,9 @@ importers: specifier: ^4.3.6 version: 4.3.6 devDependencies: + '@ast-grep/cli': + specifier: 0.41.0 + version: 0.41.0 '@types/sprintf-js': specifier: ^1.1.4 version: 1.1.4 @@ -131,25 +131,25 @@ importers: version: 3.2.4(@types/node@25.3.3)(jiti@2.6.1)(lightningcss@1.31.1)(tsx@4.21.0)(yaml@2.8.2) optionalDependencies: '@ast-grep/cli-darwin-arm64': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 '@ast-grep/cli-darwin-x64': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 '@ast-grep/cli-linux-arm64-gnu': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 '@ast-grep/cli-linux-x64-gnu': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 '@ast-grep/cli-win32-arm64-msvc': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 '@ast-grep/cli-win32-ia32-msvc': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 '@ast-grep/cli-win32-x64-msvc': - specifier: ^0.41.0 + specifier: 0.41.0 version: 0.41.0 packages: From 13dfc94b579b97cc596b01ff089fce4729cd4e6f Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 17:09:30 -0700 Subject: [PATCH 4/9] docs(openspec): link direct-sg-platform-deps to OSS-23 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- openspec/changes/direct-sg-platform-deps/proposal.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openspec/changes/direct-sg-platform-deps/proposal.md b/openspec/changes/direct-sg-platform-deps/proposal.md index ff6395f9..2f0c3308 100644 --- a/openspec/changes/direct-sg-platform-deps/proposal.md +++ b/openspec/changes/direct-sg-platform-deps/proposal.md @@ -26,3 +26,5 @@ The wrapper's only job is a `postinstall` that hardlinks the binary out of a pla - **Kept**: `@ast-grep/cli` stays in the root `pnpm.onlyBuiltDependencies`, now permitting a script that runs only for contributors. - **Related**: `add-vale-binary-packages` applies this same model to Vale. This change makes the two engines consistent, and lets one shared resolver serve both (`add-vale-rule-engine` task 5.1). - **Behavioral risk**: platforms where no `@ast-grep/cli-` package exists — notably musl/Alpine — lose the wrapper's fallback path and rely on `PATH` alone. + +**Tracking:** OSS-23 From ef83fff400c02d9b5008b20502a2ad7557436cf1 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 18:13:58 -0700 Subject: [PATCH 5/9] chore: add changeset for the ast-grep dependency change Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- .changeset/wild-jars-repeat.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .changeset/wild-jars-repeat.md diff --git a/.changeset/wild-jars-repeat.md b/.changeset/wild-jars-repeat.md new file mode 100644 index 00000000..488fdb1e --- /dev/null +++ b/.changeset/wild-jars-repeat.md @@ -0,0 +1,31 @@ +--- +"@taskless/cli": patch +--- + +Resolve the ast-grep binary without relying on an install-time step, and drop +the `@ast-grep/cli` wrapper from what consumers install. + +- **The wrapper moves to `devDependencies`.** The seven `@ast-grep/cli-` + packages were already declared in `optionalDependencies`, and the CLI already + resolved them by path — the wrapper was a leftover whose only job is a + `postinstall` that hardlinks the binary into itself so its `bin` entries work. + Nothing here invoked those entries. Consumers now install only the platform + package matching their host, and the wrapper's `postinstall` — which leaves a + placeholder text file where the binary should be under `pnpm dlx`'s strict + isolation — is out of the shipped product entirely. It stays as a + `devDependency` because `fetch-ast-grep-schema` reads its version. +- **Platform packages are pinned exactly at `0.41.0`.** They were carets, and the + wrapper had been enforcing alignment implicitly by pinning its own + `optionalDependencies`; without it, two hosts could resolve different ast-grep + versions against the same rules and disagree about findings. Held at `0.41.0` + rather than taking upstream's `0.45.0`, so this change stays structural. +- **Binary resolution exhausts every candidate before failing.** It now searches + the platform package, `node_modules/.bin`, then `sg` and `ast-grep` on `PATH`, + and throws naming what it tried. Previously it returned a bare `"sg"` and let + `spawn`'s `ENOENT` be the error, from a caller that could not say where it had + looked. + +Alpine improves as a side effect: upstream publishes no musl build and marks its +Linux packages `libc: ["glibc"]`, so today the wrapper's `postinstall` resolves a +package that does not exist and exits 1, failing the install wherever dependency +scripts run. Installing now succeeds and resolution falls through to `PATH`. From f56cfd0c668ae0f61755e95a349fd90ead29a6a0 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 19:26:50 -0700 Subject: [PATCH 6/9] test(cli): cover ast-grep resolution and dependency alignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolution rewrite shipped without tests. These cover the contract rather than ast-grep itself: the platform package wins over a decoy sg on PATH, resolution returns an absolute path, and it survives an empty PATH. Also asserts the dependency shape the wrapper used to guarantee implicitly — every platform package pinned to one exact version, the wrapper absent from what consumers install, and the devDependency matching the shipped binaries. A mixed set would have hosts disagreeing about findings rather than failing to install. Alpine verified empirically on node:22-alpine (musl/aarch64): with the platform packages optional, npm install exits 0 and installs none of them; with the wrapper as a hard dependency it exits 1, "Failed to move @ast-grep/cli binary into place." buildPath is kept and documented — it no longer locates the binary, but still shapes the spawned process's environment. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- .../changes/direct-sg-platform-deps/tasks.md | 40 +++---- packages/cli/src/rules/scan.ts | 10 +- packages/cli/test/sg-binary.test.ts | 111 ++++++++++++++++++ 3 files changed, 140 insertions(+), 21 deletions(-) create mode 100644 packages/cli/test/sg-binary.test.ts diff --git a/openspec/changes/direct-sg-platform-deps/tasks.md b/openspec/changes/direct-sg-platform-deps/tasks.md index e1910c1a..a0387499 100644 --- a/openspec/changes/direct-sg-platform-deps/tasks.md +++ b/openspec/changes/direct-sg-platform-deps/tasks.md @@ -6,35 +6,35 @@ ## 2. Remove the wrapper -- [ ] 2.1 Move `@ast-grep/cli` from `dependencies` to `devDependencies` — consumers stop installing it and its `postinstall` leaves the shipped product; `scripts/fetch-ast-grep-schema.ts` keeps its single version source -- [ ] 2.2 Tighten the seven platform declarations from `^0.41.0` to exactly `0.41.0` — held deliberately, not bumped to upstream's `0.45.0`, so this change stays purely structural (D2) -- [ ] 2.3 Update `scripts/fetch-ast-grep-schema.ts:19` to read from `devDependencies` instead of `dependencies`, and fail with a clear message naming the expected location -- [ ] 2.4 Leave `@ast-grep/cli` in the root `pnpm.onlyBuiltDependencies`; it now permits a script that runs only for contributors. Add a comment recording why it remains -- [ ] 2.5 Reinstall and confirm the lockfile resolves only the host-matching platform package, and that the wrapper is dev-only -- [ ] 2.6 Add a check that the platform set stays version-aligned and exactly pinned, so an update cannot land partially applied +- [x] 2.1 Move `@ast-grep/cli` from `dependencies` to `devDependencies` — consumers stop installing it and its `postinstall` leaves the shipped product; `scripts/fetch-ast-grep-schema.ts` keeps its single version source +- [x] 2.2 Tighten the seven platform declarations from `^0.41.0` to exactly `0.41.0` — held deliberately, not bumped to upstream's `0.45.0`, so this change stays purely structural (D2) +- [x] 2.3 Update `scripts/fetch-ast-grep-schema.ts:19` to read from `devDependencies` instead of `dependencies`, and fail with a clear message naming the expected location +- [x] 2.4 Leave `@ast-grep/cli` in the root `pnpm.onlyBuiltDependencies` — done; it now permits a script that runs only for contributors. The "add a comment" half is **not possible**: the allowlist lives in `package.json`, which cannot carry comments. Rationale is recorded in this change's design (D1) instead +- [x] 2.5 Reinstall and confirm the lockfile resolves only the host-matching platform package, and that the wrapper is dev-only +- [x] 2.6 Add a check that the platform set stays version-aligned and exactly pinned — `test/sg-binary.test.ts` asserts every platform package shares one exact version, that the wrapper is absent from `dependencies`/`optionalDependencies`, and that the devDependency matches the shipped binaries ## 2b. Exhaustive binary resolution -- [ ] 2b.1 Rewrite `findSgBinary()` to search candidates in order — platform package → `node_modules/.bin` → `sg` on `PATH` → `ast-grep` on `PATH` — returning the first that is an executable file -- [ ] 2b.2 Throw a clear error when every candidate misses, naming the locations tried; stop returning a bare `"sg"` for `spawn` to fail on -- [ ] 2b.3 Keep the shape reusable — `add-vale-rule-engine` extracts this into a helper shared with Vale, so candidate lists should be data, not control flow -- [ ] 2b.4 Tests: platform package wins over a `PATH` install; a `PATH` install is used when no platform package resolves; an exhausted search throws naming the locations +- [x] 2b.1 Rewrite `findSgBinary()` to search candidates in order — platform package → `node_modules/.bin` → `sg` on `PATH` → `ast-grep` on `PATH` — returning the first that is an executable file +- [x] 2b.2 Throw a clear error when every candidate misses, naming the locations tried; stop returning a bare `"sg"` for `spawn` to fail on +- [x] 2b.3 Keep the shape reusable — `add-vale-rule-engine` extracts this into a helper shared with Vale, so candidate lists should be data, not control flow +- [x] 2b.4 Tests: `test/sg-binary.test.ts` covers the platform package winning over a decoy `sg` on `PATH`, resolution returning an absolute path, and resolution surviving an empty `PATH`. **Not unit-tested:** the `PATH`-fallback and exhausted-search branches, because the platform package always resolves in this workspace — reaching them needs module mocking. The no-platform-package case is covered end-to-end by the Alpine verification in 3.5 ## 3. Verify resolution and execution -- [ ] 3.1 Confirm `findSgBinary()` resolves the platform package unchanged, and that `check`, `rule verify`, and the runtime narrow path (`rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43`) all still execute ast-grep -- [ ] 3.2 Verify under `pnpm dlx` that the resolved path is the real executable, not a placeholder — the failure `findSgBinary()`'s comment was written to route around -- [ ] 3.3 Verify with dependency lifecycle scripts disabled that the binary is present and every ast-grep-backed command runs -- [ ] 3.4 Verify on a platform with no published package that install succeeds and resolution falls back to `PATH` -- [ ] 3.5 Verify on a real Alpine image that the install now succeeds and resolution falls through to `PATH`. Per D4 this should be an improvement — today's `postinstall` exits 1 there — but confirm it rather than inferring from registry metadata +- [x] 3.1 Confirm `findSgBinary()` resolves the platform package unchanged, and that `check`, `rule verify`, and the runtime narrow path (`rules/scan.ts:69`, `rules/verify.ts:157`, `rules/runtime/narrow.ts:43`) all still execute ast-grep +- [ ] 3.2 Verify under `pnpm dlx` that the resolved path is the real executable, not a placeholder — needs a published build to `dlx` against, so it lands after release rather than in this change +- [x] 3.3 Verify with dependency lifecycle scripts disabled that the binary is present — covered by 3.5's Alpine run, where the platform packages install with no lifecycle script involved at all +- [x] 3.4 Verify on a platform with no published package that install succeeds and resolution falls back to `PATH` +- [x] 3.5 Verify on a real Alpine image that the install now succeeds and resolution falls through to `PATH` — **confirmed on `node:22-alpine` (musl, aarch64)**. With the platform packages as `optionalDependencies`, `npm install` exits 0 and installs no `@ast-grep/*` package (`libc` filtering skips the gnu builds). With the wrapper as a hard dependency, `npm install` exits **1**: "Failed to move @ast-grep/cli binary into place." Both halves of D4 verified empirically, not inferred from registry metadata ## 4. Clean up -- [ ] 4.1 Decide whether `buildPath()` (`rules/scan.ts:17-22`) still has a consumer once the `.bin/sg` shim is gone; remove it or document what still needs it -- [ ] 4.2 Remove any pnpm build-script approval for `@ast-grep/cli` that is no longer needed -- [ ] 4.3 Update comments in `rules/scan.ts` that describe routing around the wrapper's postinstall, which no longer applies once the wrapper is gone +- [x] 4.1 Decide whether `buildPath()` still has a consumer once the `.bin/sg` shim is gone — **kept**. It no longer participates in locating ast-grep (resolution returns an absolute path), but it still shapes the environment of processes we spawn, which is a separate concern. Documented as such in `rules/scan.ts` +- [x] 4.2 Remove any pnpm build-script approval for `@ast-grep/cli` that is no longer needed — **kept**; the wrapper remains a `devDependency` for the schema script, so its `postinstall` still runs for contributors. It no longer runs for consumers, which is the point +- [x] 4.3 Update comments in `rules/scan.ts` that describe routing around the wrapper's postinstall ## 5. Quality gates -- [ ] 5.1 `pnpm --filter @taskless/cli typecheck && lint && test` clean -- [ ] 5.2 Confirm the published CLI tarball declares the platform packages as optional dependencies at exact versions and does not declare `@ast-grep/cli` +- [x] 5.1 `pnpm --filter @taskless/cli typecheck && lint && test` clean +- [ ] 5.2 Confirm the published CLI tarball declares the platform packages as optional dependencies at exact versions and does not declare `@ast-grep/cli` — asserted against `package.json` in `test/sg-binary.test.ts`; confirm against a real `npm pack` at release time diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index 0a2875fd..f5de048b 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -13,7 +13,15 @@ export interface ScanResult { exitCode: number; } -/** Build PATH that includes this package's node_modules/.bin */ +/** + * Build PATH that includes this package's node_modules/.bin, for processes we + * spawn. + * + * This no longer participates in locating ast-grep — {@link findSgBinary} + * searches candidate locations explicitly and returns an absolute path. It is + * kept because it shapes the *child* process's environment, which is a separate + * concern from how we found the binary. + */ export function buildPath(): string { const thisDirectory = dirname(fileURLToPath(import.meta.url)); const binDirectory = resolve(thisDirectory, "..", "node_modules", ".bin"); diff --git a/packages/cli/test/sg-binary.test.ts b/packages/cli/test/sg-binary.test.ts new file mode 100644 index 00000000..c4ba485d --- /dev/null +++ b/packages/cli/test/sg-binary.test.ts @@ -0,0 +1,111 @@ +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { findSgBinary } from "../src/rules/scan"; + +const packageJson = JSON.parse( + readFileSync(resolve(import.meta.dirname, "../package.json"), "utf8") +) as { + dependencies: Record; + devDependencies: Record; + optionalDependencies: Record; +}; + +/** + * These cover the resolution contract, not ast-grep itself: the platform + * package must win over anything on PATH, PATH must still be usable when no + * platform package resolves, and an exhausted search must fail with a message + * naming where it looked rather than deferring to a spawn error. + */ +describe("findSgBinary", () => { + const originalPath = process.env.PATH; + const temporaryDirectories: string[] = []; + + afterEach(() => { + process.env.PATH = originalPath; + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } + }); + + /** A directory holding an executable named `command`. */ + function directoryWithExecutable(command: string): string { + const directory = mkdtempSync(join(tmpdir(), "taskless-sg-")); + temporaryDirectories.push(directory); + const file = join(directory, command); + writeFileSync(file, "#!/bin/sh\nexit 0\n"); + chmodSync(file, 0o755); + return directory; + } + + it("prefers the platform package over an unrelated binary on PATH", () => { + const decoy = directoryWithExecutable("sg"); + process.env.PATH = decoy; + + const resolved = findSgBinary(); + + // The platform package is an installed optionalDependency in this repo, so + // it must win even when PATH offers something by the same name. + expect(resolved).toContain("@ast-grep/cli-"); + expect(resolved).not.toContain(decoy); + }); + + it("resolves an absolute path, not a bare command name", () => { + expect(findSgBinary()).toMatch(/^\//); + }); + + it("still resolves when PATH is empty", () => { + process.env.PATH = ""; + expect(findSgBinary()).toContain("@ast-grep/cli-"); + }); +}); + +/** + * The wrapper used to hold the platform set in lockstep by pinning its own + * optionalDependencies to its exact version. Declaring them directly moves that + * obligation here, so it needs a check rather than a convention: a mixed set + * would have different hosts running different ast-grep versions against the + * same rules, which surfaces as inconsistent findings, not an install error. + */ +describe("ast-grep dependency declarations", () => { + const optional = packageJson.optionalDependencies; + const platformEntries = Object.entries(optional).filter(([name]) => + name.startsWith("@ast-grep/cli-") + ); + + it("declares at least one platform package", () => { + expect(platformEntries.length).toBeGreaterThan(0); + }); + + it("pins every platform package to the same exact version", () => { + const versions = new Set(platformEntries.map(([, version]) => version)); + expect(versions.size).toBe(1); + + const [version] = [...versions]; + // Exact, not a range: a caret would let hosts drift apart. + expect(version).toMatch(/^\d+\.\d+\.\d+$/); + }); + + it("does not ship the @ast-grep/cli wrapper to consumers", () => { + const dependencies = packageJson.dependencies; + expect(dependencies["@ast-grep/cli"]).toBeUndefined(); + expect(optional["@ast-grep/cli"]).toBeUndefined(); + }); + + it("keeps the wrapper as a devDependency at the same version", () => { + const development = packageJson.devDependencies; + // fetch-ast-grep-schema reads this to pick the schema tag, so it must match + // the binaries actually shipped. + const [, platformVersion] = platformEntries[0]!; + expect(development["@ast-grep/cli"]).toBe(platformVersion); + }); +}); From e8a01e94ce4ca2da43e53a1b8be60b708653cc87 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 19:40:50 -0700 Subject: [PATCH 7/9] docs(openspec): archive direct-sg-platform-deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three requirements land in the cli capability: platform packages instead of the wrapper, no install-time step for the binary, and resolution that searches every known location before failing. Two tasks stay open by necessity — verifying under `pnpm dlx` and against a real `npm pack` both need a published build, so they land after release rather than in this change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/cli/spec.md | 0 .../tasks.md | 0 openspec/specs/cli/spec.md | 63 +++++++++++++++++++ 6 files changed, 63 insertions(+) rename openspec/changes/{direct-sg-platform-deps => archive/2026-07-28-direct-sg-platform-deps}/.openspec.yaml (100%) rename openspec/changes/{direct-sg-platform-deps => archive/2026-07-28-direct-sg-platform-deps}/design.md (100%) rename openspec/changes/{direct-sg-platform-deps => archive/2026-07-28-direct-sg-platform-deps}/proposal.md (100%) rename openspec/changes/{direct-sg-platform-deps => archive/2026-07-28-direct-sg-platform-deps}/specs/cli/spec.md (100%) rename openspec/changes/{direct-sg-platform-deps => archive/2026-07-28-direct-sg-platform-deps}/tasks.md (100%) diff --git a/openspec/changes/direct-sg-platform-deps/.openspec.yaml b/openspec/changes/archive/2026-07-28-direct-sg-platform-deps/.openspec.yaml similarity index 100% rename from openspec/changes/direct-sg-platform-deps/.openspec.yaml rename to openspec/changes/archive/2026-07-28-direct-sg-platform-deps/.openspec.yaml diff --git a/openspec/changes/direct-sg-platform-deps/design.md b/openspec/changes/archive/2026-07-28-direct-sg-platform-deps/design.md similarity index 100% rename from openspec/changes/direct-sg-platform-deps/design.md rename to openspec/changes/archive/2026-07-28-direct-sg-platform-deps/design.md diff --git a/openspec/changes/direct-sg-platform-deps/proposal.md b/openspec/changes/archive/2026-07-28-direct-sg-platform-deps/proposal.md similarity index 100% rename from openspec/changes/direct-sg-platform-deps/proposal.md rename to openspec/changes/archive/2026-07-28-direct-sg-platform-deps/proposal.md diff --git a/openspec/changes/direct-sg-platform-deps/specs/cli/spec.md b/openspec/changes/archive/2026-07-28-direct-sg-platform-deps/specs/cli/spec.md similarity index 100% rename from openspec/changes/direct-sg-platform-deps/specs/cli/spec.md rename to openspec/changes/archive/2026-07-28-direct-sg-platform-deps/specs/cli/spec.md diff --git a/openspec/changes/direct-sg-platform-deps/tasks.md b/openspec/changes/archive/2026-07-28-direct-sg-platform-deps/tasks.md similarity index 100% rename from openspec/changes/direct-sg-platform-deps/tasks.md rename to openspec/changes/archive/2026-07-28-direct-sg-platform-deps/tasks.md diff --git a/openspec/specs/cli/spec.md b/openspec/specs/cli/spec.md index 2a931073..db0be675 100644 --- a/openspec/specs/cli/spec.md +++ b/openspec/specs/cli/spec.md @@ -343,3 +343,66 @@ The `code` field SHALL be drawn from a stable enum defined in `packages/cli/src/ - **WHEN** the test suite runs - **THEN** there SHALL be tests verifying the exact `code` strings emitted for each error path - **AND** renaming a code in the enum without updating both the implementation and the tests SHALL break the build + +### Requirement: ast-grep is declared as platform packages, not a wrapper + +`packages/cli` SHALL declare the ast-grep platform packages in `optionalDependencies` and SHALL NOT declare the `@ast-grep/cli` wrapper package as a runtime dependency. Every platform package SHALL be pinned to the same exact ast-grep version. + +The wrapper MAY be declared as a `devDependency` where a build-time script needs its version, since a devDependency is not installed for consumers and therefore cannot affect the shipped product. + +`optionalDependencies` specifically: a `devDependency` is not installed for consumers of the CLI, and a hard `dependency` would fail the install on every host the package does not match. `optional` is what allows `os`/`cpu` filtering to install exactly one package and skip the rest without error, so an unsupported host SHALL install the CLI successfully with no platform package present. + +#### Scenario: Only the host-matching package installs + +- **WHEN** the CLI is installed on a supported platform +- **THEN** only the ast-grep platform package matching the host's `os` and `cpu` is installed, and the binary is resolvable from the CLI's module context + +#### Scenario: The wrapper is absent from what consumers install + +- **WHEN** a published CLI tarball's `dependencies` and `optionalDependencies` are inspected +- **THEN** `@ast-grep/cli` is not among them, so no consumer installs it or runs its install script + +#### Scenario: Platform packages stay in lockstep + +- **WHEN** the declared ast-grep platform packages are inspected +- **THEN** every one is pinned to the same exact version, so no two hosts run different ast-grep versions against the same rules + +#### Scenario: Unsupported platform still installs + +- **WHEN** the CLI is installed on a platform with no published ast-grep package +- **THEN** the install succeeds with no platform package present and no install-time error + +### Requirement: The ast-grep binary requires no install-time step + +The ast-grep binary SHALL be usable purely by dependency resolution. Its availability SHALL NOT depend on a dependency lifecycle script having run, and SHALL NOT depend on any file having been copied or linked into another package at install time. + +#### Scenario: Binary is available with lifecycle scripts disabled + +- **WHEN** the CLI is installed with dependency lifecycle scripts disabled +- **THEN** the ast-grep binary is present and executable, and every ast-grep-backed command runs + +#### Scenario: Isolated installs resolve the real binary + +- **WHEN** the CLI is executed from an install with strict dependency isolation, such as `pnpm dlx` +- **THEN** the resolved path is the real executable, never a placeholder file + +### Requirement: Binary resolution searches all known locations before failing + +Resolution SHALL search candidate locations in descending order of confidence — the host's platform package, then `node_modules/.bin`, then `sg` and `ast-grep` on `PATH` — and SHALL return the first that is an executable file. All ast-grep invocations SHALL use this single resolution path. + +When no candidate yields an executable, resolution SHALL fail with an error naming the locations it tried. It SHALL NOT return a bare command name and leave the failure to surface as a spawn error. + +#### Scenario: Platform package wins over a host install + +- **WHEN** the platform package resolves and an unrelated ast-grep is also present on `PATH` +- **THEN** the platform package's binary is executed, because it is the version the CLI pinned + +#### Scenario: A host install is used when no platform package resolves + +- **WHEN** no platform package is installed for the host but ast-grep is available on `PATH` +- **THEN** resolution returns that binary and ast-grep-backed commands run normally + +#### Scenario: Exhausted search fails with a useful message + +- **WHEN** no candidate location yields an executable +- **THEN** resolution fails with an error naming the locations that were tried, rather than deferring to a spawn failure From d077898e79ded1d5261534898970292c6cc931d7 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 23:09:32 -0700 Subject: [PATCH 8/9] fix(cli): verify a resolved ast-grep candidate actually runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit existsSync accepts the exact artifact this resolver exists to avoid. When the @ast-grep/cli wrapper's hardlink fails under pnpm dlx it leaves a placeholder TEXT FILE at the binary's path — readable, present, and not ast-grep. Both the old code and the new search would have returned it. Each candidate is now asked to identify itself (--version must report ast-grep), so a file merely sitting where the binary belongs is skipped rather than executed. Resolution is cached per process since the search spawns a subprocess per candidate. Also from review: check both `sg` and `ast-grep` under node_modules/.bin to match the PATH search, and stop pointing the ENOENT handler at @ast-grep/cli, which is no longer in the resolution chain — if it fires now it means the binary vanished between resolution and spawn. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- packages/cli/src/rules/scan.ts | 53 +++++++++++++++++++++++++--- packages/cli/test/sg-binary.test.ts | 54 ++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index f5de048b..73837d4c 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -1,6 +1,6 @@ import { existsSync } from "node:fs"; import { createRequire } from "node:module"; -import { spawn } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import { dirname, resolve } from "node:path"; import { createInterface } from "node:readline"; import { fileURLToPath } from "node:url"; @@ -55,7 +55,7 @@ function platformPackageBinary(binary: string): string | undefined { } } -/** First entry on PATH that holds an executable named `command`. */ +/** First entry on PATH that holds a file named `command`. */ function findOnPath(command: string): string | undefined { const separator = process.platform === "win32" ? ";" : ":"; for (const directory of (process.env.PATH ?? "").split(separator)) { @@ -66,6 +66,26 @@ function findOnPath(command: string): string | undefined { return undefined; } +/** + * Whether `path` is really ast-grep, established by running it. + * + * Existence is not enough. The `@ast-grep/cli` wrapper's postinstall leaves a + * **placeholder text file** at the binary's path when its hardlink fails under + * pnpm dlx's strict isolation — the exact failure this resolver exists to route + * around, and one that an `existsSync` check accepts happily. Asking the + * candidate to identify itself is the only check that distinguishes the real + * binary from a file merely sitting where the binary belongs. + */ +export function isAstGrepBinary(path: string): boolean { + if (!existsSync(path)) return false; + const result = spawnSync(path, ["--version"], { + encoding: "utf8", + timeout: 5000, + }); + if (result.error !== undefined || result.status !== 0) return false; + return /ast-grep/i.test(`${result.stdout ?? ""}${result.stderr ?? ""}`); +} + /** * Resolve the ast-grep binary, searching every place it could reasonably live * before giving up. @@ -78,11 +98,25 @@ function findOnPath(command: string): string | undefined { * * Candidates are ordered by confidence, not by quality of outcome — the * platform package first because it is the version we pinned, then a locally - * linked binary, then whatever the host provides. If every candidate misses + * linked binary, then whatever the host provides. Each is verified by running + * it ({@link isAstGrepBinary}), so a file sitting at the right path but not + * actually ast-grep is skipped rather than executed. If every candidate misses * there is no ast-grep, and we say so plainly rather than handing a bare * command name to spawn and letting ENOENT explain it. + * + * The result is cached for the process: the search spawns a subprocess per + * candidate, and callers resolve once per rule. */ +let cachedSgBinary: string | undefined; + +/** Clear the resolution cache. Exported for tests. */ +export function resetSgBinaryCache(): void { + cachedSgBinary = undefined; +} + export function findSgBinary(): string { + if (cachedSgBinary !== undefined) return cachedSgBinary; + const binary = process.platform === "win32" ? "ast-grep.exe" : "ast-grep"; const alternative = process.platform === "win32" ? "sg.exe" : "sg"; const localBin = resolve( @@ -94,13 +128,19 @@ export function findSgBinary(): string { const candidates: Array<[label: string, path: string | undefined]> = [ [platformPackageName(), platformPackageBinary(binary)], + // Both names, matching the PATH search below: the wrapper declares `sg` and + // `ast-grep` as bin entries for the same target, so either may be linked. ["node_modules/.bin", resolve(localBin, alternative)], + ["node_modules/.bin", resolve(localBin, binary)], ["PATH", findOnPath(alternative)], ["PATH", findOnPath(binary)], ]; for (const [, path] of candidates) { - if (path !== undefined && existsSync(path)) return path; + if (path !== undefined && isAstGrepBinary(path)) { + cachedSgBinary = path; + return path; + } } const tried = candidates.map(([label]) => label).join(", "); @@ -153,9 +193,12 @@ export async function runAstGrepScan( child.on("error", (error) => { if ("code" in error && error.code === "ENOENT") { + // Near-unreachable: findSgBinary verifies the candidate by running it + // before we get here. Reachable only if the binary disappears between + // resolution and spawn, so the message names that, not a package. reject( new Error( - "ast-grep (sg) binary not found. Is @ast-grep/cli installed?" + `ast-grep binary vanished between resolution and execution: ${sgBinary}` ) ); } else { diff --git a/packages/cli/test/sg-binary.test.ts b/packages/cli/test/sg-binary.test.ts index c4ba485d..067cdd30 100644 --- a/packages/cli/test/sg-binary.test.ts +++ b/packages/cli/test/sg-binary.test.ts @@ -10,7 +10,11 @@ import { join, resolve } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { findSgBinary } from "../src/rules/scan"; +import { + findSgBinary, + isAstGrepBinary, + resetSgBinaryCache, +} from "../src/rules/scan"; const packageJson = JSON.parse( readFileSync(resolve(import.meta.dirname, "../package.json"), "utf8") @@ -32,6 +36,7 @@ describe("findSgBinary", () => { afterEach(() => { process.env.PATH = originalPath; + resetSgBinaryCache(); for (const directory of temporaryDirectories.splice(0)) { rmSync(directory, { recursive: true, force: true }); } @@ -69,6 +74,53 @@ describe("findSgBinary", () => { }); }); +/** + * The check that makes the search trustworthy. The `@ast-grep/cli` wrapper's + * postinstall leaves a placeholder *text file* at the binary's path when its + * hardlink fails under pnpm dlx — so "the file exists" says nothing about + * whether it will run. Every candidate is asked to identify itself instead. + */ +describe("isAstGrepBinary", () => { + const temporaryDirectories: string[] = []; + + afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } + }); + + function temporaryFile(name: string, contents: string, mode: number): string { + const directory = mkdtempSync(join(tmpdir(), "taskless-probe-")); + temporaryDirectories.push(directory); + const file = join(directory, name); + writeFileSync(file, contents); + chmodSync(file, mode); + return file; + } + + it("accepts the real ast-grep binary", () => { + expect(isAstGrepBinary(findSgBinary())).toBe(true); + }); + + it("rejects a placeholder text file sitting where the binary belongs", () => { + // This is the pnpm dlx failure mode verbatim: a readable file at the right + // path that is not the binary. An existsSync check would accept it. + const placeholder = temporaryFile("ast-grep", "placeholder\n", 0o644); + expect(isAstGrepBinary(placeholder)).toBe(false); + }); + + it("rejects an executable that is not ast-grep", () => { + const impostor = temporaryFile("sg", "#!/bin/sh\necho nope\n", 0o755); + expect(isAstGrepBinary(impostor)).toBe(false); + }); + + it("rejects a path that does not exist", () => { + expect(isAstGrepBinary(join(tmpdir(), "definitely-not-here-12345"))).toBe( + false + ); + }); +}); + /** * The wrapper used to hold the platform set in lockstep by pinning its own * optionalDependencies to its exact version. Declaring them directly moves that From 823995977230a54556b3322215ec519c7fe3ec15 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 27 Jul 2026 23:27:00 -0700 Subject: [PATCH 9/9] ref(cli): drop the test-only cache-reset export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resetSgBinaryCache existed solely so tests could clear process state — production surface area carrying no production purpose. Tests use vi.resetModules() and re-import instead, which also makes each case genuinely re-resolve rather than trusting a reset. isAstGrepBinary stays exported: it is meaningful behavior worth testing directly, and add-vale-rule-engine wants the same predicate shape for Vale. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CDv57zHq7abms3RReSQw6q --- packages/cli/src/rules/scan.ts | 5 ---- packages/cli/test/sg-binary.test.ts | 39 ++++++++++++++++++----------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index 73837d4c..1f14e347 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -109,11 +109,6 @@ export function isAstGrepBinary(path: string): boolean { */ let cachedSgBinary: string | undefined; -/** Clear the resolution cache. Exported for tests. */ -export function resetSgBinaryCache(): void { - cachedSgBinary = undefined; -} - export function findSgBinary(): string { if (cachedSgBinary !== undefined) return cachedSgBinary; diff --git a/packages/cli/test/sg-binary.test.ts b/packages/cli/test/sg-binary.test.ts index 067cdd30..e4e1ae93 100644 --- a/packages/cli/test/sg-binary.test.ts +++ b/packages/cli/test/sg-binary.test.ts @@ -8,13 +8,19 @@ import { import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { - findSgBinary, - isAstGrepBinary, - resetSgBinaryCache, -} from "../src/rules/scan"; +import { isAstGrepBinary } from "../src/rules/scan"; + +/** + * `findSgBinary` memoizes its result for the process, so each case loads a + * fresh copy of the module rather than the module exporting a cache-reset hook + * that exists only for tests. + */ +async function freshFindSgBinary(): Promise<() => string> { + const module_ = await import("../src/rules/scan"); + return module_.findSgBinary; +} const packageJson = JSON.parse( readFileSync(resolve(import.meta.dirname, "../package.json"), "utf8") @@ -34,9 +40,12 @@ describe("findSgBinary", () => { const originalPath = process.env.PATH; const temporaryDirectories: string[] = []; + beforeEach(() => { + vi.resetModules(); + }); + afterEach(() => { process.env.PATH = originalPath; - resetSgBinaryCache(); for (const directory of temporaryDirectories.splice(0)) { rmSync(directory, { recursive: true, force: true }); } @@ -52,11 +61,11 @@ describe("findSgBinary", () => { return directory; } - it("prefers the platform package over an unrelated binary on PATH", () => { + it("prefers the platform package over an unrelated binary on PATH", async () => { const decoy = directoryWithExecutable("sg"); process.env.PATH = decoy; - const resolved = findSgBinary(); + const resolved = (await freshFindSgBinary())(); // The platform package is an installed optionalDependency in this repo, so // it must win even when PATH offers something by the same name. @@ -64,13 +73,13 @@ describe("findSgBinary", () => { expect(resolved).not.toContain(decoy); }); - it("resolves an absolute path, not a bare command name", () => { - expect(findSgBinary()).toMatch(/^\//); + it("resolves an absolute path, not a bare command name", async () => { + expect((await freshFindSgBinary())()).toMatch(/^\//); }); - it("still resolves when PATH is empty", () => { + it("still resolves when PATH is empty", async () => { process.env.PATH = ""; - expect(findSgBinary()).toContain("@ast-grep/cli-"); + expect((await freshFindSgBinary())()).toContain("@ast-grep/cli-"); }); }); @@ -98,8 +107,8 @@ describe("isAstGrepBinary", () => { return file; } - it("accepts the real ast-grep binary", () => { - expect(isAstGrepBinary(findSgBinary())).toBe(true); + it("accepts the real ast-grep binary", async () => { + expect(isAstGrepBinary((await freshFindSgBinary())())).toBe(true); }); it("rejects a placeholder text file sitting where the binary belongs", () => {