fix(cli): os generate refuses to write TypeScript that does not parse

.changeset/generate-refuses-unparseable-emission.md

@@ -0,0 +1,12 @@
+---
+"@objectstack/cli": patch
+---
+
+`os generate <type> <name>` no longer exits 0 after writing TypeScript the compiler cannot parse.
+
+The command ran no name validation of any kind — no `validateProjectName`, no sanitiser — so the name went into a binding position untouched. `os generate object foo.bar` reported success and left two broken files behind: `const foo.bar: Data.ServiceObject = {` in `src/objects/foo.bar.object.ts`, and a matching `export { default as foo.bar } from './foo.bar.object';` appended to the barrel `src/objects/index.ts`. The author learned about it at the next `tsc`, in a file the scaffolder had just told them it created.
+
+Both emissions are now rendered once, at the single point where the derived identifier is finished, and handed to TypeScript's own parser before anything is written. If either does not parse, the command prints the compiler's own diagnostics for each affected file and exits 1 without touching the filesystem — including under `--dry-run`, where a preview of un-parseable output under exit 0 is the same defect in preview form. One check covers all 14 emission sites across all 7 generators (`object`, `view`, `action`, `flow`, `dashboard`, `app`, `skill`) plus the barrel, and a generator added later inherits it.
+
+- **The criterion is parseability, not a charset.** Nothing is rewritten and no name that already produced parseable output is refused: the accepted set moves only by the names whose emission was already broken. Which names `os generate` should accept — and whether it should normalise the ones it does, the way `os create` derives its identifier — is a separate, open decision. Deriving a legal-looking identifier from a name that should have been refused is the worse of the two failures, so this refuses loudly rather than answering that question by widening tolerance.
+- **Asking the compiler is what makes the check correct per emission position.** A rule about identifier characters, or about reserved words, gets this wrong in both directions: `os generate object class` is refused (`const class:` is not a declaration) while `os generate view class` is accepted (that generator emits `const classViews:`), and a name carrying a quote or a comment terminator breaks the emitted file without touching the identifier at all.

packages/cli/src/commands/generate.ts

@@ -20,6 +20,7 @@ import type { FieldType } from '@objectstack/spec/data';
 import { isTenancyDisabled, isUniqueDeclared } from '@objectstack/spec/data';
 import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, CLI_ALIAS } from '../utils/format.js';
 import { metadataFileName } from '../utils/metadata-file-name.js';
+import { findEmissionParseFailures } from '../utils/emitted-source-parses.js';
 
 // ─── Metadata Type Templates ────────────────────────────────────────
 
@@ -699,12 +700,87 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?:
     console.log(`  ${chalk.dim('File:')}  ${chalk.white(path.join(dir, fileName))}`);
     console.log('');
 
