fix(#3426): build-time warning for unresolvable flow templates + guard the formula re-read#3472
Merged
Conversation
…+ guard the #3426 re-read (#3426) 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: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
# Conflicts: # packages/cli/src/commands/validate.ts # packages/lint/src/index.ts
github-actions
Bot
added
size/l
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 19 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Jul 25, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 25, 2026
…e flow templates (#3475) (#3482) A record-change flow can declare `expand: ['<lookup>', …]` on its start-node config so node templates resolve `{record.<lookup>.<field>}` (e.g. `{record.account.name}` — the #3426 lookup gap). The engine re-reads the declared relations AFTER identity resolution, as the run's OWN principal (`resolveRunDataContext` honors `runAs`), and grafts the expanded objects onto the run record. So a `runAs:'user'` run reads the referenced object as the triggering USER — RLS/FLS enforced — not system- elevated, which is exactly what made expansion unsafe in the trigger's re-read (no resolved grants there). New `AutomationEngine.setRecordExpander`, bridged by the plugin to the same data engine the CRUD nodes use. Only declared keys are grafted, so bare lookup ids, `multiple` lookup arrays (#1872), and trigger-hydrated formula fields are untouched. Opt-in ⇒ zero cost otherwise; best-effort ⇒ a re-read failure leaves the record unexpanded and never breaks the flow. The `flow-template-lookup-traversal` lint warning (#3472) is suppressed once a relation is declared in `config.expand`. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 25, 2026
…ly never firing (#3481) (#3484) An array `triggerType` on a flow start node (e.g. ['record-after-create', 'record-after-delete']) — the shape an author or AI authoring pass naturally reaches for to fire on multiple events — was accepted everywhere and armed nowhere. Multi-event unions are deliberately unsupported (#3457), but nothing said so: defineFlow passed the array, the engine's `typeof === 'string'` check folded it to no trigger and misclassified the flow as manual (so it never entered the binding audit), and the flow-trigger-readiness lint used the same narrowing and produced no finding. The flow bound to nothing and never fired, with zero output at any layer — the last authoring shape still slipping past the #3427/#3472 silent-never-fire guards. Defensive fix (arrays stay unsupported; they now fail loudly): - lint (validate-flow-trigger-readiness): an array triggerType with any record-* element yields a flow-trigger-unknown-event warning at os validate time, steering to record-after-write or one flow per event. - engine (resolveTriggerBinding): route such an array to the record_change trigger — as an unmappable single token is — instead of folding to a manual flow, so it reaches the trigger's bind-time rejection and the binding audit. - trigger (record-change): the bind-time rejection detects the array shape and emits a targeted warning (names the flow, points at record-after-write and #3457) rather than the generic unknown-token line. Each new test proves red against the pre-fix code first. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-ups to #3426, after #3445 closed the formula half of
{record.<path>}template resolution. Keeps #3426 open — see the third section for the remaining lookup-expand piece and why it's deferred.1. Build-time signal for unresolvable flow templates (the issue's fallback ask)
os validatenow flags a record-change flow node whose{record.<path>}template can't resolve, turning the previous silent blank into an advisory warning. New@objectstack/lintrulevalidateFlowTemplatePaths(pure(stack) => Finding[], ADR-0019), wired intoos validateright beside the existing flow-trigger-wiring check.Two findings:
flow-template-unknown-field—{record.<x>}where<x>is neither a declared field nor a system column (a typo like{record.full_naem}).flow-template-lookup-traversal—{record.<lookup>.<field>}, a cross-object hop the seeded record carries only as a scalar id (still unsupported; see §3).Deliberately quiet to keep false positives near zero: formula fields, bare lookup ids, numeric indexes into
multiplelookups (#1872),jsonsub-paths, and system columns are not flagged, and flows bound to an object this stack doesn't define are skipped (no schema to compare against).2. Hydration re-read guards (evaluation finding on #3445)
The
trigger-record-changecomputed-field re-read from #3445 was unconditional and per-binding. Two guards keep it off the hot path:formulafield (the only thing the re-read adds;summaryis stored on write). Uses the engine's optionalgetObjectConfig; when unsure, re-reads (correctness over the optimization).HookContext(the engine passes one ctx reference to every handler for a single write). AWeakMap, so entries drop when the context is GC'd.3. Lookup expansion (
{record.account.name}) — intentionally deferred, needs a directionScoping shows the plumbing is easy (no spec/engine/automation changes:
binding.configalready carries the start-node config,findOnealready accepts anexpandmap, andexpandRelatedRecordsinfers the target from the field'sreference). But it hits a read-identity security decision: the trigger's re-read runs as an elevated system principal, so expanding a relation there would read the referenced records as system — bypassing the triggering user's RLS/FLS. For arunAs:'user'flow,{record.account.secret}in a notify template would then leak fields the user can't see. Reading the expand as the user isn't feasible in the trigger: the hook session carries onlyuserId, not the resolved grants (the engine resolves those only at flow-run time, #3356).So safe lookup expansion belongs in the automation engine's flow run (which has the user's grants), not the trigger's system re-read. That's the "decide direction first (small ADR)" call from the issue thread — deferring rather than shipping a system-read expand. The new lint rule flags the traversal as unsupported in the meantime, and the formula workaround (
{record.first_name} {record.last_name}, or a projecting formula field) still applies.Testing
@objectstack/lint: 282 tests pass (15 new forvalidateFlowTemplatePaths);tsc --noEmitclean.@objectstack/trigger-record-change: 46 tests pass (5 new for the gate + memoize, existing formula-context integration test still green);tsc --noEmitclean.@objectstack/cli:tsc --noEmitclean;os validateruns end-to-end onexamples/app-crm(new "Checking flow template references…" step present, exit 0, no crash on real metadata).Closes part of #3426 (formula-side build signal + re-read guards); the lookup-expand piece stays open pending the direction decision in §3.
🤖 Generated with Claude Code