diff --git a/.changeset/flow-template-paths-into-reference-integrity-suite.md b/.changeset/flow-template-paths-into-reference-integrity-suite.md new file mode 100644 index 0000000000..1baec9a2e7 --- /dev/null +++ b/.changeset/flow-template-paths-into-reference-integrity-suite.md @@ -0,0 +1,43 @@ +--- +"@objectstack/lint": minor +"@objectstack/cli": patch +--- + +fix(lint,cli): the flow-template-path rule reaches `os lint` and `os compile`, not just `os validate` (#3583, #3810) + +`validateFlowTemplatePaths` was wired by hand into `os validate` and nowhere +else. That is precisely the drift `REFERENCE_INTEGRITY_RULES` exists to end +(#3583 §5 D5): the same stack, checked by a different rule subset depending on +which command the author happened to run. + +It mattered more after #3861 gave the rule a gating severity. A `{record.}` +token in a CRUD node's `filter` that names an unknown field — or hops through an +un-expanded relation — makes the runtime **refuse the node** (#3810). `os +validate` failed on it; `os lint` and `os compile` did not look, so a CI job +running either one would build and ship a flow that cannot execute. + +**The rule is now a suite member.** It belongs by the suite's own admission +criterion: a `{record.}` token is a name written in metadata, resolved +against the bound object's declared fields. One line in +`REFERENCE_INTEGRITY_RULES` reaches all three commands, and the hand-wiring in +`validate.ts` is deleted rather than duplicated. + +Before landing this, the rule was run against all three stack shapes the suite +is handed — raw `config` (`os lint`), `normalizeStackInput` output, and +schema-parsed `result.data` (`os validate` / `os compile`) — across `app-todo`, +`app-crm` and `app-showcase`. All three agree finding-for-finding, so moving the +call site does not change what is reported. + +Verified end-to-end on `app-showcase`: all three commands pass unchanged on the +real stack (the four pre-existing lookup-traversal warnings still print, still +advisory), and with one filter token corrupted to `{record.idd}` **all three now +exit 1** — where previously only `validate` did. + +**Also fixed, in the same file.** On a clean run, `os validate --json` never +reported the reference-integrity suite's warnings: `refWarnings` was assembled, +printed to the console, and included in the *failure* payload, but omitted from +the success-path `warnings` array. Adding the rule to the suite would have +silently dropped its warnings from `--json` for JSON consumers, so `refWarnings` +now appears there — which also surfaces the other five rules' warnings that were +being discarded. Same shape of bug as the dropped errors #3861 fixed: computed, +then thrown away. diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 720217874c..ec9e3a536f 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -21,7 +21,6 @@ import { validateCapabilityReferences } from '@objectstack/lint'; import { validateVisibilityPredicates } from '@objectstack/lint'; import { validateSecurityPosture, validateOrgAxisRedLines } from '@objectstack/lint'; import { validateFlowTriggerReadiness } from '@objectstack/lint'; -import { validateFlowTemplatePaths } from '@objectstack/lint'; import { validateReadonlyFlowWrites } from '@objectstack/lint'; import { lintFlowPatterns } from '../utils/lint-flow-patterns.js'; import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js'; @@ -515,48 +514,13 @@ export default class Validate extends Command { } } - // 3g-bis. Flow template path references (#3426): a `{record.}` token - // in a node template that names an unknown field, or hops through a - // lookup relation the seeded record carries only as a scalar id, both - // render a SILENT empty string at runtime. Advisory in most positions: - // the head object may come from another package (skipped there), and - // the runtime still produces output (a blank), so nothing is fully - // broken. GATING inside a filter-guarded CRUD node's `filter`: since - // #3810 an unresolved token there does not blank a value, it deletes - // the condition — which WIDENS the query — so the node refuses to - // execute. The build must not ship a flow whose runtime is already - // decided. - if (!flags.json) printStep('Checking flow template references...'); - const flowTemplateFindings = validateFlowTemplatePaths(normalized as Record); - const flowTemplateErrors = flowTemplateFindings.filter((f) => f.severity === 'error'); - const flowTemplateWarnings = flowTemplateFindings.filter((f) => f.severity === 'warning'); - if (flowTemplateErrors.length > 0) { - if (flags.json) { - await emitJson({ - valid: false, - errors: flowTemplateErrors, - warnings: flowTemplateWarnings, - duration: timer.elapsed(), - }); - this.exit(1); - } - console.log(''); - printError( - `Flow template reference check failed (${flowTemplateErrors.length} issue${flowTemplateErrors.length > 1 ? 's' : ''})`, - ); - for (const f of flowTemplateErrors.slice(0, 50)) { - console.log(` • ${f.where}: ${f.message}`); - console.log(chalk.dim(` ${f.hint}`)); - console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`)); - } - this.exit(1); - } - if (!flags.json) { - for (const w of flowTemplateWarnings.slice(0, 50)) { - console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`)); - console.log(chalk.dim(` ${w.hint}`)); - } - } + // 3g-bis. Flow template path references (#3426) used to be checked here by + // hand. It is a reference rule — a `{record.}` token resolved + // against the bound object's declared fields — so it now runs as a + // member of REFERENCE_INTEGRITY_RULES in step 3 above, which reaches + // `os lint` and `os compile` at the same time. Those two accepted a + // flow whose filter token the runtime refuses (#3810) for as long as + // this call site was the only one. // 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record // that writes a static-`readonly` field is a SILENT no-op — the engine @@ -762,7 +726,13 @@ export default class Validate extends Command { valid: true, manifest: config.manifest, stats, - warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...securityAdvisories, ...capProviderWarnings], + // `refWarnings` carries the whole reference-integrity suite, which now + // includes the flow-template-path rule this list used to name directly. + // It was absent here before: on a CLEAN run `--json` reported none of + // the suite's warnings, though the failure path (above) and the console + // both did. Same shape of bug as the dropped errors — computed, then + // discarded — so it is fixed rather than reproduced under a new name. + warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...refWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...securityAdvisories, ...capProviderWarnings], conversions: conversionNotices, specVersionGap: specGap, duration: timer.elapsed(), diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts index c9d42a78ac..740b12b59a 100644 --- a/packages/lint/src/reference-integrity-suite.test.ts +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -21,6 +21,7 @@ describe('reference-integrity suite — membership', () => { 'validateChartBindings', 'validateNavAccess', 'validateTranslationReferences', + 'validateFlowTemplatePaths', ]); }); @@ -106,6 +107,25 @@ describe('reference-integrity suite — every member actually runs', () => { // validateTranslationReferences: a field the object does not declare. { en: { objects: { crm_lead: { label: 'Lead', fields: { assigned_to: { label: 'Owner' } } } } } }, ], + flows: [ + { + name: 'lead_followup', + type: 'record_change', + nodes: [ + { id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } }, + // validateFlowTemplatePaths: `budget` is not a field on crm_lead. In a + // FILTER position an erased condition widens the query rather than + // narrowing it, so the runtime refuses the node — gating, not advisory + // (#3810). This member is the suite's only source of `error` findings + // from a flow, so it also pins that both severities survive the suite. + { + id: 'fetch', + type: 'get_record', + config: { objectName: 'crm_lead', filter: { name: '{record.budget}' } }, + }, + ], + }, + ], }; it('reports at least one finding from every member', () => { @@ -118,6 +138,15 @@ describe('reference-integrity suite — every member actually runs', () => { expect(rules).toContain('chart-measure-unknown'); expect(rules).toContain('nav-object-ungranted'); expect(rules).toContain('translation-target-unknown'); + expect(rules).toContain('flow-template-unknown-field'); + }); + + it('carries a gating flow-template finding through the suite (#3810)', () => { + const findings = validateReferenceIntegrity(stack); + const flow = findings.find((f) => f.rule === 'flow-template-unknown-field'); + // A filter-position miss must reach the CLI as an ERROR, or `os lint` and + // `os compile` would print a yellow line and ship a flow that cannot run. + expect(flow?.severity).toBe('error'); }); it('concatenates in list order and carries the common finding shape', () => { @@ -131,9 +160,9 @@ describe('reference-integrity suite — every member actually runs', () => { expect(typeof f.message).toBe('string'); expect(typeof f.hint).toBe('string'); } - // Object references run first, translations last. + // Object references run first, flow template paths last. expect(findings[0].rule).toBe('object-reference-unknown'); - expect(findings[findings.length - 1].rule).toBe('translation-target-unknown'); + expect(findings[findings.length - 1].rule).toBe('flow-template-unknown-field'); }); it('returns nothing for an empty stack', () => { diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index d934fb884c..38e05df4a8 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -24,6 +24,14 @@ * shipping broken: every instance parsed, validated, and failed silently at * runtime because nothing checked that the name pointed at anything. * + * `validateFlowTemplatePaths` is a member for exactly that reason: a + * `{record.}` token is a field name written in metadata, resolved + * against the bound object's declared fields. It was wired by hand into + * `os validate` alone — the drift this suite exists to end — so `os lint` and + * `os compile` accepted a flow the runtime refuses. Its findings carry BOTH + * severities (see that module: a filter-position miss gates, every other + * position advises), which is why the suite's contract is severity-agnostic. + * * Rules that check SHAPE rather than reference (view containers, responsive * styles, seed replay safety, seed state machines, seed/security posture) stay * out — they answer a different question and have their own call sites. @@ -43,6 +51,7 @@ import { validatePageFieldBindings } from './validate-page-field-bindings.js'; import { validateChartBindings } from './validate-chart-bindings.js'; import { validateNavAccess } from './validate-nav-access.js'; import { validateTranslationReferences } from './validate-translation-references.js'; +import { validateFlowTemplatePaths } from './validate-flow-template-paths.js'; export type ReferenceIntegritySeverity = 'error' | 'warning'; @@ -84,6 +93,7 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ { name: 'validateChartBindings', run: validateChartBindings }, { name: 'validateNavAccess', run: validateNavAccess }, { name: 'validateTranslationReferences', run: validateTranslationReferences }, + { name: 'validateFlowTemplatePaths', run: validateFlowTemplatePaths }, ]; /**