Skip to content

Commit 2c6d5df

Browse files
thecodedriftclaude
andcommitted
ref(cli): share the platform-binary resolver between ast-grep and Vale
Task 1.1. `findSgBinary()` hard-coded ast-grep's packaging: the `@ast-grep/cli` prefix, the `-gnu`/`-msvc` suffixes, the two bin spellings, and an `ast-grep` identity check. Vale needs the same search with different answers to all four, so the search moves to `rules/platform-binary.ts` and each engine supplies a spec. The parameter that matters is `toolchainSuffix`. ast-grep publishes `@ast-grep/cli-linux-x64-gnu`; add-vale-binary-packages publishes `@taskless/vale-linux-x64` with no libc suffix at all. Reusing ast-grep's naming for Vale would resolve nothing on Linux and surface as the ordinary "Vale is unavailable" message — a naming bug wearing the costume of a host that never installed it. test/platform-binary.test.ts pins the naming for both engines across every published platform, and cross-checks the Vale names against the optionalDependencies actually declared in package.json so a rename on either side fails there rather than at runtime. The resolver returns `{path, tried}` instead of throwing, because the two callers need different things from a miss: ast-grep is the only executor for `sg` rules, so `findSgBinary()` keeps throwing, while `findValeBinary()` returns undefined per D6b — a missing Vale binary makes one engine unavailable and must not abort the others. `findValeBinary()` caches the miss as well as the hit, since an absent Vale is the common case and each resolution spawns a subprocess per candidate. ast-grep behaviour is unchanged: `isAstGrepBinary` and `findSgBinary` keep their signatures and their existing tests, all 456 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
1 parent fd4bb6d commit 2c6d5df

4 files changed

Lines changed: 370 additions & 73 deletions

