fix(lint,cli): a filter reference that cannot resolve fails the build, not the run (#3426, #3810)

.changeset/flow-template-filter-position-severity.md

@@ -0,0 +1,50 @@
+---
+"@objectstack/lint": minor
+"@objectstack/cli": patch
+---
+
+fix(lint,cli): a filter reference that cannot resolve fails the build, not the run (#3426, #3810)
+
+`validateFlowTemplatePaths` reported every `{record.<path>}` miss as **advisory**,
+on the reasoning that an unresolved token renders a blank and the run still
+completes. Since #3810 that reasoning no longer holds in one position: inside a
+CRUD node's `filter`, an unresolved token does not blank a value, it **deletes
+the condition** — and a removed condition matches MORE rows, not fewer. Those
+nodes now refuse to execute rather than run a widened query.
+
+So the rule was warning about metadata whose runtime is already decided: `os
+validate` printed a yellow line, exited 0, and shipped a flow that cannot run.
+Severity now follows the runtime consequence, by position:
+
+- **`filter` of `get_record` / `update_record` / `delete_record` → `error`.**
+  These are the three nodes whose filter `resolveNodeFilter` guards. The finding
+  says what the runtime will do ("the node refuses to run at execution time")
+  and why the build gates rather than warns (an absent condition *widens* the
+  query). `os validate` exits 1.
+- **Every other position → `warning`, unchanged.** A message body, an `http`
+  url, an `update_record` write payload: the token still renders a blank, the
+  run still completes, and the head object may legitimately come from another
+  installed package. `create_record` is deliberately excluded from the gating
+  set — it writes a payload and has no filter to widen.
+
+Both rules split this way (`flow-template-unknown-field` and
+`flow-template-lookup-traversal`), so a typo and a lookup hop are gated wherever
+the runtime refuses them. A reference used in both positions on one node is
+reported **once, at error severity**.
+
+**`os validate` now enforces it.** The command filtered this rule's findings for
+`severity === 'warning'` and dropped everything else on the floor, so an error
+from it would have been invisible. It now gates on errors first — printing rule
+id and config path, and emitting them under `errors` in `--json` — mirroring the
+`validateReadonlyFlowWrites` step directly below, which makes the same
+shift-left split (a certain runtime failure gates; a state-dependent one
+advises).
+
+Verified against the shipped examples: 33 flows across app-todo, app-crm and
+app-showcase produce **no new errors**; the four pre-existing lookup-traversal
+warnings sit in `script` / `notify` / `subflow` / `parallel` positions and keep
+their advisory severity.
+
+No authoring change is required for a correct filter. A filter that this rule
+now fails is one the runtime would have refused anyway — the difference is that
+you find out at `os validate` instead of at 3am.

packages/cli/src/commands/validate.ts

@@ -518,12 +518,39 @@ 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: the head object
-      //     may come from another package (skipped there), and the runtime still
-      //     produces output (a blank), so nothing is fully broken.
+      //     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}`));

packages/lint/src/validate-flow-template-paths.test.ts

@@ -232,4 +232,116 @@ describe('validateFlowTemplatePaths', () => {
   it('returns empty when there are no flows', () => {
     expect(validateFlowTemplatePaths({ objects: [LEAD_OBJECT] })).toHaveLength(0);
   });
+
+  // ── severity follows position (#3810) ───────────────────────────────────
+  // Outside a filter an unresolved token blanks a value (advisory); inside a
+  // filter-guarded CRUD node's filter it DELETES the condition, so the node
+  // refuses to run — the build must not ship it.
+  describe('filter-position severity (#3810)', () => {
+    /** A record-change flow with one CRUD node carrying the given config. */
+    function crudFlow(type: string, config: AnyRec): AnyRec {
+      return {
+        objects: [LEAD_OBJECT],
+        flows: [
+          {
+            name: 'crud_flow',
+            type: 'record_change',
+            nodes: [
+              { id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
+              { id: 'c1', type, config },
+            ],
+          },
+        ],
+      };
+    }
+
+    it.each(['get_record', 'update_record', 'delete_record'])(
+      'raises an unknown field inside a %s filter to error',
+      (type) => {
+        const findings = validateFlowTemplatePaths(
+          crudFlow(type, { objectName: 'crm_lead', filter: { company: '{record.compnay}' } }),
+        );
+        expect(findings).toHaveLength(1);
+        expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD);
+        expect(findings[0].severity).toBe('error');
+        expect(findings[0].message).toContain('refuses to run');
+      },
+    );
+
+    it('raises a lookup traversal inside a filter to error', () => {
+      const findings = validateFlowTemplatePaths(
+        crudFlow('delete_record', { objectName: 'crm_lead', filter: { company: '{record.crm_account.name}' } }),
+      );
+      expect(findings).toHaveLength(1);
+      expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL);
+      expect(findings[0].severity).toBe('error');
+    });
+
+    it('keeps a bad reference OUTSIDE the filter advisory on the same node', () => {
+      const findings = validateFlowTemplatePaths(
+        crudFlow('update_record', {
+          objectName: 'crm_lead',
+          filter: { company: '{record.company}' },
+          fields: { last_name: '{record.compnay}' },
+        }),
+      );
+      expect(findings).toHaveLength(1);
+      expect(findings[0].severity).toBe('warning');
+    });
+
+    it('reports a reference used in BOTH positions once, at error severity', () => {
+      const findings = validateFlowTemplatePaths(
+        crudFlow('update_record', {
+          objectName: 'crm_lead',
+          filter: { company: '{record.compnay}' },
+          fields: { last_name: 'echo {record.compnay}' },
+        }),
+      );
+      expect(findings).toHaveLength(1);
+      expect(findings[0].severity).toBe('error');
+    });
+
+    it('stays advisory for create_record, which has no filter to widen', () => {
+      const findings = validateFlowTemplatePaths(
+        crudFlow('create_record', { objectName: 'crm_lead', filter: { company: '{record.compnay}' } }),
+      );
+      expect(findings).toHaveLength(1);
+      expect(findings[0].severity).toBe('warning');
+    });
+
+    it('does NOT flag a valid filter reference', () => {
+      const findings = validateFlowTemplatePaths(
+        crudFlow('delete_record', {
+          objectName: 'crm_lead',
+          filter: { company: '{record.company}', owner: '{record.owner}' },
+        }),
+      );
+      expect(findings).toHaveLength(0);
+    });
+
+    it('respects declared expand for a filter traversal (#3475)', () => {
+      const findings = validateFlowTemplatePaths({
+        objects: [LEAD_OBJECT],
+        flows: [
+          {
+            name: 'expanded_filter',
+            type: 'record_change',
+            nodes: [
+              {
+                id: 'start',
+                type: 'start',
+                config: { objectName: 'crm_lead', triggerType: 'record-created', expand: ['crm_account'] },
+              },
+              {
+                id: 'c1',
+                type: 'get_record',
+                config: { objectName: 'crm_lead', filter: { company: '{record.crm_account.name}' } },
+              },
+            ],
+          },
+        ],
+      });
+      expect(findings).toHaveLength(0);
+    });
+  });
 });