You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found while implementing #6. Not a defect in any shipped metadata — a gap between what AGENTS.md promises and what the toolchain enforces, plus a wrong failure model in the doc. Filed unassigned.
What AGENTS.md says
Verify after every metadata change
Metadata mistakes fail silently at runtime. A bare field reference in a predicate (done instead of record.done) evaluates to null and hides the action on every record […]
Predicates are CEL — record.<field>, never bare <field>.
Both halves are measurably wrong for flow predicates. They remain correct for action / validation-rule predicates, which is presumably where they were written.
1. pnpm validate does not reject a bare reference in a flow predicate
Measured on main + the #6 branch, @objectstack/cli 17.2.0. Mutating a start-node condition from record.status == "dispatched" to status == "dispatched", confirmed on disk (injected 1, removed 0):
This is deliberate, not an oversight. packages/lint/src/validate-expressions.ts, collectBoundRecordReads:
Deliberately NEVER a bare identifier: in a flattened flow scope a bare name may be a flow variable, and a false finding here is the trust-killer ADR-0072 D1 names.
The rule that does run resolves each record.<field> / previous.<field> against the bound object, and it works well — misspelling one fails loudly and locates itself:
✗ Author-time rules failed (1 issue)
• flow 'duly_assignment_fanout' · edge 'e_collection' (fan_out→find_assigner_task) condition:
unknown field `needs_colection` on `duly_assignment` — did you mean `needs_collection`?
So the gate exists for qualified reads and is deliberately absent for bare ones. Nothing in this repo closes that gap.
2. A bare reference in a flow does not evaluate to null either
The automation engine flattens the trigger record's fields into top-level variables and spreads them into the CEL scope — from evaluateCondition:
Expose variables two ways under extra: as a vars namespace […] AND spread to top level (so bare identifiers like status / previous.status resolve — the natural authoring style for record-change start conditions).
So in a flow a bare status genuinely resolves. And when a name does not resolve, the engine throws rather than yielding false — explicitly, ADR-0032 §1c:
NO silent fallback. A non-ok result is a real fault […] instead of the old return false that made a broken condition indistinguishable from "condition not met" (#1491).
The documented "evaluates to null and silently hides the branch" failure therefore does not describe flows at all.
Why the rule is still right
Keep it. A bare name in a flow is ambiguous by construction: a declared flow variable is seeded before the record is flattened and shadows a record field of the same name, so status can silently mean two different things depending on a variables: entry elsewhere in the file. record.status cannot. The rule is sound — it just has no enforcement behind it here, and the reason to follow it is not the one the doc gives.
Suggested fix
Two independent pieces, either useful alone:
A repo-level test that walks dulyFlows, collects every predicate (node config.condition + edge condition, including region bodies — loop.config.body.edges is where half of them live), and asserts no bare reference to a field of the bound object. test/assignment-fanout.test.ts on the Assignment fan-out — one piece of work becomes N independent tasks #6 branch has a scoped version of exactly this walk that can be lifted out; today it only covers its own flow.
Correct the AGENTS.md wording so the stated failure mode matches flows (loud throw, not silent null) and so it does not claim pnpm validate catches this.
Worth doing before the remaining flow/job cards land — #2, #7 and #11 all author predicates, and each one currently ships with no gate on this.
Found while implementing #6. Not a defect in any shipped metadata — a gap between what
AGENTS.mdpromises and what the toolchain enforces, plus a wrong failure model in the doc. Filed unassigned.What
AGENTS.mdsaysBoth halves are measurably wrong for flow predicates. They remain correct for action / validation-rule predicates, which is presumably where they were written.
1.
pnpm validatedoes not reject a bare reference in a flow predicateMeasured on
main+ the #6 branch,@objectstack/cli17.2.0. Mutating a start-node condition fromrecord.status == "dispatched"tostatus == "dispatched", confirmed on disk (injected 1, removed 0):This is deliberate, not an oversight.
packages/lint/src/validate-expressions.ts,collectBoundRecordReads:The rule that does run resolves each
record.<field>/previous.<field>against the bound object, and it works well — misspelling one fails loudly and locates itself:So the gate exists for qualified reads and is deliberately absent for bare ones. Nothing in this repo closes that gap.
2. A bare reference in a flow does not evaluate to
nulleitherThe automation engine flattens the trigger record's fields into top-level variables and spreads them into the CEL scope — from
evaluateCondition:So in a flow a bare
statusgenuinely resolves. And when a name does not resolve, the engine throws rather than yielding false — explicitly, ADR-0032 §1c:The documented "evaluates to null and silently hides the branch" failure therefore does not describe flows at all.
Why the rule is still right
Keep it. A bare name in a flow is ambiguous by construction: a declared flow variable is seeded before the record is flattened and shadows a record field of the same name, so
statuscan silently mean two different things depending on avariables:entry elsewhere in the file.record.statuscannot. The rule is sound — it just has no enforcement behind it here, and the reason to follow it is not the one the doc gives.Suggested fix
Two independent pieces, either useful alone:
dulyFlows, collects every predicate (nodeconfig.condition+ edgecondition, including region bodies —loop.config.body.edgesis where half of them live), and asserts no bare reference to a field of the bound object.test/assignment-fanout.test.tson the Assignment fan-out — one piece of work becomes N independent tasks #6 branch has a scoped version of exactly this walk that can be lifted out; today it only covers its own flow.AGENTS.mdwording so the stated failure mode matches flows (loud throw, not silent null) and so it does not claimpnpm validatecatches this.Worth doing before the remaining flow/job cards land — #2, #7 and #11 all author predicates, and each one currently ships with no gate on this.