File tree

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { spawnSync } from "node:child_process";
2+
import { existsSync } from "node:fs";
3+
import { createRequire } from "node:module";
4+
import { dirname, resolve } from "node:path";
5+
import { fileURLToPath } from "node:url";
6+
7+
/**
8+
* Resolving a prebuilt binary that ships as a per-platform npm package.
9+
*
10+
* This was ast-grep's resolver in `scan.ts`, generalized when Vale became a
11+
* second engine that needs the same treatment. Both depend on per-platform
12+
* packages directly and exec by path, because neither can rely on an
13+
* install-time step: `@ast-grep/cli`'s postinstall hardlink fails under pnpm
14+
* dlx's strict isolation, and the `@taskless/vale-*` packages deliberately ship
15+
* no `bin` and no scripts at all.
16+
*
17+
* WHAT IS PARAMETERIZED, and why each field exists rather than being derived:
18+
*
19+
* - `toolchainSuffix` is the one that bites. ast-grep publishes
20+
* `@ast-grep/cli-linux-x64-gnu` and `-win32-x64-msvc`; the Vale packages are
21+
* `@taskless/vale-linux-x64` with **no libc suffix at all**. Reusing
22+
* ast-grep's naming for Vale would look up `@taskless/vale-linux-x64-gnu` and
23+
* miss on every Linux host — a resolution failure that reads as "Vale is not
24+
* installed" rather than as a naming bug.
25+
* - `identity` exists because existence is not proof. A file can sit exactly
26+
* where the binary belongs and not be the binary: ast-grep's failed hardlink
27+
* leaves a placeholder text file there. Asking a candidate to identify itself
28+
* is the only check that tells the two apart.
29+
*/
30+
export interface PlatformBinarySpec {
31+
/** Name used in error messages, e.g. `ast-grep`. */
32+
label: string;
33+
/** Package name up to the platform suffix, e.g. `@ast-grep/cli`. */
34+
packagePrefix: string;
35+
/**
36+
* Append the platform's toolchain suffix (`-gnu` on Linux, `-msvc` on
37+
* Windows). True for ast-grep, false for the Vale packages.
38+
*/
39+
toolchainSuffix: boolean;
40+
/**
41+
* Executable names to try, in confidence order, spelled for unix. `.exe` is
42+
* appended on Windows. More than one because ast-grep declares both
43+
* `ast-grep` and `sg` for the same target.
44+
*/
45+
binaryNames: string[];
46+
/** Pattern the candidate's own `--version` output must match. */
47+
identity: RegExp;
48+
}
49+
50+
/** The npm package carrying this host's prebuilt binary. */
51+
export function platformPackageName(spec: PlatformBinarySpec): string {
52+
const parts: string[] = [process.platform, process.arch];
53+
if (spec.toolchainSuffix) {
54+
if (process.platform === "linux") {
55+
parts.push("gnu");
56+
} else if (process.platform === "win32") {
57+
parts.push("msvc");
58+
}
59+
}
60+
return `${spec.packagePrefix}-${parts.join("-")}`;
61+
}
62+
63+
/** Executable name for this platform. */
64+
function executableName(name: string): string {
65+
return process.platform === "win32" ? `${name}.exe` : name;
66+
}
67+
68+
/** Absolute path to a binary inside the resolved platform package, if any. */
69+
function platformPackageBinary(
70+
spec: PlatformBinarySpec,
71+
binary: string
72+
): string | undefined {
73+
try {
74+
const require = createRequire(import.meta.url);
75+
const packageJsonPath = require.resolve(
76+
`${platformPackageName(spec)}/package.json`
77+
);
78+
return resolve(dirname(packageJsonPath), binary);
79+
} catch {
80+
// Not installed for this host: an unsupported arch, or musl, where neither
81+
// project publishes a build and `os`/`cpu` filtering skips the package.
82+
return undefined;
83+
}
84+
}
85+
86+
/** First entry on PATH that holds a file named `command`. */
87+
export function findOnPath(command: string): string | undefined {
88+
const separator = process.platform === "win32" ? ";" : ":";
89+
for (const directory of (process.env.PATH ?? "").split(separator)) {
90+
if (directory === "") continue;
91+
const candidate = resolve(directory, command);
92+
if (existsSync(candidate)) return candidate;
93+
}
94+
return undefined;
95+
}
96+
97+
/**
98+
* Whether `path` is really this binary, established by running it.
99+
*
100+
* See {@link PlatformBinarySpec.identity} — existence is not enough, because a
101+
* placeholder file left by a failed install sits at exactly the right path and
102+
* satisfies `existsSync` happily.
103+
*/
104+
export function isPlatformBinary(
105+
spec: PlatformBinarySpec,
106+
path: string
107+
): boolean {
108+
if (!existsSync(path)) return false;
109+
const result = spawnSync(path, ["--version"], {
110+
encoding: "utf8",
111+
timeout: 5000,
112+
});
113+
if (result.error !== undefined || result.status !== 0) return false;
114+
return spec.identity.test(`${result.stdout ?? ""}${result.stderr ?? ""}`);
115+
}
116+
117+
export interface PlatformBinaryResolution {
118+
/** Absolute path to the verified binary, or `undefined` when none resolved. */
119+
path: string | undefined;
120+
/** Locations searched, in order, for an actionable failure message. */
121+
tried: string[];
122+
}
123+
124+
/**
125+
* Search every place the binary could reasonably live, verifying each.
126+
*
127+
* Candidates are ordered by confidence rather than by convenience — the
128+
* platform package first because it is the version we pinned, then a locally
129+
* linked binary, then whatever the host provides on PATH.
130+
*
131+
* Returns rather than throws. The two callers want different things from a
132+
* miss: ast-grep cannot run at all without it, while a missing Vale binary
133+
* makes one engine unavailable and must not abort the others (D6b). Encoding
134+
* "not found" as a value rather than an exception is what lets each decide.
135+
*/
136+
export function resolvePlatformBinary(
137+
spec: PlatformBinarySpec
138+
): PlatformBinaryResolution {
139+
const localBin = resolve(
140+
dirname(fileURLToPath(import.meta.url)),
141+
"..",
142+
"node_modules",
143+
".bin"
144+
);
145+
146+
const candidates: Array<[label: string, path: string | undefined]> = [
147+
...spec.binaryNames.map((name): [string, string | undefined] => [
148+
platformPackageName(spec),
149+
platformPackageBinary(spec, executableName(name)),
150+
]),
151+
...spec.binaryNames.map((name): [string, string | undefined] => [
152+
"node_modules/.bin",
153+
resolve(localBin, executableName(name)),
154+
]),
155+
...spec.binaryNames.map((name): [string, string | undefined] => [
156+
"PATH",
157+
findOnPath(executableName(name)),
158+
]),
159+
];
160+
161+
for (const [, path] of candidates) {
162+
if (path !== undefined && isPlatformBinary(spec, path)) {
163+
return { path, tried: candidates.map(([label]) => label) };
164+
}
165+
}
166+
167+
return { path: undefined, tried: candidates.map(([label]) => label) };
168+
}

packages/cli/src/rules/scan.ts

