Skip to content

A designer configSchema and the keys its executor actually reads are still unreconciled — notify honours cfg.source, which no schema declares #4045

Description

@os-zhuang

Follow-up to #4027, which closed only the expression subset of that gap (#4040).

What #4040 fixed, and what it didn't

#4027 described two hand-written lists nobody reconciled: a node type's designer configSchema, and what the rest of the platform does with the keys it declares. #4040 built a ledger + ratchet for the expression slots — every xExpression property a builtin declares is now validated and cannot silently go unchecked again.

The other half is untouched. A configSchema is written by hand next to an executor that reads config by hand, and nothing compares them. A property can be:

  • declared but never read — an author fills in a Studio field that does nothing (the visibleWhen shape, now covered for expressions only);
  • read but never declared — the executor honours a key the designer never offers, so it is reachable only by hand-authored metadata.

The concrete instance found while doing #4040

notify's executor reads cfg.source as a nested { object, id } shape:

// packages/services/service-automation/src/builtin/notify-node.ts:92-93
const src = (cfg.source ?? null) as { object?: unknown; id?: unknown } | null;
const object = toStr(interpolate(cfg.sourceObject ?? src?.object, variables, context));

Its configSchema declares recipients, title, message, channels, topic, severity, sourceObject, sourceId, actorId, actionUrl, payloadno source. So:

  1. An author using Studio's generated form can never set the nested shape.
  2. Metadata that does carry source works at run time, with no schema describing it and no test pinning it.
  3. The cfg.sourceObject ?? src?.object line is a consumer-side fallback of exactly the kind Prime Directive Add comprehensive test suite for Zod schema validation #12 calls debt — a second de-facto contract next to the declared one. If source is legacy, it belongs in the ADR-0087 D2 conversion layer (like flow-node-notify-config-aliases already does for to/subject/body/url) so it is declared, tested and removable on a schedule. If it is current, it belongs in the configSchema.

Either way the current state is the one #4027 was filed about: two lists, no reconciliation, drift invisible until someone trips on it.

Scope caveat — this is one instance, not an audit

I only checked the four executors whose config reads were easy to enumerate (crud, screen, notify, http) while working on #4040. notify.source is what that turned up; crud and http looked consistent, and I did not check connector_action, loop, map, parallel, try_catch, subflow, wait or script. The remaining node types have the same structure, so more drift is likely.

Two non-findings worth recording so nobody re-chases them: config.expand appears in crud-nodes.ts only inside an error-message string (it is a start-node key, and start is structural with no descriptor), and update_record / delete_record neither declare nor read outputVariable — consistent, not drift.

Proposed

  1. Decide notify.source — conversion-layer alias, or declared property. Not something to guess at; it changes the designer form.
  2. A declared-vs-read ratchet. Harder than the expression one, because "what the executor reads" is not machine-readable today. Options, roughly in increasing cost:
  3. Whichever is chosen, log/report what is deliberately designer-only or read-only, rather than leaving the exemption implicit.

Related

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions