diff --git a/packages/nopy/cubes/apt/all/manifest.mjs b/packages/nopy/cubes/apt/all/manifest.mjs index 2abd002..fe52f43 100644 --- a/packages/nopy/cubes/apt/all/manifest.mjs +++ b/packages/nopy/cubes/apt/all/manifest.mjs @@ -1,6 +1,6 @@ import { cubes } from '@bitsquare/nopy'; export default cubes.Manifest({ - name: '[apt-all] Test dependencies', - dependencies: () => ['apt/more'], + name: '[test:apt-all] Test dependencies', + dependencies: () => ['test:apt-more'], }); diff --git a/packages/nopy/cubes/apt/essentials/manifest.mjs b/packages/nopy/cubes/apt/essentials/manifest.mjs index fef6d35..026b2ae 100644 --- a/packages/nopy/cubes/apt/essentials/manifest.mjs +++ b/packages/nopy/cubes/apt/essentials/manifest.mjs @@ -2,7 +2,7 @@ import { cubes } from '@bitsquare/nopy'; import { z } from 'zod'; export default cubes.Manifest({ - name: '[apt:essentials] Install essential packages', + name: '[test:apt-essentials] Install essential packages', dependencies: () => [], schema: z.object({ UPDATE: z.boolean().default(false), diff --git a/packages/nopy/cubes/apt/more/manifest.mjs b/packages/nopy/cubes/apt/more/manifest.mjs index c96956d..70bf794 100644 --- a/packages/nopy/cubes/apt/more/manifest.mjs +++ b/packages/nopy/cubes/apt/more/manifest.mjs @@ -1,6 +1,6 @@ import { cubes } from '@bitsquare/nopy'; export default cubes.Manifest({ - name: '[apt-more] Test dependencies', - dependencies: () => [['apt/essentials']], + name: '[test:apt-more] Test dependencies', + dependencies: () => [['test:apt-essentials']], }); diff --git a/packages/nopy/docs/CUBE-PACKAGES.md b/packages/nopy/docs/CUBE-PACKAGES.md index 4bbdb60..16fe2f8 100644 --- a/packages/nopy/docs/CUBE-PACKAGES.md +++ b/packages/nopy/docs/CUBE-PACKAGES.md @@ -1,6 +1,6 @@ # Cube bundles as npm packages -Status: **plan, not a record.** Nothing here is implemented yet. +Status: **Phase 0 has landed; Phases 1–6 are still a plan, not a record.** Distributing cubes as npm packages so a project can `pnpm add @acme/cubes-net` and have its cubes show up in `nopy` alongside local ones. @@ -58,39 +58,60 @@ What blocks a clean story: `isSymbolicLink()`, not `isDirectory()` — the scan would skip every package. Package roots must be resolved explicitly. -## Phase 0 — fixes that land first +## Phase 0 — fixes that land first — **done** Independent of packaging, and the duplicate-id work depends on them. -**0.1 `scanDirectory` drops subtrees on duplicates.** `loader.ts:84-87` pushes -the error and `return`s, which exits before the recursive descent at line 100. -Cubes nested below a duplicate never get scanned, so the error report is -incomplete: you fix one collision, re-run, find the next. Should record the -duplicate and keep descending. +**0.1 `scanDirectory` drops subtrees on duplicates.** `loader.ts:84-87` pushed +the error and `return`ed, which exited before the recursive descent at line 100. +Cubes nested below a duplicate never got scanned, so the error report was +incomplete: you fix one collision, re-run, find the next. -**0.2 Duplicate detection is order-dependent.** `loadCubes()` runs -`Promise.all` over folders into a shared `cubes` object, so which source is -"first" and which is "the duplicate" varies run to run. Restructure: the scan -emits a flat list of candidates, then a single grouping pass builds `cubes` and -the error list. Makes the hard-error path deterministic, which the tests need. +**0.2 Duplicate detection is order-dependent.** `loadCubes()` ran `Promise.all` +over folders into a shared `cubes` object, so which source was "first" and which +was "the duplicate" varied run to run. + +Both are one restructure. Scanning and id resolution are now separate passes: +each root fills its own `ScanResult`, the lists are concatenated in root order +(`Promise.all` preserves input order regardless of completion order), and a +grouping pass builds `cubes` and the errors. `scanDirectory` no longer decides +anything about ids, so it always descends. Directory entries are sorted, and a +directory reachable from two roots is deduped by path — one cube seen twice is +not a collision, which it used to be reported as. **0.3 `apt:essentials` is already declared twice.** `cubes/apt/essentials` -declares it via `id`; `packages/nopy/cubes/apt/essentials` declares it via the +declares it via `id`; `packages/nopy/cubes/apt/essentials` declared it via an `[apt:essentials]` prefix in `name`. `cubeDirs` merges root-first, so running -`nopy` from `packages/nopy` already collects both and errors. Rename the -`packages/nopy/cubes` fixtures (`[test:apt-essentials]`, `[test:apt-all]`, -`[test:apt-more]`) — they are dev fixtures, not real cubes, and the migration in -Phase 5 makes the collision permanent otherwise. +`nopy` from `packages/nopy` collected both and aborted. Confirmed against the +real trees before the rename: + +``` +Duplicate cube id 'apt:essentials' from 2 sources: + /…/ansiblingz/cubes/apt/essentials + /…/ansiblingz/packages/nopy/cubes/apt/essentials +Rename one of them, or remove a source from .nopyrc.json. +``` + +The three `packages/nopy/cubes` fixtures are now `[test:apt-essentials]`, +`[test:apt-all]` and `[test:apt-more]`; all 25 cubes load with no errors. Their +`dependencies` were stale too — they named `apt/more` and `apt/essentials`, +which are not ids anything declares — so they now point at the renamed ids. **0.4 `coerceValue` breaks if zod is ever duplicated.** `nopy.prompts.ts:147-154` -discriminates with `instanceof z.ZodDefault`, `z.ZodBoolean`, `z.ZodNumber` and +discriminated with `instanceof z.ZodDefault`, `z.ZodBoolean`, `z.ZodNumber` and friends — checks against the *running CLI's* zod instance. The moment a bundle resolves its own copy of zod (entirely possible once manifests arrive from `node_modules`; see Phase 4), every check returns false and `coerceValue` falls through to the raw string, silently. Booleans stop being booleans. -Rewrite against the string discriminant, which is instance-agnostic. Verified on -the installed zod 4.4.3: +`defaultValueOf` in `cubes/types.ts` had the same breakage, reached whenever a +schema has one field without a `.default()` — `getDefaults()` tries +`safeParse({})` first, which is instance-agnostic, and only then drops to the +per-field read. + +Both now discriminate on `def.type`, a plain string that holds across instances, +via two exported helpers (`zodKind`, `zodInner`). Verified on the installed zod +4.4.3: ``` z.boolean().default(false).def.type → 'default' @@ -98,8 +119,13 @@ z.boolean().default(false).def.innerType → { def: { type: 'boolean' } } z.number().def.type → 'number' ``` -Do this before anything else in Phase 4 lands, and it stops being a footgun for -the local `cubes/` tree too. +`tests/helpers/foreign-zod.ts` rebuilds a schema as plain objects carrying zod's +`def` but not its prototype — structurally what a second copy of zod produces, +and `instanceof`-blind, so neither call site can regress. + +Worth noting for Phase 4: zod 4 exposes `def.defaultValue` as a getter that +already invokes a lazily declared default, so the `typeof === 'function'` branch +in `defaultValueOf` is now dead. It is kept as insurance against that changing. ## Phase 1 — the bundle contract @@ -376,6 +402,18 @@ With the split, the hook is a convenience rather than load-bearing: bundles resolve `@bitsquare/nopy-cube` through their own `node_modules` and never reach it. +**The gotcha is bigger than CLAUDE.md says: it is two specifiers, not one.** +Measured by linking `@bitsquare/nopy` into the root `node_modules` and loading +the real tree — every manifest then failed on `Cannot find package 'zod'` +instead. Manifests import `z` directly to build their schema, and pnpm's +isolated layout puts zod under `packages/nopy/node_modules`, not the root. A +hook that only covers `@bitsquare/nopy` moves the error rather than fixing it, +so it has to fall back for `zod` too. With both linked, all 25 cubes load. + +Falling back for `zod` hands local cubes the *CLI's* zod instance, so no +duplication arises there. Bundles are the case that duplicates it, and Phase 0.4 +is what makes that safe. + New `packages/nopy/src/nopy.resolve-hook.mjs`, registered once from `loadCubes()` before the first `import(manifestPath)`: @@ -398,8 +436,9 @@ Constraints: structured-cloneable (a string URL is). - The `.mjs` must ship in `dist` and be listed in `files` — it already is, via the `dist` entry. -- It resolves `@bitsquare/nopy`, not `@bitsquare/nopy-cube`. Bundles never depend - on the hook; only the in-repo `cubes/` tree and hand-written local cubes do. +- It resolves `@bitsquare/nopy` and `zod`, not `@bitsquare/nopy-cube`. Bundles + never depend on the hook; only the in-repo `cubes/` tree and hand-written local + cubes do. ## Phase 5 — proof of concept: `packages/cubes-core` diff --git a/packages/nopy/src/cubes/index.ts b/packages/nopy/src/cubes/index.ts index 233074e..fd8bea9 100644 --- a/packages/nopy/src/cubes/index.ts +++ b/packages/nopy/src/cubes/index.ts @@ -31,6 +31,8 @@ export type { export { Cube, Manifest, + zodInner, + zodKind, } from './types.js'; // Utilities diff --git a/packages/nopy/src/cubes/loader.ts b/packages/nopy/src/cubes/loader.ts index 6cc08d8..4306dad 100644 --- a/packages/nopy/src/cubes/loader.ts +++ b/packages/nopy/src/cubes/loader.ts @@ -50,68 +50,131 @@ function extractCubeId(manifest: Manifest): string | undefined { return match ? match[1] : undefined; } +/** A cube found on disk, before ids have been checked against each other. */ +interface CubeCandidate { + id: string; + manifest: Manifest; + dir: string; + deployScript: string; +} + +/** What one root directory contributed. */ +interface ScanResult { + candidates: CubeCandidate[]; + errors: string[]; +} + /** - * Loads all cubes from discovered cube directories. + * Walks one root, collecting every cube below it. + * + * Deliberately does not decide anything about ids: a duplicate is only visible + * once every root has been walked, and stopping the descent here would hide + * whatever sits below the offending directory. */ -export async function loadCubes(): Promise { - const cubesFolders = findCubeDirectories(); - const cubes: Record = {}; - const errors: string[] = []; +async function scanDirectory(currentDir: string, result: ScanResult): Promise { + const entries = (await fs.readdir(currentDir, { withFileTypes: true })).sort((a, b) => + a.name.localeCompare(b.name) + ); - async function scanDirectory(currentDir: string, baseDir: string): Promise { - const entries = await fs.readdir(currentDir, { withFileTypes: true }); + const files = entries.filter((e) => e.isFile()); + const manifestFile = files.find( + (f) => f.name === 'manifest.mjs' || f.name.endsWith('.manifest.mjs') + ); + const deployFile = files.find((f) => f.name === 'deploy.py' || f.name.endsWith('.deploy.py')); - const files = entries.filter((e) => e.isFile()); - const manifestFile = files.find( - (f) => f.name === 'manifest.mjs' || f.name.endsWith('.manifest.mjs') - ); - const deployFile = files.find((f) => f.name === 'deploy.py' || f.name.endsWith('.deploy.py')); + if (manifestFile && deployFile) { + const manifestPath = path.join(currentDir, manifestFile.name); - if (manifestFile && deployFile) { - const cubePath = currentDir; - const manifestPath = path.join(cubePath, manifestFile.name); + try { + const manifest = (await import(manifestPath)).default as Manifest; - try { - const manifest = (await import(manifestPath)).default as Manifest; + if (!manifest || typeof manifest !== 'object') { + result.errors.push(`Invalid manifest export in ${manifestPath}`); + } else if (!manifest.name) { + result.errors.push(`Invalid manifest format in ${manifestPath}: missing 'name'`); + } else { + const cubeId = extractCubeId(manifest) || path.basename(currentDir); - if (!manifest || typeof manifest !== 'object') { - errors.push(`Invalid manifest export in ${manifestPath}`); - } else if (!manifest.name) { - errors.push(`Invalid manifest format in ${manifestPath}: missing 'name'`); - } else { - const cubeId = extractCubeId(manifest) || path.basename(cubePath); + // Ensure basic properties + manifest.id = cubeId; + manifest.schema = manifest.schema ?? z.object({}); - if (cubes[cubeId]) { - errors.push(`Duplicate cube id '${cubeId}'`); - return; - } - - // Ensure basic properties - manifest.id = cubeId; - manifest.schema = manifest.schema ?? z.object({}); - - cubes[cubeId] = new Cube(manifest, cubePath, deployFile.name); - } - } catch (err) { - errors.push(`Failed to load manifest ${manifestPath}: ${err}`); - } - } - - for (const entry of entries) { - if (entry.isDirectory() && !entry.name.startsWith('.') && entry.name !== 'node_modules') { - await scanDirectory(path.join(currentDir, entry.name), baseDir); + result.candidates.push({ + id: cubeId, + manifest, + dir: currentDir, + deployScript: deployFile.name, + }); } + } catch (err) { + result.errors.push(`Failed to load manifest ${manifestPath}: ${err}`); } } - await Promise.all( + for (const entry of entries) { + if (entry.isDirectory() && !entry.name.startsWith('.') && entry.name !== 'node_modules') { + await scanDirectory(path.join(currentDir, entry.name), result); + } + } +} + +/** The message a duplicate id produces. Aborts the run — see `nopy.main.ts`. */ +function duplicateError(id: string, group: CubeCandidate[]): string { + const where = group.map((c) => ` ${c.dir}`).join('\n'); + return ( + `Duplicate cube id '${id}' from ${group.length} sources:\n${where}\n` + + `Rename one of them, or remove a source from .nopyrc.json.` + ); +} + +/** + * Loads all cubes from discovered cube directories. + * + * Scanning and id resolution are separate passes on purpose. Each root + * contributes its own candidate list, and those lists are concatenated in + * root order rather than in whichever order the concurrent scans happened to + * finish — so which cube is reported as "the duplicate" is the same on every + * run, which is what makes the hard error testable. + */ +export async function loadCubes(): Promise { + const cubesFolders = findCubeDirectories(); + + const scans = await Promise.all( cubesFolders.map(async (folder) => { + const result: ScanResult = { candidates: [], errors: [] }; if (fs.existsSync(folder)) { - await scanDirectory(folder, folder); + await scanDirectory(folder, result); } + return result; }) ); + // Promise.all preserves input order regardless of completion order. + const errors = scans.flatMap((scan) => scan.errors); + + // One directory reachable from two roots (a `cubeDirs` entry nested under a + // `.npcubes` marker, say) is one cube seen twice, not a collision. + const seenDirs = new Set(); + const byId = new Map(); + + for (const candidate of scans.flatMap((scan) => scan.candidates)) { + if (seenDirs.has(candidate.dir)) continue; + seenDirs.add(candidate.dir); + + const group = byId.get(candidate.id); + if (group) group.push(candidate); + else byId.set(candidate.id, [candidate]); + } + + const cubes: Record = {}; + for (const [id, group] of byId) { + if (group.length > 1) errors.push(duplicateError(id, group)); + // The map is still populated for the callers that only report; a duplicate + // is fatal, so which candidate landed here never reaches a deploy. + const [first] = group; + cubes[id] = new Cube(first.manifest, first.dir, first.deployScript); + } + return { cubes, errors }; } diff --git a/packages/nopy/src/cubes/types.ts b/packages/nopy/src/cubes/types.ts index 007dbc6..4427e07 100644 --- a/packages/nopy/src/cubes/types.ts +++ b/packages/nopy/src/cubes/types.ts @@ -82,6 +82,28 @@ export namespace Manifest { } } +/** + * zod's runtime discriminant for a schema node, as a plain string. + * + * `instanceof z.ZodDefault` compares against the *running* copy of zod. A cube + * manifest is free to build its schema with a different copy — its own + * dependency, or one shipped inside a bundle — and then every `instanceof` + * quietly returns false and the caller falls through to a wrong answer instead + * of failing. `def.type` holds across instances, so nothing here may go back to + * `instanceof`. + */ +export function zodKind(zodType: unknown): string { + return (zodType as { def: { type: string } }).def.type; +} + +/** + * The type a wrapper wraps — `.default()`, `.optional()`, `.nullable()`. + * Only call this for a node whose {@link zodKind} is one of those. + */ +export function zodInner(zodType: unknown): z.ZodType { + return (zodType as { def: { innerType: z.ZodType } }).def.innerType; +} + /** * Reads the `.default()` off a schema field, unwrapping the wrappers that may * sit above it (`.default().optional()`, `.default().nullable()`). @@ -90,16 +112,32 @@ export namespace Manifest { * `requiredKeys()` recognises a field the user has to supply. */ function defaultValueOf(zodType: z.ZodType): unknown { - if (zodType instanceof z.ZodDefault) { - const { defaultValue } = zodType._def as { defaultValue: unknown }; + const kind = zodKind(zodType); + if (kind === 'default') { + // zod 4 exposes `defaultValue` as a getter that already invokes a lazily + // declared default; the function branch is insurance against that changing. + const { defaultValue } = (zodType as unknown as { def: { defaultValue: unknown } }).def; return typeof defaultValue === 'function' ? defaultValue() : defaultValue; } - if (zodType instanceof z.ZodOptional || zodType instanceof z.ZodNullable) { - return defaultValueOf(zodType._def.innerType as z.ZodType); + if (kind === 'optional' || kind === 'nullable') { + return defaultValueOf(zodInner(zodType)); } return undefined; } +/** + * Where a cube was discovered. + * + * Worth carrying because a cube's own directory does not say how it got into + * the run: `/…/node_modules/@acme/cubes-net/cubes/x` could equally have come + * from a `cubeDirs` entry pointing straight at it. + */ +export type CubeSource = + /** Found under a `cubeDirs` entry or a `.npcubes` marker, at `dir`. */ + | { type: 'dir'; dir: string } + /** Contributed by a package named in `cubePackages`. */ + | { type: 'package'; packageName: string; dir: string }; + /** * A fully loaded cube with its filesystem location and runtime state */ @@ -107,7 +145,9 @@ export class Cube { constructor( public readonly manifest: Manifest, public readonly dir: string, - public readonly deployScript: string + public readonly deployScript: string, + /** Defaults to the cube's own directory, for cubes built by hand. */ + public readonly source: CubeSource = { type: 'dir', dir } ) {} get id(): string { diff --git a/packages/nopy/src/nopy.config.ts b/packages/nopy/src/nopy.config.ts index d169ece..16a13b5 100644 --- a/packages/nopy/src/nopy.config.ts +++ b/packages/nopy/src/nopy.config.ts @@ -55,13 +55,41 @@ export type ResolutionConfig = { }; /** - * Raw config file structure (includes resolution) + * A cube package named in `cubePackages`, paired with where it was named. + * + * Node resolution has to start from the config file that asked for the package, + * not from `process.cwd()` — otherwise a package listed in `~/.nopyrc.json` + * only resolves in projects that happen to depend on it themselves. This is the + * same problem {@link PATH_PROPERTIES} solves for `cubeDirs`, except the answer + * is a reference to resolve later rather than a rewritten path. */ -export interface NopyConfigFile extends Partial { +export interface CubePackageRef { + /** The package name as written in the config, e.g. `@acme/cubes-net`. */ + spec: string; + /** Directory of the `.nopyrc.json` that named it. */ + from: string; +} + +/** + * Raw config file structure (includes resolution) + * + * Diverges from {@link NopyConfig} for `cubePackages`: a file lists plain + * package names, and loading turns each into a {@link CubePackageRef}. + */ +export interface NopyConfigFile extends Omit, 'cubePackages'> { + /** Cube packages to load, by package name */ + cubePackages?: string[]; /** Customize merge behavior for specific properties */ resolution?: ResolutionConfig; } +/** + * A config file whose paths have been resolved — what actually gets merged. + */ +type ResolvedConfigFile = Omit & { + cubePackages?: CubePackageRef[]; +}; + /** * Nopy configuration file structure */ @@ -70,6 +98,8 @@ export interface NopyConfig { hosts: string[]; /** Directories to search for cubes */ cubeDirs: string[]; + /** Installed packages to load cubes from */ + cubePackages: CubePackageRef[]; /** Global environment variables */ env: TVariables; /** Logging configuration */ @@ -86,6 +116,7 @@ export interface NopyConfig { const DEFAULT_CONFIG: NopyConfig = { hosts: [], cubeDirs: [], + cubePackages: [], env: {}, }; @@ -219,30 +250,33 @@ const PATH_PROPERTIES: (keyof NopyConfig)[] = ['cubeDirs']; * Resolves relative paths in a config file based on its location * Only resolves paths for properties that are known to contain filesystem paths */ -function resolveConfigPaths(config: NopyConfigFile, configPath: string): NopyConfigFile { +function resolveConfigPaths(config: NopyConfigFile, configPath: string): ResolvedConfigFile { const configDir = path.dirname(configPath); - const resolved: NopyConfigFile = {}; + const resolved: Record = {}; for (const [key, value] of Object.entries(config)) { if (key === 'resolution') { // Don't resolve the resolution config itself resolved[key] = value as ResolutionConfig; + } else if (key === 'cubePackages') { + // Not a path — a package name, tagged with where to resolve it from. + resolved[key] = (value as string[]).map((spec) => ({ spec, from: configDir })); } else if (PATH_PROPERTIES.includes(key as keyof NopyConfig)) { // Only resolve paths for known path properties - resolved[key as keyof NopyConfigFile] = resolveRelativePaths(value, configDir) as any; + resolved[key] = resolveRelativePaths(value, configDir); } else { // Copy other properties as-is (including hosts) - resolved[key as keyof NopyConfigFile] = value as any; + resolved[key] = value; } } - return resolved; + return resolved as ResolvedConfigFile; } /** * Merges a child config into a parent config */ -function mergeConfigs(parent: NopyConfig, childFile: NopyConfigFile): NopyConfig { +function mergeConfigs(parent: NopyConfig, childFile: ResolvedConfigFile): NopyConfig { const resolution = childFile.resolution || {}; const result: Record = { ...parent }; diff --git a/packages/nopy/src/nopy.prompts.ts b/packages/nopy/src/nopy.prompts.ts index b473acd..786e861 100644 --- a/packages/nopy/src/nopy.prompts.ts +++ b/packages/nopy/src/nopy.prompts.ts @@ -6,8 +6,8 @@ import Enquirer from 'enquirer'; import fuzzy from 'fuzzy'; import inquirer from 'inquirer'; -import { z } from 'zod'; -import type { AnyObjectSchema, Cube } from './cubes/index.js'; +import type { z } from 'zod'; +import { type AnyObjectSchema, type Cube, zodInner, zodKind } from './cubes/index.js'; import type { Variables } from './nopy.common.js'; interface CubeChoice { @@ -142,20 +142,32 @@ export async function HostSelection(hosts: string[]): Promise { return selectedHost.customHost ?? selectedHost.host; } +/** + * Turns a form answer — always a string — back into what the schema declares. + * + * Discriminates on {@link zodKind} rather than `instanceof`: the schema may + * have been built by a copy of zod that is not the one this file imported, and + * `instanceof` would then fail open and leave every value a string. + */ function coerceValue(value: unknown, zodType: z.core.$ZodType): unknown { if (typeof value !== 'string') return value; - if (zodType instanceof z.ZodDefault) return coerceValue(value, zodType._def.innerType); - if (zodType instanceof z.ZodOptional) return coerceValue(value, zodType._def.innerType); - if (zodType instanceof z.ZodNullable) { - if (value === 'null' || value === '') return null; - return coerceValue(value, zodType._def.innerType); + + switch (zodKind(zodType)) { + case 'default': + case 'optional': + return coerceValue(value, zodInner(zodType)); + case 'nullable': + if (value === 'null' || value === '') return null; + return coerceValue(value, zodInner(zodType)); + case 'boolean': + return value === 'true' || value === 'yes' || value === '1'; + case 'number': { + const num = Number(value); + return Number.isNaN(num) ? value : num; + } + default: + return value; } - if (zodType instanceof z.ZodBoolean) return value === 'true' || value === 'yes' || value === '1'; - if (zodType instanceof z.ZodNumber) { - const num = Number(value); - return Number.isNaN(num) ? value : num; - } - return value; } interface FormChoice { diff --git a/packages/nopy/tests/cubes.loader.edge.test.ts b/packages/nopy/tests/cubes.loader.edge.test.ts index dbefaeb..0c7bb6d 100644 --- a/packages/nopy/tests/cubes.loader.edge.test.ts +++ b/packages/nopy/tests/cubes.loader.edge.test.ts @@ -136,14 +136,63 @@ describe('loader edge cases', () => { expect(errors[0]).toMatch(/Failed to load manifest/); }); - it('reports duplicate cube ids', async () => { + it('reports duplicate cube ids, naming every directory that claims one', async () => { cube('first', 'export default { id: "dup", name: "First" }'); cube('second', 'export default { id: "dup", name: "Second" }'); const { cubes, errors } = await loadCubes(); expect(Object.keys(cubes)).toEqual(['dup']); - expect(errors[0]).toMatch(/Duplicate cube id 'dup'/); + expect(errors).toHaveLength(1); + expect(errors[0]).toMatch(/Duplicate cube id 'dup' from 2 sources/); + expect(errors[0]).toContain(path.join(tmpDir, 'first')); + expect(errors[0]).toContain(path.join(tmpDir, 'second')); + }); + + it('keeps scanning below a duplicate instead of dropping the subtree', async () => { + cube('first', 'export default { id: "dup", name: "First" }'); + cube('second', 'export default { id: "dup", name: "Second" }'); + cube('second/inner', 'export default { id: "buried", name: "Buried" }'); + + const { cubes, errors } = await loadCubes(); + + expect(cubes.buried).toBeDefined(); + expect(errors).toHaveLength(1); + }); + + it('reports the same duplicate whichever root is scanned first', async () => { + cube('a/one', 'export default { id: "dup", name: "One" }'); + cube('b/two', 'export default { id: "dup", name: "Two" }'); + const roots = [path.join(tmpDir, 'a'), path.join(tmpDir, 'b')]; + + fs.writeFileSync(path.join(tmpDir, '.nopyrc.json'), JSON.stringify({ cubeDirs: roots })); + const forwards = await loadCubes(); + + fs.writeFileSync( + path.join(tmpDir, '.nopyrc.json'), + JSON.stringify({ cubeDirs: [...roots].reverse() }) + ); + const backwards = await loadCubes(); + + expect(forwards.errors[0]).toContain(path.join(tmpDir, 'a', 'one')); + expect(forwards.errors[0]).toContain(path.join(tmpDir, 'b', 'two')); + expect(backwards.errors).toHaveLength(1); + expect(new Set(backwards.errors[0].split('\n'))).toEqual( + new Set(forwards.errors[0].split('\n')) + ); + }); + + it('does not call one directory a duplicate of itself when two roots reach it', async () => { + cube('nested/one', 'export default { id: "once", name: "Once" }'); + fs.writeFileSync( + path.join(tmpDir, '.nopyrc.json'), + JSON.stringify({ cubeDirs: ['./', './nested'] }) + ); + + const { cubes, errors } = await loadCubes(); + + expect(errors).toEqual([]); + expect(cubes.once).toBeDefined(); }); it('skips hidden and node_modules directories', async () => { diff --git a/packages/nopy/tests/cubes.types.test.ts b/packages/nopy/tests/cubes.types.test.ts index 420f193..2132ff1 100644 --- a/packages/nopy/tests/cubes.types.test.ts +++ b/packages/nopy/tests/cubes.types.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it } from 'vitest'; import { z } from 'zod'; import { Cube, Manifest } from '../src/cubes/types.js'; +import { foreignZodSchema } from './helpers/foreign-zod.js'; const cube = (schema: z.ZodObject) => new Cube(Manifest.create({ id: 'c', name: 'C', schema }), '/cubes/c', 'deploy.py'); @@ -64,6 +65,22 @@ describe('Cube.getDefaults', () => { it('returns an empty object for an empty schema', () => { expect(cube(z.object({})).getDefaults()).toEqual({}); }); + + it('reads defaults off a schema built by a different copy of zod', () => { + // The per-field fallback reads zod's internals directly. Under `instanceof` + // a foreign schema yields no defaults at all, without erroring. + const c = cube( + foreignZodSchema( + z.object({ + REQUIRED: z.string(), + PRIORITY: z.number().default(10), + NESTED: z.number().default(2).optional(), + }) + ) + ); + + expect(c.getDefaults()).toEqual({ PRIORITY: 10, NESTED: 2 }); + }); }); describe('Cube.requiredKeys', () => { diff --git a/packages/nopy/tests/helpers/foreign-zod.ts b/packages/nopy/tests/helpers/foreign-zod.ts new file mode 100644 index 0000000..9cb4504 --- /dev/null +++ b/packages/nopy/tests/helpers/foreign-zod.ts @@ -0,0 +1,29 @@ +/** + * A schema that behaves like zod's but does not share zod's prototypes. + * + * Once cubes arrive from `node_modules`, the schema a manifest builds may come + * from a *second* copy of zod — its own dependency, or one shipped inside a + * bundle. Such a schema is structurally identical and `instanceof` blind to it. + * Rebuilding the nodes as plain objects reproduces that from inside a single + * process, so anything that reads zod's internals stays pinned to `def.type`. + */ + +import type { z } from 'zod'; + +/** Strips the prototype off a schema node and everything it wraps. */ +function strip(node: unknown): unknown { + const def = { ...(node as { def: Record }).def }; + if (def.innerType) def.innerType = strip(def.innerType); + return { def }; +} + +export function foreignZodSchema>(schema: S): S { + return { + // Parsing is not what is under test — delegate it and keep the real + // behaviour, so only the introspection path sees the foreign nodes. + safeParse: (value: unknown) => schema.safeParse(value), + shape: Object.fromEntries( + Object.entries(schema.shape).map(([key, node]) => [key, strip(node)]) + ), + } as unknown as S; +} diff --git a/packages/nopy/tests/prompts.test.ts b/packages/nopy/tests/prompts.test.ts index 83bbfd5..158af63 100644 --- a/packages/nopy/tests/prompts.test.ts +++ b/packages/nopy/tests/prompts.test.ts @@ -46,6 +46,7 @@ import { PasswordSelection, VariableAssignment, } from '../src/nopy.prompts.js'; +import { foreignZodSchema } from './helpers/foreign-zod.js'; /** Grabs the single question object passed to the last inquirer.prompt call. */ const questions = () => inquirerPrompt.mock.calls.at(-1)?.[0] as Record[]; @@ -310,13 +311,14 @@ describe('VariableAssignment', () => { const nullableSchema = z.object({ maybe: z.number().nullable().default(1), opt: z.number().optional().default(2), + given: z.number().nullable().default(3), }); const variables = new Variables(); - formRun.mockResolvedValue({ maybe: 'null', opt: '7' }); + formRun.mockResolvedValue({ maybe: 'null', opt: '7', given: '42' }); await VariableAssignment(cube('svc', 'Service', nullableSchema), variables); - expect(variables.get('svc', 'prompts')).toEqual({ maybe: null, opt: 7 }); + expect(variables.get('svc', 'prompts')).toEqual({ maybe: null, opt: 7, given: 42 }); }); it('treats an empty string as null for a nullable field', async () => { @@ -356,4 +358,28 @@ describe('VariableAssignment', () => { ).resolves.toBeUndefined(); expect(variables.get('svc', 'prompts')).toEqual({}); }); + + it('coerces against a schema built by a different copy of zod', async () => { + // Guards the discriminant in `coerceValue`: under `instanceof` every check + // here returns false and the answers stay strings, silently. + const variables = new Variables(); + formRun.mockResolvedValue({ port: '9090', enabled: 'true', maybe: '' }); + + await VariableAssignment( + cube( + 'svc', + 'Service', + foreignZodSchema( + z.object({ + port: z.number().default(8080), + enabled: z.boolean().default(false), + maybe: z.number().nullable().default(1), + }) + ) + ), + variables + ); + + expect(variables.get('svc', 'prompts')).toEqual({ port: 9090, enabled: true, maybe: null }); + }); });