+    // Both emissions are rendered ONCE, here, and every branch below reuses
+    // them: the scaffold file, and the barrel re-export line. They are the two
+    // files one name reaches (#16541), and rendering them at the single point
+    // where the name has finished being derived is what lets one refusal cover
+    // all 14 emission sites across all 7 generators instead of 14 patches.
+    const content = generator.generate(name);
+    const exportLine = `export { default as ${toCamelCase(name)} } from '${moduleSpecifier}';`;
+
+    // ⛔ REFUSE rather than rewrite (#16541).
+    //
+    // This command ran no name validation at all, so a name that is legal as a
+    // NAME but not as an IDENTIFIER was interpolated straight into a binding
+    // position and written out under `exit 0` — `const foo.bar:
+    // Data.ServiceObject = {`, plus a matching barrel line: two files that are
+    // not TypeScript, from a command that reported success.
+    //
+    // The criterion is PARSEABILITY, not a charset. `findEmissionParseFailures`
+    // asks the compiler about the bytes above and about nothing else, which is
+    // why it also covers what a rule about identifier characters would miss —
+    // a reserved word is illegal as a `const` binding and legal as an
+    // `export { default as … }` alias, and `${toCamelCase(name)}Views` parses
+    // for a name that bare `${toCamelCase(name)}` refuses.
+    //
+    // Which names this command should ACCEPT — and whether it should normalise
+    // the ones it does, the way `os create` derives its identifier since
+    // #15892 — is an OPEN decision. Sanitising here would answer it by quietly
+    // widening tolerance, and a legal-looking identifier derived from a name
+    // that should have been refused is the worse of the two failures. So
+    // nothing is rewritten, acceptance is unchanged for every name that already
+    // produced parseable output, and the refusal is loud.
+    //
+    // Placed AHEAD of the dry-run branch on purpose: a preview that prints
+    // un-parseable TypeScript and exits 0 is the same defect in preview form.
+    const parseFailures = await findEmissionParseFailures([
+      { label: path.join(dir, fileName), source: content },
+      { label: path.join(dir, 'index.ts'), source: exportLine },
+    ]);
+    if (parseFailures.length > 0) {
+      printError('Refusing to generate — the TypeScript this would write does not parse');
+      console.log('');
+      console.log(`  ${chalk.dim('Name:')}       ${chalk.white(name)}`);
+      console.log(`  ${chalk.dim('Identifier:')} ${chalk.white(toCamelCase(name))}`);
+      console.log('');
+      for (const failure of parseFailures) {
+        console.log(`  ${chalk.white(failure.label)}`);
+        for (const diagnostic of failure.diagnostics) {
+          console.log(chalk.dim(`    ${diagnostic}`));
+        }
+      }
+      console.log('');
+      console.log(chalk.dim(
+        `  \`${CLI_ALIAS} g\` derives a TypeScript identifier from the name you give it, and`,
+      ));
+      console.log(chalk.dim(
+        '  this one is not something the compiler can parse — so what is listed above',
+      ));
+      console.log(chalk.dim(
+        '  would be written broken. Nothing was written.',
+      ));
+      console.log('');
+      console.log(chalk.dim(
+        '  It refuses instead of rewriting your name into a legal-looking identifier,',
+      ));
+      console.log(chalk.dim(
+        '  which would decide in silence which names this command accepts. Pick a name',
+      ));
+      console.log(chalk.dim(
+        `  that survives as an identifier — \`${CLI_ALIAS} g ${type} order_line\` and`,
+      ));
+      console.log(chalk.dim(
+        `  \`${CLI_ALIAS} g ${type} order-line\` both work, and both fold to \`orderLine\`.`,
+      ));
+      console.log('');
+      process.exit(1);
+    }
+
     if (flags.dryRun) {
       printInfo('Dry run — no files written');
       console.log('');
       console.log(chalk.dim('  Content:'));
       console.log(chalk.dim('  ' + '-'.repeat(38)));
-      const content = generator.generate(name);
       for (const line of content.split('\n')) {
         console.log(chalk.dim(`  ${line}`));
       }
@@ -725,25 +801,24 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?:
         fs.mkdirSync(fullDir, { recursive: true });
       }
 
-      // Write file
-      const content = generator.generate(name);
+      // Write file — the same `content` the parse check above accepted, ⛔ not
+      // a re-render: a second call to `generator.generate` would make the
+      // bytes that were checked and the bytes that land two different things.
       fs.writeFileSync(filePath, content);
       printSuccess(`Created ${path.join(dir, fileName)}`);
 
       // Check for barrel index
       const indexPath = path.join(process.cwd(), dir, 'index.ts');
       if (fs.existsSync(indexPath)) {
         const indexContent = fs.readFileSync(indexPath, 'utf-8');
-        const exportLine = `export { default as ${toCamelCase(name)} } from '${moduleSpecifier}';`;
 
         if (!indexContent.includes(toCamelCase(name))) {
           fs.appendFileSync(indexPath, exportLine + '\n');
           printSuccess(`Updated ${dir}/index.ts with export`);
         }
       } else {
         // Create barrel index
-        const exportLine = `export { default as ${toCamelCase(name)} } from '${moduleSpecifier}';\n`;
-        fs.writeFileSync(indexPath, exportLine);
+        fs.writeFileSync(indexPath, exportLine + '\n');
         printSuccess(`Created ${dir}/index.ts`);
       }
 

packages/cli/src/utils/emitted-source-parses.ts

