From 44b69078dd6369484ceac4f486a4e70fca9d975c Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 2 Sep 2026 23:07:05 -0700 Subject: [PATCH] feat(cli): publish the engine resolver as @taskless/cli/node/runtimes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rule's behaviour is a property of the engine version that executes it, not of the version it was written against. A generator sandbox pinned @ast-grep/cli 0.41.1 by hand while this package pins 0.45.2, so a capture that narrows differently across those four minors verifies clean there and matches nothing here: a rule that passes its gate and never fires, with both sides reporting success. The new entry exports resolvePlatformBinary along with AST_GREP_BINARY and VALE_BINARY, so a consumer installs the pinned CLI, lets its optional dependencies bring the platform binaries, and asks this resolver where they are. The engine version becomes a consequence of the CLI pin rather than a second fact tracked alongside it. AST_GREP_BINARY moves to rules/ast-grep-binary.ts so a consumer wanting a path does not load a scanner to get one; Vale's spec was already a leaf. The build's entry classification is now exhaustive. LIBRARY_ENTRIES is opt-in, so an export joining no list was checked by nothing and forgetting was indistinguishable from a deliberate exemption. Every published export must now name itself graph-asserted or deliberately host-bound, and the build fails on one that does neither. A host-bound entry is excused from the host-capability rule only; it still may not reach the command layer. That last rule also had to be strengthened. It compared against the bin chunk's file name, and importing src/index.ts from a library entry makes rollup hoist the CLI into a shared chunk and leave dist/index.js a 50-byte re-export facade the library entry never imports — the check reported nothing while the entry carried the whole command layer. It now compares module ids, which chunking cannot move. resolvePlatformBinary's contract is unchanged: a miss is { path: undefined, tried } rather than a throw, because check must lose one engine instead of the whole run. The entry documents that so a verifier can fail closed on it, which a sandbox must, since an optional dependency that did not install resolves nothing for the same reason a laptop does. --- .changeset/node-runtimes-export.md | 32 ++++ .../node-runtimes-export/.openspec.yaml | 2 + .../changes/node-runtimes-export/proposal.md | 74 ++++++++ .../specs/cli-runtime-binary-export/spec.md | 105 +++++++++++ .../changes/node-runtimes-export/tasks.md | 65 +++++++ packages/cli/package.json | 4 + packages/cli/src/node/runtimes/index.ts | 93 +++++++++ packages/cli/src/rules/ast-grep-binary.ts | 33 ++++ packages/cli/src/rules/scan.ts | 24 +-- .../cli/test/ast-grep-vendor-contract.test.ts | 2 +- packages/cli/test/platform-binary.test.ts | 2 +- packages/cli/tsconfig.public.json | 11 +- packages/cli/vite.config.ts | 176 +++++++++++++++--- 13 files changed, 567 insertions(+), 56 deletions(-) create mode 100644 .changeset/node-runtimes-export.md create mode 100644 openspec/changes/node-runtimes-export/.openspec.yaml create mode 100644 openspec/changes/node-runtimes-export/proposal.md create mode 100644 openspec/changes/node-runtimes-export/specs/cli-runtime-binary-export/spec.md create mode 100644 openspec/changes/node-runtimes-export/tasks.md create mode 100644 packages/cli/src/node/runtimes/index.ts create mode 100644 packages/cli/src/rules/ast-grep-binary.ts diff --git a/.changeset/node-runtimes-export.md b/.changeset/node-runtimes-export.md new file mode 100644 index 00000000..86d14acb --- /dev/null +++ b/.changeset/node-runtimes-export.md @@ -0,0 +1,32 @@ +--- +"@taskless/cli": patch +--- + +`@taskless/cli/node/runtimes` publishes the engine binary resolver. + +It exports `resolvePlatformBinary` along with the `AST_GREP_BINARY` and +`VALE_BINARY` specs, so a service that verifies a generated rule can locate the +same ast-grep and the same Vale that `taskless check` executes, instead of +pinning the engine version by hand in a second place. + +The second pin is the problem it removes. A generator sandbox pinned +`@ast-grep/cli` at 0.41.1 while this package pinned 0.45.2, and a capture that +narrows differently across those four minors verifies clean in the sandbox and +matches nothing on the machine that runs it — a rule that passes its gate and +never fires, with both sides reporting success. Installing this package and +asking it where the binary is makes the engine version a consequence of the CLI +pin rather than a fact tracked alongside it. + +`node/` is in the specifier because resolution spawns each candidate to make it +identify itself, reads the filesystem, and consults `PATH`. `/prompts` and +`/layout` remain host-free and a Worker still imports them; the build now +requires every published export to declare which of the two it is, so this +entry's exemption is written down rather than inferred from a missing list +entry. + +Resolution still reports a miss as `{ path: undefined, tried }` rather than +throwing. That is what lets `check` lose one engine instead of the whole run, +and it is why a verifier should fail closed on `undefined` itself: a sandbox +whose optional dependency did not install resolves nothing for the same reason a +laptop does, and skipping verification there reports a clean generation for a +rule nothing checked. diff --git a/openspec/changes/node-runtimes-export/.openspec.yaml b/openspec/changes/node-runtimes-export/.openspec.yaml new file mode 100644 index 00000000..032461ff --- /dev/null +++ b/openspec/changes/node-runtimes-export/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-02 diff --git a/openspec/changes/node-runtimes-export/proposal.md b/openspec/changes/node-runtimes-export/proposal.md new file mode 100644 index 00000000..c74bfad4 --- /dev/null +++ b/openspec/changes/node-runtimes-export/proposal.md @@ -0,0 +1,74 @@ +## Why + +A rule's behaviour is a property of the engine version that executes it, not of +the version it was written against. The Taskless Cloud generator verifies a +generated runtime rule in a sandbox that pins `@ast-grep/cli` **0.41.1** by hand +in a Dockerfile, while `taskless check` on a developer's machine runs this +package's pinned **0.45.2**. Four minors apart. A capture that narrows +differently across them verifies clean on their side and matches nothing on +ours: a rule that passes its gate and never fires, with both sides reporting +success. That is live right now. + +The Dockerfile's own comment says the pins exist so the check validates "with +the same ast-grep the rule was generated against". Generation never runs +ast-grep. The version that matters is the one that **executes** the rule, and +that is this CLI's. + +There is no way to close that gap by pinning harder. Two teams tracking the same +engine version in two places drift, and the drift is silent in exactly the +direction that hurts. What removes it is making the engine version a consequence +of the CLI version rather than a parallel fact: the consumer installs the pinned +CLI, its optional dependencies bring the platform binaries, and it asks our +resolver where they are. The versions then move together because they are the +same pin. + +## What Changes + +- A new published entry, `@taskless/cli/node/runtimes`, exporting the CLI's own + `resolvePlatformBinary`, the `AST_GREP_BINARY` and `VALE_BINARY` specs, and + the types a consumer needs to hold the result. +- `AST_GREP_BINARY` moves from `rules/scan.ts` to `rules/ast-grep-binary.ts`, so + a consumer that wants a path does not load a scanner to get one. Vale's spec + already sits in a leaf module. +- The build's entry classification becomes exhaustive. `LIBRARY_ENTRIES` is + opt-in, so before this change an export that joined no list was checked by + nothing, and forgetting was indistinguishable from a deliberate exemption. + Every published export must now name itself as either graph-asserted or + deliberately host-bound, and the build fails on one that does neither. +- The CLI-entry half of the graph guard now asks about the bin entry's **module + id** rather than the bin **chunk's** file name, and applies to host-bound + entries too. + +**No change to `resolvePlatformBinary`'s contract.** It returns +`{ path: undefined, tried }` rather than throwing, because `check` must lose one +engine rather than the whole run. A verifier wants the opposite and will fail +closed on `undefined` itself; the entry documents that the return is a value +precisely so each side can decide. + +## Capabilities + +### New Capabilities + +- `cli-runtime-binary-export`: the published entry that resolves the engine + binaries this CLI executes, and the build-time classification that keeps every + published entry's host access declared rather than assumed. + +## Impact + +- New public surface on `@taskless/cli`. Additive; nothing existing changes + shape. `patch` under pre-1.0 guidance, where `minor` is reserved for breaking + changes. +- `@taskless/cli/prompts` and `@taskless/cli/layout` are unaffected. Their graphs + stay host-free and the build still refuses to emit them otherwise — the new + entry is excused from that rule by name, not by absence. +- Consumers gain a reason to depend on the CLI package from a service, so the + entry's Node requirement is stated in its import path (`node/`) where a reader + meets it before a build error does. + +## Delivery shape + +**Single PR.** One entry module, one spec relocation, one build guard, and the +spec delta that records why the exemption is legitimate. The guard is the half +that makes the exemption reviewable, so splitting it out would ship a new +unchecked published surface for as long as the two halves were apart — which is +the exact failure the guard exists to prevent. diff --git a/openspec/changes/node-runtimes-export/specs/cli-runtime-binary-export/spec.md b/openspec/changes/node-runtimes-export/specs/cli-runtime-binary-export/spec.md new file mode 100644 index 00000000..e48a86b3 --- /dev/null +++ b/openspec/changes/node-runtimes-export/specs/cli-runtime-binary-export/spec.md @@ -0,0 +1,105 @@ +## ADDED Requirements + +### Requirement: The engine binary resolver is published + +The CLI SHALL publish, as an importable entry, the resolver it uses to locate the +engine binaries it executes, together with the per-engine package specs that +resolver takes. + +A consumer that verifies a generated rule has to execute it, and a rule's +behaviour is a property of the engine version that runs it. Tracking that version +separately from the CLI version means two pins in two repositories, which drift +silently: a `0.41.1` sandbox verified a capture clean that a `0.45.2` developer +machine then matched nothing with. Installing the CLI and asking it where the +binary is makes the engine version a consequence of the CLI pin rather than a +parallel fact. + +#### Scenario: A consumer resolves the same binary the CLI executes + +- **WHEN** a consumer imports the published runtimes entry and resolves an engine + spec +- **THEN** it SHALL receive the absolute path of a binary that identified itself + as that engine +- **AND** the resolver and the specs SHALL be the same modules the CLI itself + dispatches on, not a copy kept in step + +#### Scenario: The pinned platform build is preferred + +- **WHEN** the engine's pinned per-platform package is installed +- **THEN** resolution SHALL try it before a linked binary or one on `PATH` + +### Requirement: A binary that cannot be resolved is reported as a value + +Resolution SHALL report a miss by returning an absent path alongside the list of +locations searched, and SHALL NOT throw. + +The two sides want opposite things from a miss, and neither can be imposed on the +other. `taskless check` runs several engines: a host without Vale installed must +lose the Vale rules rather than the whole run. A verifier must fail closed, +because a sandbox whose optional dependency did not install resolves nothing for +the same reason a laptop does, and skipping verification there reports a clean +generation for a rule nothing checked. Encoding the miss as a value is what lets +each caller choose; the entry SHALL document that choice rather than leaving it +to be inferred. + +#### Scenario: Nothing resolves + +- **WHEN** no candidate location holds a binary that identifies itself as the + engine +- **THEN** resolution SHALL return an absent path +- **AND** it SHALL return the locations it searched, in order +- **AND** it SHALL NOT throw + +#### Scenario: A file exists but is not the binary + +- **WHEN** a candidate path exists but does not identify itself as the engine +- **THEN** resolution SHALL reject that candidate and continue searching + +### Requirement: A host-bound entry declares its host requirement in its path + +An entry that requires Node SHALL carry that requirement in its published +specifier rather than only in documentation. + +The published surface is what a consumer reads first. A specifier that names the +host states the constraint before a bundler error has to, and it distinguishes +this entry from the host-free ones beside it, which a Worker imports today. + +#### Scenario: The specifier names the host + +- **WHEN** a consumer reads the import path of an entry that spawns processes, + reads the filesystem, or consults `PATH` +- **THEN** the path SHALL identify the host it requires + +### Requirement: Every published export declares whether it is host-bound + +The build SHALL classify every entry named in the package's `exports` as either +graph-asserted host-free or deliberately host-bound, and SHALL fail when a +published export belongs to neither. + +The graph assertion is opt-in, so before this requirement an entry that joined no +list was checked by nothing, and forgetting to enroll a new surface was +indistinguishable from exempting it on purpose. Requiring a classification turns +the exemption into a claim a reviewer can disagree with. A host-bound entry is +excused only from the host-capability rule; it SHALL still be forbidden from +reaching the CLI's command layer, because needing a subprocess is not a reason to +also ship the command tree. + +#### Scenario: An unclassified published export fails the build + +- **WHEN** the package publishes an export whose entry appears in neither + classification +- **THEN** the build SHALL fail +- **AND** the artifact SHALL NOT be emitted + +#### Scenario: A published export names no built entry + +- **WHEN** a published export points at a file the build does not emit +- **THEN** the build SHALL fail + +#### Scenario: A host-bound entry reaching the command layer fails the build + +- **WHEN** a host-bound entry's graph reaches the CLI entry module +- **THEN** the build SHALL fail +- **AND** the check SHALL be made against the entry module rather than against + the emitted CLI chunk, since bundling may move the command layer into a shared + chunk the CLI entry itself merely re-exports diff --git a/openspec/changes/node-runtimes-export/tasks.md b/openspec/changes/node-runtimes-export/tasks.md new file mode 100644 index 00000000..50ba9f4c --- /dev/null +++ b/openspec/changes/node-runtimes-export/tasks.md @@ -0,0 +1,65 @@ +# Tasks + +Delivery shape: **single PR**. The entry and the guard that makes its exemption +reviewable land together; apart, the first ships an unchecked published surface. + +## 1. Give the ast-grep spec a leaf module + +- [x] 1.1 Move `AST_GREP_BINARY` from `src/rules/scan.ts` to + `src/rules/ast-grep-binary.ts`, and say in the comment why it lives alone: + `scan.ts` reaches `rules/engines.ts` and through it `node:fs/promises` and + the CLI's error type, none of which a consumer asking for a path needs. +- [x] 1.2 Point `scan.ts` and the two tests that name it at the new module. + `VALE_BINARY` needs no move; `rules/vale/binary.ts` already imports nothing + but the resolver. + +## 2. The entry + +- [x] 2.1 Add `src/node/runtimes/index.ts` re-exporting `resolvePlatformBinary`, + `platformPackageName`, `pathCommandName`, `findOnPath`, `isPlatformBinary`, + both specs, and `PlatformBinarySpec` / `PlatformBinaryResolution`. + `.js` extensions on the specifiers, as on the other published entries, or + `tsc` copies extensionless paths into the emitted `.d.ts` and a consumer on + `node16` resolution cannot follow them. +- [x] 2.2 Document that a miss is a value rather than an exception, and that a + verifier should fail closed on it while `check` deliberately does not. +- [x] 2.3 Add the `./node/runtimes` export and the vite entry. +- [x] 2.4 Add the entry to `tsconfig.public.json` so its types ship. + +## 3. Make the classification exhaustive + +- [x] 3.1 Derive the vite entry map from one `ENTRY_SOURCES` record, so the + classification check and the build read the same list. +- [x] 3.2 Add `HOST_BOUND_ENTRIES`, and `assertExportsClassified()`, which fails + the build when a published export names no built entry or appears in + neither classification. +- [x] 3.3 Apply the CLI-entry rule to host-bound entries too, and state at both + lists that absence is no longer an exemption. +- [x] 3.4 Verify it bites, three ways: an export naming an unbuilt entry; the new + entry removed from `HOST_BOUND_ENTRIES`, which is the exempt-by-omission + case; and the entry importing `src/index.ts`. + +## 4. The chunk-versus-module correction + +- [x] 4.1 The CLI-entry rule compared against the bin chunk's file name, and task + 3.4's third test found that evadable: importing `src/index.ts` from a + library entry made rollup hoist the CLI into a shared chunk and leave + `dist/index.js` a 50-byte re-export facade, which the library entry did not + import. The check reported nothing while the entry carried the whole + command layer. Compare against the bin entry's module id instead, which + chunking cannot move. + +## 5. Checks + +- [x] 5.1 `pnpm build`, `pnpm typecheck`, `pnpm lint`, + `pnpm --filter @taskless/cli exec vitest run`, `pnpm cli check`. +- [x] 5.2 Prove the built artifact, not the source: import + `@taskless/cli/node/runtimes` by its published specifier from outside the + package and resolve both engines, showing each path and that identity + verified. +- [x] 5.3 Changeset, `patch`. + +## 6. Close out + +- [ ] 6.1 Archive the change. Archiving is what promotes the delta into the + standing spec. diff --git a/packages/cli/package.json b/packages/cli/package.json index bf961096..685ba6d7 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -34,6 +34,10 @@ "./layout": { "types": "./dist/layout/index.d.ts", "import": "./dist/layout.js" + }, + "./node/runtimes": { + "types": "./dist/node/runtimes/index.d.ts", + "import": "./dist/node/runtimes.js" } }, "files": [ diff --git a/packages/cli/src/node/runtimes/index.ts b/packages/cli/src/node/runtimes/index.ts new file mode 100644 index 00000000..02a0028d --- /dev/null +++ b/packages/cli/src/node/runtimes/index.ts @@ -0,0 +1,93 @@ +/** + * Public entry for `@taskless/cli/node/runtimes`. + * + * Where the engine binaries this CLI executes actually live on this host, and + * how to find them. It is the CLI's own resolver and the CLI's own package + * specs — not a copy kept in step, the actual modules — so a consumer that + * installs this package resolves the same ast-grep and the same Vale that + * `taskless check` would run. + * + * ## Why this is published + * + * A service that verifies a generated rule has to execute it, and a rule's + * behaviour is a property of the engine version that runs it, not of the + * version it was written against. ast-grep in particular changes what a capture + * matches across minors: a rule verified under one and executed under another + * can verify clean and then match nothing, which is a rule that passes its gate + * and never fires. Nothing reports that, because both sides succeeded. + * + * The alternative to importing this is pinning the engine version by hand + * alongside the CLI version, in a second place, and keeping the two in step. + * They drift, and the drift is silent in exactly the direction that matters. + * Installing this package and asking it where the binary is makes the engine + * version a consequence of the CLI version rather than a parallel fact: the + * pinned platform packages arrive as this package's own optional dependencies, + * and they move when the pin moves. + * + * ## This entry requires Node, and says so in its path + * + * `node/` is in the specifier deliberately. Resolution spawns each candidate to + * make it identify itself, reads the filesystem, and consults `PATH`, so this + * graph reaches `node:child_process`, `node:fs`, `node:module`, `node:path` and + * `node:url`. It will not run on a Worker or in a browser, and the import path + * says that before a build error has to. + * + * This is the difference between this entry and `@taskless/cli/prompts` and + * `@taskless/cli/layout`, which are host-free and whose builds fail if their + * graphs ever reach a host capability. Every published entry is classified as + * one or the other in `vite.config.ts`, and the build fails on an export + * belonging to neither, so this exemption is a declaration rather than an + * omission. + * + * ## Resolution reports a miss as a value, and each side decides what it means + * + * {@link resolvePlatformBinary} returns `{ path: undefined, tried }` when no + * candidate verified. It does not throw, and that is deliberate rather than + * incidental: `taskless check` runs several engines, and a host without Vale + * installed must lose the Vale rules rather than the whole run. Encoding "not + * found" as a value is what lets a caller choose. + * + * A verifier should choose the opposite. A sandbox whose optional dependency + * did not install resolves `undefined` for the same reason a developer's laptop + * does — nothing is there — but the consequence is not a skipped engine, it is + * a verification that ran no engine and reported success. Fail closed on + * `undefined`, and use `tried` to say where you looked: + * + * ```ts + * import { + * AST_GREP_BINARY, + * resolvePlatformBinary, + * } from "@taskless/cli/node/runtimes"; + * + * const { path, tried } = resolvePlatformBinary(AST_GREP_BINARY); + * if (path === undefined) { + * throw new Error(`ast-grep not resolved. Looked in: ${tried.join(", ")}`); + * } + * ``` + * + * ## What a resolved path is, and is not + * + * It is a path that exists and that answered `--version` with output matching + * the spec's `identity`, so it is the binary and not a placeholder a failed + * install left behind. It is not a promise about which version answered: the + * first candidate tried is the pinned platform package, but a host that only + * has the engine on `PATH` resolves that instead. Read `--version` yourself if + * the version is what you are relying on. + */ + +// The `.js` extensions are deliberate, for the same reason they are on the +// prompts and layout entries: this module is a published entry, so `tsc` copies +// these specifiers verbatim into `dist/node/runtimes/index.d.ts`, where an +// extensionless specifier fails to resolve for a consumer on +// `moduleResolution: node16`/`nodenext`. +export { + findOnPath, + isPlatformBinary, + pathCommandName, + platformPackageName, + resolvePlatformBinary, + type PlatformBinaryResolution, + type PlatformBinarySpec, +} from "../../rules/platform-binary.js"; +export { AST_GREP_BINARY } from "../../rules/ast-grep-binary.js"; +export { VALE_BINARY } from "../../rules/vale/binary.js"; diff --git a/packages/cli/src/rules/ast-grep-binary.ts b/packages/cli/src/rules/ast-grep-binary.ts new file mode 100644 index 00000000..55cd1e8a --- /dev/null +++ b/packages/cli/src/rules/ast-grep-binary.ts @@ -0,0 +1,33 @@ +import type { PlatformBinarySpec } from "./platform-binary"; + +/** + * ast-grep's per-platform packaging, as the shared resolver understands it. + * + * `toolchainSuffix: true` is what produces `@ast-grep/cli-linux-x64-gnu` and + * `-win32-x64-msvc`. The Vale packages set it false; see + * {@link PlatformBinarySpec} for why that distinction is load-bearing. + * + * Both `ast-grep` and `sg` are listed because the wrapper declares them as bin + * entries for the same target, so either may be what got linked. `ast-grep` + * leads because it is the name the platform package ships the binary under; the + * resolver reverses the list at the link-based tiers, so `sg` is still tried + * first there and named in the PATH advice, as it was before the shared + * resolver existed. + * + * ## Why this sits in its own module rather than in `scan.ts` + * + * The spec is data: five fields describing how a package is named and how the + * binary identifies itself. `scan.ts` is the scanner — it reaches + * `rules/engines.ts`, and through it `node:fs/promises` and the CLI's error + * type. `@taskless/cli/node/runtimes` publishes this spec so a consumer can + * resolve the same binary the CLI executes, and a consumer that wants a path + * has no business loading a scanner to get one. Vale's spec is already a leaf + * for the same reason: `rules/vale/binary.ts` imports nothing but the resolver. + */ +export const AST_GREP_BINARY: PlatformBinarySpec = { + label: "ast-grep", + packagePrefix: "@ast-grep/cli", + toolchainSuffix: true, + binaryNames: ["ast-grep", "sg"], + identity: /ast-grep/i, +}; diff --git a/packages/cli/src/rules/scan.ts b/packages/cli/src/rules/scan.ts index 00e28e9a..3f5c5815 100644 --- a/packages/cli/src/rules/scan.ts +++ b/packages/cli/src/rules/scan.ts @@ -6,13 +6,13 @@ import { fileURLToPath } from "node:url"; import type { AstGrepMatch } from "../types/check"; import { toCheckResult, type CheckResult } from "../types/check"; +import { AST_GREP_BINARY } from "./ast-grep-binary"; import { ASSEMBLED_SG_CONFIG } from "./engines"; import { isWholeProjectWalk } from "./walk-scope"; import { isPlatformBinary, pathCommandName, resolvePlatformBinary, - type PlatformBinarySpec, } from "./platform-binary"; export interface ScanResult { @@ -36,28 +36,6 @@ export function buildPath(): string { return `${binDirectory}${separator}${process.env.PATH ?? ""}`; } -/** - * ast-grep's per-platform packaging, as the shared resolver understands it. - * - * `toolchainSuffix: true` is what produces `@ast-grep/cli-linux-x64-gnu` and - * `-win32-x64-msvc`. The Vale packages set it false; see - * {@link PlatformBinarySpec} for why that distinction is load-bearing. - * - * Both `ast-grep` and `sg` are listed because the wrapper declares them as bin - * entries for the same target, so either may be what got linked. `ast-grep` - * leads because it is the name the platform package ships the binary under; the - * resolver reverses the list at the link-based tiers, so `sg` is still tried - * first there and named in the PATH advice, as it was before the shared - * resolver existed. - */ -export const AST_GREP_BINARY: PlatformBinarySpec = { - label: "ast-grep", - packagePrefix: "@ast-grep/cli", - toolchainSuffix: true, - binaryNames: ["ast-grep", "sg"], - identity: /ast-grep/i, -}; - /** * ast-grep's `sg` alias prints a deprecation banner to stderr on every run, * from 0.45.0. Strip it before stderr becomes a user-facing message. diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index 53f4ac05..feded2de 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -377,7 +377,7 @@ const failingProject = () => withSg("ast-grep vendor contract", () => { it("reports its own name in --version", () => { - // Depended on by: AST_GREP_BINARY.identity (/ast-grep/i) in scan.ts. The + // Depended on by: AST_GREP_BINARY.identity (/ast-grep/i) in rules/ast-grep-binary.ts. The // resolver runs each candidate because `@ast-grep/cli`'s postinstall can // leave a placeholder text file at the binary path. If ast-grep stops // saying "ast-grep" here, findSgBinary rejects the real binary and throws diff --git a/packages/cli/test/platform-binary.test.ts b/packages/cli/test/platform-binary.test.ts index 36cdf8ad..50abb6b9 100644 --- a/packages/cli/test/platform-binary.test.ts +++ b/packages/cli/test/platform-binary.test.ts @@ -10,7 +10,7 @@ import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { AST_GREP_BINARY } from "../src/rules/scan"; +import { AST_GREP_BINARY } from "../src/rules/ast-grep-binary"; import { pathCommandName, platformPackageName, diff --git a/packages/cli/tsconfig.public.json b/packages/cli/tsconfig.public.json index 4fc83641..d6d4b6e7 100644 --- a/packages/cli/tsconfig.public.json +++ b/packages/cli/tsconfig.public.json @@ -1,5 +1,5 @@ -// Declaration emit for the public subpath exports — `@taskless/cli/prompts` -// and `@taskless/cli/layout` — and nothing else. +// Declaration emit for the public subpath exports — `@taskless/cli/prompts`, +// `@taskless/cli/layout`, and `@taskless/cli/node/runtimes` — and nothing else. // // Vite's lib build emits JavaScript and no types, so `tsc` supplies the `.d.ts` // for the entries that are public importable API. The include list is @@ -32,5 +32,10 @@ "rootDir": "src", "declarationDir": "dist" }, - "include": ["src/globals.d.ts", "src/prompts/index.ts", "src/layout/index.ts"] + "include": [ + "src/globals.d.ts", + "src/prompts/index.ts", + "src/layout/index.ts", + "src/node/runtimes/index.ts" + ] } diff --git a/packages/cli/vite.config.ts b/packages/cli/vite.config.ts index 4385fce0..3c0df974 100644 --- a/packages/cli/vite.config.ts +++ b/packages/cli/vite.config.ts @@ -19,7 +19,10 @@ import { const pkg = JSON.parse( readFileSync(resolve(import.meta.dirname, "package.json"), "utf8") -) as { version: string }; +) as { + version: string; + exports: Record; +}; // Target resolution lives in ./scripts/build-target.ts so it can be unit-tested // (test/build-target.test.ts) over an explicit environment. Everything below @@ -106,26 +109,117 @@ function assertSkillVersions(): Plugin { }; } -// The build emits three entries. `index` is the executable CLI; `prompts` and -// `layout` are library modules consumers import as `@taskless/cli/`. Only -// the first is a program, so only it gets a shebang and the executable bit — a -// `#!` line on a library entry would be a syntax error to anything importing it -// as a module. +// `index` is the executable CLI; everything else is a library module consumers +// import as `@taskless/cli/`. Only the first is a program, so only it +// gets a shebang and the executable bit — a `#!` line on a library entry would +// be a syntax error to anything importing it as a module. const BIN_ENTRY = "index"; const PROMPTS_ENTRY = "prompts"; const LAYOUT_ENTRY = "layout"; +const NODE_RUNTIMES_ENTRY = "node/runtimes"; + +/** Every entry this build emits, and the module each one is rooted at. */ +const ENTRY_SOURCES: Record = { + [BIN_ENTRY]: resolve(import.meta.dirname, "src/index.ts"), + [PROMPTS_ENTRY]: resolve(import.meta.dirname, "src/prompts/index.ts"), + [LAYOUT_ENTRY]: resolve(import.meta.dirname, "src/layout/index.ts"), + [NODE_RUNTIMES_ENTRY]: resolve( + import.meta.dirname, + "src/node/runtimes/index.ts" + ), +}; /** - * Every entry that is a library surface rather than a program, and therefore - * must not drag the CLI runtime along with it. + * Library entries whose graphs must reach no host capability at all. * - * Adding an entry here is what subjects it to {@link assertLibraryGraphs}. An - * entry absent from this list is checked by nothing, so a new published surface - * that forgets to join it leaks silently — which is the failure this guard - * exists to prevent, one level up. + * Membership here is what subjects an entry to the external-import half of + * {@link assertLibraryGraphs}. An entry absent from this list used to be + * checked by nothing, so a new published surface that forgot to join it leaked + * silently — which is the failure this guard exists to prevent, one level up. + * {@link assertExportsClassified} closes that: absence is now only meaningful + * when the entry appears in {@link HOST_BOUND_ENTRIES} instead. */ const LIBRARY_ENTRIES = [PROMPTS_ENTRY, LAYOUT_ENTRY]; +/** + * Library entries that require Node, declared rather than merely unlisted. + * + * `@taskless/cli/node/runtimes` resolves the engine binaries the CLI executes: + * it spawns candidates to verify their identity, reads the filesystem, and + * consults `PATH`. It can never satisfy the external-import rule and must not + * be made to — the point of the entry is the host access. What it must still + * not do is reach the CLI entry, so a consumer importing it gets a resolver and + * not the command layer, and that half of {@link assertLibraryGraphs} applies + * here too. + * + * The `node/` segment in the published specifier carries the same statement to + * a reader, who meets it before a build error does. + */ +const HOST_BOUND_ENTRIES = [NODE_RUNTIMES_ENTRY]; + +/** + * Refuse to build when a published export is classified as neither. + * + * Both lists above are opt-in, and opting in is a thing a person can forget. + * Before this check, forgetting was indistinguishable from a deliberate + * exemption: a new `exports` key that joined no list simply went unchecked, and + * nothing anywhere could tell the two apart. Requiring every published export + * to name its classification makes the exemption a claim someone wrote down and + * a reviewer can disagree with. + * + * The mapping runs from `package.json` rather than the other way, because + * `exports` is what a consumer can actually import. An entry built but not + * exported reaches nobody; an export with no entry behind it is the case this + * check is for. + */ +function assertExportsClassified(): Plugin { + return { + name: "assert-exports-classified", + buildStart() { + const classified = new Set([ + BIN_ENTRY, + ...LIBRARY_ENTRIES, + ...HOST_BOUND_ENTRIES, + ]); + const problems: string[] = []; + + for (const [subpath, conditions] of Object.entries(pkg.exports)) { + const target = conditions.import; + const entryName = target?.match(/^\.\/dist\/(.+)\.js$/)?.[1]; + if (entryName === undefined) { + problems.push( + ` "${subpath}" resolves to ${target ?? "(no import condition)"}, ` + + `which does not name a built entry (expected "./dist/.js")` + ); + continue; + } + if (!(entryName in ENTRY_SOURCES)) { + problems.push( + ` "${subpath}" points at dist/${entryName}.js, which this build ` + + `does not emit — add it to ENTRY_SOURCES in vite.config.ts` + ); + continue; + } + if (!classified.has(entryName)) { + problems.push( + ` "${subpath}" (entry "${entryName}") is in neither ` + + `LIBRARY_ENTRIES nor HOST_BOUND_ENTRIES` + ); + } + } + + if (problems.length > 0) { + throw new Error( + `Published exports must declare what they are.\n${problems.join("\n")}\n` + + `Add the entry to LIBRARY_ENTRIES if its graph reaches no host ` + + `capability, or to HOST_BOUND_ENTRIES if it deliberately does. ` + + `Leaving it out is not an exemption; it is an unchecked surface.` + ); + } + }, + }; +} + function shebang(): Plugin { // Typed against Rollup's own bundle union rather than a structural shape, so // an asset cannot satisfy the parameter by having none of these fields. The @@ -169,6 +263,21 @@ function findEntryChunk( /** * Walk one library entry's transitive chunk graph, failing the build on a leak. * + * `hostBound` selects which of the two rules apply. A host-free entry gets + * both; a host-bound one keeps only the CLI rule, since its externals are the + * reason it exists. It is a parameter rather than two functions because the + * traversal is the same walk either way, and the entry's classification is + * already recorded at its list. + * + * The CLI rule asks about the bin entry's **module**, not the bin **chunk**. + * Chunking is rollup's to decide, and it moves under exactly the condition this + * rule is watching for: measured here, an entry importing `src/index.ts` made + * rollup hoist the whole CLI into a shared chunk and leave `dist/index.js` a + * 50-byte re-export facade, which the library entry then did not import. A + * comparison against the bin chunk's file name reported nothing at all, while + * the emitted library entry carried the entire command layer. A module id is + * the thing that cannot be re-arranged out from under the check. + * * A plain function taking the plugin context, NOT a method on the plugin * object: rollup binds `this` in a hook to its own `PluginContext`, so a helper * hung off the plugin is not reachable as `this.helper` and fails at build time @@ -178,13 +287,14 @@ function checkLibraryEntry( context: Rollup.PluginContext, bundle: Rollup.OutputBundle, entryName: string, - binFile: string | undefined + hostBound: boolean ): void { const entry = findEntryChunk(bundle, entryName); // Not an error: `build:self` and any future single-entry build legitimately // emit no library entries. Nothing to check, not a failure to check it. if (entry === undefined) return; + const binModule = ENTRY_SOURCES[BIN_ENTRY]; const seen = new Set(); const queue = [entry.fileName]; while (queue.length > 0) { @@ -196,16 +306,20 @@ function checkLibraryEntry( const chunk = bundle[imported]; if (chunk === undefined || chunk.type !== "chunk") { - // Resolved to something outside the bundle: an external module. + // Resolved to something outside the bundle: an external module. For a + // host-bound entry that is the point, and there is no chunk to walk into, + // so the traversal stops here rather than failing here. + if (hostBound) continue; context.error( `${entryName} entry graph imports ${imported}; a library entry ` + `must not reach a host capability` ); } - if (imported === binFile) { + if (binModule !== undefined && chunk.moduleIds.includes(binModule)) { context.error( - `${entryName} entry graph reaches the CLI entry (${imported}); ` + - `importing @taskless/cli/${entryName} would load the command layer` + `${entryName} entry graph reaches the CLI entry module ` + + `(${binModule}, bundled into ${imported}); importing ` + + `@taskless/cli/${entryName} would load the command layer` ); } queue.push(...chunk.imports, ...chunk.dynamicImports); @@ -218,17 +332,24 @@ function checkLibraryEntry( * `@taskless/cli/prompts` renders recipe text; `@taskless/cli/layout` is the * rule layout table a service builds payloads against. Both are imported by * consumers that are not this CLI — a Worker among them — so neither may pull - * in the command layer or reach a host capability. The graph is allowed to - * touch embedded text and the pure values it exports, and nothing else. + * in the command layer or reach a host capability. Their graphs are allowed to + * touch embedded text and the pure values they export, and nothing else. * * Two rules, both checked over the entry's transitive chunk graph: * * - **No external imports at all.** Everything but node builtins is bundled * (see `rollupOptions.external`), so a bare specifier surviving here is a - * builtin — `node:fs`, `node:child_process` — and a library entry has no - * business with any of them. + * builtin — `node:fs`, `node:child_process` — and a host-free library entry + * has no business with any of them. * - **Never reach the bin entry.** That chunk is the CLI itself. * + * The second rule applies to every published library entry, including the + * host-bound ones in {@link HOST_BOUND_ENTRIES}. Needing `node:child_process` + * is not a reason to also ship the command tree, and a resolver that dragged in + * the CLI would give a consumer telemetry and a command layer it did not ask + * for. The first rule is the one host-bound entries are excused from, by name, + * in a list {@link assertExportsClassified} requires them to appear in. + * * ENFORCED IN THE BUILD, DELIBERATELY, rather than asserted by a test over the * artifact. A build that refuses to emit a leaking bundle makes the bad artifact * unproducible; a test that inspects one afterwards only notices. It is also @@ -243,9 +364,11 @@ function assertLibraryGraphs(): Plugin { return { name: "assert-library-graphs", generateBundle(_options, bundle) { - const binFile = findEntryChunk(bundle, BIN_ENTRY)?.fileName; for (const entryName of LIBRARY_ENTRIES) { - checkLibraryEntry(this, bundle, entryName, binFile); + checkLibraryEntry(this, bundle, entryName, false); + } + for (const entryName of HOST_BOUND_ENTRIES) { + checkLibraryEntry(this, bundle, entryName, true); } }, }; @@ -285,17 +408,14 @@ export default defineConfig({ plugins: [ tsconfigPaths(), assertSkillVersions(), + assertExportsClassified(), shebang(), assertLibraryGraphs(), ], build: { outDir, lib: { - entry: { - [BIN_ENTRY]: resolve(import.meta.dirname, "src/index.ts"), - [PROMPTS_ENTRY]: resolve(import.meta.dirname, "src/prompts/index.ts"), - [LAYOUT_ENTRY]: resolve(import.meta.dirname, "src/layout/index.ts"), - }, + entry: ENTRY_SOURCES, formats: ["es"], fileName: (_format, entryName) => `${entryName}.js`, },