diff --git a/README.md b/README.md index 8d02dff..3f32d24 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ What makes it worth wiring in: - **Auto-fixes locally, fails in CI (same command).** Run it on your machine and it _fixes_ what it can (lint, formatting) instead of just complaining. Run it under CI and the identical command is check-only, so a PR can't merge with problems that should have been fixed. - **Silent when green, so it's cheap to loop.** A passing run prints nothing and exits `0`, with no output to burn an agent's tokens or bury the one failure that matters. Agents can run it as often as they like. -- **Failure output written for an agent to act on.** When a check fails it names the tool it ran, the exact command, and a docs link, so the agent (or you) knows what to fix and how instead of guessing. +- **Failure output written for an agent to act on.** When a check fails it names the tool it ran, the exact argv, and a docs link, so the agent (or you) knows what to fix and how instead of guessing. - **Convention over configuration.** Checks are just `verify:*` npm scripts run in parallel. Add, drop, or override any of them; there's no bespoke config format to learn. ## Install @@ -127,11 +127,11 @@ Flags on the bare `verifyx` command: | `circular-deps` | external | Circular dependencies ([skott](https://github.com/antoine-coulon/skott)). | | `duplicate-code` | external | Copy-paste detection ([jscpd](https://github.com/kucherenko/jscpd)). | -External checks shell out to their tool and **skip gracefully when it is not installed**; `verifyx init` installs the ones you opt into. They run the tool from your local `node_modules/.bin` regardless of how `verifyx` was invoked. `oxlint`/`oxfmt`/`tsc` are resolved if present; the rest are declared as optional `peerDependencies`. +External checks spawn their tool and **skip gracefully when it is not installed**; `verifyx init` installs the ones you opt into. They run the tool from your local `node_modules/.bin` regardless of how `verifyx` was invoked. `oxlint`/`oxfmt`/`tsc` are resolved if present; the rest are declared as optional `peerDependencies`. -Because checks are named for their function, **on failure** an external check prints the tool it used, the exact command it ran, and a docs link, so you (or an agent) can add the tool's config (e.g. `knip.json`) without guessing. On success it prints nothing (output is buffered and flushed only on failure, to keep runs quiet and cheap). If you override a check with your own `verify:` script, a failure shows that `npm run verify:` was what ran. +Because checks are named for their function, **on failure** an external check prints the tool it used, the exact argv it ran, and a docs link, so you (or an agent) can add the tool's config (e.g. `knip.json`) without guessing. On success it prints nothing (output is buffered and flushed only on failure, to keep runs quiet and cheap). If you override a check with your own `verify:` script, a failure shows that `npm run verify:` was what ran. -**Passing extra arguments through.** When you run an external check directly, anything after `--` is forwarded verbatim to the underlying tool, so you can tweak an invocation without ejecting it: `verifyx circular-deps -- src/*.ts` runs skott against `src/*.ts`, `verifyx lint -- --quiet` passes `--quiet` to oxlint. `verifyx init` scaffolds `verify:circular-deps` as `verifyx circular-deps -- src/*.ts` (skott needs a target), so the default is visible in your `package.json` and easy to point at your own source layout. +**Passing extra arguments through.** When you run an external check directly, anything after `--` is forwarded as literal argv entries to the underlying tool, so you can tweak an invocation without ejecting it: `verifyx circular-deps -- src/index.ts` asks skott to start at that file, while `verifyx lint -- --quiet` passes `--quiet` to oxlint. With no entrypoint, skott scans the current project, so `verifyx init` scaffolds the cross-platform default `verifyx circular-deps` without a shell glob. Each external check is configured through its **tool's own config file**, exactly as you would use that tool standalone: @@ -326,7 +326,7 @@ import { analyzeComplexity, getCheck, runAll } from '@makerx/verify' // Run the maintainability analysis directly. const { failing } = analyzeComplexity({ pattern: 'src/**/*.ts', threshold: 50 }) -// Run any single check by name, including the ones that shell out to an external tool. +// Run any single check by name, including the ones that spawn an external tool. const lint = await getCheck('lint')?.runDefault() const unused = await getCheck('unused-code')?.runDefault({ maxWarnings: 5 }) diff --git a/package-lock.json b/package-lock.json index 065a714..e4565a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,15 +10,17 @@ "license": "MIT", "dependencies": { "commander": "^15.0.0", + "cross-spawn": "^7.0.6", "enquirer": "^2.4.1", "minimatch": "^10.2.5", "typescript": "^6.0.3" }, "bin": { - "verify": "dist/cli.mjs" + "verifyx": "dist/cli.mjs" }, "devDependencies": { "@rollup/plugin-typescript": "^12.3.0", + "@types/cross-spawn": "^6.0.6", "@types/node": "^26.1.1", "@vitest/coverage-v8": "^4.1.10", "better-npm-audit": "^3.11.0", @@ -2710,6 +2712,16 @@ "assertion-error": "^2.0.1" } }, + "node_modules/@types/cross-spawn": { + "version": "6.0.6", + "resolved": "https://registry.npmjs.org/@types/cross-spawn/-/cross-spawn-6.0.6.tgz", + "integrity": "sha512-fXRhhUkG4H3TQk5dBhQ7m/JDdSNHKwR2BBia62lhwEIq9xGiQKLxd6LymNhn47SjXhsUEPmxi+PKw2OkW4LLjA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/deep-eql": { "version": "4.0.2", "dev": true, @@ -3416,7 +3428,8 @@ }, "node_modules/cross-spawn": { "version": "7.0.6", - "dev": true, + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", + "integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==", "license": "MIT", "dependencies": { "path-key": "^3.1.0", @@ -3429,12 +3442,10 @@ }, "node_modules/cross-spawn/node_modules/isexe": { "version": "2.0.0", - "dev": true, "license": "ISC" }, "node_modules/cross-spawn/node_modules/which": { "version": "2.0.2", - "dev": true, "license": "ISC", "dependencies": { "isexe": "^2.0.0" @@ -5074,7 +5085,6 @@ }, "node_modules/path-key": { "version": "3.1.1", - "dev": true, "license": "MIT", "engines": { "node": ">=8" @@ -5432,7 +5442,6 @@ }, "node_modules/shebang-command": { "version": "2.0.0", - "dev": true, "license": "MIT", "dependencies": { "shebang-regex": "^3.0.0" @@ -5443,7 +5452,6 @@ }, "node_modules/shebang-regex": { "version": "3.0.0", - "dev": true, "license": "MIT", "engines": { "node": ">=8" diff --git a/package.json b/package.json index 5f57fff..f0da1d7 100644 --- a/package.json +++ b/package.json @@ -58,12 +58,14 @@ }, "dependencies": { "commander": "^15.0.0", + "cross-spawn": "^7.0.6", "enquirer": "^2.4.1", "minimatch": "^10.2.5", "typescript": "^6.0.3" }, "devDependencies": { "@rollup/plugin-typescript": "^12.3.0", + "@types/cross-spawn": "^6.0.6", "@types/node": "^26.1.1", "@vitest/coverage-v8": "^4.1.10", "better-npm-audit": "^3.11.0", diff --git a/src/checks/external.test.ts b/src/checks/external.test.ts index ab7ee62..98e6751 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -1,94 +1,93 @@ import { describe, expect, it, vi } from 'vitest' import { runCaptured } from '../shared/output.ts' -import { appendArgs, defineExternalCheck, externalFailureHint, runCountedBudget, selectCommand } from './external.ts' +import { defineExternalCheck, externalFailureHint, runCountedBudget, selectCommand } from './external.ts' -const fixable = { checkCommand: 'oxfmt --check .', fixCommand: 'oxfmt .' } -const notFixable = { checkCommand: 'tsc --noEmit' } +const fixable = { checkCommand: ['oxfmt', '--check', '.'], fixCommand: ['oxfmt', '.'] } +const notFixable = { checkCommand: ['tsc', '--noEmit'] } describe('selectCommand', () => { it('uses the fix command in fix mode when the check is fixable', () => { - expect(selectCommand(fixable, 'fix')).toBe('oxfmt .') + expect(selectCommand(fixable, 'fix')).toEqual(['oxfmt', '.']) }) it('uses the check command in check mode', () => { - expect(selectCommand(fixable, 'check')).toBe('oxfmt --check .') + expect(selectCommand(fixable, 'check')).toEqual(['oxfmt', '--check', '.']) }) it('always uses the check command when the check has no fix command', () => { - expect(selectCommand(notFixable, 'fix')).toBe('tsc --noEmit') - expect(selectCommand(notFixable, 'check')).toBe('tsc --noEmit') + expect(selectCommand(notFixable, 'fix')).toEqual(['tsc', '--noEmit']) + expect(selectCommand(notFixable, 'check')).toEqual(['tsc', '--noEmit']) }) }) describe('externalFailureHint', () => { - it('names the tool, the exact command that ran, and the docs link', () => { - const hint = externalFailureHint({ name: 'unused-code', bin: 'knip', docs: 'https://knip.dev' }, 'knip --no-progress') + it('names the tool, the exact argv that ran, and the docs link', () => { + const hint = externalFailureHint({ name: 'unused-code', bin: 'knip', docs: 'https://knip.dev' }, ['knip', '--no-progress']) expect(hint).toContain('unused-code') expect(hint).toContain('knip') - expect(hint).toContain('knip --no-progress') + expect(hint).toContain('["knip","--no-progress"]') expect(hint).toContain('https://knip.dev') }) it('still names the tool and command when no docs link is set', () => { - const hint = externalFailureHint({ name: 'circular-deps', bin: 'skott' }, 'skott src') + const hint = externalFailureHint({ name: 'circular-deps', bin: 'skott' }, ['skott', 'src']) expect(hint).toContain('skott') - expect(hint).toContain('skott src') + expect(hint).toContain('["skott","src"]') expect(hint).not.toContain('undefined') }) }) -describe('appendArgs', () => { - it('appends passthrough args verbatim, unquoted (so shell globs still expand)', () => { - expect(appendArgs('skott --showCircularDependencies', ['src/*.ts'])).toBe('skott --showCircularDependencies src/*.ts') - }) - - it('returns the command unchanged when there are no extra args', () => { - expect(appendArgs('oxlint .', [])).toBe('oxlint .') - expect(appendArgs('oxlint .')).toBe('oxlint .') - }) -}) - describe('defineExternalCheck', () => { it('exposes raw commands for eject, including the fix variant when fixable', () => { const lint = defineExternalCheck({ name: 'lint', description: '', bin: 'oxlint', - checkCommand: 'oxlint .', - fixCommand: 'oxlint --fix .', + checkCommand: ['oxlint', '.'], + fixCommand: ['oxlint', '--fix', '.'], devDeps: [], }) expect(lint.eject).toEqual({ check: 'oxlint .', fix: 'oxlint --fix .' }) }) - it('scaffolds default trailing args after `--` so a consumer can see and tweak them', () => { + it('scaffolds a bare CLI call and serializes argv only at the eject boundary', () => { const circular = defineExternalCheck({ name: 'circular-deps', description: '', bin: 'skott', - checkCommand: 'skott', + checkCommand: ['skott'], devDeps: [], - scaffoldArgs: 'src/*.ts', }) - expect(circular.scaffold.script).toBe('verifyx circular-deps -- src/*.ts') + expect(circular.scaffold.script).toBe('verifyx circular-deps') expect(circular.eject).toEqual({ check: 'skott', fix: undefined }) }) it('scaffolds a bare CLI call when there are no default args', () => { - const knip = defineExternalCheck({ name: 'unused-code', description: '', bin: 'knip', checkCommand: 'knip', devDeps: [] }) + const knip = defineExternalCheck({ name: 'unused-code', description: '', bin: 'knip', checkCommand: ['knip'], devDeps: [] }) expect(knip.scaffold.script).toBe('verifyx unused-code') }) + + it('serializes eject lazily, so an unserializable command fails eject rather than check construction', () => { + const check = defineExternalCheck({ name: 'x', description: '', bin: 'tool', checkCommand: ['tool', '$HOME'], devDeps: [] }) + expect(() => check.eject).toThrow('cannot safely serialize') + }) }) describe('runCountedBudget', () => { - const spec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } + const spec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: ['jscpd', 'src'], docs: 'https://x' } const budget = (count: number, report = '') => ({ strategy: 'count' as const, unit: 'clone', count: async () => ({ count, report }) }) it('passes when the finding count is at or below the budget', async () => { expect(await runCountedBudget(spec, budget(5), 5, [], {})).toEqual({ name: 'duplicate-code', ok: true }) }) + it('passes one prebuilt argv, including passthrough args, to the counter', async () => { + const count = vi.fn(async () => ({ count: 0, report: '' })) + await runCountedBudget(spec, { strategy: 'count', unit: 'clone', count }, 5, ['--ignore', 'has space'], {}) + expect(count).toHaveBeenCalledWith({ argv: ['jscpd', 'src', '--ignore', 'has space'], env: {} }) + }) + it('fails over budget, printing the counting run’s report instead of re-running the tool', async () => { const transformingSpec = { ...spec, transformOutput: (output: string) => output.toUpperCase() } const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) diff --git a/src/checks/external.ts b/src/checks/external.ts index e43230f..9ab4c1b 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -4,13 +4,12 @@ import path from 'node:path' import { color } from '../shared/color.ts' import { resolveMode } from '../shared/mode.ts' import { emit } from '../shared/output.ts' -import { appendArgs, runCommand } from '../shared/spawn.ts' +import { appendArgv, formatArgv, formatShellCommand, runArgvCommand } from '../shared/spawn.ts' import { type MaxWarningsSupport, withinBudget } from './maxWarnings.ts' import type { Check, CheckMode, CheckResult, RunDefaultOptions } from './types.ts' -export { appendArgs } from '../shared/spawn.ts' - -const BIN_EXTENSIONS = ['', '.cmd', '.ps1', '.exe'] +// Mirror the launcher's resolution (cross-spawn → which + PATHEXT) so "installed" and "runnable" agree. +const BIN_EXTENSIONS = process.platform === 'win32' ? (process.env.PATHEXT ?? '.EXE;.CMD;.BAT;.COM').split(';').filter(Boolean) : [''] /** True when a project-local binary is installed under node_modules/.bin (cross-platform). */ function hasLocalBin(bin: string, cwd: string = process.cwd()): boolean { @@ -34,9 +33,9 @@ export type ExternalCheckSpec = { /** The node_modules/.bin executable that must be present for the check to run. */ bin: string /** Command run in check mode (report + fail, never rewrite). */ - checkCommand: string + checkCommand: readonly string[] /** Command run in fix mode. When omitted, the check is not fixable and always runs `checkCommand`. */ - fixCommand?: string + fixCommand?: readonly string[] devDeps: string[] recommended?: boolean /** Docs / config reference for the underlying tool, surfaced when the check fails. */ @@ -45,26 +44,21 @@ export type ExternalCheckSpec = { canRun?: () => boolean /** Rewrite the tool's captured output before it is printed, e.g. to strip a tool's own hardcoded colouring. */ transformOutput?: (output: string) => string - /** - * Default trailing args scaffolded after `--` (e.g. skott's `src/*.ts` target), surfaced in the `verify:` - * script so a consumer can see and tweak them. Not baked into `runDefault`; only the scaffolded script carries them. - */ - scaffoldArgs?: string maxWarnings?: MaxWarningsSupport } /** Pick the command for the run mode: the fix command only in fix mode and only when the check is fixable. */ -export function selectCommand(spec: Pick, mode: CheckMode): string { +export function selectCommand(spec: Pick, mode: CheckMode): readonly string[] { return mode === 'fix' && spec.fixCommand ? spec.fixCommand : spec.checkCommand } /** * The line printed when an external check fails: names the tool (checks are named for their function, so the - * tool is otherwise hidden), the exact command that ran, and where to configure it — so an agent can set up + * tool is otherwise hidden), the exact argv that ran, and where to configure it — so an agent can set up * the tool's config file (e.g. knip.json) without guessing. */ -export function externalFailureHint(spec: Pick, command: string): string { - return `↳ ${spec.name} uses ${spec.bin}: ran \`${command}\`. Configure ${spec.bin}${spec.docs ? ` — ${spec.docs}` : ''}.` +export function externalFailureHint(spec: Pick, argv: readonly string[]): string { + return `↳ ${spec.name} uses ${spec.bin}: ran argv \`${formatArgv(argv)}\`. Configure ${spec.bin}${spec.docs ? ` — ${spec.docs}` : ''}.` } type CountBudget = Extract @@ -77,9 +71,10 @@ export async function runCountedBudget( extraArgs: string[], env: Record, ): Promise { + const argv = appendArgv(spec.checkCommand, extraArgs) let counted: { count: number; report: string } try { - counted = await budget.count({ extraArgs, env, checkCommand: spec.checkCommand }) + counted = await budget.count({ argv, env }) } catch (error) { const docs = spec.docs ? ` — ${spec.docs}` : '' console.error( @@ -95,11 +90,11 @@ export async function runCountedBudget( const unit = count === 1 ? budget.unit : `${budget.unit}s` console.error(color.dim(`↳ ${spec.name}: ${count} ${unit} found, exceeds --max-warnings ${maxWarnings}.`)) if (report) emit(spec.transformOutput ? spec.transformOutput(report) : report) - console.error(color.dim(externalFailureHint(spec, appendArgs(spec.checkCommand, extraArgs)))) + console.error(color.dim(externalFailureHint(spec, argv))) return { name: spec.name, ok: false } } -/** Build a Check that shells out to an external tool, skipping gracefully when the tool cannot run. */ +/** Build a Check that spawns an external tool, skipping gracefully when the tool cannot run. */ export function defineExternalCheck(spec: ExternalCheckSpec): Check { return { name: spec.name, @@ -109,11 +104,17 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { supportsMaxWarnings: !!spec.maxWarnings, // Scaffold as a call into this CLI so fix-vs-check lives in one place, not the consumer's script. scaffold: { - script: spec.scaffoldArgs ? `verifyx ${spec.name} -- ${spec.scaffoldArgs}` : `verifyx ${spec.name}`, + script: `verifyx ${spec.name}`, devDeps: spec.devDeps, }, - // `verifyx eject ` inlines these raw commands into the consumer's verify:* scripts. - eject: { check: spec.checkCommand, fix: spec.fixCommand }, + // `verifyx eject ` inlines these raw commands into the consumer's verify:* scripts. Lazy so + // formatShellCommand's unserializable-arg throw fires on eject only, not when CHECKS is built at import. + get eject() { + return { + check: formatShellCommand(spec.checkCommand), + fix: spec.fixCommand ? formatShellCommand(spec.fixCommand) : undefined, + } + }, async runDefault({ extraArgs = [], maxWarnings }: RunDefaultOptions = {}): Promise { if (!hasLocalBin(spec.bin)) { console.log(color.dim(`${spec.name}: ${spec.bin} not installed — skipping (add it with \`npx verifyx init\`)`)) @@ -124,17 +125,17 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { return { name: spec.name, ok: true, skipped: true } } // quiet: buffer the tool's output and flush only on failure (streamed live under --verbose). - const runReport = async (command: string): Promise => { - const code = await runCommand(command, { env: envWithLocalBin(), quiet: true, transform: spec.transformOutput }) - if (code !== 0) console.error(color.dim(externalFailureHint(spec, command))) + const runReport = async (argv: string[]): Promise => { + const code = await runArgvCommand(argv, { env: envWithLocalBin(), quiet: true, transform: spec.transformOutput }) + if (code !== 0) console.error(color.dim(externalFailureHint(spec, argv))) return { name: spec.name, ok: code === 0 } } if (maxWarnings !== undefined && spec.maxWarnings) { const budget = spec.maxWarnings if (budget.strategy === 'count') return runCountedBudget(spec, budget, maxWarnings, extraArgs, envWithLocalBin()) - return runReport(appendArgs(selectCommand(spec, resolveMode()), [...extraArgs, ...budget.toArgs(maxWarnings)])) + return runReport(appendArgv(selectCommand(spec, resolveMode()), [...extraArgs, ...budget.toArgs(maxWarnings)])) } - return runReport(appendArgs(selectCommand(spec, resolveMode()), extraArgs)) + return runReport(appendArgv(selectCommand(spec, resolveMode()), extraArgs)) }, } } diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts index 3cf852b..fda53fa 100644 --- a/src/checks/maxWarnings.ts +++ b/src/checks/maxWarnings.ts @@ -2,9 +2,9 @@ import fs from 'node:fs' import os from 'node:os' import path from 'node:path' -import { appendArgs, captureCommand } from '../shared/spawn.ts' +import { captureArgvCommand } from '../shared/spawn.ts' -type MaxWarningsCountContext = { extraArgs: string[]; env: Record; checkCommand: string } +type MaxWarningsCountContext = { argv: readonly string[]; env: Record } /** The finding count plus the tool's rendered console report, so a failing budget can print it without a second run. */ export type CountResult = { count: number; report: string } @@ -29,8 +29,8 @@ export function countJscpdClones(report: { statistics?: { total?: { clones?: unk export async function jscpdCount(ctx: MaxWarningsCountContext): Promise { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verifyx-jscpd-')) try { - const command = `${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}"` - const { code, stdout, stderr } = await captureCommand(command, { env: ctx.env }) + const argv = [...ctx.argv, '--reporters', 'json', '--output', dir] + const { code, stdout, stderr } = await captureArgvCommand(argv, { env: ctx.env }) const reportPath = path.join(dir, 'jscpd-report.json') if (!fs.existsSync(reportPath)) throw new Error(`jscpd produced no report (exit ${code})${stderr.trim() ? `: ${stderr.trim()}` : ''}`) const count = countJscpdClones(JSON.parse(fs.readFileSync(reportPath, 'utf8'))) diff --git a/src/checks/registry.test.ts b/src/checks/registry.test.ts index 4dd066c..3383509 100644 --- a/src/checks/registry.test.ts +++ b/src/checks/registry.test.ts @@ -39,8 +39,8 @@ describe('check registry', () => { expect(getCheck('complexity')?.scaffold.script).toBe('verifyx complexity') }) - it('scaffolds circular-deps with a default skott target after `--`', () => { - expect(getCheck('circular-deps')?.scaffold.script).toBe('verifyx circular-deps -- src/*.ts') + it('scaffolds circular-deps without a shell-dependent glob', () => { + expect(getCheck('circular-deps')?.scaffold.script).toBe('verifyx circular-deps') }) it('exposes raw tool commands for eject on external checks, but not native ones', () => { diff --git a/src/checks/registry.ts b/src/checks/registry.ts index a89826f..9daf54a 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -37,8 +37,8 @@ export const CHECKS: Check[] = [ name: 'lint', description: 'Lint — auto-fixes locally, checks in CI', bin: 'oxlint', - checkCommand: 'oxlint .', - fixCommand: 'oxlint --fix .', + checkCommand: ['oxlint', '.'], + fixCommand: ['oxlint', '--fix', '.'], devDeps: ['oxlint'], recommended: true, docs: 'https://oxc.rs/docs/guide/usage/linter.html', @@ -47,8 +47,8 @@ export const CHECKS: Check[] = [ name: 'format', description: 'Formatting — writes locally, checks in CI', bin: 'oxfmt', - checkCommand: 'oxfmt --check .', - fixCommand: 'oxfmt .', + checkCommand: ['oxfmt', '--check', '.'], + fixCommand: ['oxfmt', '.'], devDeps: ['oxfmt'], recommended: true, docs: 'https://oxc.rs', @@ -57,7 +57,7 @@ export const CHECKS: Check[] = [ name: 'check-types', description: 'TypeScript type check', bin: 'tsc', - checkCommand: 'tsc --noEmit', + checkCommand: ['tsc', '--noEmit'], devDeps: ['typescript'], canRun: () => fs.existsSync('tsconfig.json'), recommended: true, @@ -67,7 +67,7 @@ export const CHECKS: Check[] = [ name: 'unused-code', description: 'Unused files, exports and dependencies', bin: 'knip', - checkCommand: 'knip --no-progress --treat-config-hints-as-errors', + checkCommand: ['knip', '--no-progress', '--treat-config-hints-as-errors'], devDeps: ['knip'], docs: 'https://knip.dev/reference/configuration', maxWarnings: { strategy: 'flag', toArgs: (n) => ['--max-issues', String(n)] }, @@ -76,17 +76,15 @@ export const CHECKS: Check[] = [ name: 'circular-deps', description: 'Circular dependency detection', bin: 'skott', - checkCommand: 'skott --displayMode=raw --showCircularDependencies --exitCodeOnCircularDependencies=1', + checkCommand: ['skott', '--displayMode=raw', '--showCircularDependencies', '--exitCodeOnCircularDependencies=1'], devDeps: ['skott'], docs: 'https://github.com/antoine-coulon/skott', - // skott needs a target; scaffold it after `--` so consumers can see and adjust it (e.g. to their source layout). - scaffoldArgs: 'src/*.ts', }), defineExternalCheck({ name: 'duplicate-code', description: 'Copy-paste / duplicate-code detection', bin: 'jscpd', - checkCommand: 'jscpd --format typescript,tsx --exit-code 1 --ignore "**/*.test.*" -r consoleFull src', + checkCommand: ['jscpd', '--format', 'typescript,tsx', '--exit-code', '1', '--ignore', '**/*.test.*', '-r', 'consoleFull', 'src'], devDeps: ['jscpd'], // jscpd hardcodes red header cells in its stats table (even on a clean run) — reads like a failure. It ignores // NO_COLOR/FORCE_COLOR, so strip the red foreground from its output; the table renders in the default colour. diff --git a/src/checks/types.ts b/src/checks/types.ts index 0a248fd..4167564 100644 --- a/src/checks/types.ts +++ b/src/checks/types.ts @@ -13,12 +13,12 @@ export type CheckResult = { /** Per-run options for a check. `extraArgs` are the tokens a user passes after `--` (external checks only). */ export type RunDefaultOptions = { - /** Extra arguments appended verbatim to an external check's underlying command (e.g. `verifyx circular-deps -- src/*.ts`). */ + /** Extra arguments appended verbatim to an external check's underlying argv (e.g. `verifyx circular-deps -- src/index.ts`). */ extraArgs?: string[] maxWarnings?: number } -/** A single verification. Native checks run in-process; external checks shell out to a tool. */ +/** A single verification. Native checks run in-process; external checks spawn a tool. */ export type Check = { name: string description: string diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index 8ef879c..0217b05 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -82,7 +82,7 @@ export function registerChecks(program: Command): void { const command = program .command(check.name) .description(check.description) - // Everything after `--` is forwarded verbatim to the underlying tool (e.g. `verifyx circular-deps -- src/*.ts`). + // Everything after `--` is forwarded verbatim to the underlying tool (e.g. `verifyx unused-code -- --production`). .argument('[toolArgs...]', 'extra arguments passed through to the underlying tool (after `--`)') if (check.supportsMaxWarnings) { command.option('--max-warnings ', 'tolerate up to n findings before failing (counts findings)', parseMaxWarnings) diff --git a/src/orchestrator/run.ts b/src/orchestrator/run.ts index 117ddcf..a7695de 100644 --- a/src/orchestrator/run.ts +++ b/src/orchestrator/run.ts @@ -1,5 +1,5 @@ import { configureMode, resolveMode } from '../shared/mode.ts' -import { runCommand, setVerbose } from '../shared/spawn.ts' +import { runShellCommand, setVerbose } from '../shared/spawn.ts' import { type MeasureRecord, printMeasureTable } from './measure.ts' import { chatty, reportOutcomes } from './report.ts' import { entryCheckName, resolveEntries, selectEntries, type VerifyEntry } from './resolveEntries.ts' @@ -16,7 +16,7 @@ export type OrchestrateOptions = { async function runEntry(entry: VerifyEntry): Promise { const startTime = Date.now() - const code = await runCommand(entry.command, { cwd: entry.cwd, quiet: true }) + const code = await runShellCommand(entry.command, { cwd: entry.cwd, quiet: true }) return { script: entry.name, code, durationMs: Date.now() - startTime } } diff --git a/src/orchestrator/runAll.ts b/src/orchestrator/runAll.ts index 07ff7dd..8cecd68 100644 --- a/src/orchestrator/runAll.ts +++ b/src/orchestrator/runAll.ts @@ -2,7 +2,7 @@ import { CHECKS } from '../checks/registry.ts' import { color, paintRed } from '../shared/color.ts' import { resolveMode } from '../shared/mode.ts' import { installConsoleCapture, runCaptured } from '../shared/output.ts' -import { runCommand } from '../shared/spawn.ts' +import { runShellCommand } from '../shared/spawn.ts' import { type MeasureRecord, printMeasureTable } from './measure.ts' import { reportOutcomes } from './report.ts' import { entryCheckName, resolveEntries, resolveOverride, selectEntries } from './resolveEntries.ts' @@ -19,7 +19,7 @@ function buildTasks(opts: RunAllOptions): Task[] { const spawn = (name: string, command: string, cwd: string, note?: string): Task => ({ name, note, - run: async () => (await runCommand(command, { cwd, quiet: true })) === 0, + run: async () => (await runShellCommand(command, { cwd, quiet: true })) === 0, }) const tasks: Task[] = CHECKS.map((check) => { @@ -29,7 +29,7 @@ function buildTasks(opts: RunAllOptions): Task[] { name: check.name, note: 'overridden', run: async () => { - const ok = (await runCommand(override.command, { cwd: override.cwd, quiet: true })) === 0 + const ok = (await runShellCommand(override.command, { cwd: override.cwd, quiet: true })) === 0 if (!ok) console.error(color.dim(`↳ ${check.name}: ran \`${override.command}\` (override)`)) return ok }, diff --git a/src/scaffold/eject.ts b/src/scaffold/eject.ts index e0340e6..11178fa 100644 --- a/src/scaffold/eject.ts +++ b/src/scaffold/eject.ts @@ -12,7 +12,7 @@ export type EjectResult = { export function ejectScripts(name: string): Record { const check = getCheck(name) if (!check) { - const known = CHECKS.filter((c) => c.eject) + const known = CHECKS.filter((c) => c.kind === 'external') .map((c) => c.name) .join(', ') throw new Error(`Unknown check "${name}". Ejectable checks: ${known}.`) diff --git a/src/scaffold/installDeps.ts b/src/scaffold/installDeps.ts index 976e228..9a53a1a 100644 --- a/src/scaffold/installDeps.ts +++ b/src/scaffold/installDeps.ts @@ -1,9 +1,9 @@ import fs from 'node:fs' import path from 'node:path' -import { runCommand } from '../shared/spawn.ts' +import { runShellCommand } from '../shared/spawn.ts' -/** Signature of the command runner (defaults to `runCommand`); injectable so tests don't shell out. */ +/** Signature of the command runner (defaults to `runShellCommand`); injectable so tests don't shell out. */ export type Runner = (command: string, opts: { cwd: string }) => Promise export type InstallReport = { @@ -41,7 +41,7 @@ function isInstalled(name: string, cwd: string, declared: Set): boolean * fails — typically a single peer-dependency conflict — falls back to per-package installs so one bad package * can't block the rest, and the exact failures are reported instead of aborting `init`. */ -export async function installDevDeps(devDeps: readonly string[], cwd: string, run: Runner = runCommand): Promise { +export async function installDevDeps(devDeps: readonly string[], cwd: string, run: Runner = runShellCommand): Promise { const declared = declaredDeps(cwd) const skipped: string[] = [] const toInstall: string[] = [] diff --git a/src/shared/spawn.test.ts b/src/shared/spawn.test.ts index ee7b992..67b0853 100644 --- a/src/shared/spawn.test.ts +++ b/src/shared/spawn.test.ts @@ -1,22 +1,98 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + import { describe, expect, it } from 'vitest' -import { captureCommand } from './spawn.ts' +import { appendArgv, captureArgvCommand, formatArgv, formatShellCommand } from './spawn.ts' -describe('captureCommand', () => { +describe('captureArgvCommand', () => { it('returns the captured stdout and a zero exit code for a successful command', async () => { - const { code, stdout } = await captureCommand(`node -e "process.stdout.write('captured-output')"`) + const command = + process.platform === 'win32' ? [process.execPath, '-e', "process.stdout.write('captured-output')"] : ['printf', 'captured-output'] + const { code, stdout } = await captureArgvCommand(command) expect(code).toBe(0) expect(stdout).toBe('captured-output') }) it('surfaces a non-zero exit code without throwing', async () => { - const { code } = await captureCommand(`node -e "process.exit(3)"`) + const command = process.platform === 'win32' ? [process.execPath, '-e', 'process.exit(3)'] : ['sh', '-c', 'exit 3'] + const { code } = await captureArgvCommand(command) expect(code).toBe(3) }) it('captures stderr separately from stdout', async () => { - const { stdout, stderr } = await captureCommand(`node -e "process.stderr.write('to-stderr')"`) + const command = + process.platform === 'win32' ? [process.execPath, '-e', "process.stderr.write('to-stderr')"] : ['sh', '-c', 'printf to-stderr >&2'] + const { stdout, stderr } = await captureArgvCommand(command) expect(stdout).toBe('') expect(stderr).toContain('to-stderr') }) + + it('returns the spawn error when an argv executable cannot be launched', async () => { + const { code, stderr } = await captureArgvCommand(['verifyx-command-that-does-not-exist']) + expect(code).toBe(127) + expect(stderr).toContain('ENOENT') + }) + + // Regression: forwarded passthrough args used to be joined into a shell string, so the spawn shell + // glob-expanded/word-split them before the tool saw them. An argv array must reach the tool verbatim. + it('passes an argv array to the process with no shell, so globs and metacharacters arrive literally', async () => { + const passthrough = ['--ignore', '**/generated/**', 'has space', '$HOME', ';whoami'] + const command = + process.platform === 'win32' + ? [process.execPath, '-e', 'process.stdout.write(JSON.stringify(process.argv.slice(1)))', '--', ...passthrough] + : ['printf', '%s\\n', ...passthrough] + const { code, stdout } = await captureArgvCommand(command) + expect(code).toBe(0) + expect(process.platform === 'win32' ? JSON.parse(stdout) : stdout.split('\n').slice(0, -1)).toEqual(passthrough) + }) + + it.runIf(process.platform === 'win32')('launches npm-style .cmd shims without interpreting their arguments', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'verifyx-spawn-')) + const binDir = path.join(root, 'node_modules', '.bin') + fs.mkdirSync(binDir, { recursive: true }) + fs.writeFileSync(path.join(binDir, 'verifyx-echo-argv.cjs'), 'process.stdout.write(JSON.stringify(process.argv.slice(2)))') + fs.writeFileSync(path.join(binDir, 'verifyx-echo-argv.cmd'), '@ECHO off\r\nnode "%~dp0\\verifyx-echo-argv.cjs" %*\r\n') + + const pathKey = Object.keys(process.env).find((key) => key.toLowerCase() === 'path') ?? 'PATH' + const env = { [pathKey]: `${binDir}${path.delimiter}${process.env[pathKey] ?? ''}` } + const passthrough = ['**/*.ts', 'has space', '$HOME', '%PATH%', ';whoami', 'a&b'] + + try { + const { code, stdout, stderr } = await captureArgvCommand(['verifyx-echo-argv', ...passthrough], { env }) + expect({ code, stderr }).toEqual({ code: 0, stderr: '' }) + expect(JSON.parse(stdout)).toEqual(passthrough) + } finally { + fs.rmSync(root, { recursive: true, force: true, maxRetries: 3 }) + } + }) +}) + +describe('appendArgv', () => { + it('appends passthrough args as their own literal entries', () => { + expect(appendArgv(['jscpd', 'src'], ['--ignore', '**/generated/**', 'has space'])).toEqual([ + 'jscpd', + 'src', + '--ignore', + '**/generated/**', + 'has space', + ]) + }) +}) + +describe('formatArgv', () => { + it('renders exact argv without making platform-specific shell quoting claims', () => { + expect(formatArgv(['tool', 'C:\\src path', '$HOME', '%PATH%'])).toBe('["tool","C:\\\\src path","$HOME","%PATH%"]') + }) +}) + +describe('formatShellCommand', () => { + it('serializes built-in argv for package.json scripts on POSIX and Windows', () => { + expect(formatShellCommand(['jscpd', '--ignore', '**/*.test.*', 'src'])).toBe('jscpd --ignore "**/*.test.*" src') + }) + + it('rejects arguments that cannot be serialized consistently for both shells', () => { + expect(() => formatShellCommand(['tool', '$HOME'])).toThrow('cannot safely serialize') + }) }) diff --git a/src/shared/spawn.ts b/src/shared/spawn.ts index df141ed..e908615 100644 --- a/src/shared/spawn.ts +++ b/src/shared/spawn.ts @@ -1,10 +1,36 @@ -import { spawn } from 'node:child_process' +import spawn from 'cross-spawn' import { emit, isCapturing } from './output.ts' -// Keep passthrough globs unquoted for shell expansion. -export function appendArgs(command: string, extraArgs: readonly string[] = []): string { - return extraArgs.length > 0 ? `${command} ${extraArgs.join(' ')}` : command +/** Append user-supplied arguments without parsing, splitting, or shell expansion. */ +export function appendArgv(command: readonly string[], extraArgs: readonly string[] = []): string[] { + return [...command, ...extraArgs] +} + +/** Render the exact argv used by a process in a platform-neutral diagnostic form. */ +export function formatArgv(argv: readonly string[]): string { + return JSON.stringify(argv) +} + +const SAFE_SHELL_ARG = /^[A-Za-z0-9_@+=:,./-]+$/ +const PORTABLE_QUOTED_ARG = /^[^"\\$`%!\r\n]*$/ + +function formatShellArg(arg: string): string { + if (arg.length > 0 && SAFE_SHELL_ARG.test(arg)) return arg + if (!PORTABLE_QUOTED_ARG.test(arg)) { + throw new Error(`cannot safely serialize argument for both POSIX and Windows npm scripts: ${JSON.stringify(arg)}`) + } + return `"${arg}"` +} + +function assertNonEmptyArgv(argv: readonly string[]): asserts argv is readonly [string, ...string[]] { + if (argv.length === 0) throw new Error('argv command must have at least one entry (the executable)') +} + +/** Serialize a trusted built-in argv for a cross-platform package.json script. */ +export function formatShellCommand(argv: readonly string[]): string { + assertNonEmptyArgv(argv) + return argv.map(formatShellArg).join(' ') } let verboseMode = false @@ -23,27 +49,39 @@ function shouldSuppress(quiet?: boolean): boolean { return !!quiet || !!process.env.CLAUDECODE } -export type RunCommandOptions = { - cwd?: string - env?: Record +type SpawnInvocation = { file: string; args: string[]; shell: boolean } + +function shellInvocation(command: string): SpawnInvocation { + return { file: command, args: [], shell: true } +} + +function argvInvocation(argv: readonly string[]): SpawnInvocation { + assertNonEmptyArgv(argv) + const [file, ...args] = argv + return { file, args, shell: false } +} + +type SpawnOptions = { cwd?: string; env?: Record } + +function spawnChild(invocation: SpawnInvocation, stdio: 'inherit' | 'pipe' | ['ignore', 'pipe', 'pipe'], opts: SpawnOptions) { + return spawn(invocation.file, invocation.args, { + stdio, + shell: invocation.shell, + cwd: opts.cwd ?? process.cwd(), + env: opts.env ? { ...process.env, ...opts.env } : undefined, + }) +} + +export type RunCommandOptions = SpawnOptions & { quiet?: boolean /** Rewrite the command's buffered output before it is emitted (only applies when output is suppressed/buffered). */ transform?: (output: string) => string } -/** - * Run a shell command, returning its exit code. Suppressed output (quiet, or under Claude Code) is buffered - * and flushed to stdout only if the command fails, keeping passing runs quiet. - */ -export function runCommand(command: string, opts: RunCommandOptions = {}): Promise { +function runInvocation(invocation: SpawnInvocation, opts: RunCommandOptions): Promise { return new Promise((resolve) => { const suppress = shouldSuppress(opts.quiet) - const child = spawn(command, [], { - stdio: suppress ? 'pipe' : 'inherit', - shell: true, - cwd: opts.cwd ?? process.cwd(), - env: opts.env ? { ...process.env, ...opts.env } : undefined, - }) + const child = spawnChild(invocation, suppress ? 'pipe' : 'inherit', opts) const chunks: Buffer[] = [] if (suppress) { child.stdout?.on('data', (data: Buffer) => chunks.push(data)) @@ -58,21 +96,30 @@ export function runCommand(command: string, opts: RunCommandOptions = {}): Promi } resolve(exitCode) }) - child.on('error', () => resolve(127)) + child.on('error', (error) => { + emit(`${String(error)}\n`, 'err') + resolve(127) + }) }) } -export function captureCommand( - command: string, - opts: { cwd?: string; env?: Record } = {}, +/** Run a consumer-owned command through the platform shell. */ +export function runShellCommand(command: string, opts: RunCommandOptions = {}): Promise { + return runInvocation(shellInvocation(command), opts) +} + +/** Run a structured argv directly, using no shell. */ +export function runArgvCommand(argv: readonly string[], opts: RunCommandOptions = {}): Promise { + return runInvocation(argvInvocation(argv), opts) +} + +/** Run a structured argv directly and capture stdout/stderr separately. */ +export function captureArgvCommand( + argv: readonly string[], + opts: SpawnOptions = {}, ): Promise<{ code: number; stdout: string; stderr: string }> { return new Promise((resolve) => { - const child = spawn(command, [], { - stdio: ['ignore', 'pipe', 'pipe'], - shell: true, - cwd: opts.cwd ?? process.cwd(), - env: opts.env ? { ...process.env, ...opts.env } : undefined, - }) + const child = spawnChild(argvInvocation(argv), ['ignore', 'pipe', 'pipe'], opts) const out: Buffer[] = [] const err: Buffer[] = [] child.stdout?.on('data', (data: Buffer) => out.push(data)) @@ -80,6 +127,6 @@ export function captureCommand( child.on('close', (code) => { resolve({ code: code ?? 1, stdout: Buffer.concat(out).toString(), stderr: Buffer.concat(err).toString() }) }) - child.on('error', () => resolve({ code: 127, stdout: '', stderr: '' })) + child.on('error', (error) => resolve({ code: 127, stdout: '', stderr: String(error) })) }) }