From 659f44134317552eddf3c985f0aa2a41aaf3a702 Mon Sep 17 00:00:00 2001 From: Jake Jarvis Date: Tue, 26 May 2026 19:20:01 -0400 Subject: [PATCH] fix: harden `applyModule` ordering, partial-init diagnostics, and `consumesPackages` context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `consumesPackages` field to `Module` and thread it into `buildRenderContext` / `TemplateContext` so templates can reference sibling package names; update `manifest.ts`, `package-json.ts`, `template.ts`, and their tests - Queue all file/package.json/env writes behind a `deferredWrites` array so `writeManifest` always persists before any disk mutation — a mid-flush crash leaves the manifest as ground truth with no orphan files it can't account for - Move codemod-catalog ID validation before any writes so `--dry-run` catches unknown codemod IDs without touching the filesystem; codemods now run in a second phase after deferred writes flush, with a follow-up `writeManifest` after claim gathering - Wrap `init`'s module-apply loop in try/catch; on failure, log which modules were already applied and leave the partial project on disk for inspection rather than silently exiting --- apps/cli/src/commands/init.ts | 99 +++++++++------ apps/cli/src/lib/codemod-runner.ts | 117 ++++++++++-------- .../src/components/builder/module-cards.tsx | 2 +- apps/web/src/lib/selection.ts | 15 +++ apps/web/src/routes/index.tsx | 19 +-- .../web/src/server/builder-state.functions.ts | 17 +-- .../web/src/server/registry-modules.server.ts | 19 ++- packages/registry/src/manifest.test.ts | 58 +++++++++ packages/registry/src/manifest.ts | 78 ++++++++---- packages/registry/src/module.ts | 11 +- packages/registry/src/package-json.test.ts | 58 +++++++++ packages/registry/src/package-json.ts | 27 +++- packages/registry/src/synthesize.ts | 2 + packages/registry/src/template.test.ts | 30 ++++- packages/registry/src/template.ts | 25 +++- 15 files changed, 422 insertions(+), 155 deletions(-) diff --git a/apps/cli/src/commands/init.ts b/apps/cli/src/commands/init.ts index ec8731b..4354012 100644 --- a/apps/cli/src/commands/init.ts +++ b/apps/cli/src/commands/init.ts @@ -124,48 +124,69 @@ export async function cmdInit(args: CliArgs): Promise { // Collected as each module applies; feeds `synthesizeReadme` at the end // (`Module` objects are already in memory from the wizard — no reload needed). const resolvedAll: Resolved = {}; + const applied: string[] = []; - for (const category of categoryOrder) { - const home = categoryHome(category); - for (const mod of result.selections[category] ?? []) { - spinner.start(`Installing ${mod.label}`); - const adapter = resolveAdapter(mod, { - manifest, - pending: fullPending, - targetAppId: home.kind === "app" ? appHomeTarget[0]!.id : undefined, - }); - if (!adapter.ok) { - spinner.stop(`${mod.label} ${pc.red("failed")}`); - throw new Error( - `Could not resolve adapter for ${category}/${mod.id}: ${adapter.error.kind}`, - ); + try { + for (const category of categoryOrder) { + const home = categoryHome(category); + for (const mod of result.selections[category] ?? []) { + spinner.start(`Installing ${mod.label}`); + const adapter = resolveAdapter(mod, { + manifest, + pending: fullPending, + targetAppId: home.kind === "app" ? appHomeTarget[0]!.id : undefined, + }); + if (!adapter.ok) { + spinner.stop(`${mod.label} ${pc.red("failed")}`); + throw new Error( + `Could not resolve adapter for ${category}/${mod.id}: ${adapter.error.kind}`, + ); + } + // Validate appKind when the module declares one (typically frameworks). + if (home.kind === "app" && mod.appKind && mod.appKind !== appHomeTarget[0]!.kind) { + spinner.stop(`${mod.label} ${pc.red("failed")}`); + throw new Error( + `${category}/${mod.id} requires an app of kind "${mod.appKind}" but "${appHomeTarget[0]!.id}" is "${appHomeTarget[0]!.kind}".`, + ); + } + let r; + try { + r = await applyModule({ + projectRoot, + manifest, + module: mod, + adapter: adapter.adapter, + // home: "app" → single target app + // home: "package" → ship shims into every app (today: just one) + // home: "repo" → seed app for render context (the first app) + targetApps: home.kind === "app" ? appHomeTarget : targetApps, + registryRoot, + dryRun, + }); + } catch (err) { + spinner.stop(`${mod.label} ${pc.red("failed")}`); + throw err; + } + manifest = r.manifest; + (resolvedAll[category] ??= []).push({ + module: mod, + adapter: adapter.adapter, + } satisfies ResolvedEntry); + applied.push(`${category}/${mod.id}`); + telemetry.capture("cli_module", { action: "install", group: category, module: mod.id }); + spinner.stop(`${pc.green("✓")} ${mod.label}`); } - // Validate appKind when the module declares one (typically frameworks). - if (home.kind === "app" && mod.appKind && mod.appKind !== appHomeTarget[0]!.kind) { - throw new Error( - `${category}/${mod.id} requires an app of kind "${mod.appKind}" but "${appHomeTarget[0]!.id}" is "${appHomeTarget[0]!.kind}".`, - ); - } - const r = await applyModule({ - projectRoot, - manifest, - module: mod, - adapter: adapter.adapter, - // home: "app" → single target app - // home: "package" → ship shims into every app (today: just one) - // home: "repo" → seed app for render context (the first app) - targetApps: home.kind === "app" ? appHomeTarget : targetApps, - registryRoot, - dryRun, - }); - manifest = r.manifest; - (resolvedAll[category] ??= []).push({ - module: mod, - adapter: adapter.adapter, - } satisfies ResolvedEntry); - telemetry.capture("cli_module", { action: "install", group: category, module: mod.id }); - spinner.stop(`${pc.green("✓")} ${mod.label}`); } + } catch (err) { + // Leave the partial project on disk — explicit is better than `rm -rf`-ing + // the user's cwd, and they may want to inspect what got applied. + if (!dryRun) { + p.log.error( + `Init failed after ${applied.length} module(s): ${applied.join(", ") || "(none)"}.\n` + + `Project left at ${projectRoot} for inspection. Remove it manually to retry.`, + ); + } + throw err; } // Final step: synthesize the project README from the assembled selection and diff --git a/apps/cli/src/lib/codemod-runner.ts b/apps/cli/src/lib/codemod-runner.ts index 42551e3..8b1d26b 100644 --- a/apps/cli/src/lib/codemod-runner.ts +++ b/apps/cli/src/lib/codemod-runner.ts @@ -137,9 +137,15 @@ export async function applyModule(args: { packageName, peers: activePeerIds(manifest, app.id), envNames: declaredEnvNames(manifest), + consumesPackages: module.consumesPackages, }); const seedApp = targetApps[0]!; + // Manifest writes lead disk writes: claims are staged in-memory, persisted, + // then flushed. A mid-flush crash leaves orphan files the manifest already + // knows how to sweep — the old single-pass ordering didn't. + const deferredWrites: Array<() => void> = []; + // 1. Templates (claim regions per-template-file). App-scoped templates loop // over `targetApps`; package/repo-scoped templates emit once. for (const tpl of adapter.templates ?? []) { @@ -151,8 +157,10 @@ export async function applyModule(args: { if (!dryRun) { const source = readTemplateSource(tpl, moduleDir); const rendered = tpl.template ? renderTemplate(source, renderContextFor(app)) : source; - fs.mkdirSync(path.dirname(dest), { recursive: true }); - fs.writeFileSync(dest, rendered, "utf8"); + deferredWrites.push(() => { + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.writeFileSync(dest, rendered, "utf8"); + }); } touchedFiles.add(rel); } @@ -170,8 +178,10 @@ export async function applyModule(args: { if (!dryRun) { const source = readTemplateSource(tpl, moduleDir); const rendered = tpl.template ? renderTemplate(source, renderContextFor(seedApp)) : source; - fs.mkdirSync(path.dirname(dest), { recursive: true }); - fs.writeFileSync(dest, rendered, "utf8"); + deferredWrites.push(() => { + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.writeFileSync(dest, rendered, "utf8"); + }); } touchedFiles.add(rel); } @@ -211,15 +221,16 @@ export async function applyModule(args: { const pkgJsonPath = path.join(projectRoot, target); for (const [name, range] of Object.entries(deps)) { manifest = claim(manifest, target, `dependencies.${name}`, owner); - if (!dryRun) addPackageDependency(pkgJsonPath, name, range); + if (!dryRun) deferredWrites.push(() => addPackageDependency(pkgJsonPath, name, range)); } for (const [name, range] of Object.entries(devDeps)) { manifest = claim(manifest, target, `devDependencies.${name}`, owner); - if (!dryRun) addPackageDependency(pkgJsonPath, name, range, { dev: true }); + if (!dryRun) + deferredWrites.push(() => addPackageDependency(pkgJsonPath, name, range, { dev: true })); } for (const [name, command] of Object.entries(installFields.scripts)) { manifest = claim(manifest, target, `scripts.${name}`, owner); - if (!dryRun) addPackageScript(pkgJsonPath, name, command); + if (!dryRun) deferredWrites.push(() => addPackageScript(pkgJsonPath, name, command)); } touchedFiles.add(target); } @@ -230,7 +241,7 @@ export async function applyModule(args: { const envFile = path.join(projectRoot, ".env.example"); for (const v of installFields.env) { manifest = claim(manifest, ".env.example", v.name, owner); - if (!dryRun) addEnvVar(envFile, v.name, v.example, v.description); + if (!dryRun) deferredWrites.push(() => addEnvVar(envFile, v.name, v.example, v.description)); } touchedFiles.add(".env.example"); } @@ -263,15 +274,16 @@ export async function applyModule(args: { const pkgJsonPath = path.join(projectRoot, target); for (const [name, range] of Object.entries(appDeps)) { manifest = claim(manifest, target, `app.dependencies.${name}`, owner); - if (!dryRun) addPackageDependency(pkgJsonPath, name, range); + if (!dryRun) deferredWrites.push(() => addPackageDependency(pkgJsonPath, name, range)); } for (const [name, range] of Object.entries(appDevDeps)) { manifest = claim(manifest, target, `app.devDependencies.${name}`, owner); - if (!dryRun) addPackageDependency(pkgJsonPath, name, range, { dev: true }); + if (!dryRun) + deferredWrites.push(() => addPackageDependency(pkgJsonPath, name, range, { dev: true })); } for (const [name, command] of Object.entries(appFields.scripts)) { manifest = claim(manifest, target, `app.scripts.${name}`, owner); - if (!dryRun) addPackageScript(pkgJsonPath, name, command); + if (!dryRun) deferredWrites.push(() => addPackageScript(pkgJsonPath, name, command)); } touchedFiles.add(target); } @@ -282,48 +294,17 @@ export async function applyModule(args: { const envFile = path.join(projectRoot, ".env.example"); for (const v of appFields.env) { manifest = claim(manifest, ".env.example", v.name, owner); - if (!dryRun) addEnvVar(envFile, v.name, v.example, v.description); + if (!dryRun) deferredWrites.push(() => addEnvVar(envFile, v.name, v.example, v.description)); } touchedFiles.add(".env.example"); } - // 4. Imperative codemods — dispatched through the CLI's generic catalog. - // For app-home and package-home modules, run once per targeted app so - // codemods that edit files inside an app dir see the right app context. - // For repo-home modules, run once with the seed app. - if (adapter.codemods?.length) { - const dispatchApps = home.kind === "repo" ? [seedApp] : targetApps; - for (const app of dispatchApps) { - const appRoot = path.join(projectRoot, app.dir); - const project = lazyProject(appRoot); - const ctx = buildContext({ - projectRoot, - app, - appRoot, - manifest, - module, - adapter, - project: project.get, - touchedFiles, - dryRun, - onClaim: (file, region) => { - manifest = claim(manifest, file, region, owner); - }, - }); - for (const invocation of adapter.codemods) { - const fn = CODEMOD_CATALOG[invocation.id]; - if (!fn) { - throw new Error( - `Codemod "${invocation.id}" referenced by ${module.category}/${module.id} (adapter "${adapter.key}") is not in the catalog. Add it to packages/codemods/src/builtins/ and register in builtins/index.ts.`, - ); - } - if (!dryRun) { - const renderedArgs = renderArgs(invocation.args ?? {}, renderContextFor(app)); - const result = await fn.apply(ctx, renderedArgs); - result.touchedFiles.forEach((f) => touchedFiles.add(f)); - } - } - if (!dryRun) await project.save(); + // Validate up front so dry-run catches missing ids too. + for (const invocation of adapter.codemods ?? []) { + if (!CODEMOD_CATALOG[invocation.id]) { + throw new Error( + `Codemod "${invocation.id}" referenced by ${module.category}/${module.id} (adapter "${adapter.key}") is not in the catalog. Add it to packages/codemods/src/builtins/ and register in builtins/index.ts.`, + ); } } @@ -343,6 +324,43 @@ export async function applyModule(args: { }, }; writeManifest(projectRoot, manifest); + + for (const write of deferredWrites) write(); + + // Codemods dispatch once per targeted app (or once with the seed app for + // repo-home). Manifest re-persists after claims are gathered, before + // `project.save()` flushes — same disk-leads-manifest contract as above. + if (adapter.codemods?.length) { + const dispatchApps = home.kind === "repo" ? [seedApp] : targetApps; + const saves: Array<() => Promise> = []; + for (const app of dispatchApps) { + const appRoot = path.join(projectRoot, app.dir); + const project = lazyProject(appRoot); + const ctx = buildContext({ + projectRoot, + app, + appRoot, + manifest, + module, + adapter, + project: project.get, + touchedFiles, + dryRun, + onClaim: (file, region) => { + manifest = claim(manifest, file, region, owner); + }, + }); + for (const invocation of adapter.codemods) { + const fn = CODEMOD_CATALOG[invocation.id]!; + const renderedArgs = renderArgs(invocation.args ?? {}, renderContextFor(app)); + const result = await fn.apply(ctx, renderedArgs); + result.touchedFiles.forEach((f) => touchedFiles.add(f)); + } + saves.push(() => project.save()); + } + writeManifest(projectRoot, manifest); + for (const save of saves) await save(); + } } return { manifest, touchedFiles: [...touchedFiles], dryRun, bootstrappedPackage }; @@ -664,6 +682,7 @@ export async function revertCodemods(args: { packageName, peers: activePeerIds(manifest, app.id), envNames: declaredEnvNames(manifest), + consumesPackages: module.consumesPackages, }); const project = lazyProject(appRoot); const ctx = buildContext({ diff --git a/apps/web/src/components/builder/module-cards.tsx b/apps/web/src/components/builder/module-cards.tsx index 67707fd..d2135ad 100644 --- a/apps/web/src/components/builder/module-cards.tsx +++ b/apps/web/src/components/builder/module-cards.tsx @@ -196,7 +196,7 @@ const ModuleCard = memo(function ModuleCard({ params={{ category: m.category, id: m.id }} aria-label={`${m.label} details`} onClick={(e) => e.stopPropagation()} - className="inline-flex shrink-0 translate-y-[-2px] items-center justify-center text-foreground/40 transition-colors hover:text-foreground" + className="inline-flex shrink-0 translate-y-[-1px] items-center justify-center text-foreground/40 transition-colors hover:text-foreground" /> } > diff --git a/apps/web/src/lib/selection.ts b/apps/web/src/lib/selection.ts index fbacbb2..25b5e01 100644 --- a/apps/web/src/lib/selection.ts +++ b/apps/web/src/lib/selection.ts @@ -23,6 +23,21 @@ export type BuilderSearch = { name?: string; pm?: string } & Partial): BuilderSearch { + const out: BuilderSearch = {}; + for (const key of SEARCH_KEYS) { + const v = input[key]; + if (typeof v === "string" && v.length > 0) out[key] = v; + } + return out; +} + /** * Parse URL search params into a name + per-category selections. Every category * is a comma-joined list (`?testing=vitest,playwright`, `?framework=next`) — the diff --git a/apps/web/src/routes/index.tsx b/apps/web/src/routes/index.tsx index b06215a..219a426 100644 --- a/apps/web/src/routes/index.tsx +++ b/apps/web/src/routes/index.tsx @@ -1,27 +1,12 @@ -import { KNOWN_CATEGORIES } from "@stanza/registry"; import { createFileRoute, Link } from "@tanstack/react-router"; import { Builder } from "@/components/builder"; -import type { BuilderSearch } from "@/lib/selection"; +import { validateBuilderSearch } from "@/lib/selection"; import { buildHead, getWebSiteJsonLd } from "@/lib/seo"; import { getBuilderState } from "@/server/builder-state.functions"; -// Keys are derived from the canonical slot + add-on tuples so this never -// drifts when a slot or category is added. `parseSelections` splits the -// comma-joined add-on values downstream. -const SEARCH_KEYS = ["name", "pm", ...KNOWN_CATEGORIES] as const; - -function validateSearch(input: Record): BuilderSearch { - const out: BuilderSearch = {}; - for (const key of SEARCH_KEYS) { - const v = input[key]; - if (typeof v === "string" && v.length > 0) out[key] = v; - } - return out; -} - export const Route = createFileRoute("/")({ - validateSearch, + validateSearch: validateBuilderSearch, loaderDeps: ({ search }) => search, loader: ({ deps }) => getBuilderState({ data: deps }), staleTime: Infinity, diff --git a/apps/web/src/server/builder-state.functions.ts b/apps/web/src/server/builder-state.functions.ts index b5111f1..b172ecc 100644 --- a/apps/web/src/server/builder-state.functions.ts +++ b/apps/web/src/server/builder-state.functions.ts @@ -13,8 +13,8 @@ import { parseSelections, resolveSelected, selectedPeerIds, + validateBuilderSearch, } from "@/lib/selection"; -import type { BuilderSearch } from "@/lib/selection"; import type { Preview } from "@/server/highlighter"; import { getHighlighter, renderPreview } from "@/server/highlighter.server"; import { loadRegistryFile } from "@/server/registry-base.server"; @@ -36,7 +36,9 @@ export type BuilderState = { * list, and pre-renders Shiki HTML for each — keeping shiki off the client. */ export const getBuilderState = createServerFn({ method: "GET" }) - .inputValidator((data: BuilderSearch) => data) + // Server functions are HTTP-reachable; share the route's allow-list so a + // direct POST can't bypass it. + .inputValidator(validateBuilderSearch) .handler(async ({ data }): Promise => { // Warm the Shiki singleton during the initial empty-state load (which // renders zero previews and so would otherwise never touch the @@ -45,11 +47,12 @@ export const getBuilderState = createServerFn({ method: "GET" }) // warm highlighter instead of paying ~hundreds of ms of cold-start. void getHighlighter(); - const index = await loadRegistryFile("index.json"); - - // Module catalog is cached per-process — registry data is immutable per - // deployment, so toggles pay this cost exactly once per server instance. - const modules = await getAllModules(index); + // Module catalog is cached per-process; `getAllModules` reads `index.json` + // internally, we still load it here for the BuilderState response. + const [index, modules] = await Promise.all([ + loadRegistryFile("index.json"), + getAllModules(), + ]); const { name, pm, selections } = parseSelections(data); const resolved = resolveSelected(modules, selections); diff --git a/apps/web/src/server/registry-modules.server.ts b/apps/web/src/server/registry-modules.server.ts index 2f75638..68be4b7 100644 --- a/apps/web/src/server/registry-modules.server.ts +++ b/apps/web/src/server/registry-modules.server.ts @@ -6,21 +6,18 @@ import { loadRegistryFile } from "@/server/registry-base.server"; let modulesPromise: Promise> | undefined; /** - * Module-singleton cache for the full module catalog. Mirrors the - * `getHighlighter()` pattern: load once per server instance, reuse across every - * `getBuilderState` invocation. Registry data is immutable per deployment, so - * cache lifetime = process lifetime (deploy-scoped on Vercel; restart the dev - * server after `vp run @stanza/web#prebuild` to pick up local edits). - * - * Per-module failures are isolated — the failing module is dropped from the - * result and reported, the rest still resolve. + * Per-process cache for the full module catalog. Registry data is immutable + * per deployment, so cache lifetime = process lifetime (restart the dev server + * after `vp run @stanza/web#prebuild` to pick up local edits). Per-module + * failures are isolated — the failing module is dropped and reported. */ -export function getAllModules(index: RegistryIndex): Promise> { - if (!modulesPromise) modulesPromise = loadAll(index); +export function getAllModules(): Promise> { + if (!modulesPromise) modulesPromise = loadAll(); return modulesPromise; } -async function loadAll(index: RegistryIndex): Promise> { +async function loadAll(): Promise> { + const index = await loadRegistryFile("index.json"); const settled = await Promise.all( index.modules.map(async (summary): Promise => { try { diff --git a/packages/registry/src/manifest.test.ts b/packages/registry/src/manifest.test.ts index 108483d..36da5ac 100644 --- a/packages/registry/src/manifest.test.ts +++ b/packages/registry/src/manifest.test.ts @@ -76,6 +76,64 @@ describe("StanzaManifestSchema", () => { }), ).toThrow(/apps/); }); + + it("rejects two home:app framework records targeting the same app", () => { + const result = StanzaManifestSchema.safeParse({ + ...emptyManifest({ + name: "acme", + apps: [{ id: "web", dir: "apps/web", kind: "web" }], + }), + modules: { + framework: [ + { id: "next", version: "0.1.0", adapter: "default", apps: ["web"] }, + { id: "tanstack-start", version: "0.1.0", adapter: "default", apps: ["web"] }, + ], + }, + }); + expect(result.success).toBe(false); + const issues = result.success ? [] : result.error.issues; + expect(issues.some((i) => /≤ 1 module per app/.test(i.message))).toBe(true); + }); + + it("accepts two home:app framework records on disjoint apps", () => { + const result = StanzaManifestSchema.safeParse({ + ...emptyManifest({ + name: "acme", + apps: [ + { id: "web", dir: "apps/web", kind: "web" }, + { id: "native", dir: "apps/native", kind: "native" }, + ], + }), + modules: { + framework: [ + { id: "next", version: "0.1.0", adapter: "default", apps: ["web"] }, + { id: "expo", version: "0.1.0", adapter: "default", apps: ["native"] }, + ], + }, + }); + expect(result.success).toBe(true); + }); + + it("rejects two home:package auth records (per-project cardinality)", () => { + const result = StanzaManifestSchema.safeParse({ + ...emptyManifest({ + name: "acme", + apps: [ + { id: "web", dir: "apps/web", kind: "web" }, + { id: "native", dir: "apps/native", kind: "native" }, + ], + }), + modules: { + auth: [ + { id: "better-auth", version: "0.1.0", adapter: "default", apps: ["web"] }, + { id: "clerk", version: "0.1.0", adapter: "default", apps: ["native"] }, + ], + }, + }); + expect(result.success).toBe(false); + const issues = result.success ? [] : result.error.issues; + expect(issues.some((i) => /≤ 1 module per project/.test(i.message))).toBe(true); + }); }); describe("app-aware selectors", () => { diff --git a/packages/registry/src/manifest.ts b/packages/registry/src/manifest.ts index ba2fa20..238b403 100644 --- a/packages/registry/src/manifest.ts +++ b/packages/registry/src/manifest.ts @@ -3,6 +3,8 @@ import { z } from "zod"; import { APP_KINDS, type AppKind, + categoryCardinality, + categoryHome, type CategoryId, KNOWN_CATEGORIES, type ModuleId, @@ -95,29 +97,61 @@ const appSpecSchema = z.object({ kind: z.enum(APP_KINDS), }) satisfies z.ZodType; -export const StanzaManifestSchema = z.object({ - $schema: z.string().optional(), - version: z.literal(CURRENT_MANIFEST_VERSION), - projectShape: z.literal("monorepo"), - packageManager: z.enum(["pnpm", "bun", "npm"]), - name: z.string(), - apps: z.array(appSpecSchema).min(1), - // Zod 4: partialRecord because not every category is filled. Every category - // holds an array (single-choice categories carry 0 or 1 records per app). - modules: z.partialRecord( - z.enum(KNOWN_CATEGORIES), - z.array( - z.object({ - id: z.string(), - version: z.string(), - adapter: z.string(), - apps: z.array(z.string()).optional(), - }), +export const StanzaManifestSchema = z + .object({ + $schema: z.string().optional(), + version: z.literal(CURRENT_MANIFEST_VERSION), + projectShape: z.literal("monorepo"), + packageManager: z.enum(["pnpm", "bun", "npm"]), + name: z.string(), + apps: z.array(appSpecSchema).min(1), + // Zod 4: partialRecord because not every category is filled. Every category + // holds an array (single-choice categories carry 0 or 1 records per app). + modules: z.partialRecord( + z.enum(KNOWN_CATEGORIES), + z.array( + z.object({ + id: z.string(), + version: z.string(), + adapter: z.string(), + apps: z.array(z.string()).optional(), + }), + ), ), - ), - regions: z.record(z.string(), z.record(z.string(), z.string())), - readmeChecksum: z.string().optional(), -}) satisfies z.ZodType; + regions: z.record(z.string(), z.record(z.string(), z.string())), + readmeChecksum: z.string().optional(), + }) + .superRefine((m, ctx) => { + // The CLI enforces cardinality at apply-time; this catches hand-edited or + // third-party-synthesized manifests so `selectedOne` callers can rely on it. + const appIds = m.apps.map((a) => a.id); + for (const category of KNOWN_CATEGORIES) { + const records = m.modules[category]; + if (!records || categoryCardinality(category) !== "one") continue; + if (categoryHome(category).kind === "app") { + const counts = new Map(); + for (const r of records) { + const targets = r.apps?.length ? r.apps : appIds; + for (const id of targets) counts.set(id, (counts.get(id) ?? 0) + 1); + } + for (const [id, count] of counts) { + if (count > 1) { + ctx.addIssue({ + code: "custom", + message: `Category "${category}" allows ≤ 1 module per app, but app "${id}" has ${count}.`, + path: ["modules", category], + }); + } + } + } else if (records.length > 1) { + ctx.addIssue({ + code: "custom", + message: `Category "${category}" allows ≤ 1 module per project, found ${records.length}.`, + path: ["modules", category], + }); + } + } + }) satisfies z.ZodType; /** * Built-in default app: a single web app at `apps/web` with id `"web"`. The diff --git a/packages/registry/src/module.ts b/packages/registry/src/module.ts index 7f92a54..fc76440 100644 --- a/packages/registry/src/module.ts +++ b/packages/registry/src/module.ts @@ -417,7 +417,9 @@ const installFieldsSchema = { const adapterSchema = z.object({ key: z.string(), - match: z.record(z.string(), z.string()), + // Keys must be valid category ids — typos would silently score specificity 0 + // (resolver iterates `KNOWN_CATEGORIES`) and lose to less-specific siblings. + match: z.partialRecord(z.enum(KNOWN_CATEGORIES), z.string()), templates: z .array( z.object({ @@ -499,7 +501,12 @@ const categorySchema = z.object({ /** Index summary: full module metadata with adapters reduced to `key` + `match`. */ export const ModuleSummarySchema = z.object({ ...moduleBaseShape, - adapters: z.array(z.object({ key: z.string(), match: z.record(z.string(), z.string()) })), + adapters: z.array( + z.object({ + key: z.string(), + match: z.partialRecord(z.enum(KNOWN_CATEGORIES), z.string()), + }), + ), }) satisfies z.ZodType; /** Runtime-validatable schema for the registry `index.json` payload. */ diff --git a/packages/registry/src/package-json.test.ts b/packages/registry/src/package-json.test.ts index b1a7663..6522378 100644 --- a/packages/registry/src/package-json.test.ts +++ b/packages/registry/src/package-json.test.ts @@ -286,6 +286,50 @@ describe("ModuleSchema (Zod) app-overlay validation", () => { expect(issues.some((i) => /forbidden for `home: "repo"`/.test(i.message))).toBe(true); }); + it("synthesize picks the higher range when two modules in the same category declare the same dep", () => { + // Two testing add-ons (home:app, many-cardinality) both contribute @types/node. + const a = defineModule({ + id: "vitest-like", + category: "testing" as const, + label: "A", + description: "", + version: "0.1.0", + devDependencies: { "@types/node": "^20.0.0" }, + adapters: [{ key: "default", match: {} }], + }); + const b = defineModule({ + id: "playwright-like", + category: "testing" as const, + label: "B", + description: "", + version: "0.1.0", + devDependencies: { "@types/node": "^24.0.0" }, + adapters: [{ key: "default", match: {} }], + }); + const appPkg = synthesizePackageJsons( + { + testing: [ + { module: a, adapter: a.adapters[0]! }, + { module: b, adapter: b.adapters[0]! }, + ], + }, + { name: "acme" }, + )["apps/web/package.json"]; + expect(appPkg?.devDependencies?.["@types/node"]).toBe("^24.0.0"); + + // Swap iteration order: same winner — synth is module-order-independent. + const swapped = synthesizePackageJsons( + { + testing: [ + { module: b, adapter: b.adapters[0]! }, + { module: a, adapter: a.adapters[0]! }, + ], + }, + { name: "acme" }, + )["apps/web/package.json"]; + expect(swapped?.devDependencies?.["@types/node"]).toBe("^24.0.0"); + }); + it("accepts manifests with app fields on package-home modules", () => { const ok = { id: "x", @@ -298,4 +342,18 @@ describe("ModuleSchema (Zod) app-overlay validation", () => { }; expect(ModuleSchema.safeParse(ok).success).toBe(true); }); + + it("rejects adapter match keys that aren't valid category ids", () => { + // A typo'd key (`frameworks` instead of `framework`) would silently score + // specificity 0 in the resolver — make sure the schema fails loud instead. + const bad = { + id: "vitest", + category: "testing" as const, + label: "Vitest", + description: "", + version: "0.1.0", + adapters: [{ key: "next", match: { frameworks: "next" } }], + }; + expect(ModuleSchema.safeParse(bad).success).toBe(false); + }); }); diff --git a/packages/registry/src/package-json.ts b/packages/registry/src/package-json.ts index 1596c25..fda3187 100644 --- a/packages/registry/src/package-json.ts +++ b/packages/registry/src/package-json.ts @@ -209,7 +209,32 @@ export type Resolved = Partial>; function addDep(pkg: PackageJson, name: string, range: string, dev = false): void { const key = dev ? "devDependencies" : "dependencies"; const map = (pkg[key] ??= {}); - if (map[name] === undefined) map[name] = range; + const existing = map[name]; + if (existing === undefined || existing === range) { + map[name] = range; + return; + } + // Conflicting ranges across modules: pick the higher version so synth output + // is module-order-independent. The CLI's region tracker hard-fails on the + // same case, so genuine conflicts still surface at install time. + map[name] = pickHigherRange(existing, range); +} + +/** Non-semver ranges (`workspace:*`, git URLs) compare lexically — fine in practice. */ +function pickHigherRange(a: string, b: string): string { + return compareSemver(a.replace(/^[\^~]/, ""), b.replace(/^[\^~]/, "")) >= 0 ? a : b; +} + +function compareSemver(a: string, b: string): number { + const pa = a.split("."); + const pb = b.split("."); + for (let i = 0; i < Math.max(pa.length, pb.length); i++) { + const na = Number(pa[i] ?? "0"); + const nb = Number(pb[i] ?? "0"); + if (Number.isNaN(na) || Number.isNaN(nb)) return a < b ? -1 : a > b ? 1 : 0; + if (na !== nb) return na - nb; + } + return 0; } function applyFields(pkg: PackageJson, fields: MergedInstallFields): void { diff --git a/packages/registry/src/synthesize.ts b/packages/registry/src/synthesize.ts index 9143198..ab9895c 100644 --- a/packages/registry/src/synthesize.ts +++ b/packages/registry/src/synthesize.ts @@ -194,6 +194,7 @@ export function synthesizeTemplates( packageManager: opts.packageManager, peers: opts.peers, envNames, + consumesPackages: entry.module.consumesPackages, }); for (const tpl of entry.adapter.templates ?? []) { @@ -333,6 +334,7 @@ export function synthesizeReadme( packageManager: pm, peers, envNames, + consumesPackages: entry.module.consumesPackages, }); lines.push(`### ${categoryLabel(category)} — ${entry.module.label}`); lines.push(""); diff --git a/packages/registry/src/template.test.ts b/packages/registry/src/template.test.ts index 2cc6dd2..fa24001 100644 --- a/packages/registry/src/template.test.ts +++ b/packages/registry/src/template.test.ts @@ -14,6 +14,7 @@ describe("renderTemplate", () => { projectName: "acme", app: webApp, packageName: "@acme/auth", + consumesPackages: ["db"], }); it("substitutes project.name", () => { @@ -156,15 +157,36 @@ describe("renderTemplate", () => { }); describe("buildRenderContext", () => { - it("populates packages..name for every PACKAGE_DIR", () => { + it("populates packages..name only for declared consumesPackages", () => { + const ctx = buildRenderContext({ + projectName: "acme", + app: webApp, + packageName: "", + consumesPackages: ["db"], + }); + expect(ctx.packages.db?.name).toBe("@acme/db"); + // `auth` is a valid PACKAGE_DIR but wasn't declared — must not leak in. + expect(ctx.packages.auth).toBeUndefined(); + }); + + it("ignores consumesPackages entries that aren't valid PACKAGE_DIRS", () => { + const ctx = buildRenderContext({ + projectName: "acme", + app: webApp, + packageName: "", + consumesPackages: ["db", "definitely-not-a-package"], + }); + expect(ctx.packages.db?.name).toBe("@acme/db"); + expect(Object.keys(ctx.packages)).toEqual(["db"]); + }); + + it("defaults packages to {} when consumesPackages is omitted", () => { const ctx = buildRenderContext({ projectName: "acme", app: webApp, packageName: "", }); - // PACKAGE_DIRS is derived from CATEGORIES; auth + db are both package homes. - expect(ctx.packages.auth?.name).toBe("@acme/auth"); - expect(ctx.packages.db?.name).toBe("@acme/db"); + expect(ctx.packages).toEqual({}); }); it("materializes every PEER_CATEGORIES key under `peers` (undefined when unset)", () => { diff --git a/packages/registry/src/template.ts b/packages/registry/src/template.ts index 5f07465..cc4bdd5 100644 --- a/packages/registry/src/template.ts +++ b/packages/registry/src/template.ts @@ -70,6 +70,15 @@ export function pmRecursive(pm: PackageManager, script: string): string { return `npm run ${script} --workspaces --if-present`; } +/** `@acme/auth` → `auth`; `undefined` for repo/app-home modules (empty packageName). */ +function packageDirFromName(packageName: string): string | undefined { + if (!packageName) return undefined; + const slash = packageName.lastIndexOf("/"); + if (slash < 0) return undefined; + const dir = packageName.slice(slash + 1); + return PACKAGE_DIRS.has(dir) ? dir : undefined; +} + // Module-singleton instance so registered helpers don't leak into other // Handlebars consumers (the global is shared otherwise). const hb = Handlebars.create(); @@ -119,6 +128,10 @@ export function renderTemplate(source: string, context: TemplateContext): string * when no module is selected) so templates can safely reference any peer slot * — that lets `{{peers.framework}}` resolve to `undefined` rather than walk * off the end of a missing path. + * + * `packages` only exposes the module's own home plus any `consumesPackages` + * it declared. Unknown keys render to empty strings, so a typo'd peer + * reference fails at build rather than producing a phantom import path. */ export function buildRenderContext(opts: { projectName: string; @@ -132,10 +145,18 @@ export function buildRenderContext(opts: { * the rendered output stays stable across module-order permutations. */ envNames?: readonly string[]; + /** Pass the active module's `consumesPackages` to make `{{packages..name}}` resolve. */ + consumesPackages?: readonly string[]; }): TemplateContext { const packages: Record = {}; - for (const dir of PACKAGE_DIRS) { - packages[dir] = { name: `@${opts.projectName}/${dir}` }; + // Own home is always exposed so a module can self-reference without having + // to list itself in `consumesPackages`. + const ownDir = packageDirFromName(opts.packageName); + if (ownDir) packages[ownDir] = { name: opts.packageName }; + for (const dir of opts.consumesPackages ?? []) { + if (PACKAGE_DIRS.has(dir) && !packages[dir]) { + packages[dir] = { name: `@${opts.projectName}/${dir}` }; + } } const peers: Partial> = {}; for (const cat of PEER_CATEGORIES) peers[cat] = opts.peers?.[cat];