diff --git a/.changeset/generate-name-charset-gate.md b/.changeset/generate-name-charset-gate.md new file mode 100644 index 0000000000..025e61854b --- /dev/null +++ b/.changeset/generate-name-charset-gate.md @@ -0,0 +1,13 @@ +--- +"@objectstack/cli": minor +--- + +feat(cli)!: `os generate` refuses a metadata name outside the charset `packages/spec` declares for an object `name`, before it derives anything from it (#16726) + +Maintainer ruling, decision batch #82 (2026-09-08), option A — **a gate, not a sanitiser**. `os generate ` used to accept any name at all; since #16724 it has refused names whose emitted TypeScript does not parse. It now also refuses, ahead of that check and ahead of every derivation, any name the object-`name` declaration in `@objectstack/spec` rejects. The refusal names the value and quotes the schema's own rule, and writes nothing. + +⛔ Nothing is rewritten. The rejected alternative was to derive a legal identifier the way `os create` does, which decouples the name the author wrote from the name that gets emitted with nothing announcing it — the failure mode that multiplies silently when metadata is written in bulk. So the name you author and the name that lands in the file are always the same string. + +**What this narrows:** kebab-case (`order-line`), uppercase (`Order`), dotted (`foo.bar`) and digit-initial (`2fast`) names were accepted before and are refused now — `order-line` used to generate `order_line.object.ts` binding `orderLine`. Write the snake_case name directly (`os g object order_line`). ⛔ No new charset was minted and no flag bypasses the gate; #16724's parse check is unchanged and stays as the backstop behind it (`class` passes the charset and is still refused for `object`, because `const class:` is not a declaration). + + diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 92a2ffe8df..e134514da3 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1259,6 +1259,13 @@ only. Generates properly typed metadata files with barrel index management. +`` is held to the charset `@objectstack/spec` declares for an object +`name` — lowercase letters, digits and `_`, never starting with a digit. A name +outside it is refused before anything is derived or written, naming the value +and the rule; it is **never** rewritten into one that fits, so the name you +write is the name that lands in the file (`os g object order_line`, not +`order-line`). + ```bash os g object customer # Generate a Customer object os g view customer # Generate a Customer list view @@ -1266,7 +1273,7 @@ os g action approve # Generate an action os g flow customer # Generate an automation flow os g dashboard sales # Generate a dashboard os g app crm # Generate an app definition -os g skill lead-qual # Generate an AI skill +os g skill lead_qual # Generate an AI skill os g object task -d lib/ # Override target directory os g object task --dry-run # Preview without writing @@ -1894,7 +1901,7 @@ os g object contact os g object opportunity # 3. Add business logic -os g flow lead-qualification +os g flow lead_qualification # 4. Validate everything os validate diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index e1fa07b059..d723d2d56e 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -24,6 +24,10 @@ import { isTenancyDisabled, isUniqueDeclared, numericColumnFor, + // #16726 — the name gate below. IMPORTED for the same reason as the five + // above: it asks the schema whether a name is legal instead of restating + // the charset the schema declares. + ObjectSchema, } from '@objectstack/spec/data'; import { printHeader, printSuccess, printError, printInfo, printStep, createTimer, isReportedError, CLI_ALIAS } from '../utils/format.js'; import { metadataFileName } from '../utils/metadata-file-name.js'; @@ -472,6 +476,54 @@ function toSnakeCase(str: string): string { return str.replace(/[-]/g, '_').replace(/[A-Z]/g, c => `_${c.toLowerCase()}`).replace(/^_/, ''); } +/** + * Is this a name `os generate` accepts? (#16726) + * + * ## The declared answer, asked rather than restated + * + * The accepted set is the charset `packages/spec` ALREADY declares for an + * object `name` — maintainer ruling, decision batch #82 (2026-09-08, option + * A): a gate, ⛔ no sanitiser, and ⛔ no third charset. So the judge here is + * that declaration itself (`ObjectSchema.shape.name`), reached through the + * package's exported surface. Nothing in this file states what the charset + * IS: a transcription is a second declaration that can drift green while spec + * moves, and the ruling asks for the spec's rule, not for a copy of today's + * reading of it. The refusal even quotes the schema's own message, so the + * pattern the author is shown is the pattern that judged them. + * + * ## ⛔ Why it returns a REASON and never a repaired name + * + * The rejected option (B) was to derive a legal identifier the way + * `os create` has since #15892. It was refused because it decouples the name + * the author wrote from the name that gets emitted, silently: write + * `foo.bar`, get `fooBar` in the file, and every later reference the author + * types by hand is wrong with nothing announcing it. For metadata written in + * bulk that divergence multiplies unseen. So this answers only *may this name + * through*, and the caller refuses loudly — ⛔ it never rewrites, and no flag + * bypasses it. + * + * ## What it deliberately does NOT decide + * + * Whether the TypeScript the accepted name would produce actually PARSES. + * That is #16541's check (`findEmissionParseFailures`), it stays exactly where + * it landed, and it is a genuinely different question: `class` is inside this + * charset and is still refused by the compiler in a `const` binding position, + * while `order-line` emits a perfectly parseable `orderLine` and is refused + * here. Neither layer shadows the other — `generate-refuses-name-outside-charset.test.ts` + * measures both directions. + * + * @returns `null` when the name is accepted, or the schema's own reason when + * it is not. + */ +function nameCharsetRefusal(name: string): string | null { + // Reached lazily, inside the call: `ObjectSchema` is a lazy schema, and a + // module-top `.shape` read would materialize it for every CLI command + // including the ones that never generate anything. + const verdict = ObjectSchema.shape.name.safeParse(name); + if (verdict.success) return null; + return verdict.error.issues[0]?.message ?? 'not a legal object name'; +} + // ─── Field Type Mapping ───────────────────────────────────────────── /** @@ -669,6 +721,58 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?: process.exit(1); } + // ⛔ REFUSE a name outside the declared charset, BEFORE anything is + // derived from it (#16726). + // + // Placed here on purpose, and the position is the ruling: every derivation + // this command performs — `toSnakeCase` for the metadata name and the + // filename, `toCamelCase` for the binding and the barrel alias, + // `toTitleCase` for the labels — happens BELOW this line, so a refused + // name is never folded into a legal-looking one on the way to a + // diagnostic. It sits after the type roster so that `os g + // ` still answers about the type, which is the more useful answer. + // + // What it is NOT: a sanitiser (option B was refused — see + // `nameCharsetRefusal`), a charset of this command's own (the judge is + // spec's object-`name` declaration), and not a replacement for the parse + // check further down, which stays as the backstop it was built to be. + const charsetRefusal = nameCharsetRefusal(name); + if (charsetRefusal) { + printError(`Refusing to generate — \`${name}\` is not a name this command accepts`); + console.log(''); + console.log(` ${chalk.dim('Name:')} ${chalk.white(name)}`); + console.log(` ${chalk.dim('Rule:')} ${chalk.white(charsetRefusal)}`); + console.log(''); + console.log(chalk.dim( + ` That rule is not \`${CLI_ALIAS} g\`'s own: it is the charset \`@objectstack/spec\``, + )); + console.log(chalk.dim( + ' declares for an object `name`, asked of the schema itself. A metadata name', + )); + console.log(chalk.dim( + ' that is refused there has no business being scaffolded here.', + )); + console.log(''); + console.log(chalk.dim( + ' It refuses instead of folding your name into one that fits, so the name you', + )); + console.log(chalk.dim( + ' write and the name that lands in the file are always the same string.', + )); + console.log(chalk.dim( + // ⛔ The examples are deliberately NOT built from what the author + // typed. A suggestion derived from the refused name is option (B) + // wearing a prompt: the author accepts it, and the divergence this + // gate exists to prevent arrives one keystroke later. + ` Nothing was written. Names like \`${CLI_ALIAS} g ${type} customer\` or`, + )); + console.log(chalk.dim( + ` \`${CLI_ALIAS} g ${type} sales_order\` are accepted.`, + )); + console.log(''); + process.exit(1); + } + const dir = flags.dir || generator.defaultDir; // The written name comes from the registry's `filePatterns` for this type // — see `metadataFileName`, which carries why it is derived rather than @@ -774,10 +878,14 @@ async function runMetadataGeneration(type: string, name: string, flags: { dir?: ' 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`, + // ⛔ This line used to offer `order-line` as an equal alternative. The + // #16726 gate above refuses that spelling before this check is ever + // reached, so offering it here would send the author to a second + // refusal. The CHECK is untouched — only the advice it prints. + ` that survives as an identifier — \`${CLI_ALIAS} g ${type} order_line\` works,`, )); console.log(chalk.dim( - ` \`${CLI_ALIAS} g ${type} order-line\` both work, and both fold to \`orderLine\`.`, + ' and binds `orderLine`.', )); console.log(''); process.exit(1); @@ -2598,7 +2706,11 @@ export default class Generate extends Command { static override args = { type: Args.string({ description: 'Metadata type to generate (object, view, action, flow, dashboard, app)', required: true }), - name: Args.string({ description: 'Name for the metadata (use kebab-case)', required: false }), + // ⛔ NOT "use kebab-case" any more (#16726): a name outside the charset + // spec declares for an object `name` is refused at the door, and + // kebab-case is outside it. What this string advertises and what the + // command accepts have to be the same set. + name: Args.string({ description: 'Name for the metadata (snake_case)', required: false }), }; static override flags = { diff --git a/packages/cli/test/generate-refuses-name-outside-charset.test.ts b/packages/cli/test/generate-refuses-name-outside-charset.test.ts new file mode 100644 index 0000000000..a730226586 --- /dev/null +++ b/packages/cli/test/generate-refuses-name-outside-charset.test.ts @@ -0,0 +1,291 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#16726) — `os generate ` refuses a name outside the + * charset `packages/spec` declares for an object `name`, refuses it BEFORE it + * derives anything from that name, and refuses it without rewriting it. + * + * ## The ruling this measures + * + * Decision batch #82 (2026-09-08), maintainer 「同意」 on option A — a GATE: + * + * > `os generate` refuses, before deriving anything, any name outside the + * > charset `packages/spec` already declares for an object `name` + * > (`^[a-z_][a-z0-9_]*$` — the implementer re-reads the schema rather than + * > trusting this transcription). The refusal names the value and the rule. + * > PR #16724's parse check stays as the backstop. ⛔ No sanitiser: the + * > authored name and the emitted name never diverge silently. ⛔ No third + * > charset. + * + * So: the refusal is asserted here, and the ⛔ half is asserted with it — a + * gate that also quietly repaired the name would satisfy "exit 1" and would be + * the option (B) the ruling refused. + * + * ⛔ The charset is NOT transcribed in this file either. The rule the command + * prints is compared against the message the SPEC schema itself produces for + * the same input, so a command that invented its own wording, or a gate wired + * to a copy of the charset, reddens here — and a deliberate move in spec moves + * both sides at once instead of leaving a stale literal to be argued with. + * + * ## THE TWO LAYERS ARE DISTINCT — measured in both directions + * + * The gate sits in front of #16541's parse check, and neither shadows the + * other. That is not an opinion about where they sit; it is a property with + * witnesses, and both are exercised below: + * + * - `order-line` — REFUSED by the gate, and the parse check would have + * ACCEPTED it (its emission parses clean, asserted here against the very + * instrument the command runs). Delete the gate and this name generates. + * - `class` — ADMITTED by the gate (every character is in the charset), and + * REFUSED by the parse check for `object`, because `const class:` is not a + * declaration. Delete the parse check and this name generates. + * + * ## Why a child process + * + * Same two reasons as `generate-refuses-unparseable-name.test.ts`: the defect + * class here is an EXIT CODE plus bytes on disk, `process.exitCode` inside a + * vitest worker is not an exit status, and these commands print through + * `utils/format.ts`, whose `printError` writes to stdout. Spawned through + * `bin/run-dev.js` + tsx so the suite does not depend on `packages/cli/dist`. + * + * ## ⛔ Why this file is NOT named `.e2e` + * + * The same sanctioned combination its sibling documents: the BEHAVIOUR + * predicate in `vitest-tiers.ts` puts a spawning file in the `integration` + * project, while the NAME decides which RUN collects it. What is pinned here + * is a published command's accepted set, so it belongs in the run that gates + * the merge queue, not in the nightly one. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { GENERATOR_SCAFFOLD_TARGETS } from '../src/commands/generate.js'; +import { metadataFileName } from '../src/utils/metadata-file-name.js'; +import { findEmissionParseFailures } from '../src/utils/emitted-source-parses.js'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** oclif + tsx cold start, with every command module loaded; ~2-10 s when healthy. */ +const RUN_TIMEOUT_MS = 240_000; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runTsx(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + args, + { cwd, maxBuffer: 8 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + // `err.code` is the real exit status; null/undefined means the child + // was signalled — a different failure, never reported as 0. + code: err + ? typeof (err as { code?: unknown }).code === 'number' + ? (err as unknown as { code: number }).code + : 1 + : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** + * What the SPEC schema says about `name`, asked the same way the command asks + * it. The command's refusal is compared against this, so neither side is a + * transcription of the other. + */ +function specVerdict(name: string): { accepted: boolean; message: string } { + const verdict = ObjectSchema.shape.name.safeParse(name); + return verdict.success + ? { accepted: true, message: '' } + : { accepted: false, message: verdict.error.issues[0]?.message ?? '' }; +} + +/** `toSnakeCase` / `toCamelCase` as `generate.ts` spells them (same reason as the sibling pin: not exported for a test's convenience). */ +function toSnakeCase(str: string): string { + return str.replace(/[-]/g, '_').replace(/[A-Z]/g, (c) => `_${c.toLowerCase()}`).replace(/^_/, ''); +} +function toCamelCase(str: string): string { + return str.replace(/[-_]([a-z])/g, (_, c: string) => c.toUpperCase()); +} + +/** The two emissions one `os generate ` produces, built the way the command builds them. */ +function emissionsFor(type: string, name: string) { + const target = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === type); + if (!target) throw new Error(`no generator for type: ${type}`); + const fileName = metadataFileName(type, toSnakeCase(name)); + if (fileName === null) throw new Error(`no file naming convention for type: ${type}`); + return [ + { label: fileName, source: target.generate(name) }, + { + label: 'index.ts', + source: `export { default as ${toCamelCase(name)} } from './${fileName.replace(/\.ts$/, '')}';`, + }, + ]; +} + +let gatedDir: string; +let dryRunDir: string; +let backstopDir: string; +let controlDir: string; + +let gated: Run; +let dryRun: Run; +let backstop: Run; +let control: Run; + +beforeAll(async () => { + gatedDir = mkdtempSync(join(tmpdir(), 'os-g-charset-')); + dryRunDir = mkdtempSync(join(tmpdir(), 'os-g-charset-dry-')); + backstopDir = mkdtempSync(join(tmpdir(), 'os-g-charset-backstop-')); + controlDir = mkdtempSync(join(tmpdir(), 'os-g-charset-control-')); + + // Sequential on purpose: cold tsx starts, each loading every command module, + // in a container several agents share. + gated = await runTsx([CLI, 'generate', 'object', 'order-line'], gatedDir); + dryRun = await runTsx([CLI, 'generate', 'flow', 'lead-qual', '--dry-run'], dryRunDir); + backstop = await runTsx([CLI, 'generate', 'object', 'class'], backstopDir); + control = await runTsx([CLI, 'generate', 'object', 'order_line'], controlDir); +}, RUN_TIMEOUT_MS); + +afterAll(() => { + for (const dir of [gatedDir, dryRunDir, backstopDir, controlDir]) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('[#16726] a name outside the declared charset is refused at the door', () => { + it('exits non-zero — `order-line` used to exit 0', () => { + expect(gated.code).toBe(1); + }); + + it('names the VALUE it refused', () => { + expect(gated.stdout).toContain('order-line'); + }); + + it('names the RULE — and it is the schema`s own, not this command`s wording', () => { + const verdict = specVerdict('order-line'); + expect(verdict.accepted).toBe(false); + // Non-empty guard: a schema that stopped producing a message would make + // the assertion below vacuously true. + expect(verdict.message.length).toBeGreaterThan(0); + expect(gated.stdout).toContain(verdict.message); + }); + + it('⛔ does not rewrite the name into one that fits', () => { + // Option B, refused by the ruling: the author writes one name and a + // different one lands in the file. Neither the folded metadata name nor + // the derived binding may appear anywhere in a refusal. + expect(gated.stdout).not.toContain('order_line'); + expect(gated.stdout).not.toContain('orderLine'); + expect(gated.stdout).not.toContain('Created'); + }); + + it('writes nothing — no scaffold, no barrel, no directory', () => { + expect(existsSync(join(gatedDir, 'src'))).toBe(false); + }); + + it('is the CHARSET refusal, not the parse check speaking', () => { + // If this said "does not parse", the gate would not exist and #16541's + // backstop would be answering — for a name whose emission parses fine. + expect(gated.stdout).not.toContain('does not parse'); + }); +}); + +describe('[#16726] the gate is one chokepoint, and it fires before the preview', () => { + it('`os generate flow lead-qual --dry-run` is refused too', () => { + // A second generator AND the dry-run branch in one run: a preview that + // renders a scaffold for a name the command would refuse to write is the + // same divergence in preview form. + expect(dryRun.code).toBe(1); + expect(dryRun.stdout).toContain('lead-qual'); + expect(dryRun.stdout).not.toContain('Automation.Flow'); + expect(dryRun.stdout).not.toContain('leadQual'); + expect(existsSync(join(dryRunDir, 'src'))).toBe(false); + }); +}); + +describe('[#16726] the gate and #16541`s parse check are DISTINCT layers', () => { + it('the gate refuses a name the parse check would have accepted', async () => { + // `order-line`: the emission is parseable — measured with the instrument + // the command itself runs — so the ONLY thing standing between it and a + // generated file is the gate asserted above. + const failures = await findEmissionParseFailures(emissionsFor('object', 'order-line')); + expect(failures).toEqual([]); + expect(specVerdict('order-line').accepted).toBe(false); + expect(gated.code).toBe(1); + }); + + it('the parse check refuses a name the gate admits', async () => { + // `class` is inside the charset — every character is a lowercase letter — + // so the gate lets it through and the backstop is what refuses it. + expect(specVerdict('class').accepted).toBe(true); + const failures = await findEmissionParseFailures(emissionsFor('object', 'class')); + expect(failures.length).toBeGreaterThan(0); + expect(backstop.code).toBe(1); + expect(backstop.stdout).toContain('does not parse'); + // ⛔ The gate must not have shadowed it: the author gets the COMPILER's + // reason, and the compiler's reason for this name is specific to it — + // a hand-written "invalid name" line would satisfy every other + // assertion in this block. + expect(backstop.stdout).toContain("'class' is not allowed as a variable declaration name."); + // ⛔ And it is NOT the charset refusal: `class` is inside the charset. + expect(backstop.stdout).not.toContain('must match pattern'); + }); + + it('⚠️ records what the pair actually does with `os generate view class`', () => { + // The card called this row "the decision in miniature", and the ruling + // comment's closing line states it "is therefore refused at the door". + // ⚠️ That does not follow from the mechanism the same ruling specifies: + // `class` is INSIDE the charset spec declares for an object `name`, so a + // charset gate admits it, and the `view` generator emits `const + // classViews:` plus an `export { default as class }` alias, both of which + // parse. The pair therefore still accepts it. + // + // Recorded rather than legislated: refusing reserved words is a THIRD + // rule, and the ruling's other half is ⛔ no third charset. Reported on + // #16726 for the maintainer; this assertion exists so that whichever way + // that is answered, the answer is a deliberate edit here. + expect(specVerdict('class').accepted).toBe(true); + return findEmissionParseFailures(emissionsFor('view', 'class')).then((failures) => { + expect(failures).toEqual([]); + }); + }); +}); + +describe('[#16726] CONTROL — a name inside the charset still generates', () => { + it('still exits 0 and reports both writes', () => { + // A gate that refused everything would satisfy every assertion above. + expect(control.code).toBe(0); + expect(control.stdout).toContain('Created src/objects/order_line.object.ts'); + expect(control.stdout).toContain('Created src/objects/index.ts'); + expect(existsSync(join(controlDir, 'src', 'objects', 'order_line.object.ts'))).toBe(true); + }); + + it('a leading underscore is inside the charset too', () => { + // The charset spec declares is `[a-z_]` first, not `[a-z]` — asserted + // through the schema rather than by spawning a fifth child process, so a + // gate quietly narrowed to "letters only" is still caught. + expect(specVerdict('_internal').accepted).toBe(true); + expect(specVerdict('order_line_2').accepted).toBe(true); + expect(specVerdict('Order').accepted).toBe(false); + expect(specVerdict('2fast').accepted).toBe(false); + expect(specVerdict('').accepted).toBe(false); + }); +}); diff --git a/packages/cli/test/generate-refuses-unparseable-name.test.ts b/packages/cli/test/generate-refuses-unparseable-name.test.ts index 2209ff9036..788cd4a4b5 100644 --- a/packages/cli/test/generate-refuses-unparseable-name.test.ts +++ b/packages/cli/test/generate-refuses-unparseable-name.test.ts @@ -31,13 +31,46 @@ * `integration` (behaviour)"), and ⛔ nothing is renamed to make the two cuts * agree. * + * ## ⚠️ WHICH NAME REACHES THIS CHECK CHANGED (#16726) + * + * A charset gate now sits in FRONT of this one: a name outside the charset + * `packages/spec` declares for an object `name` is refused before anything is + * derived from it, so it never reaches the compiler at all. `foo.bar` — the + * card's measured name — is one of those, and it still exits 1 having written + * nothing, which is the defect #16541 was filed about. What it no longer + * demonstrates is THIS check: the refusal it now meets is the gate's. + * + * So the parse check is measured through `class`, added here for that purpose. + * It is inside the charset (every character is a lowercase letter), the gate + * admits it, and `const class:` is still not a declaration — so it is the name + * that proves this command consults the compiler before it writes, and that + * the layer in front did not swallow the layer behind. ⛔ Nothing was deleted + * to make room for it: every `foo.bar` assertion that is still about the + * COMMAND (exit code, no rewrite, nothing on disk) is asserted below unchanged. + * + * One property genuinely stopped being reachable from here: a name that breaks + * the BARREL line as well as the scaffold. A reserved word is legal as an + * `export { default as … }` alias, so no charset-legal name breaks both, and + * every name that does is now stopped one layer earlier. That half stays + * pinned where it still runs — `generate-emission-parses.test.ts`'s CANARY row + * measures `foo.bar` against both emissions, with the instrument this command + * calls. + * * ## The control is load-bearing * * A refusal that fires on everything would satisfy every assertion about - * `foo.bar` and would be a worse command than the broken one. `order-line` + * `foo.bar` and would be a worse command than the broken one. `order_line` * runs the whole path — writes the scaffold, writes the barrel — and both of * its files are re-read and re-parsed here, so "still works" is a reading * rather than an exit code. + * + * ⚠️ The control was spelled `order-line` until #16726 put a charset gate in + * front of this check, and kebab-case is outside the charset spec declares for + * an object `name` — so that spelling now stops one layer earlier and would + * have made this control measure the OTHER refusal. The two spellings derive + * the same everything (`order_line.object.ts`, `orderLine`), so every + * assertion below is the one #16541 wrote, byte for byte; only the authored + * input moved to a name this command still accepts. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -107,16 +140,19 @@ function parseErrors(source: string): string[] { let refusedDir: string; let controlDir: string; let dryRunDir: string; +let unparseableDir: string; let refused: Run; let refusedAgain: Run; let control: Run; let dryRun: Run; +let unparseable: Run; beforeAll(async () => { refusedDir = mkdtempSync(join(tmpdir(), 'os-g-refuse-')); controlDir = mkdtempSync(join(tmpdir(), 'os-g-control-')); dryRunDir = mkdtempSync(join(tmpdir(), 'os-g-dryrun-')); + unparseableDir = mkdtempSync(join(tmpdir(), 'os-g-unparseable-')); // Sequential on purpose: cold tsx starts, each loading every command module, // in a container several agents share. @@ -124,11 +160,14 @@ beforeAll(async () => { // A second generator, to show the refusal is not one patched call site. refusedAgain = await runTsx([CLI, 'generate', 'flow', 'foo.bar'], refusedDir); dryRun = await runTsx([CLI, 'generate', 'object', 'foo.bar', '--dry-run'], dryRunDir); - control = await runTsx([CLI, 'generate', 'object', 'order-line'], controlDir); + // Inside the charset, outside the grammar — the name that reaches THIS + // check now that a charset gate stands in front of it (#16726). + unparseable = await runTsx([CLI, 'generate', 'object', 'class'], unparseableDir); + control = await runTsx([CLI, 'generate', 'object', 'order_line'], controlDir); }, RUN_TIMEOUT_MS); afterAll(() => { - for (const dir of [refusedDir, controlDir, dryRunDir]) { + for (const dir of [refusedDir, controlDir, dryRunDir, unparseableDir]) { rmSync(dir, { recursive: true, force: true }); } }); @@ -139,21 +178,9 @@ describe('[#16541] `os generate object foo.bar` refuses instead of exiting 0', ( expect(refused.code).toBe(1); }); - it('says it is refusing, and says the emission does not parse', () => { + it('says it is refusing, and names the value it refused', () => { expect(refused.stdout).toContain('Refusing to generate'); - expect(refused.stdout).toContain('does not parse'); - }); - - it('names BOTH files the name would have corrupted', () => { - expect(refused.stdout).toContain('foo.bar.object.ts'); - expect(refused.stdout).toContain('index.ts'); - }); - - it('quotes the compiler`s own diagnostic rather than a restatement of it', () => { - // The message TypeScript emits for a property access in a binding - // position. Asserted because a hand-written "invalid name" line would pass - // every other assertion in this block. - expect(refused.stdout).toContain("',' expected."); + expect(refused.stdout).toContain('foo.bar'); }); it('⛔ does not rewrite the name into a legal-looking identifier', () => { @@ -170,6 +197,33 @@ describe('[#16541] `os generate object foo.bar` refuses instead of exiting 0', ( }); }); +describe('[#16541] the parse check still refuses, with the compiler`s own words', () => { + // `class`, not `foo.bar` — see the header. Everything asserted here is what + // #16541 asserted about `foo.bar` before the #16726 gate started answering + // for that spelling first. + it('exits non-zero and says the emission does not parse', () => { + expect(unparseable.code).toBe(1); + expect(unparseable.stdout).toContain('Refusing to generate'); + expect(unparseable.stdout).toContain('does not parse'); + }); + + it('names the file the name would have corrupted', () => { + expect(unparseable.stdout).toContain('class.object.ts'); + }); + + it('quotes the compiler`s own diagnostic rather than a restatement of it', () => { + // The message TypeScript emits for a reserved word in a binding position. + // Asserted because a hand-written "invalid name" line would pass every + // other assertion in this block — and because it proves the gate in front + // did not answer for this name. + expect(unparseable.stdout).toContain("'class' is not allowed as a variable declaration name."); + }); + + it('writes nothing', () => { + expect(existsSync(join(unparseableDir, 'src'))).toBe(false); + }); +}); + describe('[#16541] the refusal is one chokepoint, not one patched generator', () => { it('`os generate flow foo.bar` is refused the same way', () => { expect(refusedAgain.code).toBe(1); diff --git a/packages/cli/test/generate-skill.e2e.test.ts b/packages/cli/test/generate-skill.e2e.test.ts index 108a24220c..d7425f3bf8 100644 --- a/packages/cli/test/generate-skill.e2e.test.ts +++ b/packages/cli/test/generate-skill.e2e.test.ts @@ -163,7 +163,10 @@ beforeAll(async () => { // Sequential on purpose: cold tsx starts, each loading every command module, // in a container several agents share. - generated = await runTsx([CLI, 'g', 'skill', 'lead-qualification'], dir); + // `lead_qualification`, not `lead-qualification`: the #16726 charset gate + // refuses kebab-case at the door. Both spellings derive the same written + // file and the same barrel alias, so what this file measures is unchanged. + generated = await runTsx([CLI, 'g', 'skill', 'lead_qualification'], dir); control = await runTsx([CLI, 'g', 'object', 'customer'], dir); const skillDir = join(dir, 'src', 'skills');