@@ -0,0 +1,127 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Does the TypeScript a scaffolder is ABOUT TO WRITE actually parse? (#16541)
+ *
+ * ## The defect this exists to end
+ *
+ * `os generate <type> <name>` derives a code identifier from `name` by
+ * camel-casing it, and ran NO validation of any kind on the result. So a name
+ * that is legal as a *name* but not as an *identifier* was interpolated
+ * straight into a binding position:
+ *
+ *     os generate object foo.bar          exit 0
+ *     src/objects/foo.bar.object.ts   ->  const foo.bar: Data.ServiceObject = {
+ *     src/objects/index.ts            ->  export { default as foo.bar } from './foo.bar.object';
+ *
+ * Two files, neither of them TypeScript, and a command that reported success.
+ * The next `tsc` is where the author finds out.
+ *
+ * ## ⛔ What this is NOT
+ *
+ * It is NOT a sanitiser and it is NOT a charset. It answers exactly one
+ * question — *do the bytes we are about to write parse?* — and the answer
+ * comes from the compiler rather than from an opinion about which characters
+ * are tasteful. Which names `os generate` should ACCEPT, and whether it should
+ * normalise the ones it does, is an open decision (#16541): deriving a
+ * legal-looking identifier from a name that should have been refused is the
+ * worse of the two failures, so this refuses loudly and rewrites nothing.
+ *
+ * ## Why the instrument is TypeScript's own parser
+ *
+ * "Does it look like an identifier" is the judgement that produced the defect
+ * in the first place, and every restatement of the grammar is a fresh chance
+ * to get it wrong: `${camel}Views` is fine for a name that `const ${camel}`
+ * refuses (`class`), a reserved word is legal as an `export { default as … }`
+ * alias and illegal as a `const` binding, and a name carrying a quote or a
+ * comment terminator breaks the emission without touching the identifier at
+ * all. Asking `ts` about the ACTUAL EMITTED BYTES answers all of those at once
+ * and restates none of them — the same reasoning, and the same instrument,
+ * that `create-plugin-identifier-parses.test.ts` (#15892) uses to pin the
+ * sibling door.
+ *
+ * Syntactic diagnostics only: `noLib` and `noResolve` keep the verdict about
+ * the grammar of these bytes. A scaffold references types it cannot resolve in
+ * a temp directory by design, and a resolution-aware verdict would refuse
+ * every name.
+ *
+ * ## Why `ts` arrives through a lazy import
+ *
+ * `ts-morph` is already a CLI runtime dependency and re-exports the full
+ * TypeScript compiler namespace, so we use its `ts` rather than adding a
+ * direct `typescript` dependency — the same call `detect-free-identifiers.ts`
+ * makes, and for the same reason. It is imported *inside* the check rather
+ * than at module top because the compiler is a heavy load and `os generate`
+ * has no other use for it: a command that is refused pays for the parser, a
+ * command that never reaches here does not.
+ */
+
+import type { ts as TS } from 'ts-morph';
+
+/** One file a command is about to write, and the bytes it would contain. */
+export interface EmittedSource {
+  /** Path as the author will see it, e.g. `src/objects/foo.bar.object.ts`. */
+  label: string;
+  /** The exact bytes that would be written. */
+  source: string;
+}
+
+/** An emission the compiler cannot parse, with the compiler's own reasons. */
+export interface EmissionParseFailure {
+  label: string;
+  /** TypeScript's syntactic diagnostics, flattened to text, in source order. */
+  diagnostics: string[];
+}
+
+/**
+ * The syntactic diagnostics TypeScript reports for `source`, flattened to text.
+ *
+ * Exported so a pin can reach the instrument the command actually uses instead
+ * of a second copy of it that could drift green.
+ */
+export function syntacticDiagnostics(ts: typeof TS, source: string): string[] {
+  const fileName = 'emitted.ts';
+  const sourceFile = ts.createSourceFile(
+    fileName,
+    source,
+    ts.ScriptTarget.Latest,
+    true,
+    ts.ScriptKind.TS,
+  );
+  const host: TS.CompilerHost = {
+    getSourceFile: (requested) => (requested === fileName ? sourceFile : undefined),
+    getDefaultLibFileName: () => 'lib.d.ts',
+    writeFile: () => {},
+    getCurrentDirectory: () => '/',
+    getCanonicalFileName: (f) => f,
+    useCaseSensitiveFileNames: () => true,
+    getNewLine: () => '\n',
+    fileExists: (f) => f === fileName,
+    readFile: (f) => (f === fileName ? source : undefined),
+  };
+  const program = ts.createProgram(
+    [fileName],
+    { noLib: true, noResolve: true, target: ts.ScriptTarget.Latest },
+    host,
+  );
+  return program
+    .getSyntacticDiagnostics(sourceFile)
+    .map((d) => ts.flattenDiagnosticMessageText(d.messageText, ' '));
+}
+
+/**
+ * Every emission in `emissions` the compiler refuses to parse, in the order
+ * given. An empty array means all of them parse — ⛔ never that the check was
+ * skipped: `emissions` is built from the same values the command writes.
+ */
+export async function findEmissionParseFailures(
+  emissions: readonly EmittedSource[],
+): Promise<EmissionParseFailure[]> {
+  const { ts } = await import('ts-morph');
+  const failures: EmissionParseFailure[] = [];
+  for (const { label, source } of emissions) {
+    const diagnostics = syntacticDiagnostics(ts, source);
+    if (diagnostics.length > 0) failures.push({ label, diagnostics });
+  }
+  return failures;
+}