From 93cd9f03c4d129248955462508033fac92f7459a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:04:11 +0000 Subject: [PATCH 1/2] fix(cli): scaffold a flow that `os validate` accepts (#14087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `flow` template emitted a top-level `trigger: { type, object, events }` block, nodes carrying `name`/`next`, and no `edges` — four refusals against `FlowSchema`, which is `.strict()` and binds a record-change flow on the START node's `config` (`{ objectName, triggerType, condition }`), where `AutomationEngine.resolveTriggerBinding` reads it from. The template now writes that shape. `generate-scaffold-validates.test.ts` puts every generator's output through the two steps `os validate` runs — schema parse, then the author-time rule registry — loaded through the same `bundle-require` path `loadConfig` uses, since a node `config` is an open slot (ADR-0018) and the schema alone cannot judge the `record-*` trigger grammar. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- .../generate-flow-scaffold-validates.md | 48 ++++ packages/cli/src/commands/generate.ts | 77 ++++- .../test/generate-scaffold-validates.test.ts | 265 ++++++++++++++++++ 3 files changed, 375 insertions(+), 15 deletions(-) create mode 100644 .changeset/generate-flow-scaffold-validates.md create mode 100644 packages/cli/test/generate-scaffold-validates.test.ts diff --git a/.changeset/generate-flow-scaffold-validates.md b/.changeset/generate-flow-scaffold-validates.md new file mode 100644 index 0000000000..8a0d993c59 --- /dev/null +++ b/.changeset/generate-flow-scaffold-validates.md @@ -0,0 +1,48 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): `os generate flow` scaffolds a flow `os validate` accepts (#14087) + +The `flow` scaffold could not survive its own toolchain. Measured on 17.2.0, +`os g flow my_flow` followed by `os validate` produced four refusals in one +parse: + +``` +flows[0].nodes[0].label expected string, received undefined +flows[0].nodes[0] unrecognized key(s): `name`, `next` +flows[0].edges expected array, received undefined +flows[0] unrecognized key(s): `trigger` +``` + +`FlowSchema` is `.strict()` and has never declared a top-level `trigger` on +protocol 17. A record-change flow binds on the START node's `config` — +`{ objectName, triggerType, condition }` — which is where +`AutomationEngine.resolveTriggerBinding` reads it from. The scaffold also wrote +an `events: ['after_insert', 'after_update']` vocabulary that exists nowhere on +the current surface, and named a single node it then pointed at a node it never +emitted. + +So the first flow anybody scaffolded was a file their own `os validate` +rejected — against a `.strict()` error enumerating what is allowed rather than +saying where the trigger had moved to. + +The template now emits the shape the schema accepts and the engine binds: +`type: 'record_change'`, a labelled START node carrying +`config: { objectName, triggerType: 'record-after-write' }`, a labelled END +node, and the `edges` array joining them. `status` stays `'draft'` — the arming +decision is the author's, and `os validate` says so. + +`generate-scaffold-validates.test.ts` puts every generator's output through the +two steps `os validate` runs on an authored stack (schema parse, then the +author-time rule registry), loaded through the same `bundle-require` path +`loadConfig` uses, so the generator and the schema cannot drift apart again +silently. Both layers are needed: a flow node's `config` is an open slot +(ADR-0018), so the schema cannot judge the trigger vocabulary at all — the +`record-*` grammar is held by `validate-flow-trigger-readiness` one layer +later. + +No other generator's output changed. Four of them (`object`, `view`, `action`, +`app`) are refused for unrelated reasons of their own; the new test records +them in a shrink-only ledger that fails when one is repaired and its entry is +left behind. diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index daabda2d4c..3dae193374 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -113,6 +113,29 @@ export default ${toCamelCase(name)}Action; flow: { description: 'Automation flow', defaultDir: 'src/flows', + /** + * A record-change flow in the shape `FlowSchema` accepts (#14087). + * + * What this template used to write refused to load: a top-level `trigger: + * { type, object, events }` block, nodes carrying `name`/`next`, and no + * `edges`. `FlowSchema` is `.strict()` and declares none of that, so the + * FIRST flow anybody scaffolded was a file their own `os validate` + * rejected — with an error enumerating what is allowed rather than saying + * where the trigger had moved to. + * + * The binding lives on the START node's `config`, which is where + * `AutomationEngine.resolveTriggerBinding` reads it from: `objectName`, + * one `record-*` `triggerType` token, and an optional bare-CEL + * `condition`. `triggerType` is NOT judged by the schema — a node `config` + * is an open slot (ADR-0018) — so the token's grammar is held by + * `validate-flow-trigger-readiness`, an author-time rule `os validate` + * gates on. `generate-scaffold-validates.test.ts` puts this output through + * both layers, which is the drift this template is not allowed to repeat. + * + * `status` stays `'draft'`: the scaffold fixes the SHAPE and leaves the + * arming decision to the author (`os validate` says so — draft flows do + * fire, so declare `'active'` to arm deliberately). + */ generate: (name: string) => `import * as Automation from '@objectstack/spec/automation'; /** @@ -121,20 +144,32 @@ export default ${toCamelCase(name)}Action; const ${toCamelCase(name)}Flow: Automation.Flow = { name: '${toSnakeCase(name)}_flow', label: '${toTitleCase(name)} Flow', - type: 'autolaunched', + type: 'record_change', status: 'draft', - trigger: { - type: 'record_change', - object: '${toSnakeCase(name)}', - events: ['after_insert', 'after_update'], - }, nodes: [ { id: 'start', type: 'start', - name: 'Start', - next: 'end', + label: 'Start', + // A record-change flow binds its trigger HERE, on the START node's + // config — there is no top-level \`trigger\` key. + // objectName the object whose writes fire this flow + // triggerType one record-{before,after}-{create,update,delete,write} + // token ('write' is create OR update, in one flow) + // condition optional bare-CEL gate, e.g. 'record.amount >= 500' + config: { + objectName: '${toSnakeCase(name)}', + triggerType: 'record-after-write', + }, }, + { + id: 'end', + type: 'end', + label: 'End', + }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'end', type: 'default' }, ], }; @@ -257,15 +292,27 @@ export default ${toCamelCase(name)}Skill; }; /** - * Every metadata type `os generate` can scaffold, with the directory it - * scaffolds into — derived from `GENERATORS` rather than restated. + * Every metadata type `os generate` can scaffold — the directory it scaffolds + * into, and the source it writes — derived from `GENERATORS` rather than + * restated. * - * Exported for `generate-file-name-registry-parity.test.ts`, and derived on - * purpose: the pin's job is to hold for the NEXT generator somebody adds, and - * a hand-kept list would leave that one unmeasured while still reading green. + * Exported for `generate-file-name-registry-parity.test.ts` (which reads + * `type` / `defaultDir`) and `generate-scaffold-validates.test.ts` (which + * reads `generate` to materialize each scaffold and put it through the schema + * `os validate` parses it with). Derived on purpose: each pin's job is to hold + * for the NEXT generator somebody adds, and a hand-kept list would leave that + * one unmeasured while still reading green. */ -export const GENERATOR_SCAFFOLD_TARGETS: readonly { type: string; defaultDir: string }[] = - Object.entries(GENERATORS).map(([type, gen]) => ({ type, defaultDir: gen.defaultDir })); +export const GENERATOR_SCAFFOLD_TARGETS: readonly { + type: string; + defaultDir: string; + generate: (name: string) => string; +}[] = + Object.entries(GENERATORS).map(([type, gen]) => ({ + type, + defaultDir: gen.defaultDir, + generate: gen.generate, + })); // ─── Retired Generators ───────────────────────────────────────────── diff --git a/packages/cli/test/generate-scaffold-validates.test.ts b/packages/cli/test/generate-scaffold-validates.test.ts new file mode 100644 index 0000000000..5d278f3f13 --- /dev/null +++ b/packages/cli/test/generate-scaffold-validates.test.ts @@ -0,0 +1,265 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#14087) — what `os generate` writes, `os validate` accepts. + * + * ## The defect + * + * The `flow` scaffold emitted a shape `FlowSchema` refuses. Measured on + * `origin/main` d63c8a2, `os g flow probe_thing` produced four refusals in one + * parse: + * + * flows[0].nodes[0].label invalid_type — expected string, received undefined + * flows[0].nodes[0] unrecognized_keys — `name`, `next` + * flows[0].edges invalid_type — expected array, received undefined + * flows[0] unrecognized_keys — `trigger` + * + * A record-change flow binds its trigger on the START node's `config` + * (`{ objectName, triggerType, condition }`) — the same place + * `AutomationEngine.resolveTriggerBinding` reads it from. There is no + * top-level `trigger` key and never was one on protocol 17. So a newcomer's + * FIRST flow was a file their own toolchain refused. + * + * ## Why the test loads the scaffold the way `os validate` loads it + * + * A scaffold is TypeScript, and `os validate` does not read it as text: it + * hands the authored source to `bundle-require` (`loadConfig`, + * `packages/cli/src/utils/config.ts`) and validates the RUNTIME object that + * comes back. So this file materializes each scaffold through that same + * loader, with that same `external` list, and then re-runs the two steps + * `Validate.run()` performs on the result: + * + * step 2 — `normalizeStackInput` → the unknown-key lints → `ObjectStackDefinitionSchema.safeParse` + * step 3 — `runAuthoringRules('validate')`, gating on the error-severity half + * + * Both steps are load-bearing, and step 3 is the half a schema-only assertion + * would miss. A flow node's `config` is an OPEN slot by design (ADR-0018), so + * `FlowSchema` cannot judge the trigger vocabulary at all: a start node + * carrying `triggerType: 'record_change'` (the engine routes only `record-*`) + * parses green and is caught one layer later, by + * `validate-flow-trigger-readiness`. Asserting the schema alone would let the + * scaffold's own trigger token drift back to a spelling that never fires. + * + * ## The roster is derived, and so is each artifact's stack slot + * + * `GENERATOR_SCAFFOLD_TARGETS` is built from `GENERATORS` itself, and the + * collection each artifact lands in comes from `singularToPlural` — the map + * `defineStack` and the metadata registry already share. Nothing here restates + * either, so a generator added tomorrow is measured by this file on the day it + * lands rather than the day someone remembers to extend a hand-kept list. + * + * ## The ledger, and why this card did not empty it + * + * Running the roster is how it emerged that `flow` is not the only scaffold + * `os validate` refuses. Measured on the same commit, same harness: + * + * object parses, then FAILS the author-time rules — `security-owd-unset` + * view `views[0].list.pageSize`, and `type` / `objectName` on the container + * action `type: 'custom'` is not an Action type; `handler` is not an Action key + * app `navigation` takes an array, the scaffold writes an object + * dashboard clean + * skill clean + * + * Those four are a separate card by triage's own fence — a census of the other + * artifacts is explicitly NOT folded into #14087 — so this file RECORDS them + * instead of fixing them, in the shrink-only shape this repo uses elsewhere + * (`KNOWN_UNALIASED_TEST_IMPORTS`, the type-check debt ledger). Two properties + * follow, and both are asserted below: + * + * - a kind NOT in the ledger must validate clean. That is the pin. + * - a kind IN the ledger must still FAIL. So whoever repairs one of them + * turns this file red and deletes its entry in the same PR; the ledger + * cannot quietly outlive the defect it records, and it can never grow to + * cover a regression (a newly-broken kind is not in it, so it just fails). + * + * `flow` is additionally asserted to be absent from the ledger, so this card's + * own defect cannot be re-admitted by adding a line to a table. + */ + +import { afterAll, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { bundleRequire } from 'bundle-require'; +import { + ObjectStackDefinitionSchema, + normalizeStackInput, + lintUnknownStackKeys, + lintUnknownAuthoringKeys, +} from '@objectstack/spec'; +import { singularToPlural } from '@objectstack/spec/shared'; +import { runAuthoringRules, splitBySeverity } from '@objectstack/lint'; +import { GENERATOR_SCAFFOLD_TARGETS } from '../src/commands/generate.js'; +import { BUNDLE_REQUIRE_EXTERNALS } from '../src/utils/config.js'; + +/** + * Scaffolds `os validate` still refuses, with the measured reason. SHRINK-ONLY + * — see the header. Adding an entry to silence a failure is the one edit this + * table must never receive; the assertions below make a stale entry fail too. + */ +const KNOWN_UNVALIDATED_SCAFFOLDS: Record = { + object: + 'parses, then fails the author-time rules: `security-owd-unset` (no sharingModel authored).', + view: + 'unrecognized `pageSize` on the list view, and `type` / `objectName` on the view container.', + action: + "`type: 'custom'` is not an Action type, and `handler` is not an Action key.", + app: + '`navigation` takes an array of nav items; the scaffold writes a `{ type, items }` object.', +}; + +/** The name `os g ` is invoked with throughout this file. */ +const STEM = 'probe_thing'; + +/** + * Where materialized scaffolds are written. + * + * Inside the package's own `node_modules` on purpose, and both halves matter: + * it is git-ignored (a materialized scaffold is a build artifact, not a + * fixture), and it sits under `packages/cli`, so a scaffold's own + * `import … from '@objectstack/spec/…'` resolves from there exactly as it + * would for a file the author had scaffolded into this package — which is the + * resolution `bundle-require` performs for any specifier kept `external`. + */ +const TMP_ROOT = fs.mkdtempSync( + path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'node_modules', '.scaffold-validate-'), +); + +afterAll(() => { + fs.rmSync(TMP_ROOT, { recursive: true, force: true }); +}); + +/** A legal, minimal host stack. Only the collection under test is populated. */ +const hostStack = (collection: string, artifact: unknown) => ({ + manifest: { + id: 'com.example.scaffold', + name: 'scaffold', + version: '1.0.0', + type: 'app' as const, + namespace: 'scaffold', + }, + [collection]: [artifact], +}); + +/** + * Load a scaffold the way `os validate` loads authored TypeScript, then run + * the two steps `Validate.run()` runs on it. + */ +async function validateScaffold(type: string, source: string) { + const file = path.join(TMP_ROOT, `${type}.scaffold.ts`); + fs.writeFileSync(file, source, 'utf8'); + + const { mod } = await bundleRequire({ filepath: file, external: BUNDLE_REQUIRE_EXTERNALS }); + const artifact = (mod as { default?: unknown }).default ?? mod; + + const normalized = normalizeStackInput( + hostStack(singularToPlural(type), artifact) as Record, + ) as Record; + + const unknownKeys = [ + ...lintUnknownStackKeys(normalized, ObjectStackDefinitionSchema), + ...lintUnknownAuthoringKeys(normalized, ObjectStackDefinitionSchema), + ]; + const parsed = ObjectStackDefinitionSchema.safeParse(normalized); + if (!parsed.success) { + return { artifact, unknownKeys, parsed, ruleErrors: null, advisories: null }; + } + + const findings = runAuthoringRules('validate', { + normalized, + parsed: parsed.data as Record, + }); + const { errors, advisories } = splitBySeverity(findings); + return { artifact, unknownKeys, parsed, ruleErrors: errors, advisories }; +} + +/** Everything `os validate` would refuse this artifact for, as one string. */ +const refusals = (r: Awaited>): string[] => [ + ...r.unknownKeys.map((k) => `unknown-key ${JSON.stringify(k)}`), + ...(r.parsed.success + ? [] + : r.parsed.error.issues.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`)), + ...(r.ruleErrors ?? []).map((f) => `${f.rule} at ${f.path}: ${f.message}`), +]; + +describe('[#14087] every `os generate` scaffold passes `os validate`', () => { + it('has generators to measure at all', () => { + // Guards every `it.each` below against silently iterating nothing if the + // export ever stops being derived from `GENERATORS`. + expect(GENERATOR_SCAFFOLD_TARGETS.length).toBeGreaterThan(0); + }); + + it('the ledger names only real generator types', () => { + const roster = new Set(GENERATOR_SCAFFOLD_TARGETS.map((t) => t.type)); + for (const type of Object.keys(KNOWN_UNVALIDATED_SCAFFOLDS)) { + expect(roster.has(type), `ledger entry '${type}' is not a generator type`).toBe(true); + } + }); + + it("`flow` is not in the ledger — this card's own defect cannot be re-admitted", () => { + expect(Object.keys(KNOWN_UNVALIDATED_SCAFFOLDS)).not.toContain('flow'); + }); + + const clean = GENERATOR_SCAFFOLD_TARGETS.filter((t) => !(t.type in KNOWN_UNVALIDATED_SCAFFOLDS)); + const known = GENERATOR_SCAFFOLD_TARGETS.filter((t) => t.type in KNOWN_UNVALIDATED_SCAFFOLDS); + + it.each(clean)('`os g $type` writes a stack `os validate` accepts', async (target) => { + const result = await validateScaffold(target.type, target.generate(STEM)); + expect(refusals(result), `os validate refuses the ${target.type} scaffold`).toEqual([]); + }); + + it.each(clean)('`os g $type` writes no key any layer drops silently', async (target) => { + // The pre-parse lint, run for the reason `validate.ts` runs it there: the + // parse is what strips an undeclared key, so a surface that is not + // `.strict()` reports here and nowhere else. + const result = await validateScaffold(target.type, target.generate(STEM)); + expect(result.unknownKeys).toEqual([]); + }); + + it.each(known)( + '`os g $type` is still refused — delete its ledger entry when you fix it', + async (target) => { + const result = await validateScaffold(target.type, target.generate(STEM)); + expect( + refusals(result), + `the ${target.type} scaffold now validates clean. Delete its ` + + `KNOWN_UNVALIDATED_SCAFFOLDS entry in the same PR that fixed it.`, + ).not.toEqual([]); + }, + ); +}); + +describe('[#14087] the flow scaffold binds its trigger where the engine reads it', () => { + it('declares the binding on the START node config, not at the flow top level', async () => { + const target = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === 'flow'); + expect(target, 'the flow generator must exist').toBeDefined(); + + const flow = (await validateScaffold('flow', target!.generate(STEM))).artifact as { + trigger?: unknown; + object?: unknown; + nodes: { id: string; type: string; label?: string; config?: Record }[]; + edges: unknown[]; + }; + + // The two keys the refusal named. Asserted on the artifact rather than + // inferred from the parse, because `.strict()` only fails while nothing + // ELSE about the flow changes — this says the keys are gone for good. + expect(flow.trigger).toBeUndefined(); + expect(flow.object).toBeUndefined(); + + const start = flow.nodes.find((n) => n.type === 'start'); + expect(start, 'a record-change flow needs a START node to bind on').toBeDefined(); + // `resolveTriggerBinding` claims a record-change flow only for a token + // starting with `record-`, and `validate-flow-trigger-readiness` gates the + // grammar; both read exactly these two keys off `start.config`. + expect(start!.config?.objectName).toBe(STEM); + expect(String(start!.config?.triggerType)).toMatch( + /^record-(?:before|after)-(?:create|insert|update|delete|write)$/, + ); + + // Every node labelled, and the graph declared — the other three refusals. + for (const node of flow.nodes) expect(typeof node.label).toBe('string'); + expect(Array.isArray(flow.edges)).toBe(true); + expect(flow.edges.length).toBeGreaterThan(0); + }); +}); From c19eb733102eae66df9b6630e330631caebd3c5b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:38:37 +0000 Subject: [PATCH 2/2] docs(cli): name the card that owns the scaffold-validates ledger (#14087) The four `KNOWN_UNVALIDATED_SCAFFOLDS` entries were measured while implementing this card and filed as #14336; the ledger now says so, so whoever repairs one of those templates has a route from the entry to the card rather than only to the failure. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --- packages/cli/test/generate-scaffold-validates.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/generate-scaffold-validates.test.ts b/packages/cli/test/generate-scaffold-validates.test.ts index 5d278f3f13..f9ab230ecc 100644 --- a/packages/cli/test/generate-scaffold-validates.test.ts +++ b/packages/cli/test/generate-scaffold-validates.test.ts @@ -61,8 +61,9 @@ * skill clean * * Those four are a separate card by triage's own fence — a census of the other - * artifacts is explicitly NOT folded into #14087 — so this file RECORDS them - * instead of fixing them, in the shrink-only shape this repo uses elsewhere + * artifacts is explicitly NOT folded into #14087 — and are filed as #14336, so + * this file RECORDS them instead of fixing them, in the shrink-only shape this + * repo uses elsewhere * (`KNOWN_UNALIASED_TEST_IMPORTS`, the type-check debt ledger). Two properties * follow, and both are asserted below: * @@ -96,6 +97,8 @@ import { BUNDLE_REQUIRE_EXTERNALS } from '../src/utils/config.js'; * Scaffolds `os validate` still refuses, with the measured reason. SHRINK-ONLY * — see the header. Adding an entry to silence a failure is the one edit this * table must never receive; the assertions below make a stale entry fail too. + * + * All four are #14336. Repair the template, delete the line, same PR. */ const KNOWN_UNVALIDATED_SCAFFOLDS: Record = { object: