diff --git a/packages/agents/package.json b/packages/agents/package.json index 49c9c9c2..09ede584 100644 --- a/packages/agents/package.json +++ b/packages/agents/package.json @@ -32,8 +32,7 @@ "dist" ], "scripts": { - "build": "nmr compile && tsx scripts/bundle-skill-helpers.ts && tsx scripts/copy-content.ts", - "build:post": "rdy compile", + "build:post": "tsx scripts/bundle-skill-helpers.ts && tsx scripts/copy-content.ts && rdy compile", "build:pre": "rdy verify", "prepublishOnly": "nmr build", "test:post": "pnpm run test:sh && pnpm run test:smoke", diff --git a/packages/kb/package.json b/packages/kb/package.json index 8f1df5ae..d3dd6c21 100644 --- a/packages/kb/package.json +++ b/packages/kb/package.json @@ -96,7 +96,6 @@ "dist" ], "scripts": { - "build": "nmr compile", "prepublishOnly": "nmr build" }, "dependencies": { diff --git a/packages/lifecycle/package.json b/packages/lifecycle/package.json index 664e218f..9e032bf6 100644 --- a/packages/lifecycle/package.json +++ b/packages/lifecycle/package.json @@ -32,7 +32,6 @@ "dist" ], "scripts": { - "build": "nmr compile", "prepublishOnly": "nmr build" }, "devDependencies": { diff --git a/packages/mcp/package.json b/packages/mcp/package.json index 214b4b6e..f92c7388 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -11,9 +11,6 @@ "bin", "dist" ], - "scripts": { - "build": "nmr compile" - }, "dependencies": { "@modelcontextprotocol/sdk": "1.30.0", "@williamthorsen/toolbelt.errors": "catalog:", diff --git a/packages/run-core/package.json b/packages/run-core/package.json index 0998a94f..d57bcde0 100644 --- a/packages/run-core/package.json +++ b/packages/run-core/package.json @@ -30,9 +30,6 @@ "files": [ "dist" ], - "scripts": { - "build": "nmr compile" - }, "dependencies": { "@williamthorsen/toolbelt.errors": "catalog:", "yaml": "2.9.0", diff --git a/scripts/__tests__/package-contents.tool.test.ts b/scripts/__tests__/package-contents.tool.test.ts index 5da1910d..e5d762c9 100644 --- a/scripts/__tests__/package-contents.tool.test.ts +++ b/scripts/__tests__/package-contents.tool.test.ts @@ -1,12 +1,10 @@ import { execFileSync } from 'node:child_process'; -import { readdirSync, readFileSync } from 'node:fs'; +import { readFileSync } from 'node:fs'; import path from 'node:path'; +import { findMonorepoRoot, getWorkspacePackageDirs } from '@williamthorsen/nmr/workspace'; import { describe, expect, it } from 'vitest'; -/** Absolute path to the workspace directory holding every package. */ -const packagesDir = path.resolve(import.meta.dirname, '..', '..', 'packages'); - /** * Paths a named package must ship beyond the invariants every publishable package satisfies. A value matches by * prefix, so an exact file path names itself and a directory names everything under it. @@ -58,21 +56,18 @@ interface PublishablePackage { * covered without editing this suite. */ function findPublishablePackages(): ReadonlyArray { - const packages = readdirSync(packagesDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) - .flatMap((entry) => { - const dir = path.join(packagesDir, entry.name); - const manifest: unknown = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); - if (typeof manifest !== 'object' || manifest === null) { - return []; - } - const isPrivate = 'private' in manifest && manifest.private === true; - const name = 'name' in manifest && typeof manifest.name === 'string' ? manifest.name : undefined; - if (isPrivate || name === undefined) { - return []; - } - return [{ bins: readBinPaths(manifest), dir, name }]; - }); + const packages = getWorkspacePackageDirs(findMonorepoRoot(import.meta.dirname)).flatMap((dir) => { + const manifest: unknown = JSON.parse(readFileSync(path.join(dir, 'package.json'), 'utf8')); + if (typeof manifest !== 'object' || manifest === null) { + return []; + } + const isPrivate = 'private' in manifest && manifest.private === true; + const name = 'name' in manifest && typeof manifest.name === 'string' ? manifest.name : undefined; + if (isPrivate || name === undefined) { + return []; + } + return [{ bins: readBinPaths(manifest), dir, name }]; + }); return packages.toSorted((a, b) => a.name.localeCompare(b.name)); } diff --git a/scripts/__tests__/package-scripts.unit.test.ts b/scripts/__tests__/package-scripts.unit.test.ts new file mode 100644 index 00000000..9548c1c6 --- /dev/null +++ b/scripts/__tests__/package-scripts.unit.test.ts @@ -0,0 +1,138 @@ +import { readFileSync } from 'node:fs'; +import path from 'node:path'; + +import { findMonorepoRoot, getWorkspacePackageDirs } from '@williamthorsen/nmr/workspace'; +import { describe, expect, it } from 'vitest'; + +/** + * Script names npm and pnpm invoke on their own schedule, which nmr never resolves as a command. A name nmr does + * carry — `test` among them — stays out of this set, so an nmr call added there is still reported. + */ +const lifecycleScriptNames: ReadonlySet = new Set([ + 'postinstall', + 'postpack', + 'preinstall', + 'prepack', + 'prepare', + 'prepublish', + 'prepublishOnly', +]); + +/** + * Script names the root manifest may reach nmr from. nmr resolves that manifest as a tier-3 override the way it + * resolves a package's, so the rest of it is guarded too. `bootstrap` builds and deploys from a fresh checkout, + * where the bare `nmr` binary is not yet on the path, and so reaches nmr through pnpm by design. + */ +const rootExemptScriptNames: ReadonlySet = new Set([...lifecycleScriptNames, 'bootstrap']); + +/** + * Matches a command reaching `nmr` as a whole token, which covers any runner that fronts it — `npx nmr`, + * `pnpm exec nmr`, `pnpm --filter x exec nmr`. nmr's own warning inspects only a command's first token, so it + * backstops none of those forms. The lookahead spares the leaf bins (`nmr-compile`, `nmr-fmt`) and a path segment + * such as `nmr/bin`, none of which start a nested run. + */ +const shelledNmrPattern = /(?:^|[\s;&|])nmr(?![\w\-/])/; + +/** One manifest the guard reads, with the script names it may reach nmr from. */ +interface GuardedManifest { + readonly exemptNames: ReadonlySet; + readonly manifestPath: string; + readonly scripts: Readonly>; +} + +describe.each(findGuardedManifests())('$manifestPath scripts', ({ exemptNames, manifestPath, scripts }) => { + it('reach nmr through no shell', () => { + // A shelled step puts the nested run's output on the channels a tool's takes, so a quiet failure surrenders the + // whole subtree instead of the failing step. + expect( + findShelledNmrScripts(scripts, exemptNames), + `${manifestPath} invokes nmr through a shell. Delete the override where it restates nmr's own default, or move the steps into a hook such as \`build:post\`.`, + ).toEqual([]); + }); +}); + +describe('findShelledNmrScripts', () => { + it.each([ + ['a bare invocation', 'nmr compile'], + ['a chained invocation', 'tsx scripts/build.ts && nmr compile'], + ['an unspaced operator', 'tsx scripts/build.ts&&nmr compile'], + ['a pnpm exec runner', 'pnpm exec nmr build'], + ['a filtered pnpm runner', 'pnpm --filter codeassembly exec nmr build'], + ['a recursive pnpm runner', 'pnpm --recursive exec nmr build'], + ['an npx runner', 'npx nmr build'], + ])('reports %s', (_label, command) => { + expect(findShelledNmrScripts({ build: command }, lifecycleScriptNames)).toEqual(['build']); + }); + + it.each([ + ['a leaf bin', 'nmr-compile'], + ['a flagged leaf bin', 'nmr-fmt --check'], + ['a path segment', 'node node_modules/@williamthorsen/nmr/bin/cli.js'], + ['an unrelated command', 'tsx scripts/build.ts && rdy compile'], + ])('passes over %s', (_label, command) => { + expect(findShelledNmrScripts({ build: command }, lifecycleScriptNames)).toEqual([]); + }); + + it('passes over an exempt script name', () => { + expect(findShelledNmrScripts({ prepublishOnly: 'nmr build' }, lifecycleScriptNames)).toEqual([]); + }); + + it('reports every offending name in sorted order', () => { + const scripts = { build: 'nmr compile', check: 'npx nmr lint', fmt: 'nmr-fmt --write' }; + expect(findShelledNmrScripts(scripts, lifecycleScriptNames)).toEqual(['build', 'check']); + }); +}); + +// region | Helpers + +/** Lists the manifests the guard reads: the monorepo root's, then every workspace package's. */ +function findGuardedManifests(): ReadonlyArray { + const monorepoRoot = findMonorepoRoot(import.meta.dirname); + const packageManifests = getWorkspacePackageDirs(monorepoRoot).map((dir) => ({ + exemptNames: lifecycleScriptNames, + manifestPath: path.relative(monorepoRoot, path.join(dir, 'package.json')), + scripts: readScripts(path.join(dir, 'package.json')), + })); + return [ + { + exemptNames: rootExemptScriptNames, + manifestPath: 'package.json', + scripts: readScripts(path.join(monorepoRoot, 'package.json')), + }, + ...packageManifests, + ]; +} + +/** Lists the script names whose values reach nmr through a shell, passing over the exempt names. */ +function findShelledNmrScripts( + scripts: Readonly>, + exemptNames: ReadonlySet, +): ReadonlyArray { + return Object.entries(scripts) + .filter(([name, value]) => !exemptNames.has(name) && shelledNmrPattern.test(value)) + .map(([name]) => name) + .toSorted(); +} + +/** Reads a manifest's `scripts` field, keeping the string values nmr can resolve as a command. */ +function readScripts(manifestPath: string): Readonly> { + const manifest: unknown = JSON.parse(readFileSync(manifestPath, 'utf8')); + if (typeof manifest !== 'object' || manifest === null || !('scripts' in manifest)) { + return {}; + } + const { scripts } = manifest; + if (typeof scripts !== 'object' || scripts === null) { + return {}; + } + // The annotation is what keeps `Object.entries` from widening each value to `any` and defeating the check below. + const entries: ReadonlyArray = Object.entries(scripts); + const result: Record = {}; + for (const [name, value] of entries) { + if (typeof value === 'string') { + result[name] = value; + } + } + return result; +} + +// endregion | Helpers