From cb5849a64ee9d3c749992303a995050e1def3009 Mon Sep 17 00:00:00 2001 From: "Rob Moore (MakerX)" Date: Sat, 11 Jul 2026 12:55:07 +0800 Subject: [PATCH] fix(init): skip installed deps, isolate install failures, pre-select prompt defaults Make `verifyx init` robust to real-world install and prompt issues: - Skip devDeps already declared in package.json (deps/dev/peer/optional) or present in node_modules, so an existing typescript/oxlint is never re-installed or version-bumped and known peer conflicts (e.g. oxlint/oxlint-tsgolint) are sidestepped when the tool is already present. - Install missing deps in one batch; on failure fall back to per-package installs to isolate the culprit(s), install the rest, and report exactly what to install manually at the end instead of aborting init. - Report the scaffold (scripts + agent files) before the install so a failing install can no longer hide or prevent the .claude/CLAUDE.md output. - Fix the interactive prompts: enquirer multiselect ignores each choice's `enabled` for the submitted result (a plain enter returns []), so agent files and recommended checks were silently dropped. Pass the enabled names as `initial` to actually pre-select them. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 +- src/commands/registerInit.test.ts | 26 ++++++++++ src/commands/registerInit.ts | 45 +++++++++++++--- src/scaffold/installDeps.test.ts | 85 +++++++++++++++++++++++++++++++ src/scaffold/installDeps.ts | 64 +++++++++++++++++++++++ 5 files changed, 213 insertions(+), 9 deletions(-) create mode 100644 src/commands/registerInit.test.ts create mode 100644 src/scaffold/installDeps.test.ts create mode 100644 src/scaffold/installDeps.ts diff --git a/README.md b/README.md index 6c7ea97..befd121 100644 --- a/README.md +++ b/README.md @@ -232,7 +232,7 @@ verifyx init It first asks how `verify` should run: **run all built-in checks** (`verifyx all`, no `verify:*` scripts) or **pick specific checks** to wire up. Then you multi-select **agent targets** (Claude and/or other agents), and, if you chose to pick, the **checks**. After that it: - writes the selected `verify:*` scripts to `package.json` (never clobbering existing ones), -- installs the external checks' tools as `--save-dev`, +- installs the external checks' tools as `--save-dev`, **skipping any already declared in `package.json` or present in `node_modules`** (so an existing `typescript`/`oxlint` is never re-installed or version-bumped). If the install hits a conflict (e.g. a peer-dependency clash), it isolates the failing package(s), installs the rest, and reports what to install manually at the end instead of aborting, - writes the **`verify` skill**: the same `SKILL.md` to `.claude/skills/verify/` (Claude) and `.agent-skills/verify/` (cross-vendor), so the integration is identical everywhere, - appends a one-line pointer to `CLAUDE.md` / `AGENTS.md` (only if not already present; existing content is never rewritten), - if `unused-code` is selected, adds the other external tools (`oxlint`/`oxfmt`/`skott`/`jscpd`) to knip's `ignoreDependencies` (verifyx runs them at runtime, so knip can't see them and would otherwise report them as unused). Merged into `knip.json` or `package.json#knip` (created if neither exists), adding only what's missing; a code-based `knip.ts`/`knip.js` is left for you to edit. diff --git a/src/commands/registerInit.test.ts b/src/commands/registerInit.test.ts new file mode 100644 index 0000000..cea5b38 --- /dev/null +++ b/src/commands/registerInit.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest' + +import { type Choice, preSelected } from './registerInit.ts' + +describe('preSelected', () => { + // The interactive target/check prompts rely on this to pre-select defaults. enquirer's multiselect ignores each + // choice's `enabled` for the submitted result (it only paints a marker) — a plain enter returns []; the enabled + // names must be handed back as `initial`. See the comment in registerInit's `ask`. Verified end-to-end manually. + it('returns the names of the enabled choices as the `initial` selection', () => { + const choices: Choice[] = [ + { name: 'claude', message: 'Claude', enabled: true }, + { name: 'agents', message: 'Other agents', enabled: false }, + ] + expect(preSelected(choices)).toEqual(['claude']) + }) + + it('returns every enabled choice and nothing else', () => { + const choices: Choice[] = [ + { name: 'a', message: 'A', enabled: true }, + { name: 'b', message: 'B' }, + { name: 'c', message: 'C', enabled: false }, + { name: 'd', message: 'D', enabled: true }, + ] + expect(preSelected(choices)).toEqual(['a', 'd']) + }) +}) diff --git a/src/commands/registerInit.ts b/src/commands/registerInit.ts index d962c9c..32aa9a7 100644 --- a/src/commands/registerInit.ts +++ b/src/commands/registerInit.ts @@ -4,18 +4,27 @@ import enquirer from 'enquirer' import { CHECKS, recommendedChecks } from '../checks/registry.ts' import type { AgentTarget } from '../scaffold/agentFiles.ts' import { applyInit, type InitResult } from '../scaffold/init.ts' +import { type InstallReport, installDevDeps } from '../scaffold/installDeps.ts' import { ACTION_MARK } from '../scaffold/writeManaged.ts' import { color } from '../shared/color.ts' -import { runCommand } from '../shared/spawn.ts' -type Choice = { name: string; message: string; enabled?: boolean } +export type Choice = { name: string; message: string; enabled?: boolean } function collect(value: string, previous: string[]): string[] { return [...previous, value] } +/** + * enquirer's multiselect ignores each choice's `enabled` for the submitted result — it only renders a marker, so a + * user who just hits enter gets []. The enabled names must be passed as `initial` to actually pre-select them. + */ +export function preSelected(choices: readonly Choice[]): string[] { + return choices.filter((c) => c.enabled).map((c) => c.name) +} + async function ask(type: 'select' | 'multiselect', message: string, choices: Choice[]): Promise { - const response = (await enquirer.prompt({ type, name: 'selected', message, choices })) as { selected: T } + const initial = type === 'multiselect' ? preSelected(choices) : undefined + const response = (await enquirer.prompt({ type, name: 'selected', message, choices, initial })) as { selected: T } return response.selected } @@ -73,7 +82,26 @@ function report(result: InitResult, defaultsOnly: boolean): void { if (file.action === 'unchanged') continue console.log(` ${ACTION_MARK[file.action]} ${file.path} (${file.action})`) } - console.log(color.dim('\nRun `npm run verify` (or `npx verifyx`) to run your verifications.')) +} + +/** Summarise the dependency install: what was skipped, installed, and — last, so it's the takeaway — what failed. */ +function reportInstall(install: InstallReport): void { + if (install.skipped.length > 0) { + console.log(color.dim(`\nSkipped ${install.skipped.length} already-installed devDependenc(ies): ${install.skipped.join(', ')}`)) + } + if (install.installed.length > 0) { + console.log(color.green(`Installed ${install.installed.length} devDependenc(ies): ${install.installed.join(', ')}`)) + } + if (install.failed.length > 0) { + console.error( + color.yellow( + `\n⚠ Failed to install ${install.failed.length} devDependenc(ies): ${install.failed.join(', ')}` + + `\n init finished; the rest is wired up. Resolve these yourself — e.g. a peer-dependency conflict may need` + + `\n a compatible version or --legacy-peer-deps. Install manually with:` + + `\n npm install --save-dev ${install.failed.join(' ')}`, + ), + ) + } } /** `verifyx init` — interactively scaffold checks + agent files into the current project. */ @@ -91,12 +119,13 @@ export function registerInit(program: Command): void { const { checks, targets, defaultsOnly } = await resolveSelections(opts) const result = applyInit({ cwd, checks, targets, defaultsOnly }) + report(result, defaultsOnly) if (result.devDeps.length > 0) { - console.log(`Installing ${result.devDeps.length} devDependenc(ies): ${result.devDeps.join(', ')}`) - const code = await runCommand(`npm install --save-dev ${result.devDeps.join(' ')}`, { cwd }) - if (code !== 0) console.error(color.yellow('npm install failed — install those devDependencies manually.')) + console.log(color.dim(`\nResolving ${result.devDeps.length} devDependenc(ies): ${result.devDeps.join(', ')}`)) + reportInstall(await installDevDeps(result.devDeps, cwd)) } - report(result, defaultsOnly) + + console.log(color.dim('\nRun `npm run verify` (or `npx verifyx`) to run your verifications.')) }) } diff --git a/src/scaffold/installDeps.test.ts b/src/scaffold/installDeps.test.ts new file mode 100644 index 0000000..0f82646 --- /dev/null +++ b/src/scaffold/installDeps.test.ts @@ -0,0 +1,85 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { installDevDeps, type Runner } from './installDeps.ts' + +let dir: string + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verify-install-')) + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'scratch' })) +}) +afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }) +}) + +/** A runner that records the commands it saw and returns 0 unless the command mentions a `fail`-listed package. */ +function fakeRunner(failFor: string[] = []): Runner & { commands: string[] } { + const commands: string[] = [] + const run: Runner = (command) => { + commands.push(command) + return Promise.resolve(failFor.some((pkg) => command.includes(pkg)) ? 1 : 0) + } + return Object.assign(run, { commands }) +} + +describe('installDevDeps', () => { + it('installs missing deps in a single batch on the happy path', async () => { + const run = fakeRunner() + const report = await installDevDeps(['oxlint', 'knip'], dir, run) + + expect(report).toEqual({ skipped: [], installed: ['oxlint', 'knip'], failed: [] }) + expect(run.commands).toEqual(['npm install --save-dev oxlint knip']) + }) + + it('skips deps already declared in package.json (any dependency field)', async () => { + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ devDependencies: { typescript: '^5.0.0' }, peerDependencies: { oxlint: '*' } }), + ) + const run = fakeRunner() + const report = await installDevDeps(['typescript', 'oxlint', 'knip'], dir, run) + + expect(report.skipped).toEqual(['typescript', 'oxlint']) + expect(report.installed).toEqual(['knip']) + expect(run.commands).toEqual(['npm install --save-dev knip']) + }) + + it('skips deps present in node_modules even when not declared', async () => { + fs.mkdirSync(path.join(dir, 'node_modules', 'oxlint'), { recursive: true }) + fs.writeFileSync(path.join(dir, 'node_modules', 'oxlint', 'package.json'), '{}') + const run = fakeRunner() + const report = await installDevDeps(['oxlint', 'knip'], dir, run) + + expect(report.skipped).toEqual(['oxlint']) + expect(report.installed).toEqual(['knip']) + }) + + it('runs no install command when everything is already present', async () => { + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ devDependencies: { oxlint: '*', knip: '*' } })) + const run = fakeRunner() + const report = await installDevDeps(['oxlint', 'knip'], dir, run) + + expect(report).toEqual({ skipped: ['oxlint', 'knip'], installed: [], failed: [] }) + expect(run.commands).toEqual([]) + }) + + it('falls back to per-package installs and isolates the failure when the batch fails', async () => { + const run = fakeRunner(['oxlint']) + const report = await installDevDeps(['oxlint', 'knip', 'skott'], dir, run) + + expect(report.skipped).toEqual([]) + expect(report.installed).toEqual(['knip', 'skott']) + expect(report.failed).toEqual(['oxlint']) + // batch first, then one command per package to isolate the culprit + expect(run.commands).toEqual([ + 'npm install --save-dev oxlint knip skott', + 'npm install --save-dev oxlint', + 'npm install --save-dev knip', + 'npm install --save-dev skott', + ]) + }) +}) diff --git a/src/scaffold/installDeps.ts b/src/scaffold/installDeps.ts new file mode 100644 index 0000000..976e228 --- /dev/null +++ b/src/scaffold/installDeps.ts @@ -0,0 +1,64 @@ +import fs from 'node:fs' +import path from 'node:path' + +import { runCommand } from '../shared/spawn.ts' + +/** Signature of the command runner (defaults to `runCommand`); injectable so tests don't shell out. */ +export type Runner = (command: string, opts: { cwd: string }) => Promise + +export type InstallReport = { + /** Already declared in package.json or present in node_modules — left untouched (no version bump). */ + skipped: string[] + /** Newly installed successfully. */ + installed: string[] + /** Attempted but npm exited non-zero (e.g. a peer-dependency conflict) — surfaced for the user to resolve. */ + failed: string[] +} + +const DEP_FIELDS = ['dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies'] as const + +type PackageJson = Partial>> + +function declaredDeps(cwd: string): Set { + try { + const pkg = JSON.parse(fs.readFileSync(path.join(cwd, 'package.json'), 'utf-8')) as PackageJson + return new Set(DEP_FIELDS.flatMap((field) => Object.keys(pkg[field] ?? {}))) + } catch { + return new Set() + } +} + +/** + * True when the dep is already available — declared in package.json (any dependency field) or resolvable + * under node_modules. Either way we skip it so `init` never re-installs and bumps an existing version. + */ +function isInstalled(name: string, cwd: string, declared: Set): boolean { + return declared.has(name) || fs.existsSync(path.join(cwd, 'node_modules', name, 'package.json')) +} + +/** + * Install the given devDeps, skipping any already present. Tries one batch install first (fast path); if that + * 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 { + const declared = declaredDeps(cwd) + const skipped: string[] = [] + const toInstall: string[] = [] + for (const dep of devDeps) (isInstalled(dep, cwd, declared) ? skipped : toInstall).push(dep) + + if (toInstall.length === 0) return { skipped, installed: [], failed: [] } + + // Fast path: one install for everything missing. + if ((await run(`npm install --save-dev ${toInstall.join(' ')}`, { cwd })) === 0) { + return { skipped, installed: toInstall, failed: [] } + } + + // The batch is atomic, so nothing was written — retry each package alone to isolate the culprit(s). + const installed: string[] = [] + const failed: string[] = [] + for (const dep of toInstall) { + ;((await run(`npm install --save-dev ${dep}`, { cwd })) === 0 ? installed : failed).push(dep) + } + return { skipped, installed, failed } +}