diff --git a/.changeset/scaffold-remote-template-namespace.md b/.changeset/scaffold-remote-template-namespace.md new file mode 100644 index 0000000000..161f679f3f --- /dev/null +++ b/.changeset/scaffold-remote-template-namespace.md @@ -0,0 +1,26 @@ +--- +'create-objectstack': patch +--- + +fix(create-objectstack): scaffolding a remote template no longer produces a project that cannot build (#4926) + +`npx create-objectstack@latest my-app -t todo` (and `compliance`, `content`, +`contracts`, `procurement`) generated a project that failed `objectstack build` +immediately — 5 of the 6 offered templates. Only the bundled `blank` worked. + +The scaffolder read the template's original namespace from +`objectstack.manifest.json`, and that filename names two different documents. +The bundled template's is app-shaped and carries `namespace`; a remote +template's is the template-registry document +(`$schema: …/template-manifest.json`) and carries none — its namespace lives +only in `objectstack.config.ts`. So the value came back `undefined` for every +remote template and the object-name rewrite was skipped, while the config's +`namespace:` was rewritten anyway. The result was `namespace: 'my_app'` sitting +next to `name: 'todo_task'`, which the `${namespace}_${shortName}` rule rejects. +Across the five templates, 74 object names were left unrewritten. + +`objectstack.config.ts` is now the authority for the template namespace (it +holds the very literal the scaffolder overwrites, so the two cannot disagree), +with the manifest as fallback. The rewrite also verifies itself: any surviving +stale prefix throws at the scaffold, naming the files and lines, instead of +surfacing as a build failure on the user's first command. diff --git a/packages/create-objectstack/src/index.ts b/packages/create-objectstack/src/index.ts index 07b871162b..48ff631a1a 100644 --- a/packages/create-objectstack/src/index.ts +++ b/packages/create-objectstack/src/index.ts @@ -47,6 +47,11 @@ import * as tar from 'tar'; import { syncObjectStackDeps } from './pkg-utils.js'; import { copyDir } from './template-copy.js'; +import { + readTemplateNamespace, + rewriteObjectNamePrefix, + findStaleNamespacePrefixes, +} from './rewrite-identity.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -227,26 +232,9 @@ async function loadRemote(pkgName: string, targetDir: string): Promise } // ─── Field-aware rewrites ─────────────────────────────────────────── - -/** - * Walk every `*.ts` file under `dir` and apply `fn` to its contents. - * Used to swap the bundled template's literal `blank_` object-name prefix - * for the user-supplied namespace so the rendered objects satisfy the - * `${namespace}_${shortName}` rule enforced by `objectstack validate`. - */ -function walkAndRewriteTs(dir: string, fn: (src: string) => string) { - if (!fs.existsSync(dir)) return; - for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { - const full = path.join(dir, entry.name); - if (entry.isDirectory()) { - walkAndRewriteTs(full, fn); - } else if (entry.isFile() && entry.name.endsWith('.ts')) { - const before = fs.readFileSync(full, 'utf8'); - const after = fn(before); - if (after !== before) fs.writeFileSync(full, after); - } - } -} +// +// The object-name prefix walk moved to rewrite-identity.ts so it can be tested +// without importing this module (which calls program.parse() on import). function rewriteProjectIdentity( targetDir: string, @@ -255,18 +243,14 @@ function rewriteProjectIdentity( ) { const title = toTitleCase(projectName); - // Read the template's *original* namespace from the manifest before we - // overwrite it — we use this as the prefix to swap in src/**/*.ts files. - let templateNamespace: string | undefined; - const manifestPathPre = path.join(targetDir, 'objectstack.manifest.json'); - if (fs.existsSync(manifestPathPre)) { - try { - const m = JSON.parse(fs.readFileSync(manifestPathPre, 'utf8')); - if (typeof m.namespace === 'string') templateNamespace = m.namespace; - } catch { - // ignore - } - } + // The template's *original* namespace, read before we overwrite it — this is + // the prefix we swap in src/**/*.ts. It comes from objectstack.config.ts + // first: a REMOTE template's objectstack.manifest.json is the template- + // REGISTRY document and carries no `namespace` at all, so reading only the + // manifest silently yielded undefined and skipped the whole rewrite below — + // shipping every remote template with a rewritten manifest namespace next to + // untouched object names (#4902). See rewrite-identity.ts for the account. + const templateNamespace = readTemplateNamespace(targetDir); // package.json — set .name and pin @objectstack/* deps to this scaffolder's // own release line. All @objectstack packages (including create-objectstack) @@ -312,19 +296,32 @@ function rewriteProjectIdentity( fs.writeFileSync(configPath, cfg); } - // src/**/*.ts — swap the bundled template's `${templateNamespace}_` object-name - // prefix for the user's sanitized namespace so rendered objects satisfy - // the `${namespace}_${shortName}` rule. No-op if namespace already matches. - if (namespace !== templateNamespace && templateNamespace) { - const prefixRe = new RegExp( - `(\\bname:\\s*)(['"\`])${templateNamespace}_([a-z0-9_]+)\\2`, - 'g', - ); - walkAndRewriteTs(path.join(targetDir, 'src'), (src) => - src.replace(prefixRe, (_m, prefix: string, q: string, rest: string) => - `${prefix}${q}${namespace}_${rest}${q}`, - ), - ); + // src/**/*.ts — swap the template's `${templateNamespace}_` object-name prefix + // for the user's sanitized namespace so rendered objects satisfy the + // `${namespace}_${shortName}` rule. No-op if the namespace already matches. + // + // Then VERIFY. A prefix rewrite that quietly does nothing looks exactly like + // one that was not needed, and that ambiguity is what let five broken + // templates ship (#4902). If any stale literal survives, the scaffold has + // produced a project that cannot build — fail here, where the cause is still + // legible, rather than in the user's first `objectstack build`. + if (templateNamespace && namespace !== templateNamespace) { + const srcDir = path.join(targetDir, 'src'); + rewriteObjectNamePrefix(srcDir, templateNamespace, namespace); + const stale = findStaleNamespacePrefixes(srcDir, templateNamespace); + if (stale.length > 0) { + const shown = stale + .slice(0, 5) + .map((s) => ` src/${s.file}:${s.line} ${s.text}`) + .join('\n'); + const more = stale.length > 5 ? `\n …and ${stale.length - 5} more` : ''; + throw new Error( + `Scaffolding rewrote the namespace to '${namespace}' but ${stale.length} object ` + + `name(s) still carry the template's '${templateNamespace}_' prefix:\n${shown}${more}\n` + + `The generated project would fail 'objectstack build' on the ` + + `\${namespace}_\${shortName} rule. This is a bug in the scaffolder, not in your input.`, + ); + } } // README.md — rewrite first H1 diff --git a/packages/create-objectstack/src/rewrite-identity.test.ts b/packages/create-objectstack/src/rewrite-identity.test.ts new file mode 100644 index 0000000000..1b41ce72d0 --- /dev/null +++ b/packages/create-objectstack/src/rewrite-identity.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. +// +// Regression cover for #4902: every published remote template scaffolded into a +// project that could not build, because the object-name prefix rewrite was +// guarded on a field only the BUNDLED template's manifest has. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { + readTemplateNamespace, + rewriteObjectNamePrefix, + findStaleNamespacePrefixes, +} from './rewrite-identity.js'; + +let dir: string; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'os-rewrite-')); + fs.mkdirSync(path.join(dir, 'src', 'objects'), { recursive: true }); +}); +afterEach(() => fs.rmSync(dir, { recursive: true, force: true })); + +const writeConfig = (ns: string) => + fs.writeFileSync( + path.join(dir, 'objectstack.config.ts'), + `export default defineStack({\n manifest: {\n id: 'x',\n namespace: '${ns}',\n },\n});\n`, + ); + +const writeObject = (file: string, name: string) => + fs.writeFileSync( + path.join(dir, 'src', 'objects', file), + `export const o = {\n name: '${name}',\n label: 'X',\n};\n`, + ); + +describe('readTemplateNamespace', () => { + it('reads a REMOTE template shape: registry manifest with no namespace, config has it', () => { + // The exact shape every template in objectstack-ai/templates ships: + // $schema template-manifest.json, no `namespace` key anywhere in it. + fs.writeFileSync( + path.join(dir, 'objectstack.manifest.json'), + JSON.stringify({ + $schema: 'https://schemas.objectstack.dev/template-manifest.json', + name: 'todo', + displayName: 'Todo', + category: 'productivity', + skills: ['objectstack-platform'], + }), + ); + writeConfig('todo'); + // Reading the manifest alone yields undefined — that was the bug. + expect(readTemplateNamespace(dir)).toBe('todo'); + }); + + it('reads a BUNDLED template shape: app manifest carrying namespace', () => { + fs.writeFileSync( + path.join(dir, 'objectstack.manifest.json'), + JSON.stringify({ name: 'blank', namespace: 'blank' }), + ); + writeConfig('blank'); + expect(readTemplateNamespace(dir)).toBe('blank'); + }); + + it('falls back to the manifest when the config declares no namespace', () => { + fs.writeFileSync( + path.join(dir, 'objectstack.manifest.json'), + JSON.stringify({ namespace: 'fallback' }), + ); + fs.writeFileSync( + path.join(dir, 'objectstack.config.ts'), + 'export default defineStack({ manifest: { id: "x" } });\n', + ); + expect(readTemplateNamespace(dir)).toBe('fallback'); + }); + + it('is undefined when neither source declares one', () => { + expect(readTemplateNamespace(dir)).toBeUndefined(); + }); + + it('survives an unparseable manifest', () => { + fs.writeFileSync(path.join(dir, 'objectstack.manifest.json'), '{ not json'); + writeConfig('todo'); + expect(readTemplateNamespace(dir)).toBe('todo'); + }); +}); + +describe('rewriteObjectNamePrefix', () => { + it('moves every object name onto the new namespace', () => { + writeObject('todo_task.object.ts', 'todo_task'); + writeObject('todo_label.object.ts', 'todo_label'); + const n = rewriteObjectNamePrefix(path.join(dir, 'src'), 'todo', 'my_app'); + expect(n).toBe(2); + const read = (f: string) => + fs.readFileSync(path.join(dir, 'src', 'objects', f), 'utf8'); + expect(read('todo_task.object.ts')).toContain("name: 'my_app_task'"); + expect(read('todo_label.object.ts')).toContain("name: 'my_app_label'"); + }); + + it('leaves names that do not carry the template prefix alone', () => { + writeObject('other.object.ts', 'sys_user'); + expect(rewriteObjectNamePrefix(path.join(dir, 'src'), 'todo', 'my_app')).toBe(0); + expect( + fs.readFileSync(path.join(dir, 'src', 'objects', 'other.object.ts'), 'utf8'), + ).toContain("name: 'sys_user'"); + }); + + it('is a no-op on a missing directory rather than throwing', () => { + expect(rewriteObjectNamePrefix(path.join(dir, 'nope'), 'todo', 'my_app')).toBe(0); + }); +}); + +describe('findStaleNamespacePrefixes', () => { + it('reports nothing once the rewrite has run', () => { + writeObject('todo_task.object.ts', 'todo_task'); + rewriteObjectNamePrefix(path.join(dir, 'src'), 'todo', 'my_app'); + expect(findStaleNamespacePrefixes(path.join(dir, 'src'), 'todo')).toEqual([]); + }); + + it('reports what a skipped rewrite leaves behind — the #4902 failure state', () => { + writeObject('todo_task.object.ts', 'todo_task'); + writeObject('todo_label.object.ts', 'todo_label'); + // No rewrite at all: exactly what the old manifest-only guard produced. + const stale = findStaleNamespacePrefixes(path.join(dir, 'src'), 'todo'); + expect(stale).toHaveLength(2); + expect(stale.map((s) => s.file).sort()).toEqual([ + path.join('objects', 'todo_label.object.ts'), + path.join('objects', 'todo_task.object.ts'), + ]); + expect(stale[0].line).toBeGreaterThan(0); + }); +}); diff --git a/packages/create-objectstack/src/rewrite-identity.ts b/packages/create-objectstack/src/rewrite-identity.ts new file mode 100644 index 0000000000..41e7bb1b70 --- /dev/null +++ b/packages/create-objectstack/src/rewrite-identity.ts @@ -0,0 +1,147 @@ +// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license. +// +// Namespace rewriting for a scaffolded project. Kept out of index.ts because +// that module calls `program.parse()` on import — anything a test needs must be +// importable without running the CLI (same reason as pkg-utils.ts). +// +// WHY THIS IS ITS OWN MODULE, AND WHY IT VERIFIES ITSELF +// ----------------------------------------------------- +// A scaffolded project must satisfy the `${manifest.namespace}_${shortName}` +// rule (packages/spec/src/kernel/namespace-prefix.ts). Scaffolding rewrites the +// manifest namespace to the user's project name, so every `object.name` literal +// carrying the TEMPLATE's namespace prefix has to move with it. Rewriting one +// without the other produces a project that fails `objectstack build` on the +// user's very first command. +// +// That is exactly what shipped. The old code read the template's namespace from +// `objectstack.manifest.json` only: +// +// if (typeof m.namespace === 'string') templateNamespace = m.namespace; +// ... +// if (namespace !== templateNamespace && templateNamespace) { ...rewrite... } +// +// and two different file formats answer to that name: +// +// - the BUNDLED `blank` template's manifest is app-shaped and HAS `namespace`; +// - a REMOTE template's manifest is the template-REGISTRY document +// (`$schema: .../template-manifest.json` — name/displayName/category/skills/ +// translations) and has NO `namespace` at all. The real namespace lives in +// `objectstack.config.ts`. +// +// So for every remote template the guard fell through, the object-name rewrite +// was silently skipped, while the config's `namespace:` was rewritten anyway — +// leaving `namespace: 'my_app'` next to `name: 'todo_task'`. All five published +// remote templates (todo, compliance, content, contracts, procurement) failed +// this way, and the nightly registry canary had been red on every one of them +// for weeks with nobody watching (#4902). +// +// Hence two rules here: +// 1. `objectstack.config.ts` is the AUTHORITY for the template's namespace — +// it is the same value the scaffolder overwrites, so the two can never +// disagree. The manifest is a fallback, not the source. +// 2. The rewrite VERIFIES ITSELF. A prefix rewrite that silently does nothing +// is indistinguishable from one that was not needed, and that ambiguity is +// what made this ship. `findStaleNamespacePrefixes` turns it into an error. + +import fs from 'node:fs'; +import path from 'node:path'; + +/** `namespace: 'x'` in a stack config — first occurrence, single/double/backtick. */ +const CONFIG_NAMESPACE_RE = /\bnamespace:\s*(['"`])([a-z0-9_]+)\1/i; + +/** + * The namespace a template ships with, read BEFORE any rewrite. + * + * Prefers `objectstack.config.ts` (authoritative — the scaffolder rewrites that + * exact literal) and falls back to an app-shaped `objectstack.manifest.json`. + * Returns undefined when the template declares no namespace anywhere, which is + * the one case where there is genuinely nothing to move. + */ +export function readTemplateNamespace(targetDir: string): string | undefined { + const configPath = path.join(targetDir, 'objectstack.config.ts'); + if (fs.existsSync(configPath)) { + const m = CONFIG_NAMESPACE_RE.exec(fs.readFileSync(configPath, 'utf8')); + if (m) return m[2]; + } + + // Fallback: an app-shaped manifest. A template-registry manifest has no + // `namespace` key and correctly yields undefined here. + const manifestPath = path.join(targetDir, 'objectstack.manifest.json'); + if (fs.existsSync(manifestPath)) { + try { + const m = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + if (typeof m.namespace === 'string' && m.namespace) return m.namespace; + } catch { + // unparseable manifest → no opinion, fall through + } + } + return undefined; +} + +/** Every `*.ts` file under `dir`, recursively. Missing dir → empty. */ +function tsFiles(dir: string, out: string[] = []): string[] { + if (!fs.existsSync(dir)) return out; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === 'node_modules') continue; + tsFiles(full, out); + } else if (entry.isFile() && entry.name.endsWith('.ts')) { + out.push(full); + } + } + return out; +} + +const namePrefixRe = (ns: string, flags: string) => + new RegExp(`(\\bname:\\s*)(['"\`])${ns}_([a-z0-9_]+)\\2`, flags); + +/** + * Rewrite `name: '_x'` → `name: '_x'` in every `*.ts` under `dir`. + * Returns the number of literals rewritten. + */ +export function rewriteObjectNamePrefix( + dir: string, + from: string, + to: string, +): number { + let rewritten = 0; + for (const file of tsFiles(dir)) { + const before = fs.readFileSync(file, 'utf8'); + const after = before.replace( + namePrefixRe(from, 'g'), + (_m, prefix: string, q: string, rest: string) => { + rewritten++; + return `${prefix}${q}${to}_${rest}${q}`; + }, + ); + if (after !== before) fs.writeFileSync(file, after); + } + return rewritten; +} + +/** + * Any `name: '_…'` literal still present after a rewrite — i.e. metadata + * the scaffolded project will be rejected for. Each entry is `file:line` + * relative to `dir`, plus the offending text. + */ +export function findStaleNamespacePrefixes( + dir: string, + oldNs: string, +): { file: string; line: number; text: string }[] { + const re = namePrefixRe(oldNs, ''); + const stale: { file: string; line: number; text: string }[] = []; + for (const file of tsFiles(dir)) { + const lines = fs.readFileSync(file, 'utf8').split('\n'); + for (let i = 0; i < lines.length; i++) { + if (re.test(lines[i])) { + stale.push({ + file: path.relative(dir, file), + line: i + 1, + text: lines[i].trim(), + }); + } + } + } + return stale; +}