Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .changeset/flow-template-paths-into-reference-integrity-suite.md
Original file line number Diff line number Diff line change
@@ -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.<path>}`
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.<field>}` 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.
58 changes: 14 additions & 44 deletions packages/cli/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -515,48 +514,13 @@ export default class Validate extends Command {
}
}

// 3g-bis. Flow template path references (#3426): a `{record.<path>}` 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<string, unknown>);
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.<field>}` 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
Expand Down Expand Up @@ -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(),
Expand Down
33 changes: 31 additions & 2 deletions packages/lint/src/reference-integrity-suite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ describe('reference-integrity suite — membership', () => {
'validateChartBindings',
'validateNavAccess',
'validateTranslationReferences',
'validateFlowTemplatePaths',
]);
});

Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/lint/src/reference-integrity-suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.<field>}` 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.
Expand All @@ -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';

Expand Down Expand Up @@ -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 },
];

/**
Expand Down
Loading