From 4dd81c275632903540ed43c1e8bdcdc74e744dc6 Mon Sep 17 00:00:00 2001 From: Hoang Dinh Date: Tue, 21 Jul 2026 14:14:12 +1000 Subject: [PATCH 1/5] feat: reduce false positives in unused-code and duplicate-code checks - duplicate-code --max-warnings now gates on distinct duplicated regions: jscpd pairs whose ranges overlap on either side are merged (union-find), collapsing overlapping re-reports and N-file patterns reported as N-1 pairs. Failure output shows both the deduped and raw counts. Benchmark: 106 raw clones -> 78 distinct regions on a real repo. - unused-code failures now print false-positive advice: check for dynamic loading (directory scan + require(), glob-registered ORM entities) and system binaries before deleting; suppress via knip entry/ignoreBinaries. - verifyx init detects system binaries invoked by package.json scripts that knip's IGNORED_GLOBAL_BINARIES doesn't cover (uv, az, python, terraform, ...) and adds them to knip ignoreBinaries. --- README.md | 4 +- src/checks/external.test.ts | 5 +++ src/checks/external.ts | 12 ++++-- src/checks/maxWarnings.test.ts | 39 +++++++++++++++++- src/checks/maxWarnings.ts | 53 +++++++++++++++++++++++- src/checks/registry.ts | 4 +- src/scaffold/init.ts | 4 +- src/scaffold/knipConfig.test.ts | 46 ++++++++++++++++++++- src/scaffold/knipConfig.ts | 73 +++++++++++++++++++++++++++------ 9 files changed, 216 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 3f32d24..7aeaca8 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ Each external check is configured through its **tool's own config file**, exactl `unused-code` and `duplicate-code` accept `--max-warnings ` and fail when findings exceed `n`. - `unused-code` passes the value to knip's [`--max-issues`](https://knip.dev/reference/cli#--max-issues). Knip configuration errors still fail. -- `duplicate-code` counts jscpd clones. Do not pass jscpd's `--reporters`, `--output`, or `--silent` options with `--max-warnings`; `verifyx` sets them to produce the count. +- `duplicate-code` counts **distinct duplicated regions**, not raw jscpd clones: jscpd reports the same duplication several times (overlapping ranges, and one pattern across N files as N−1 pairs), so clones whose ranges overlap are merged before the count is compared to the budget. The failure output shows both numbers. Do not pass jscpd's `--reporters`, `--output`, or `--silent` options with `--max-warnings`; `verifyx` sets them to produce the count. ```sh verifyx unused-code --max-warnings 5 @@ -247,7 +247,7 @@ It first asks how `verify` should run: **run all built-in checks** (`verifyx all - 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. +- 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), and adds system binaries your scripts invoke (`uv`, `az`, `python`, `terraform`, …) that knip's own global list doesn't cover to `ignoreBinaries`. 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. The skill auto-triggers on "verify"/"run checks", so agents run the checks proactively; the pointer reinforces it for tools that read `CLAUDE.md`/`AGENTS.md` as standing instructions. diff --git a/src/checks/external.test.ts b/src/checks/external.test.ts index 98e6751..65e945c 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -36,6 +36,11 @@ describe('externalFailureHint', () => { expect(hint).toContain('["skott","src"]') expect(hint).not.toContain('undefined') }) + + it('appends the failure advice when the check declares one', () => { + const hint = externalFailureHint({ name: 'unused-code', bin: 'knip', failureAdvice: 'check for dynamic loading' }, ['knip']) + expect(hint).toContain('check for dynamic loading') + }) }) describe('defineExternalCheck', () => { diff --git a/src/checks/external.ts b/src/checks/external.ts index 9ab4c1b..6519930 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -40,6 +40,8 @@ export type ExternalCheckSpec = { recommended?: boolean /** Docs / config reference for the underlying tool, surfaced when the check fails. */ docs?: string + /** Extra guidance printed on failure, e.g. how to recognise and suppress the tool's known false positives. */ + failureAdvice?: string /** Extra guard beyond bin presence (e.g. require a tsconfig). */ canRun?: () => boolean /** Rewrite the tool's captured output before it is printed, e.g. to strip a tool's own hardcoded colouring. */ @@ -57,12 +59,16 @@ export function selectCommand(spec: Pick, argv: readonly string[]): string { - return `↳ ${spec.name} uses ${spec.bin}: ran argv \`${formatArgv(argv)}\`. Configure ${spec.bin}${spec.docs ? ` — ${spec.docs}` : ''}.` +export function externalFailureHint( + spec: Pick, + argv: readonly string[], +): string { + const hint = `↳ ${spec.name} uses ${spec.bin}: ran argv \`${formatArgv(argv)}\`. Configure ${spec.bin}${spec.docs ? ` — ${spec.docs}` : ''}.` + return spec.failureAdvice ? `${hint}\n↳ ${spec.failureAdvice}` : hint } type CountBudget = Extract -type CountableSpec = Pick +type CountableSpec = Pick export async function runCountedBudget( spec: CountableSpec, diff --git a/src/checks/maxWarnings.test.ts b/src/checks/maxWarnings.test.ts index d5b1256..5e1f21d 100644 --- a/src/checks/maxWarnings.test.ts +++ b/src/checks/maxWarnings.test.ts @@ -2,8 +2,15 @@ import { describe, expect, it } from 'vitest' import { countJscpdClones, withinBudget } from './maxWarnings.ts' +function clone(aName: string, aStart: number, aEnd: number, bName: string, bStart: number, bEnd: number) { + return { + firstFile: { name: aName, start: aStart, end: aEnd }, + secondFile: { name: bName, start: bStart, end: bEnd }, + } +} + describe('countJscpdClones', () => { - it('reads the total clone count from statistics', () => { + it('reads the total clone count from statistics when duplicates lack range info', () => { const report = { duplicates: [{}, {}, {}], statistics: { total: { clones: 3 } } } expect(countJscpdClones(report)).toBe(3) }) @@ -12,6 +19,36 @@ describe('countJscpdClones', () => { expect(countJscpdClones({ duplicates: [{}, {}] })).toBe(2) }) + it('counts disjoint clones as separate regions', () => { + const report = { duplicates: [clone('a.ts', 1, 10, 'b.ts', 1, 10), clone('a.ts', 50, 60, 'c.ts', 1, 10)] } + expect(countJscpdClones(report)).toBe(2) + }) + + it('merges clones reported over overlapping ranges into one region', () => { + const report = { duplicates: [clone('a.ts', 100, 120, 'b.ts', 10, 30), clone('a.ts', 110, 130, 'b.ts', 20, 40)] } + expect(countJscpdClones(report)).toBe(1) + }) + + it('merges the same pattern repeated across N files into one region', () => { + const report = { + duplicates: [clone('x.ts', 1, 20, 'y.ts', 1, 20), clone('x.ts', 1, 20, 'z.ts', 1, 20), clone('y.ts', 1, 20, 'z.ts', 1, 20)], + } + expect(countJscpdClones(report)).toBe(1) + }) + + it('does not merge clones in the same file at non-overlapping ranges', () => { + const report = { duplicates: [clone('a.ts', 979, 986, 'a.ts', 1085, 1092), clone('a.ts', 1008, 1023, 'a.ts', 1101, 1115)] } + expect(countJscpdClones(report)).toBe(2) + }) + + it('prefers the deduped region count over the raw statistics total', () => { + const report = { + duplicates: [clone('a.ts', 1, 20, 'b.ts', 1, 20), clone('a.ts', 5, 25, 'c.ts', 1, 20)], + statistics: { total: { clones: 2 } }, + } + expect(countJscpdClones(report)).toBe(1) + }) + it('returns 0 for a clean report', () => { expect(countJscpdClones({ duplicates: [], statistics: { total: { clones: 0 } } })).toBe(0) }) diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts index fda53fa..163befd 100644 --- a/src/checks/maxWarnings.ts +++ b/src/checks/maxWarnings.ts @@ -17,7 +17,53 @@ export function withinBudget(count: number, maxWarnings: number): boolean { return count <= maxWarnings } +type CloneRange = { name: string; start: number; end: number } +type CloneRanges = [CloneRange, CloneRange] + +function parseCloneRange(value: unknown): CloneRange | undefined { + if (typeof value !== 'object' || value === null) return undefined + const { name, start, end } = value as { name?: unknown; start?: unknown; end?: unknown } + if (typeof name !== 'string' || typeof start !== 'number' || typeof end !== 'number') return undefined + return { name, start, end } +} + +function parseCloneRangePairs(duplicates: readonly unknown[]): CloneRanges[] | undefined { + const pairs: CloneRanges[] = [] + for (const entry of duplicates) { + const { firstFile, secondFile } = (entry ?? {}) as { firstFile?: unknown; secondFile?: unknown } + const a = parseCloneRange(firstFile) + const b = parseCloneRange(secondFile) + if (!a || !b) return undefined + pairs.push([a, b]) + } + return pairs +} + +function rangesOverlap(a: CloneRange, b: CloneRange): boolean { + return a.name === b.name && a.start <= b.end && b.start <= a.end +} + +function clonesShareRegion(a: CloneRanges, b: CloneRanges): boolean { + return a.some((aRange) => b.some((bRange) => rangesOverlap(aRange, bRange))) +} + +/** One pattern across N files yields N-1 jscpd pairs (plus overlapping re-reports), so the budget gates on connected components, not raw pairs. */ +function countDistinctCloneRegions(pairs: readonly CloneRanges[]): number { + const regionOf = pairs.map((_, i) => i) + const rootOf = (i: number): number => (regionOf[i] === i ? i : (regionOf[i] = rootOf(regionOf[i] as number))) + for (let i = 0; i < pairs.length; i++) { + for (let j = i + 1; j < pairs.length; j++) { + if (clonesShareRegion(pairs[i] as CloneRanges, pairs[j] as CloneRanges)) regionOf[rootOf(j)] = rootOf(i) + } + } + return new Set(pairs.map((_, i) => rootOf(i))).size +} + export function countJscpdClones(report: { statistics?: { total?: { clones?: unknown } }; duplicates?: unknown }): number { + if (Array.isArray(report.duplicates)) { + const pairs = parseCloneRangePairs(report.duplicates) + if (pairs) return countDistinctCloneRegions(pairs) + } const clones = report.statistics?.total?.clones if (typeof clones === 'number' && Number.isInteger(clones) && clones >= 0) return clones if (Array.isArray(report.duplicates)) return report.duplicates.length @@ -33,12 +79,15 @@ export async function jscpdCount(ctx: MaxWarningsCountContext): Promise" line — the temp dir is deleted before anyone could read it. - const report = (stdout + stderr) + let report = (stdout + stderr) .split('\n') .filter((line) => !line.includes(dir)) .join('\n') + if (rawClones > count) report += `\n↳ ${count} distinct duplicated region(s) merged from ${rawClones} raw jscpd clone(s).\n` return { count, report } } finally { // Retry transient Windows locks on the new report. diff --git a/src/checks/registry.ts b/src/checks/registry.ts index 9daf54a..17ded98 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -70,6 +70,8 @@ export const CHECKS: Check[] = [ checkCommand: ['knip', '--no-progress', '--treat-config-hints-as-errors'], devDeps: ['knip'], docs: 'https://knip.dev/reference/configuration', + failureAdvice: + 'An "unused" finding can be a false positive when the file is loaded dynamically (directory scan + require(), glob-registered ORM entities) or a script calls a system binary. Verify before deleting: suppress genuinely runtime-loaded files via knip `entry` globs and system tools via `ignoreBinaries` — never delete a file to satisfy this check without checking how it is loaded.', maxWarnings: { strategy: 'flag', toArgs: (n) => ['--max-issues', String(n)] }, }), defineExternalCheck({ @@ -90,7 +92,7 @@ export const CHECKS: Check[] = [ // NO_COLOR/FORCE_COLOR, so strip the red foreground from its output; the table renders in the default colour. transformOutput: withoutRed, docs: 'https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config', - maxWarnings: { strategy: 'count', unit: 'clone', count: jscpdCount }, + maxWarnings: { strategy: 'count', unit: 'duplicated region', count: jscpdCount }, }), ] diff --git a/src/scaffold/init.ts b/src/scaffold/init.ts index 8b2c496..ad2e803 100644 --- a/src/scaffold/init.ts +++ b/src/scaffold/init.ts @@ -3,7 +3,7 @@ import path from 'node:path' import { getCheck } from '../checks/registry.ts' import type { Check } from '../checks/types.ts' import { type AgentTarget, writeAgentFiles } from './agentFiles.ts' -import { ensureKnipIgnores } from './knipConfig.ts' +import { detectSystemBinaries, ensureKnipIgnores } from './knipConfig.ts' import { addVerifyScripts } from './packageScripts.ts' import type { ManagedFileResult } from './writeManaged.ts' @@ -47,7 +47,7 @@ export function applyInit(opts: InitOptions): InitResult { .filter((check): check is Check => !!check && check.kind === 'external' && check.name !== 'unused-code') .flatMap((check) => check.scaffold.devDeps ?? []) .filter((dep) => dep !== 'typescript') - ensureKnipIgnores(opts.cwd, [...new Set(toolDeps)], agentFiles) + ensureKnipIgnores(opts.cwd, [...new Set(toolDeps)], agentFiles, detectSystemBinaries(opts.cwd)) } return { addedScripts, devDeps: [...new Set(devDeps)], agentFiles } diff --git a/src/scaffold/knipConfig.test.ts b/src/scaffold/knipConfig.test.ts index 1089249..d1e159e 100644 --- a/src/scaffold/knipConfig.test.ts +++ b/src/scaffold/knipConfig.test.ts @@ -4,7 +4,7 @@ import path from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { ensureKnipIgnores } from './knipConfig.ts' +import { detectSystemBinaries, ensureKnipIgnores } from './knipConfig.ts' import type { ManagedFileResult } from './writeManaged.ts' let dir: string @@ -68,4 +68,48 @@ describe('ensureKnipIgnores', () => { expect(results).toEqual([]) expect(fs.existsSync(path.join(dir, 'knip.json'))).toBe(false) }) + + it('writes detected system binaries to ignoreBinaries', () => { + const results: ManagedFileResult[] = [] + ensureKnipIgnores(dir, [], results, ['uv']) + expect(results[0]?.action).toBe('created') + expect((readKnip() as { ignoreBinaries?: string[] }).ignoreBinaries).toEqual(['uv']) + }) + + it('merges binaries into an existing knip.json alongside dependencies', () => { + fs.writeFileSync(path.join(dir, 'knip.json'), JSON.stringify({ ignoreDependencies: ['oxlint'], ignoreBinaries: ['az'] })) + const results: ManagedFileResult[] = [] + ensureKnipIgnores(dir, ['jscpd'], results, ['az', 'uv']) + const cfg = readKnip() as { ignoreDependencies?: string[]; ignoreBinaries?: string[] } + expect(cfg.ignoreDependencies).toEqual(['oxlint', 'jscpd']) + expect(cfg.ignoreBinaries).toEqual(['az', 'uv']) + }) +}) + +describe('detectSystemBinaries', () => { + function writeScripts(scripts: Record) { + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'scratch', scripts })) + } + + it('finds a known system binary invoked in a compound script', () => { + writeScripts({ 'evals:local': 'cd evals && uv run deepeval test run .' }) + expect(detectSystemBinaries(dir)).toEqual(['uv']) + }) + + it('ignores binaries knip already ignores globally and npm-installed bins', () => { + writeScripts({ up: 'docker compose up', tf: 'terraform apply' }) + fs.mkdirSync(path.join(dir, 'node_modules', '.bin'), { recursive: true }) + fs.writeFileSync(path.join(dir, 'node_modules', '.bin', 'terraform'), '') + expect(detectSystemBinaries(dir)).toEqual([]) + }) + + it('skips env-var assignments to find the command word', () => { + writeScripts({ gen: 'NODE_ENV=test uv run pytest' }) + expect(detectSystemBinaries(dir)).toEqual(['uv']) + }) + + it('does not report a system binary that only appears as an argument', () => { + writeScripts({ docs: 'echo uv is required' }) + expect(detectSystemBinaries(dir)).toEqual([]) + }) }) diff --git a/src/scaffold/knipConfig.ts b/src/scaffold/knipConfig.ts index 13a337c..85eb3a4 100644 --- a/src/scaffold/knipConfig.ts +++ b/src/scaffold/knipConfig.ts @@ -7,7 +7,46 @@ import type { ManagedFileResult } from './writeManaged.ts' const CODE_CONFIGS = ['knip.ts', 'knip.js', 'knip.config.ts', 'knip.config.js', 'knip.jsonc'] const SCHEMA = 'https://unpkg.com/knip/schema.json' -type KnipConfig = { ignoreDependencies?: string[] } & Record +type KnipConfig = { ignoreDependencies?: string[]; ignoreBinaries?: string[] } & Record + +// context: tools knip flags as unlisted binaries because they are system-installed, not npm bins, and are +// missing from knip's own IGNORED_GLOBAL_BINARIES (which already covers git, docker, aws, cargo, ...). +const SYSTEM_BINARIES = [ + 'az', + 'dotnet', + 'gcloud', + 'go', + 'helm', + 'kubectl', + 'make', + 'pip', + 'pip3', + 'pipx', + 'poetry', + 'python', + 'python3', + 'ruby', + 'terraform', + 'uv', + 'uvx', +] + +export function detectSystemBinaries(cwd: string): string[] { + const pkgPath = path.join(cwd, 'package.json') + if (!fs.existsSync(pkgPath)) return [] + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) as { scripts?: Record } + const invoked = new Set() + for (const script of Object.values(pkg.scripts ?? {})) { + for (const segment of script.split(/&&|\|\||;|\|/)) { + const command = segment + .trim() + .split(/\s+/) + .find((word) => word !== '' && !word.includes('=')) + if (command) invoked.add(command) + } + } + return SYSTEM_BINARIES.filter((bin) => invoked.has(bin) && !fs.existsSync(path.join(cwd, 'node_modules', '.bin', bin))) +} /** Append any missing `deps` to `existing`, preserving order; report whether anything changed. */ function addMissing(existing: string[] | undefined, deps: readonly string[]): { list: string[]; changed: boolean } { @@ -22,26 +61,33 @@ function addMissing(existing: string[] | undefined, deps: readonly string[]): { return { list, changed } } -function mergeInto(config: KnipConfig, deps: readonly string[]): boolean { - const { list, changed } = addMissing(config.ignoreDependencies, deps) - if (changed) config.ignoreDependencies = list - return changed +function mergeInto(config: KnipConfig, deps: readonly string[], binaries: readonly string[]): boolean { + const depMerge = addMissing(config.ignoreDependencies, deps) + if (depMerge.changed) config.ignoreDependencies = depMerge.list + const binMerge = addMissing(config.ignoreBinaries, binaries) + if (binMerge.changed) config.ignoreBinaries = binMerge.list + return depMerge.changed || binMerge.changed } /** - * Ensure the project's knip config ignores `deps` — the tools verifyx invokes at runtime (via node_modules/.bin), - * which knip can't see and would otherwise flag as unused. Adds only what's missing (idempotent), never removes + * Ensure the project's knip config ignores `deps` (the tools verifyx invokes at runtime, which knip can't see) + * and `binaries` (system tools its scripts call). Adds only what's missing (idempotent), never removes * or rewrites unrelated content. Merges into an existing `knip.json` or `package.json#knip`, creates a minimal * `knip.json` if there's no config, and leaves code-based configs (knip.ts/js) untouched. */ -export function ensureKnipIgnores(cwd: string, deps: readonly string[], results: ManagedFileResult[]): void { - if (deps.length === 0) return +export function ensureKnipIgnores( + cwd: string, + deps: readonly string[], + results: ManagedFileResult[], + binaries: readonly string[] = [], +): void { + if (deps.length === 0 && binaries.length === 0) return if (CODE_CONFIGS.some((file) => fs.existsSync(path.join(cwd, file)))) return const jsonPath = path.join(cwd, 'knip.json') if (fs.existsSync(jsonPath)) { const config = JSON.parse(fs.readFileSync(jsonPath, 'utf-8')) as KnipConfig - const changed = mergeInto(config, deps) + const changed = mergeInto(config, deps, binaries) if (changed) fs.writeFileSync(jsonPath, `${JSON.stringify(config, null, 2)}\n`) results.push({ path: jsonPath, action: changed ? 'updated' : 'unchanged' }) return @@ -50,12 +96,15 @@ export function ensureKnipIgnores(cwd: string, deps: readonly string[], results: const pkgPath = path.join(cwd, 'package.json') const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) as { knip?: KnipConfig } & Record if (pkg.knip && typeof pkg.knip === 'object') { - const changed = mergeInto(pkg.knip, deps) + const changed = mergeInto(pkg.knip, deps, binaries) if (changed) fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`) results.push({ path: pkgPath, action: changed ? 'updated' : 'unchanged' }) return } - fs.writeFileSync(jsonPath, `${JSON.stringify({ $schema: SCHEMA, ignoreDependencies: [...deps] }, null, 2)}\n`) + const fresh: KnipConfig = { $schema: SCHEMA } + if (deps.length > 0) fresh.ignoreDependencies = [...deps] + if (binaries.length > 0) fresh.ignoreBinaries = [...binaries] + fs.writeFileSync(jsonPath, `${JSON.stringify(fresh, null, 2)}\n`) results.push({ path: jsonPath, action: 'created' }) } From b7fd92f235cb58d4da2aa514b29eab1dde2a7e24 Mon Sep 17 00:00:00 2001 From: Hoang Dinh Date: Tue, 21 Jul 2026 15:16:33 +1000 Subject: [PATCH 2/5] fix: align binary detection with knip --- package-lock.json | 4 +- package.json | 3 +- src/scaffold/detectSystemBinaries.ts | 140 +++++++++++++++++++++++++++ src/scaffold/init.ts | 3 +- src/scaffold/knipConfig.test.ts | 35 ++++++- src/scaffold/knipConfig.ts | 39 -------- 6 files changed, 180 insertions(+), 44 deletions(-) create mode 100644 src/scaffold/detectSystemBinaries.ts diff --git a/package-lock.json b/package-lock.json index e4565a9..b497ddd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,8 @@ "cross-spawn": "^7.0.6", "enquirer": "^2.4.1", "minimatch": "^10.2.5", - "typescript": "^6.0.3" + "typescript": "^6.0.3", + "unbash": "^4.0.2" }, "bin": { "verifyx": "dist/cli.mjs" @@ -5826,7 +5827,6 @@ }, "node_modules/unbash": { "version": "4.0.2", - "dev": true, "license": "ISC", "engines": { "node": ">=14" diff --git a/package.json b/package.json index f0da1d7..802da61 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,8 @@ "cross-spawn": "^7.0.6", "enquirer": "^2.4.1", "minimatch": "^10.2.5", - "typescript": "^6.0.3" + "typescript": "^6.0.3", + "unbash": "^4.0.2" }, "devDependencies": { "@rollup/plugin-typescript": "^12.3.0", diff --git a/src/scaffold/detectSystemBinaries.ts b/src/scaffold/detectSystemBinaries.ts new file mode 100644 index 0000000..88ebb9a --- /dev/null +++ b/src/scaffold/detectSystemBinaries.ts @@ -0,0 +1,140 @@ +import fs from 'node:fs' +import path from 'node:path' + +import { type Command, type Node, parse, type Script, type Word } from 'unbash' + +type PackageJson = { + scripts?: Record +} & Partial>> + +const DEP_FIELDS = ['dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies'] as const + +// context: tools knip flags as unlisted binaries because they are system-installed, not npm bins, and are +// missing from knip's own IGNORED_GLOBAL_BINARIES (which already covers git, docker, aws, cargo, ...). +const SYSTEM_BINARIES = [ + 'az', + 'dotnet', + 'gcloud', + 'go', + 'helm', + 'kubectl', + 'make', + 'pip', + 'pip3', + 'pipx', + 'poetry', + 'python', + 'python3', + 'ruby', + 'terraform', + 'uv', + 'uvx', +] + +const SPAWNING_BINARIES = new Set(['cross-env', 'retry-cli']) + +export function detectSystemBinaries(cwd: string): string[] { + const pkgPath = path.join(cwd, 'package.json') + if (!fs.existsSync(pkgPath)) return [] + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) as PackageJson + const invoked = new Set() + for (const script of Object.values(pkg.scripts ?? {})) { + collectInvokedBinaries(script, invoked) + } + const declared = new Set(DEP_FIELDS.flatMap((field) => Object.keys(pkg[field] ?? {}))) + return SYSTEM_BINARIES.filter( + (bin) => invoked.has(bin) && !declared.has(bin) && !fs.existsSync(path.join(cwd, 'node_modules', '.bin', bin)), + ) +} + +function collectInvokedBinaries(script: string, invoked: Set): void { + try { + collectInvokedBinariesFromScript(parse(script), invoked, new Set()) + } catch { + return + } +} + +function collectInvokedBinariesFromScript(script: Script, invoked: Set, definedFunctions: Set): void { + for (const statement of script.commands) { + if (statement.command.type === 'Function') definedFunctions.add(statement.command.name.text) + } + + for (const statement of script.commands) { + for (const command of walkCommands(statement)) { + if (command.name) collectExpansionBinaries(command.name, invoked, definedFunctions) + for (const prefix of command.prefix) { + if (prefix.value) collectExpansionBinaries(prefix.value, invoked, definedFunctions) + } + for (const suffix of command.suffix) collectExpansionBinaries(suffix, invoked, definedFunctions) + + const binary = command.name?.value + if (!binary || definedFunctions.has(binary)) continue + invoked.add(binary) + if (SPAWNING_BINARIES.has(binary)) { + collectInvokedBinaries( + command.suffix + .filter((word) => word.text !== '--') + .map((word) => word.text) + .join(' '), + invoked, + ) + } + } + } +} + +function* walkCommands(node: Node): Generator { + switch (node.type) { + case 'Command': + yield node + break + case 'AndOr': + case 'Pipeline': + for (const command of node.commands) yield* walkCommands(command) + break + case 'If': + yield* walkCommands(node.clause) + yield* walkCommands(node.then) + if (node.else) yield* walkCommands(node.else) + break + case 'For': + case 'ArithmeticFor': + case 'Select': + case 'Subshell': + case 'BraceGroup': + yield* walkCommands(node.body) + break + case 'While': + yield* walkCommands(node.clause) + yield* walkCommands(node.body) + break + case 'CompoundList': + for (const statement of node.commands) yield* walkCommands(statement) + break + case 'Case': + for (const item of node.items) yield* walkCommands(item.body) + break + case 'Function': + case 'Coproc': + yield* walkCommands(node.body) + break + case 'Statement': + yield* walkCommands(node.command) + break + } +} + +function collectExpansionBinaries(word: Word, invoked: Set, definedFunctions: Set): void { + for (const part of word.parts ?? []) { + if ((part.type === 'CommandExpansion' || part.type === 'ProcessSubstitution') && part.script) { + collectInvokedBinariesFromScript(part.script, invoked, definedFunctions) + } else if (part.type === 'DoubleQuoted' || part.type === 'LocaleString') { + for (const child of part.parts) { + if (child.type === 'CommandExpansion' && child.script) { + collectInvokedBinariesFromScript(child.script, invoked, definedFunctions) + } + } + } + } +} diff --git a/src/scaffold/init.ts b/src/scaffold/init.ts index ad2e803..53695f0 100644 --- a/src/scaffold/init.ts +++ b/src/scaffold/init.ts @@ -3,7 +3,8 @@ import path from 'node:path' import { getCheck } from '../checks/registry.ts' import type { Check } from '../checks/types.ts' import { type AgentTarget, writeAgentFiles } from './agentFiles.ts' -import { detectSystemBinaries, ensureKnipIgnores } from './knipConfig.ts' +import { detectSystemBinaries } from './detectSystemBinaries.ts' +import { ensureKnipIgnores } from './knipConfig.ts' import { addVerifyScripts } from './packageScripts.ts' import type { ManagedFileResult } from './writeManaged.ts' diff --git a/src/scaffold/knipConfig.test.ts b/src/scaffold/knipConfig.test.ts index d1e159e..9a7d657 100644 --- a/src/scaffold/knipConfig.test.ts +++ b/src/scaffold/knipConfig.test.ts @@ -4,7 +4,8 @@ import path from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { detectSystemBinaries, ensureKnipIgnores } from './knipConfig.ts' +import { detectSystemBinaries } from './detectSystemBinaries.ts' +import { ensureKnipIgnores } from './knipConfig.ts' import type { ManagedFileResult } from './writeManaged.ts' let dir: string @@ -108,8 +109,40 @@ describe('detectSystemBinaries', () => { expect(detectSystemBinaries(dir)).toEqual(['uv']) }) + it('finds a system binary invoked through a spawning wrapper', () => { + writeScripts({ test: 'cross-env NODE_ENV=test uv run pytest' }) + expect(detectSystemBinaries(dir)).toEqual(['uv']) + }) + + it('finds system binaries invoked in expansions throughout a command', () => { + writeScripts({ + expansions: '$(python --version) VERSION=$(python3 --version) echo "$(ruby --version)" <(terraform version)', + }) + expect(detectSystemBinaries(dir)).toEqual(['python', 'python3', 'ruby', 'terraform']) + }) + + it('ignores same-named packages declared in any dependency field before installation', () => { + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ + name: 'scratch', + scripts: { tools: 'python --version && python3 --version && ruby --version && terraform version && uv --version' }, + dependencies: { python: '*' }, + devDependencies: { python3: '*' }, + peerDependencies: { ruby: '*' }, + optionalDependencies: { terraform: '*' }, + }), + ) + expect(detectSystemBinaries(dir)).toEqual(['uv']) + }) + it('does not report a system binary that only appears as an argument', () => { writeScripts({ docs: 'echo uv is required' }) expect(detectSystemBinaries(dir)).toEqual([]) }) + + it('does not report calls to shell functions as system binaries', () => { + writeScripts({ local: 'python() { echo hi; }; python' }) + expect(detectSystemBinaries(dir)).toEqual([]) + }) }) diff --git a/src/scaffold/knipConfig.ts b/src/scaffold/knipConfig.ts index 85eb3a4..5e7d1eb 100644 --- a/src/scaffold/knipConfig.ts +++ b/src/scaffold/knipConfig.ts @@ -9,45 +9,6 @@ const SCHEMA = 'https://unpkg.com/knip/schema.json' type KnipConfig = { ignoreDependencies?: string[]; ignoreBinaries?: string[] } & Record -// context: tools knip flags as unlisted binaries because they are system-installed, not npm bins, and are -// missing from knip's own IGNORED_GLOBAL_BINARIES (which already covers git, docker, aws, cargo, ...). -const SYSTEM_BINARIES = [ - 'az', - 'dotnet', - 'gcloud', - 'go', - 'helm', - 'kubectl', - 'make', - 'pip', - 'pip3', - 'pipx', - 'poetry', - 'python', - 'python3', - 'ruby', - 'terraform', - 'uv', - 'uvx', -] - -export function detectSystemBinaries(cwd: string): string[] { - const pkgPath = path.join(cwd, 'package.json') - if (!fs.existsSync(pkgPath)) return [] - const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) as { scripts?: Record } - const invoked = new Set() - for (const script of Object.values(pkg.scripts ?? {})) { - for (const segment of script.split(/&&|\|\||;|\|/)) { - const command = segment - .trim() - .split(/\s+/) - .find((word) => word !== '' && !word.includes('=')) - if (command) invoked.add(command) - } - } - return SYSTEM_BINARIES.filter((bin) => invoked.has(bin) && !fs.existsSync(path.join(cwd, 'node_modules', '.bin', bin))) -} - /** Append any missing `deps` to `existing`, preserving order; report whether anything changed. */ function addMissing(existing: string[] | undefined, deps: readonly string[]): { list: string[]; changed: boolean } { const list = [...(existing ?? [])] From 036b9a468f218fe9ae92dfe31dfe6d430342409f Mon Sep 17 00:00:00 2001 From: Hoang Dinh Date: Tue, 21 Jul 2026 15:45:32 +1000 Subject: [PATCH 3/5] test: cover jscpd merged-count output --- src/checks/maxWarnings.test.ts | 43 ++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/checks/maxWarnings.test.ts b/src/checks/maxWarnings.test.ts index 5e1f21d..50aa2db 100644 --- a/src/checks/maxWarnings.test.ts +++ b/src/checks/maxWarnings.test.ts @@ -1,6 +1,13 @@ -import { describe, expect, it } from 'vitest' +import fs from 'node:fs' +import path from 'node:path' -import { countJscpdClones, withinBudget } from './maxWarnings.ts' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const captureArgvCommand = vi.hoisted(() => vi.fn()) + +vi.mock('../shared/spawn.ts', () => ({ captureArgvCommand })) + +import { countJscpdClones, jscpdCount, withinBudget } from './maxWarnings.ts' function clone(aName: string, aStart: number, aEnd: number, bName: string, bStart: number, bEnd: number) { return { @@ -9,6 +16,18 @@ function clone(aName: string, aStart: number, aEnd: number, bName: string, bStar } } +function mockJscpdReport(duplicates: ReturnType[]) { + captureArgvCommand.mockImplementationOnce(async (argv: readonly string[]) => { + const outputIndex = argv.indexOf('--output') + const outputDir = argv[outputIndex + 1] + if (!outputDir) throw new Error('missing jscpd output directory') + fs.writeFileSync(path.join(outputDir, 'jscpd-report.json'), JSON.stringify({ duplicates })) + return { code: 1, stdout: 'clone report\n', stderr: '' } + }) +} + +afterEach(() => captureArgvCommand.mockReset()) + describe('countJscpdClones', () => { it('reads the total clone count from statistics when duplicates lack range info', () => { const report = { duplicates: [{}, {}, {}], statistics: { total: { clones: 3 } } } @@ -64,6 +83,26 @@ describe('countJscpdClones', () => { }) }) +describe('jscpdCount', () => { + it('reports both counts when raw clones merge into fewer regions', async () => { + mockJscpdReport([clone('a.ts', 1, 20, 'b.ts', 1, 20), clone('a.ts', 5, 25, 'c.ts', 1, 20)]) + + const result = await jscpdCount({ argv: ['jscpd', 'src'], env: {} }) + + expect(result.count).toBe(1) + expect(result.report).toContain('1 distinct duplicated region(s) merged from 2 raw jscpd clone(s).') + }) + + it('omits the merged-count summary when no clones were merged', async () => { + mockJscpdReport([clone('a.ts', 1, 20, 'b.ts', 1, 20)]) + + const result = await jscpdCount({ argv: ['jscpd', 'src'], env: {} }) + + expect(result.count).toBe(1) + expect(result.report).toBe('clone report\n') + }) +}) + describe('withinBudget', () => { it('passes when the count is at or below the budget (budget is inclusive)', () => { expect(withinBudget(3, 5)).toBe(true) From 019df4ba1cb4543e8ae96f4bfe7c4b9175999346 Mon Sep 17 00:00:00 2001 From: Hoang Dinh Date: Wed, 22 Jul 2026 09:07:45 +1000 Subject: [PATCH 4/5] fix: detect system binaries in redirect expansions --- src/scaffold/detectSystemBinaries.ts | 41 ++++++++++++++++------------ src/scaffold/knipConfig.test.ts | 9 ++++++ 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/scaffold/detectSystemBinaries.ts b/src/scaffold/detectSystemBinaries.ts index 88ebb9a..cbd94c9 100644 --- a/src/scaffold/detectSystemBinaries.ts +++ b/src/scaffold/detectSystemBinaries.ts @@ -1,7 +1,7 @@ import fs from 'node:fs' import path from 'node:path' -import { type Command, type Node, parse, type Script, type Word } from 'unbash' +import { type Node, parse, type Script, type Word } from 'unbash' type PackageJson = { scripts?: Record @@ -61,7 +61,16 @@ function collectInvokedBinariesFromScript(script: Script, invoked: Set, } for (const statement of script.commands) { - for (const command of walkCommands(statement)) { + for (const node of walkNodes(statement)) { + if ('redirects' in node) { + for (const redirect of node.redirects) { + if (redirect.target) collectExpansionBinaries(redirect.target, invoked, definedFunctions) + if (redirect.body) collectExpansionBinaries(redirect.body, invoked, definedFunctions) + } + } + if (node.type !== 'Command') continue + + const command = node if (command.name) collectExpansionBinaries(command.name, invoked, definedFunctions) for (const prefix of command.prefix) { if (prefix.value) collectExpansionBinaries(prefix.value, invoked, definedFunctions) @@ -84,43 +93,41 @@ function collectInvokedBinariesFromScript(script: Script, invoked: Set, } } -function* walkCommands(node: Node): Generator { +function* walkNodes(node: Node): Generator { + yield node switch (node.type) { - case 'Command': - yield node - break case 'AndOr': case 'Pipeline': - for (const command of node.commands) yield* walkCommands(command) + for (const command of node.commands) yield* walkNodes(command) break case 'If': - yield* walkCommands(node.clause) - yield* walkCommands(node.then) - if (node.else) yield* walkCommands(node.else) + yield* walkNodes(node.clause) + yield* walkNodes(node.then) + if (node.else) yield* walkNodes(node.else) break case 'For': case 'ArithmeticFor': case 'Select': case 'Subshell': case 'BraceGroup': - yield* walkCommands(node.body) + yield* walkNodes(node.body) break case 'While': - yield* walkCommands(node.clause) - yield* walkCommands(node.body) + yield* walkNodes(node.clause) + yield* walkNodes(node.body) break case 'CompoundList': - for (const statement of node.commands) yield* walkCommands(statement) + for (const statement of node.commands) yield* walkNodes(statement) break case 'Case': - for (const item of node.items) yield* walkCommands(item.body) + for (const item of node.items) yield* walkNodes(item.body) break case 'Function': case 'Coproc': - yield* walkCommands(node.body) + yield* walkNodes(node.body) break case 'Statement': - yield* walkCommands(node.command) + yield* walkNodes(node.command) break } } diff --git a/src/scaffold/knipConfig.test.ts b/src/scaffold/knipConfig.test.ts index 9a7d657..f72d1de 100644 --- a/src/scaffold/knipConfig.test.ts +++ b/src/scaffold/knipConfig.test.ts @@ -121,6 +121,15 @@ describe('detectSystemBinaries', () => { expect(detectSystemBinaries(dir)).toEqual(['python', 'python3', 'ruby', 'terraform']) }) + it('finds system binaries invoked in redirect expansions', () => { + writeScripts({ + compound: '{ echo ok; } > >(ruby --version)', + heredoc: 'cat < >(terraform fmt)', + }) + expect(detectSystemBinaries(dir)).toEqual(['python', 'ruby', 'terraform', 'uv']) + }) + it('ignores same-named packages declared in any dependency field before installation', () => { fs.writeFileSync( path.join(dir, 'package.json'), From 1b00604491059ec1067e78d186dbbcb1b7df2354 Mon Sep 17 00:00:00 2001 From: Hoang Dinh Date: Wed, 22 Jul 2026 09:32:05 +1000 Subject: [PATCH 5/5] fix: resolve fast-uri audit vulnerability --- package-lock.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index b497ddd..fbb2ef6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3713,7 +3713,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.3", + "version": "3.1.4", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.4.tgz", + "integrity": "sha512-8JnbkQ4juDyvYs4mgFGQqg4yCYtFDtUtmp2QIQq11ZZe5CFQ5wcqm1rqDgAh/QdMySuBnPzMUiJUNZG5N/AiQw==", "dev": true, "funding": [ {