feat(automation,formula): warn on undeclared flow-node config keys (#4045) - #4059
Merged
Merged
Conversation
…4045) `FlowNodeSchema.config` is `z.record(z.unknown())`, so a misspelled or invented config key was accepted in total silence. `visibleIf` instead of `visibleWhen` registered cleanly, was never read, and the only symptom was a feature that quietly did not happen — the diagnostic vacuum that made #3528 take three passes and two wrong diagnoses. `registerFlow` now walks each node's `config` against its descriptor's `configSchema` and warns on anything undeclared, located to the exact path with the declared set listed. The walk descends where the schema declares structure and STOPS at free-form keyValue maps, whose keys are author data (`filter: { status: 'stale' }`). Descending is load-bearing, not thoroughness: `visibleWhen` is a property of `screen`'s `fields[].items`, NOT of config's top level, so a top-level-only comparison would miss the exact typo class this exists to catch. A test pins `config.fields[0].visibleIf`. Warn, never reject. An undeclared key is one of three things and this seam cannot yet tell them apart: an author typo (reject-worthy), a key the executor genuinely reads that its hand-written configSchema never declared (`notify.source` was exactly this until #4050 — rejecting those breaks working apps), or dead config. Only 4 of the 13 schema-carrying builtins have been audited for the second population, so hard-failing would gamble on the other nine. This warning is what measures the distribution; tightening is a later per-key decision, and belongs with a tombstone carrying the prescription (the `UNKNOWN_KEY_GUIDANCE` pattern). Soft-fail matches `validateNodeTypes` immediately above — same function, same channel, same reasoning. The published `configSchema` is unchanged, so no consumer sees a different shape. `@objectstack/formula` exports `nearestName`, the edit-distance helper already behind unknown-field and unknown-role suggestions, so these diagnostics share one threshold rather than each inventing its own. It is deliberately a bonus, not the mechanism: `visibleIf` → `visibleWhen` is distance 4 against a threshold of 3, so the declared set is printed ALWAYS. Loosening the shared threshold would mean confidently wrong suggestions over the hundreds of candidates the field-ref diagnostic faces; a config object declares at most a dozen keys, so listing them is cheap and complete. Both branches are tested. Measured against the repo's own apps before shipping: 30 flows across app-crm and app-showcase produce exactly ONE warning — no flood — and that one is a real finding, fixed here. `showcase_inquiry_purge`'s `get_record` carried `mode: 'records'`, which no executor reads (`if (limit && limit > 1)` selects the list read), with a comment crediting `mode` for what `limit` actually does. Left as is, our own showcase would warn on every dev boot and keep advertising a `mode` idiom an AI author would copy. Test plan: formula 265 passed, service-automation 448 passed (8 new), app-showcase `validate` + `typecheck` clean, ESLint clean on all changed files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoA8AV99Ss1RRkLLAYmDzq
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 07:39
This was referenced Jul 30, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…config with what the executors read (#4045) (#4228) Completes the declared-vs-read reconciliation across the flat builtins, after notify/http/connector in #4210. Six executor-derived Zod contracts land in automation/builtin-node-config.zod.ts (get/create/update/delete_record, screen + its field item, map), each written by READING the executor rather than transcribing the descriptor literal — so the new bidirectional ledger test is evidence, not a tautology. Contract exports only; nothing parses with them yet (step 3b, gated on the #4059 warning data). Writing them against the executors surfaced seven live capabilities that no form offered, online or offline — authorable only by hand-written metadata: get_record.fields (the projection passed into find/findOne), screen.recordId (the record `mode: 'edit'` opens — the form declared the mode but not its target), screen.fields[].options/defaultValue/placeholder (all three forwarded into the ScreenSpec the client renders, so a select field's choices could not be authored in Studio at all — the same nested repeater position as #3528), and map.indexVariable/map.input. All are declared on their descriptors now. map's undeclared `flow` alias went the other route: the executor carried `cfg.flowName ?? cfg.flow` for a spelling no schema ever described — the notify.source shape (PD #12). The bare fallback is deleted and flow-node-map-flow-alias (protocol 17, retires at 18) renames it at load, including the registerFlow rehydration seam. assignment is pinned as deliberately un-reconcilable with its reason on record: with no `assignments` wrapper its top-level config keys ARE the author's variable names, so no fixed key set describes it and a catchall Zod would reconcile vacuously. The ledger pins what CAN be pinned — the form offers exactly the canonical assignments map, and that map stays open. Every builtin publishing a configSchema is now reconciled against its executor; those publishing none each have a recorded reason. Claude-Session: https://claude.ai/code/session_01UDhMtxPLLoFaGtdpNA7xTU Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Jul 31, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…4332) * feat(automation,spec): flow executors parse() their config; undeclared config keys reject at registration (#4277) Half (a): the 12 contract-carrying builtins (CRUD quartet, screen, map, notify, http, loop/parallel/try_catch) now parse node.config against their Zod contracts before executing (builtin/parse-config.ts). A type or missing-required violation refuses the node as a guard — not routable via fault edges. Templates stay legal: string slots parse the raw config; http parses post-interpolation, the shape its executor reads. A legacy flat-graph loop (no config.body) stays exempt. LoopConfigSchema.collection widened to string|array — the executor always accepted inline arrays (map.collection already declared the union), so string-only under-declared. Half (b): registerFlow now REJECTS config keys the node type's descriptor configSchema does not declare (tightening the #4059 warning), naming the path, the declared key set, a did-you-mean, and per-key tombstones (the UNKNOWN_KEY_GUIDANCE pattern). assignment stays exempt wholesale (its top-level keys are author variable names); schemaless types and keyValue maps are unchanged. Example metadata fixed at the producer (PD #12): two showcase notify nodes missing the required recipients/title (both already failed at run time), and app-todo's dead getAll/message/buttons keys plus bare-string select options. Closes #4277 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CW8ZP3zUuC7ovSxqnN5o77 * docs(automation): flows.mdx stops claiming node config is unchecked (#4277) The docs-drift advisory on #4332 flagged flows.mdx, and it was right: the node-property table and the strict-shells callout both described config as an open record with no enforcement, which #4277 changed — undeclared keys reject at registerFlow() and the contract-carrying builtins parse their config at execute time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CW8ZP3zUuC7ovSxqnN5o77 --------- Co-authored-by: Claude <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
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.
Step 3a of #4045's staged plan, after #4050 (step 1). This is the half that changes the feedback an author gets; the rest of #4045 is internal de-duplication.
The vacuum
FlowNodeSchema.configisz.record(z.unknown()), so a misspelled or invented config key is accepted in total silence. Measured on mergedmainbefore this change:registerFlowdoes not complain. The key is never read, so there is no runtime error to trace back — the only symptom is a feature that quietly does not happen. That is the exact shape that made #3528 take three passes and two wrong diagnoses.What this adds
registerFlowwalks each node'sconfigagainst its descriptor'sconfigSchema:Two things I got wrong first, both caught by the tests
1. A top-level-only check would have missed the motivating case. My first version compared only
Object.keys(config).visibleWhenis a property ofscreen'sfields[].items, not of config's top level — sovisibleIfgot no report at all. The walk now descends where the schema declares structure (properties, anditemsof an array) and stops at free-form keyValue maps, whose keys are author data (filter: { status: 'stale' }). Both behaviours are pinned by tests.2. The did-you-mean heuristic misses this very typo.
nearestName('visibleIf', …)returns nothing: edit distance is 4 against a threshold of 3. Loosening the shared helper is the wrong fix — it also backs the unknown-field and unknown-role diagnostics, where a looser threshold means confidently wrong suggestions across hundreds of candidates. A config object declares at most a dozen keys, so the declared set is now printed always and the suggestion is a bonus for the cases it does catch (titl→title). Both branches are tested.Warn, never reject — and why that is not timidity
An undeclared key is one of three things, and this seam cannot yet tell them apart:
visibleIfnotify.source(until #4050)mode: 'records'(found below)Only 4 of the 13 schema-carrying builtins have been audited for the middle population. Hard-failing now would gamble on the other nine, and the first customer to upgrade would find the answer as
registerFlowthrowing. The warning is what measures that distribution; tightening is a later per-key decision, and belongs with a tombstone that carries the prescription (theUNKNOWN_KEY_GUIDANCEpattern inobject.zod.ts).Soft-fail matches
validateNodeTypesimmediately above it — same function, samelogger.warnchannel, same reasoning. The publishedconfigSchemais unchanged, so no consumer — designer form generation included — sees a different shape. That is what makes this step trivially revertible.Measured against our own apps before shipping
A diagnostic that floods is worse than none, so I ran it over every flow in
app-crmandapp-showcase:One warning, and it is a real finding — fixed here.
showcase_inquiry_purge'sget_recordnode carried:No executor reads
mode.if (limit && limit > 1)is what selects the list read (findvsfindOne), so the flow works because oflimit: 200— while the comment above it creditedmodefor that behaviour. Left in place, our own showcase would warn on every dev boot and keep advertising amode: 'records'idiom that does nothing, which is precisely the kind of thing an AI author copies.Scope note
Engine seam only. I said earlier this would cover
objectstack validatetoo, and that was wrong:packages/lintcannot see the descriptor registry, so doing it now needs either anAutomationEnginebooted inside the CLI with agetService(){throw}cast (registering executors as a side effect to read metadata — a smell) or splitting declaration from registration across 12register*functions. Both are worse than waiting: onceconfigSchemas move tospecin steps 2/4,packages/lintcan import them directly and the author-time seam is free. I would rather say so than quietly narrow the claim.Test plan
@objectstack/formula@objectstack/service-automationapp-showcasevalidateapp-showcasetypecheckNew coverage pins: the nested typo is located to
config.fields[0].visibleIf; the flow still registers (getFlowreturns it); every undeclared key is reported, not just the first; a fully declared config is silent; a schemaless type (decision) is silent; keyValue map keys are not flagged; and the did-you-mean fires when the distance is actually within threshold.🤖 Generated with Claude Code
https://claude.ai/code/session_01QoA8AV99Ss1RRkLLAYmDzq
Generated by Claude Code