Skip to content

Commit fd7cfde

Browse files
authored
fix(lint,cli): the flow-template-path rule reaches os lint and os compile (#3583, #3810) (#3874)
`validateFlowTemplatePaths` was wired by hand into `os validate` and nowhere else — the drift `REFERENCE_INTEGRITY_RULES` exists to end (#3583 §5 D5): one stack, a different rule subset depending on which command the author ran. It mattered more once #3861 gave the rule a gating severity. A `{record.<path>}` token in a CRUD node's `filter` naming an unknown field, or hopping 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 built and shipped a flow that cannot execute. The rule is now a suite member — it meets the suite's own admission criterion, since a `{record.<field>}` token is a name resolved against the bound object's declared fields. One line reaches all three commands; the hand-wiring in `validate.ts` is deleted rather than duplicated. Checked the three stack shapes the suite is handed before moving the call site: raw `config` (lint), `normalizeStackInput` output, and schema-parsed `result.data` (validate/compile), across all three example apps. They agree finding-for-finding, so the move does not change what is reported. End-to-end on app-showcase: all three commands pass unchanged on the real stack, and with one filter token corrupted to `{record.idd}` all three exit 1 — where previously only `validate` did. Also fixed in the same file: on a clean run `os validate --json` never reported the suite's warnings. `refWarnings` was assembled, printed, and included in the FAILURE payload, but omitted from the success-path `warnings` array — so adding the rule to the suite would have silently dropped its warnings for JSON consumers. Same bug shape as the dropped errors #3861 fixed: computed, then discarded.
1 parent 0bfdf46 commit fd7cfde

4 files changed

Lines changed: 98 additions & 46 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"@objectstack/lint": minor
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(lint,cli): the flow-template-path rule reaches `os lint` and `os compile`, not just `os validate` (#3583, #3810)
7+
8+
`validateFlowTemplatePaths` was wired by hand into `os validate` and nowhere
9+
else. That is precisely the drift `REFERENCE_INTEGRITY_RULES` exists to end
10+
(#3583 §5 D5): the same stack, checked by a different rule subset depending on
11+
which command the author happened to run.
12+
13+
It mattered more after #3861 gave the rule a gating severity. A `{record.<path>}`
14+
token in a CRUD node's `filter` that names an unknown field — or hops through an
15+
un-expanded relation — makes the runtime **refuse the node** (#3810). `os
16+
validate` failed on it; `os lint` and `os compile` did not look, so a CI job
17+
running either one would build and ship a flow that cannot execute.
18+
19+
**The rule is now a suite member.** It belongs by the suite's own admission
20+
criterion: a `{record.<field>}` token is a name written in metadata, resolved
21+
against the bound object's declared fields. One line in
22+
`REFERENCE_INTEGRITY_RULES` reaches all three commands, and the hand-wiring in
23+
`validate.ts` is deleted rather than duplicated.
24+
25+
Before landing this, the rule was run against all three stack shapes the suite
26+
is handed — raw `config` (`os lint`), `normalizeStackInput` output, and
27+
schema-parsed `result.data` (`os validate` / `os compile`) — across `app-todo`,
28+
`app-crm` and `app-showcase`. All three agree finding-for-finding, so moving the
29+
call site does not change what is reported.
30+
31+
Verified end-to-end on `app-showcase`: all three commands pass unchanged on the
32+
real stack (the four pre-existing lookup-traversal warnings still print, still
33+
advisory), and with one filter token corrupted to `{record.idd}` **all three now
34+
exit 1** — where previously only `validate` did.
35+
36+
**Also fixed, in the same file.** On a clean run, `os validate --json` never
37+
reported the reference-integrity suite's warnings: `refWarnings` was assembled,
38+
printed to the console, and included in the *failure* payload, but omitted from
39+
the success-path `warnings` array. Adding the rule to the suite would have
40+
silently dropped its warnings from `--json` for JSON consumers, so `refWarnings`
41+
now appears there — which also surfaces the other five rules' warnings that were
42+
being discarded. Same shape of bug as the dropped errors #3861 fixed: computed,
43+
then thrown away.

packages/cli/src/commands/validate.ts

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import { validateCapabilityReferences } from '@objectstack/lint';
2121
import { validateVisibilityPredicates } from '@objectstack/lint';
2222
import { validateSecurityPosture, validateOrgAxisRedLines } from '@objectstack/lint';
2323
import { validateFlowTriggerReadiness } from '@objectstack/lint';
24-
import { validateFlowTemplatePaths } from '@objectstack/lint';
2524
import { validateReadonlyFlowWrites } from '@objectstack/lint';
2625
import { lintFlowPatterns } from '../utils/lint-flow-patterns.js';
2726
import { lintAutonumberFormats } from '../utils/lint-autonumber-formats.js';
@@ -515,48 +514,13 @@ export default class Validate extends Command {
515514
}
516515
}
517516

518-
// 3g-bis. Flow template path references (#3426): a `{record.<path>}` token
519-
// in a node template that names an unknown field, or hops through a
520-
// lookup relation the seeded record carries only as a scalar id, both
521-
// render a SILENT empty string at runtime. Advisory in most positions:
522-
// the head object may come from another package (skipped there), and
523-
// the runtime still produces output (a blank), so nothing is fully
524-
// broken. GATING inside a filter-guarded CRUD node's `filter`: since
525-
// #3810 an unresolved token there does not blank a value, it deletes
526-
// the condition — which WIDENS the query — so the node refuses to
527-
// execute. The build must not ship a flow whose runtime is already
528-
// decided.
529-
if (!flags.json) printStep('Checking flow template references...');
530-
const flowTemplateFindings = validateFlowTemplatePaths(normalized as Record<string, unknown>);
531-
const flowTemplateErrors = flowTemplateFindings.filter((f) => f.severity === 'error');
532-
const flowTemplateWarnings = flowTemplateFindings.filter((f) => f.severity === 'warning');
533-
if (flowTemplateErrors.length > 0) {
534-
if (flags.json) {
535-
await emitJson({
536-
valid: false,
537-
errors: flowTemplateErrors,
538-
warnings: flowTemplateWarnings,
539-
duration: timer.elapsed(),
540-
});
541-
this.exit(1);
542-
}
543-
console.log('');
544-
printError(
545-
`Flow template reference check failed (${flowTemplateErrors.length} issue${flowTemplateErrors.length > 1 ? 's' : ''})`,
546-
);
547-
for (const f of flowTemplateErrors.slice(0, 50)) {
548-
console.log(` • ${f.where}: ${f.message}`);
549-
console.log(chalk.dim(` ${f.hint}`));
550-
console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`));
551-
}
552-
this.exit(1);
553-
}
554-
if (!flags.json) {
555-
for (const w of flowTemplateWarnings.slice(0, 50)) {
556-
console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`));
557-
console.log(chalk.dim(` ${w.hint}`));
558-
}
559-
}
517+
// 3g-bis. Flow template path references (#3426) used to be checked here by
518+
// hand. It is a reference rule — a `{record.<field>}` token resolved
519+
// against the bound object's declared fields — so it now runs as a
520+
// member of REFERENCE_INTEGRITY_RULES in step 3 above, which reaches
521+
// `os lint` and `os compile` at the same time. Those two accepted a
522+
// flow whose filter token the runtime refuses (#3810) for as long as
523+
// this call site was the only one.
560524

561525
// 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record
562526
// that writes a static-`readonly` field is a SILENT no-op — the engine
@@ -762,7 +726,13 @@ export default class Validate extends Command {
762726
valid: true,
763727
manifest: config.manifest,
764728
stats,
765-
warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...securityAdvisories, ...capProviderWarnings],
729+
// `refWarnings` carries the whole reference-integrity suite, which now
730+
// includes the flow-template-path rule this list used to name directly.
731+
// It was absent here before: on a CLEAN run `--json` reported none of
732+
// the suite's warnings, though the failure path (above) and the console
733+
// both did. Same shape of bug as the dropped errors — computed, then
734+
// discarded — so it is fixed rather than reproduced under a new name.
735+
warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...refWarnings, ...readonlyWriteWarnings, ...authoringLintWarnings, ...securityAdvisories, ...capProviderWarnings],
766736
conversions: conversionNotices,
767737
specVersionGap: specGap,
768738
duration: timer.elapsed(),

packages/lint/src/reference-integrity-suite.test.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ describe('reference-integrity suite — membership', () => {
2121
'validateChartBindings',
2222
'validateNavAccess',
2323
'validateTranslationReferences',
24+
'validateFlowTemplatePaths',
2425
]);
2526
});
2627

@@ -106,6 +107,25 @@ describe('reference-integrity suite — every member actually runs', () => {
106107
// validateTranslationReferences: a field the object does not declare.
107108
{ en: { objects: { crm_lead: { label: 'Lead', fields: { assigned_to: { label: 'Owner' } } } } } },
108109
],
110+
flows: [
111+
{
112+
name: 'lead_followup',
113+
type: 'record_change',
114+
nodes: [
115+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
116+
// validateFlowTemplatePaths: `budget` is not a field on crm_lead. In a
117+
// FILTER position an erased condition widens the query rather than
118+
// narrowing it, so the runtime refuses the node — gating, not advisory
119+
// (#3810). This member is the suite's only source of `error` findings
120+
// from a flow, so it also pins that both severities survive the suite.
121+
{
122+
id: 'fetch',
123+
type: 'get_record',
124+
config: { objectName: 'crm_lead', filter: { name: '{record.budget}' } },
125+
},
126+
],
127+
},
128+
],
109129
};
110130

111131
it('reports at least one finding from every member', () => {
@@ -118,6 +138,15 @@ describe('reference-integrity suite — every member actually runs', () => {
118138
expect(rules).toContain('chart-measure-unknown');
119139
expect(rules).toContain('nav-object-ungranted');
120140
expect(rules).toContain('translation-target-unknown');
141+
expect(rules).toContain('flow-template-unknown-field');
142+
});
143+
144+
it('carries a gating flow-template finding through the suite (#3810)', () => {
145+
const findings = validateReferenceIntegrity(stack);
146+
const flow = findings.find((f) => f.rule === 'flow-template-unknown-field');
147+
// A filter-position miss must reach the CLI as an ERROR, or `os lint` and
148+
// `os compile` would print a yellow line and ship a flow that cannot run.
149+
expect(flow?.severity).toBe('error');
121150
});
122151

123152
it('concatenates in list order and carries the common finding shape', () => {
@@ -131,9 +160,9 @@ describe('reference-integrity suite — every member actually runs', () => {
131160
expect(typeof f.message).toBe('string');
132161
expect(typeof f.hint).toBe('string');
133162
}
134-
// Object references run first, translations last.
163+
// Object references run first, flow template paths last.
135164
expect(findings[0].rule).toBe('object-reference-unknown');
136-
expect(findings[findings.length - 1].rule).toBe('translation-target-unknown');
165+
expect(findings[findings.length - 1].rule).toBe('flow-template-unknown-field');
137166
});
138167

139168
it('returns nothing for an empty stack', () => {

packages/lint/src/reference-integrity-suite.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@
2424
* shipping broken: every instance parsed, validated, and failed silently at
2525
* runtime because nothing checked that the name pointed at anything.
2626
*
27+
* `validateFlowTemplatePaths` is a member for exactly that reason: a
28+
* `{record.<field>}` token is a field name written in metadata, resolved
29+
* against the bound object's declared fields. It was wired by hand into
30+
* `os validate` alone — the drift this suite exists to end — so `os lint` and
31+
* `os compile` accepted a flow the runtime refuses. Its findings carry BOTH
32+
* severities (see that module: a filter-position miss gates, every other
33+
* position advises), which is why the suite's contract is severity-agnostic.
34+
*
2735
* Rules that check SHAPE rather than reference (view containers, responsive
2836
* styles, seed replay safety, seed state machines, seed/security posture) stay
2937
* out — they answer a different question and have their own call sites.
@@ -43,6 +51,7 @@ import { validatePageFieldBindings } from './validate-page-field-bindings.js';
4351
import { validateChartBindings } from './validate-chart-bindings.js';
4452
import { validateNavAccess } from './validate-nav-access.js';
4553
import { validateTranslationReferences } from './validate-translation-references.js';
54+
import { validateFlowTemplatePaths } from './validate-flow-template-paths.js';
4655

4756
export type ReferenceIntegritySeverity = 'error' | 'warning';
4857

@@ -84,6 +93,7 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
8493
{ name: 'validateChartBindings', run: validateChartBindings },
8594
{ name: 'validateNavAccess', run: validateNavAccess },
8695
{ name: 'validateTranslationReferences', run: validateTranslationReferences },
96+
{ name: 'validateFlowTemplatePaths', run: validateFlowTemplatePaths },
8797
];
8898

8999
/**

0 commit comments

Comments
 (0)