Lines changed: 33 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1-
import { existsSync } from "node:fs";
2-
import { createRequire } from "node:module";
3-
import { spawn, spawnSync } from "node:child_process";
1+
import { spawn } from "node:child_process";
42
import { dirname, resolve } from "node:path";
53
import { createInterface } from "node:readline";
64
import { fileURLToPath } from "node:url";
75

86
import type { AstGrepMatch } from "../types/check";
97
import { toCheckResult, type CheckResult } from "../types/check";
108
import { COMMITTED_SG_CONFIG } from "./engines";
9+
import {
10+
isPlatformBinary,
11+
resolvePlatformBinary,
12+
type PlatformBinarySpec,
13+
} from "./platform-binary";
1114

1215
export interface ScanResult {
1316
results: CheckResult[];
@@ -30,42 +33,23 @@ export function buildPath(): string {
3033
return `${binDirectory}${separator}${process.env.PATH ?? ""}`;
3134
}
3235

33-
/** The npm package carrying this host's prebuilt ast-grep binary. */
34-
function platformPackageName(): string {
35-
const parts: string[] = [process.platform, process.arch];
36-
if (process.platform === "linux") {
37-
parts.push("gnu");
38-
} else if (process.platform === "win32") {
39-
parts.push("msvc");
40-
}
41-
return `@ast-grep/cli-${parts.join("-")}`;
42-
}
43-
44-
/** Absolute path to the binary inside the resolved platform package, if any. */
45-
function platformPackageBinary(binary: string): string | undefined {
46-
try {
47-
const require = createRequire(import.meta.url);
48-
const packageJsonPath = require.resolve(
49-
`${platformPackageName()}/package.json`
50-
);
51-
return resolve(dirname(packageJsonPath), binary);
52-
} catch {
53-
// Not installed for this host (unsupported arch, or musl — upstream
54-
// publishes no musl package and marks the gnu ones `libc: [glibc]`).
55-
return undefined;
56-
}
57-
}
58-
59-
/** First entry on PATH that holds a file named `command`. */
60-
function findOnPath(command: string): string | undefined {
61-
const separator = process.platform === "win32" ? ";" : ":";
62-
for (const directory of (process.env.PATH ?? "").split(separator)) {
63-
if (directory === "") continue;
64-
const candidate = resolve(directory, command);
65-
if (existsSync(candidate)) return candidate;
66-
}
67-
return undefined;
68-
}
36+
/**
37+
* ast-grep's per-platform packaging, as the shared resolver understands it.
38+
*
39+
* `toolchainSuffix: true` is what produces `@ast-grep/cli-linux-x64-gnu` and
40+
* `-win32-x64-msvc`. The Vale packages set it false; see
41+
* {@link PlatformBinarySpec} for why that distinction is load-bearing.
42+
*
43+
* Both `ast-grep` and `sg` are listed because the wrapper declares them as bin
44+
* entries for the same target, so either may be what got linked.
45+
*/
46+
export const AST_GREP_BINARY: PlatformBinarySpec = {
47+
label: "ast-grep",
48+
packagePrefix: "@ast-grep/cli",
49+
toolchainSuffix: true,
50+
binaryNames: ["ast-grep", "sg"],
51+
identity: /ast-grep/i,
52+
};
6953

7054
/**
7155
* Whether `path` is really ast-grep, established by running it.
@@ -78,13 +62,7 @@ function findOnPath(command: string): string | undefined {
7862
* binary from a file merely sitting where the binary belongs.
7963
*/
8064
export function isAstGrepBinary(path: string): boolean {
81-
if (!existsSync(path)) return false;
82-
const result = spawnSync(path, ["--version"], {
83-
encoding: "utf8",
84-
timeout: 5000,
85-
});
86-
if (result.error !== undefined || result.status !== 0) return false;
87-
return /ast-grep/i.test(`${result.stdout ?? ""}${result.stderr ?? ""}`);
65+
return isPlatformBinary(AST_GREP_BINARY, path);
8866
}
8967

9068
/**
@@ -113,36 +91,18 @@ let cachedSgBinary: string | undefined;
11391
export function findSgBinary(): string {
11492
if (cachedSgBinary !== undefined) return cachedSgBinary;
11593

116-
const binary = process.platform === "win32" ? "ast-grep.exe" : "ast-grep";
117-
const alternative = process.platform === "win32" ? "sg.exe" : "sg";
118-
const localBin = resolve(
119-
dirname(fileURLToPath(import.meta.url)),
120-
"..",
121-
"node_modules",
122-
".bin"
123-
);
124-
125-
const candidates: Array<[label: string, path: string | undefined]> = [
126-
[platformPackageName(), platformPackageBinary(binary)],
127-
// Both names, matching the PATH search below: the wrapper declares `sg` and
128-
// `ast-grep` as bin entries for the same target, so either may be linked.
129-
["node_modules/.bin", resolve(localBin, alternative)],
130-
["node_modules/.bin", resolve(localBin, binary)],
131-
["PATH", findOnPath(alternative)],
132-
["PATH", findOnPath(binary)],
133-
];
134-
135-
for (const [, path] of candidates) {
136-
if (path !== undefined && isAstGrepBinary(path)) {
137-
cachedSgBinary = path;
138-
return path;
139-
}
94+
const { path, tried } = resolvePlatformBinary(AST_GREP_BINARY);
95+
if (path !== undefined) {
96+
cachedSgBinary = path;
97+
return path;
14098
}
14199

142-
const tried = candidates.map(([label]) => label).join(", ");
100+
// ast-grep, unlike Vale, has no degraded mode: it is the executor for every
101+
// `sg` rule, so a miss is fatal for this command rather than one engine
102+
// reporting itself unavailable.
143103
throw new Error(
144-
`ast-grep binary not found. Looked in: ${tried}. Install a supported ` +
145-
`platform build, or put \`${alternative}\` on your PATH.`
104+
`ast-grep binary not found. Looked in: ${tried.join(", ")}. Install a ` +
105+
`supported platform build, or put \`sg\` on your PATH.`
146106
);
147107
}
148108

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import {
2+
resolvePlatformBinary,
3+
type PlatformBinarySpec,
4+
} from "../platform-binary";
5+
6+
/**
7+
* Vale's per-platform packaging.
8+
*
9+
* `toolchainSuffix: false` is the whole reason this spec exists separately from
10+
* ast-grep's. `add-vale-binary-packages` publishes `@taskless/vale-<os>-<cpu>`
11+
* — `@taskless/vale-linux-x64`, not `-linux-x64-gnu`. ast-grep's resolver
12+
* appends `-gnu` on every Linux, so reusing its naming here would resolve
13+
* nothing on Linux while reporting the ordinary "Vale is unavailable" message,
14+
* making a naming bug indistinguishable from a host without the binary.
15+
*
16+
* One binary name, unlike ast-grep's two: these packages ship `vale` (or
17+
* `vale.exe`) as pure payload, with no `bin` entry and no lifecycle script, so
18+
* there is no wrapper spelling to also try.
19+
*/
20+
export const VALE_BINARY: PlatformBinarySpec = {
21+
label: "vale",
22+
packagePrefix: "@taskless/vale",
23+
toolchainSuffix: false,
24+
binaryNames: ["vale"],
25+
identity: /vale/i,
26+
};
27+
28+
/**
29+
* Resolution is cached for the process, including a miss.
30+
*
31+
* Caching the miss matters as much as caching the hit: resolution spawns a
32+
* subprocess per candidate, and an absent Vale is the common case on a host
33+
* that never installed it. Without this, every rule would re-run the whole
34+
* search to rediscover the same nothing.
35+
*/
36+
let cached: { path: string | undefined; tried: string[] } | undefined;
37+
38+
/**
39+
* Locate the Vale binary, or report that it is unavailable.
40+
*
41+
* Returns `undefined` rather than throwing, per D6b: a missing Vale binary
42+
* makes the Vale engine unavailable and must not abort the other engines. The
43+
* caller turns that into a reported-but-not-fatal outcome; ast-grep's resolver
44+
* throws instead, because it has no degraded mode.
45+
*/
46+
export function findValeBinary(): {
47+
path: string | undefined;
48+
tried: string[];
49+
} {
50+
cached ??= resolvePlatformBinary(VALE_BINARY);
51+
return cached;
52+
}
53+
54+
/** Reset the process cache. Tests only. */
55+
export function resetValeBinaryCache(): void {
56+
cached = undefined;
57+
}
58+
59+
/** An actionable message naming where we looked. */
60+
export function valeUnavailableMessage(tried: string[]): string {
61+
return (
62+
`Vale binary not found. Looked in: ${tried.join(", ")}. Install a ` +
63+
`supported platform build, or put \`vale\` on your PATH. Other engines ` +
64+
`still ran.`
65+
);
66+
}

0 commit comments

Comments
 (0)