Skip to content

Commit 169b58a

Browse files
os-zhuangclaude
andauthored
fix(lint,trigger-record-change): warn on unresolvable flow templates + guard the #3426 re-read (#3426) (#3472)
Two follow-ups to #3426, after #3445 closed the formula half. Build-time signal (the issue's fallback ask): `os validate` now flags a record-change flow node whose `{record.<path>}` template can't resolve — the previous SILENT blank becomes an advisory warning. New @objectstack/lint rule `validateFlowTemplatePaths` with two findings: flow-template-unknown-field (a typo) and flow-template-lookup-traversal (a cross-object hop still unsupported). Wired into `os validate` beside the flow-trigger-wiring check. Hydration re-read guards: the trigger-record-change computed-field re-read (#3445) is now skipped when the object declares no formula field (via the engine's optional getObjectConfig) and memoized per write on the shared HookContext, so N flows on one written record share one re-read instead of N. Lookup expansion ({record.account.name}) is intentionally NOT implemented here: scoping shows it needs a read-identity design decision — expanding in the trigger's elevated (system) re-read would bypass the triggering user's RLS/FLS on the referenced object. Left for an ADR; the lint rule flags it unsupported. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c2d9098 commit 169b58a

7 files changed

Lines changed: 756 additions & 17 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
"@objectstack/lint": patch
3+
"@objectstack/cli": patch
4+
"@objectstack/trigger-record-change": patch
5+
---
6+
7+
fix(#3426): build-time warning for unresolvable flow template paths + guard the formula re-read
8+
9+
Two follow-ups to #3426 (the formula/lookup `{record.<path>}` template gap that #3445 began closing).
10+
11+
**Build-time signal (the issue's fallback ask).** `os validate` now flags a
12+
record-change flow node whose `{record.<path>}` template cannot resolve —
13+
turning the previous SILENT blank into an advisory warning. Two cases, via the
14+
new `@objectstack/lint` rule `validateFlowTemplatePaths`:
15+
16+
- `flow-template-unknown-field``{record.<x>}` where `<x>` is neither a
17+
declared field nor a system column (a typo like `{record.full_naem}`).
18+
- `flow-template-lookup-traversal``{record.<lookup>.<field>}`, a cross-object
19+
hop the seeded record carries only as a scalar id (still unsupported; tracked
20+
on #3426).
21+
22+
Deliberately quiet: formula fields, bare lookup ids, numeric indexes into
23+
`multiple` lookups (#1872), `json` sub-paths, and system columns are NOT flagged,
24+
and flows bound to an object this stack does not define are skipped (no schema to
25+
compare against).
26+
27+
**Hydration re-read guards.** The `trigger-record-change` computed-field re-read
28+
(#3445) is now (a) skipped when the object declares no `formula` field — the only
29+
thing it adds — via the engine's optional `getObjectConfig`, and (b) memoized per
30+
write on the shared HookContext, so N flows on one written record share ONE
31+
re-read instead of N. Any uncertainty falls back to the prior unconditional
32+
re-read (correctness over the optimization).

packages/cli/src/commands/validate.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { validateCapabilityReferences } from '@objectstack/lint';
1919
import { validateVisibilityPredicates } from '@objectstack/lint';
2020
import { validateSecurityPosture } from '@objectstack/lint';
2121
import { validateFlowTriggerReadiness } from '@objectstack/lint';
22+
import { validateFlowTemplatePaths } from '@objectstack/lint';
2223
import { validateReadonlyFlowWrites } from '@objectstack/lint';
2324
import { preflightRequiredCapabilities, renderCapabilityMessage } from '../utils/capability-preflight.js';
2425
import {
@@ -428,6 +429,22 @@ export default class Validate extends Command {
428429
}
429430
}
430431

432+
// 3g-bis. Flow template path references (#3426): a `{record.<path>}` token
433+
// in a node template that names an unknown field, or hops through a
434+
// lookup relation the seeded record carries only as a scalar id, both
435+
// render a SILENT empty string at runtime. Advisory: the head object
436+
// may come from another package (skipped there), and the runtime still
437+
// produces output (a blank), so nothing is fully broken.
438+
if (!flags.json) printStep('Checking flow template references...');
439+
const flowTemplateFindings = validateFlowTemplatePaths(normalized as Record<string, unknown>);
440+
const flowTemplateWarnings = flowTemplateFindings.filter((f) => f.severity === 'warning');
441+
if (!flags.json) {
442+
for (const w of flowTemplateWarnings.slice(0, 50)) {
443+
console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`));
444+
console.log(chalk.dim(` ${w.hint}`));
445+
}
446+
}
447+
431448
// 3e2. [#3425] Readonly flow-write guardrail. A `runAs:user` update_record
432449
// that writes a static-`readonly` field is a SILENT no-op — the engine
433450
// strips it from the UPDATE payload (#2948) yet the step reports
@@ -545,7 +562,7 @@ export default class Validate extends Command {
545562
valid: true,
546563
manifest: config.manifest,
547564
stats,
548-
warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...securityAdvisories, ...capProviderWarnings],
565+
warnings: [...exprWarnings, ...widgetWarnings, ...actionRefWarnings, ...styleWarnings, ...jsxWarnings, ...capWarnings, ...flowReadinessWarnings, ...flowTemplateWarnings, ...securityAdvisories, ...capProviderWarnings],
549566
conversions: conversionNotices,
550567
specVersionGap: specGap,
551568
duration: timer.elapsed(),

packages/lint/src/index.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@ export type {
3939
FlowTriggerReadinessSeverity,
4040
} from './validate-flow-trigger-readiness.js';
4141

42+
export {
43+
validateFlowTemplatePaths,
44+
FLOW_TEMPLATE_UNKNOWN_FIELD,
45+
FLOW_TEMPLATE_LOOKUP_TRAVERSAL,
46+
} from './validate-flow-template-paths.js';
47+
export type {
48+
FlowTemplatePathFinding,
49+
FlowTemplatePathSeverity,
50+
} from './validate-flow-template-paths.js';
51+
4252
export {
4353
validateReadonlyFlowWrites,
4454
FLOW_UPDATE_READONLY_FIELD,
Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
validateFlowTemplatePaths,
6+
FLOW_TEMPLATE_UNKNOWN_FIELD,
7+
FLOW_TEMPLATE_LOOKUP_TRAVERSAL,
8+
} from './validate-flow-template-paths.js';
9+
10+
type AnyRec = Record<string, unknown>;
11+
12+
/** A crm_lead object with a scalar, a formula, a lookup, and a multi-lookup. */
13+
const LEAD_OBJECT: AnyRec = {
14+
name: 'crm_lead',
15+
fields: {
16+
first_name: { name: 'first_name', type: 'text' },
17+
last_name: { name: 'last_name', type: 'text' },
18+
company: { name: 'company', type: 'text' },
19+
full_name: { name: 'full_name', type: 'formula' },
20+
crm_account: { name: 'crm_account', type: 'lookup', reference_to: 'crm_account' },
21+
target_channels: { name: 'target_channels', type: 'lookup', reference_to: 'channel', multiple: true },
22+
payload: { name: 'payload', type: 'json' },
23+
},
24+
};
25+
26+
/** Build a record-change flow with one notify node carrying the given templates. */
27+
function flowWith(notify: AnyRec, objectName = 'crm_lead'): AnyRec {
28+
return {
29+
objects: [LEAD_OBJECT],
30+
flows: [
31+
{
32+
name: 'notify_lead',
33+
type: 'record_change',
34+
nodes: [
35+
{ id: 'start', type: 'start', config: { objectName, triggerType: 'record-created' } },
36+
{ id: 'n1', type: 'notify', notify },
37+
],
38+
},
39+
],
40+
};
41+
}
42+
43+
describe('validateFlowTemplatePaths', () => {
44+
it('flags an unknown field in a {record.<x>} template (typo)', () => {
45+
const findings = validateFlowTemplatePaths(
46+
flowWith({ title: 'New lead: {record.full_naem}', body: 'x' }),
47+
);
48+
expect(findings).toHaveLength(1);
49+
expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD);
50+
expect(findings[0].severity).toBe('warning');
51+
expect(findings[0].message).toContain('full_naem');
52+
});
53+
54+
it('flags a lookup cross-object hop {record.<lookup>.<field>}', () => {
55+
const findings = validateFlowTemplatePaths(
56+
flowWith({ title: 'From {record.crm_account.name}', body: 'x' }),
57+
);
58+
expect(findings).toHaveLength(1);
59+
expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL);
60+
expect(findings[0].message).toContain('crm_account.name');
61+
});
62+
63+
it('does NOT flag a formula field (valid, hydrated since #3445)', () => {
64+
const findings = validateFlowTemplatePaths(
65+
flowWith({ title: 'New lead: {record.full_name}', body: '{record.company}' }),
66+
);
67+
expect(findings).toHaveLength(0);
68+
});
69+
70+
it('does NOT flag a plain scalar field', () => {
71+
const findings = validateFlowTemplatePaths(
72+
flowWith({ title: '{record.first_name} {record.last_name}', body: '{record.company}' }),
73+
);
74+
expect(findings).toHaveLength(0);
75+
});
76+
77+
it('does NOT flag a bare lookup id (no sub-path)', () => {
78+
const findings = validateFlowTemplatePaths(
79+
flowWith({ title: 'acct {record.crm_account}', body: 'x' }),
80+
);
81+
expect(findings).toHaveLength(0);
82+
});
83+
84+
it('does NOT flag a numeric index into a multiple lookup (#1872)', () => {
85+
const findings = validateFlowTemplatePaths(
86+
flowWith({ title: 'ch {record.target_channels.0}', body: 'x' }),
87+
);
88+
expect(findings).toHaveLength(0);
89+
});
90+
91+
it('does NOT flag a sub-path into a json field', () => {
92+
const findings = validateFlowTemplatePaths(
93+
flowWith({ title: '{record.payload.foo}', body: 'x' }),
94+
);
95+
expect(findings).toHaveLength(0);
96+
});
97+
98+
it('does NOT flag system/audit columns', () => {
99+
const findings = validateFlowTemplatePaths(
100+
flowWith({ title: '{record.id} {record.created_at} {record.owner}', body: 'x' }),
101+
);
102+
expect(findings).toHaveLength(0);
103+
});
104+
105+
it('ignores non-record tokens (flow vars, NOW(), $User)', () => {
106+
const findings = validateFlowTemplatePaths(
107+
flowWith({ title: '{some_var.field} {NOW()} {$User.Email}', body: 'x' }),
108+
);
109+
expect(findings).toHaveLength(0);
110+
});
111+
112+
it('skips a flow whose object is not defined in this stack', () => {
113+
const findings = validateFlowTemplatePaths({
114+
objects: [LEAD_OBJECT],
115+
flows: [
116+
{
117+
name: 'external',
118+
type: 'record_change',
119+
nodes: [
120+
{ id: 'start', type: 'start', config: { objectName: 'sys_user', triggerType: 'record-created' } },
121+
{ id: 'n1', type: 'notify', notify: { title: '{record.anything.deep}', body: 'x' } },
122+
],
123+
},
124+
],
125+
});
126+
expect(findings).toHaveLength(0);
127+
});
128+
129+
it('skips non-record-triggered flows', () => {
130+
const findings = validateFlowTemplatePaths({
131+
objects: [LEAD_OBJECT],
132+
flows: [
133+
{
134+
name: 'manual',
135+
type: 'screen',
136+
nodes: [
137+
{ id: 'start', type: 'start', config: {} },
138+
{ id: 'n1', type: 'notify', notify: { title: '{record.full_naem}', body: 'x' } },
139+
],
140+
},
141+
],
142+
});
143+
expect(findings).toHaveLength(0);
144+
});
145+
146+
it('dedupes a repeated bad reference to one finding per node', () => {
147+
const findings = validateFlowTemplatePaths(
148+
flowWith({ title: '{record.full_naem}', body: 'again {record.full_naem}' }),
149+
);
150+
expect(findings).toHaveLength(1);
151+
});
152+
153+
it('resolves objectName from the typed start block too', () => {
154+
const findings = validateFlowTemplatePaths({
155+
objects: [LEAD_OBJECT],
156+
flows: [
157+
{
158+
name: 'typed_start',
159+
type: 'record_change',
160+
nodes: [
161+
{ id: 'start', type: 'start', start: { objectName: 'crm_lead', triggerType: 'record-created' } },
162+
{ id: 'n1', type: 'notify', notify: { title: '{record.crm_account.name}', body: 'x' } },
163+
],
164+
},
165+
],
166+
});
167+
expect(findings).toHaveLength(1);
168+
expect(findings[0].rule).toBe(FLOW_TEMPLATE_LOOKUP_TRAVERSAL);
169+
});
170+
171+
it('detects references in freeform config and other node types (http url)', () => {
172+
const findings = validateFlowTemplatePaths({
173+
objects: [LEAD_OBJECT],
174+
flows: [
175+
{
176+
name: 'webhook',
177+
type: 'record_change',
178+
nodes: [
179+
{ id: 'start', type: 'start', config: { objectName: 'crm_lead', triggerType: 'record-created' } },
180+
{ id: 'h1', type: 'http', http: { url: 'https://x.test/{record.full_naem}', method: 'GET' } },
181+
],
182+
},
183+
],
184+
});
185+
expect(findings).toHaveLength(1);
186+
expect(findings[0].rule).toBe(FLOW_TEMPLATE_UNKNOWN_FIELD);
187+
});
188+
189+
it('returns empty when there are no flows', () => {
190+
expect(validateFlowTemplatePaths({ objects: [LEAD_OBJECT] })).toHaveLength(0);
191+
});
192+
});

0 commit comments

Comments
 (0)