From e415c8ce9cba4b81a312bd3faa04571855168ef1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:04:18 +0000 Subject: [PATCH 1/9] feat(service-automation): canonicalizeStoredFlow lists every site registerFlow refuses after the parse registerFlow's registration checks (undeclared plugin config keys, decision modes, expressions and template holes, the api trigger secret) now return their refusal with its sites instead of throwing. registerFlow throws the first refusal, byte-identical, and canonicalizeStoredFlow lists every site as a TODO under 'registerFlow', so the stored preview reads such a flow as refused rather than on protocol. A site a conversion TODO already names is not listed twice. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../services/service-automation/src/engine.ts | 448 ++++++++++++++---- 1 file changed, 348 insertions(+), 100 deletions(-) diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index a597cd20f6e..46984b1903d 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -65,13 +65,13 @@ import { builtinNodeConfigKeysJudged } from '@objectstack/spec/automation'; // hand-written `trim()` here is exactly the drift the #15662 campaign built the // shared refusal to prevent. See `checkStructuralCondition` in `registerFlow`. import { EvaluatedExpressionInputSchema, EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec'; -import { applyConversionsToFlow, type ConversionNotice, type ConversionConflictNotice, type ConversionTodoNotice } from '@objectstack/spec'; +import { applyConversionsToFlow, CONVERSION_TODO_CODE, type ConversionNotice, type ConversionConflictNotice, type ConversionTodoNotice } from '@objectstack/spec'; // [ADR-0126 §7.3, #20761] "Does a code package ship this flow?" is NOT read // off a definition here. The subflow guards, the arming gate and the // activation door ask {@link AutomationEngine.packagedFlowOwner} — the loader's // set, attached by the host — because a definition reaches `registerFlow` // through authoring doors too, and its provenance stamps are the caller's. -import type { FlowRegionParsed } from '@objectstack/spec/automation'; +import type { FlowGraph, FlowRegionParsed } from '@objectstack/spec/automation'; import type { Connector, ConnectorActionEffect, @@ -2251,6 +2251,100 @@ const CONVERSIONS_NOT_REPLAYED_AT_REHYDRATION: readonly string[] = [ 'flow-decision-mode-inclusive-explicit', ]; +/** + * One site a registration check refuses (#23317): where it is, the shape + * found there, and the check's own words for it. + * + * A check does not throw: it returns a {@link FlowRegistrationRefusal}, which + * {@link AutomationEngine.registerFlow} throws and + * {@link AutomationEngine.canonicalizeStoredFlow} lists site by site. So the + * stored pass (`os migrate meta --stored`) previews exactly what registration + * refuses, through the same checks, and a check added to registration is + * previewed with no second change. + */ +interface FlowRegistrationSite { + /** + * The site's path in the stack the stored pass wraps one flow in — + * `flows[0].nodes[2].config.title`, `flows[0].edges[1].condition` — the + * spelling `applyConversionsToFlow` gives a TODO's path, so a site a + * conversion TODO already names is recognised as the same site. + */ + readonly path: string; + /** The shape found at the site, serialized: a source text, a key, a declared value. */ + readonly from: string; + /** The site's refusal on one line: where it is, then the check's message. */ + readonly detail: string; +} + +/** + * What one registration check refuses in a flow: the error `registerFlow` + * throws, byte for byte, and every site it names. + */ +interface FlowRegistrationRefusal { + readonly message: string; + readonly sites: readonly FlowRegistrationSite[]; +} + +/** + * The id a registration refusal is listed under in the stored pass — a + * `ConversionTodoNotice.conversionId` that names the refusing door, for a site + * no conversion and no D3 entry of the migration chain covers. + */ +export const FLOW_REGISTRATION_TODO_ID = 'registerFlow'; + +/** What a TODO from {@link FLOW_REGISTRATION_TODO_ID} says the site is on. */ +const FLOW_REGISTRATION_TODO_SURFACE = 'flow — a site AutomationEngine.registerFlow refuses'; + +/** + * One key the undeclared-config-key walk finds: the key, its path from the + * node (`config.fields[0].visibleIf`), and the line its refusal prints. + */ +interface UndeclaredConfigKey { + readonly key: string; + readonly at: string; + readonly line: string; +} + +/** The flow's own path in the stack the stored pass wraps it in: `{ flows: [flow] }`. */ +const STORED_FLOW_PATH = 'flows[0]'; + +/** + * The stored-pass path of one graph's container — `flows[0]` for the flow's + * own, `flows[0].nodes[1].config.body` for a loop body — built from + * {@link FlowGraph.path}, so a nested node is named where the conversion walk + * (`mapFlowNodeList`) names it. + */ +function storedGraphPath(graph: FlowGraph): string { + let out = STORED_FLOW_PATH; + for (const segment of graph.path) out += typeof segment === 'number' ? `[${segment}]` : `.${segment}`; + return out; +} + +/** The stored-pass path of one region-scoped list member, by its index in the graph. */ +function storedGraphMemberPath(graph: FlowGraph, list: 'nodes' | 'edges', index: number): string { + return `${storedGraphPath(graph)}.${list}[${index}]`; +} + +/** + * One registration refusal site as a stored-pass TODO. The fate leads and the + * check's own words follow, as a refused-shape TODO's do: the site stays as + * stored, and the flow does not register. + */ +function flowRegistrationTodo(flowName: string, site: FlowRegistrationSite): ConversionTodoNotice { + const reason = 'No conversion rewrites it, and registerFlow refuses it: left as stored, the flow is not ' + + `registered at boot and never runs. Flow \`${flowName}\`, ${site.detail}`; + return { + code: CONVERSION_TODO_CODE, + conversionId: FLOW_REGISTRATION_TODO_ID, + surface: FLOW_REGISTRATION_TODO_SURFACE, + from: site.from, + path: site.path, + reason, + message: `[protocol] left ${FLOW_REGISTRATION_TODO_SURFACE} at ${site.path} as stored — ` + + `AutomationEngine.registerFlow refuses it and no conversion rewrites it: ${reason}`, + }; +} + /** * Lift the `{ dialect, source }` envelopes the flow schema derives for edge * `condition`s back onto the conversion output — and take nothing else with @@ -4358,8 +4452,22 @@ export class AutomationEngine implements IAutomationService { * template token, a CRUD filter operand among them, which * {@link registerFlow} goes on to refuse. The parse accepts such a flow, so * without them the stored pass would read a flow the boot refuses as on - * protocol. They change nothing in `parsed` or `storable`; `registerFlow` - * ignores them, since its own refusal names the same sites. + * protocol. + * + * [#23317] `todos` also carries every site {@link registerFlow} refuses + * once the parse has accepted the flow: the same registration checks, run + * here without registering ({@link flowRegistrationRefusals}), each site + * listed under {@link FLOW_REGISTRATION_TODO_ID}. A text slot's template + * hole, a decision's `mode`, a malformed predicate, a plugin node's + * undeclared config key and an `api` flow with no secret are among them. + * A site a conversion TODO already names is not listed twice. What the + * parse itself refuses is not a TODO: this method throws it, and the + * stored pass reports that row `failed` with the parse's words. + * + * None of it changes `parsed` or `storable`, and nothing here throws for + * it, so a caller that reads no `todos` (the save door, a package + * duplication) behaves as before. `registerFlow` reads none either: it + * runs the same checks itself and throws the first refusal. */ canonicalizeStoredFlow(name: string, definition: unknown): { parsed: FlowParsed; @@ -4367,6 +4475,34 @@ export class AutomationEngine implements IAutomationService { notices: ConversionNotice[]; conflicts: ConversionConflictNotice[]; todos: ConversionTodoNotice[]; + } { + const canonical = this.canonicalizeFlow(name, definition); + // One listing per site: a site a conversion TODO names (the value-slot + // refusal the spec lists under its D3 entry) keeps that TODO, which + // names the retirement; the registration check's line for it is the + // same refusal and is not repeated. + const listed = new Set(canonical.todos.map((todo) => todo.path)); + for (const refusal of this.flowRegistrationRefusals(name, canonical.parsed)) { + for (const site of refusal.sites) { + if (listed.has(site.path)) continue; + canonical.todos.push(flowRegistrationTodo(name, site)); + } + } + return canonical; + } + + /** + * {@link canonicalizeStoredFlow}'s pass without the registration checks: + * convert, parse, and refuse what the parse, the cycle check and the + * region validator refuse. {@link registerFlow} calls this and then runs + * the checks itself, so registration judges a flow once. + */ + private canonicalizeFlow(name: string, definition: unknown): { + parsed: FlowParsed; + storable: unknown; + notices: ConversionNotice[]; + conflicts: ConversionConflictNotice[]; + todos: ConversionTodoNotice[]; } { // ADR-0087 D2 — the runtime load seam. A stored flow authored against an // old shape (a `webhook`/`http_request` callout node, a `delete_record` @@ -4451,7 +4587,7 @@ export class AutomationEngine implements IAutomationService { // One canonicalization policy, shared with the stored-row migration so // the two can never disagree about what "canonical" means (#4454). // Execution takes the parsed shape (schema defaults materialized). - const { parsed } = this.canonicalizeStoredFlow(name, definition); + const { parsed } = this.canonicalizeFlow(name, definition); // ADR-0018 §M1 — node types are validated against the live action // registry, but NOT here: the check moved to the moment the vocabulary @@ -4461,34 +4597,15 @@ export class AutomationEngine implements IAutomationService { // soft-fail — a flow authored against a currently-absent plugin must // still register, and executeNode() throws NO_EXECUTOR at run time. - // #4277 — REJECT config keys the node's descriptor does not declare - // (the tightened #4059 warning; a `visibleIf` typo used to register in - // silence). Hard-fail with per-key prescriptions: see - // validateNodeConfigKeys for why the #4045 reconciliation made this - // safe, and for the deliberate exemptions (`assignment`, schemaless - // types, keyValue maps). [#21982] Only for the types the spec does not - // judge — plugin node types (#22343: `try_catch` was the last builtin - // here); every builtin's undeclared key was refused by the - // `FlowSchema.parse` above. - this.validateNodeConfigKeys(name, parsed); - - // #15429 — parse every `decision` node's config against the spec's - // `DecisionConfigSchema` and refuse the flow on an invalid `mode`, with - // the schema's own sentence — the same answer `os validate` gives. - this.validateDecisionModes(name, parsed); - - // ADR-0032 §Decision 1a — parse-validate every predicate at registration, - // so a malformed condition (e.g. the #1491 `{record.x}` template-brace-in- - // CEL mistake) is a LOUD registration error with the offending source, - // not a silent runtime `false`. Hard-fail: a broken predicate is never - // safe to run. - this.validateFlowExpressions(name, parsed); - - // ADR-0041 — an `api` flow's inbound hook requires a per-flow secret. - // Refused here, at the publish seam, so the author learns before - // deploying; `trigger-api`'s own `start()` refuses the same binding for - // a host that binds without this engine. - this.validateApiTriggerSecret(name, parsed); + // [#23317] Every registration check, in order, through the ONE list + // the stored pass previews with ({@link registrationChecks}). The first + // refusal is thrown, so a flow is refused exactly as before; a check + // belongs in that list, never as a second call here, or the stored + // pass would read a flow this method refuses as on protocol. + for (const check of this.registrationChecks({ advisory: true })) { + const refusal = check(name, parsed); + if (refusal) throw new Error(refusal.message); + } // Version history management const history = this.flowVersionHistory.get(name) ?? []; @@ -10339,6 +10456,74 @@ export class AutomationEngine implements IAutomationService { ); } + /** + * [#23317] What {@link registerFlow} judges a canonical flow by after the + * parse — every registration check, in the order it runs them. The ONE + * list: `registerFlow` throws the first refusal, and + * {@link flowRegistrationRefusals} collects them all for the stored pass, + * so a check added here is refused at registration and previewed by + * `os migrate meta --stored` alike. + * + * `advisory` turns on the schema-aware expression pass, which logs and + * never refuses: registration runs it, the stored pass does not, so the + * doors that canonicalize without registering log nothing new. + */ + private registrationChecks( + { advisory }: { advisory: boolean }, + ): ReadonlyArray<(flowName: string, flow: FlowParsed) => FlowRegistrationRefusal | undefined> { + return [ + // #4277 — config keys the node's descriptor does not declare (the + // tightened #4059 warning; a `visibleIf` typo used to register in + // silence), with per-key prescriptions: see validateNodeConfigKeys + // for why the #4045 reconciliation made this safe, and for the + // deliberate exemptions (`assignment`, schemaless types, keyValue + // maps). [#21982] Only for the types the spec does not judge — + // plugin node types (#22343: `try_catch` was the last builtin here); + // every builtin's undeclared key was refused by `FlowSchema.parse`. + (flowName, flow) => this.validateNodeConfigKeys(flowName, flow), + // #15429 — every `decision` node's config against the spec's + // `DecisionConfigSchema`, refused on an invalid `mode` with the + // schema's own sentence — the same answer `os validate` gives. + (flowName, flow) => this.validateDecisionModes(flowName, flow), + // ADR-0032 §Decision 1a — every predicate parse-validated, so a + // malformed condition (e.g. the #1491 `{record.x}` template-brace-in- + // CEL mistake) is a LOUD registration error with the offending + // source, not a silent runtime `false`: a broken predicate is never + // safe to run. + (flowName, flow) => this.validateFlowExpressions(flowName, flow, { advisory }), + // ADR-0041 — an `api` flow's inbound hook requires a per-flow + // secret, refused at the publish seam so the author learns before + // deploying; `trigger-api`'s own `start()` refuses the same binding + // for a host that binds without this engine. + (flowName, flow) => this.validateApiTriggerSecret(flowName, flow), + ]; + } + + /** + * [#23317] Every refusal {@link registerFlow} would raise for a flow that + * has passed the parse, without registering it — what the stored pass + * lists through {@link canonicalizeStoredFlow}. + * + * Never throws. A check that throws instead of refusing still names a flow + * `registerFlow` does not register (it throws the same error there), so it + * is listed as one refusal at the flow itself: the doors that canonicalize + * without registering must not start throwing for a check they never ran. + */ + private flowRegistrationRefusals(flowName: string, flow: FlowParsed): FlowRegistrationRefusal[] { + const refusals: FlowRegistrationRefusal[] = []; + for (const check of this.registrationChecks({ advisory: false })) { + let refusal: FlowRegistrationRefusal | undefined; + try { + refusal = check(flowName, flow); + } catch (e) { + const message = e instanceof Error ? e.message : String(e); + refusal = { message, sites: [{ path: STORED_FLOW_PATH, from: JSON.stringify(flowName), detail: message }] }; + } + if (refusal) refusals.push(refusal); + } + return refusals; + } + /** * REJECT node `config` keys the node type's descriptor does not declare * (#4277 — the error half of the #4045 unknown-key ladder; #4059 shipped @@ -10402,10 +10587,12 @@ export class AutomationEngine implements IAutomationService { * Hard-fail matches {@link validateFlowExpressions} below — a flow whose * metadata is wrong never registers, and every `registerFlow` call site * already try/catches per flow, so a bad flow is skipped loudly at boot - * rather than crashing the kernel. + * rather than crashing the kernel. [#23317] The refusal is returned, not + * thrown: `registerFlow` throws it ({@link registrationChecks}). */ - private validateNodeConfigKeys(flowName: string, flow: FlowParsed): void { + private validateNodeConfigKeys(flowName: string, flow: FlowParsed): FlowRegistrationRefusal | undefined { const violations: string[] = []; + const sites: FlowRegistrationSite[] = []; // #4389 — every graph, not just the flow's own. `visibleIf` is the typo // this check exists to catch, and moving the node into a `loop` body // used to restore the silence #4277 closed. No double-reporting from the @@ -10413,22 +10600,32 @@ export class AutomationEngine implements IAutomationService { // slot as a bare `nodes: { type: 'array' }` with no `items`, so the // schema-lockstep walk stops there rather than descending twice. for (const graph of collectFlowGraphs(flow)) { - const scoped: string[] = []; - for (const node of graph.nodes) { + graph.nodes.forEach((node, index) => { // `assignment` config keys are the author's variable names — see the // exemption note above. - if (node.type === 'assignment') continue; + if (node.type === 'assignment') return; // [#21982] The spec judged this type's keys in `FlowSchema.parse`: // one judge per type, never two. - if (builtinNodeConfigKeysJudged(node.type)) continue; + if (builtinNodeConfigKeysJudged(node.type)) return; const schema = this.actionDescriptors.get(node.type)?.configSchema as ConfigSchemaNode | undefined; - if (!schema) continue; - this.collectUndeclaredConfigKeys(node, schema, node.config, 'config', scoped); - } - for (const v of scoped) violations.push(graph.scope ? `${graph.scope} · ${v}` : v); + if (!schema) return; + const found: UndeclaredConfigKey[] = []; + this.collectUndeclaredConfigKeys(node, schema, node.config, 'config', found); + for (const key of found) { + const v = graph.scope ? `${graph.scope} · ${key.line}` : key.line; + violations.push(v); + sites.push({ + path: `${storedGraphMemberPath(graph, 'nodes', index)}.${key.at}`, + from: JSON.stringify(key.key), + detail: v, + }); + } + }); } - if (violations.length > 0) { - throw new Error( + if (violations.length === 0) return undefined; + return { + sites, + message: `Flow '${flowName}' rejected: ${violations.length} undeclared config key(s).\n` + violations.map((v) => ` - ${v}`).join('\n') + `\nAn undeclared key is never read, so it can only be a typo or dead config — fix the ` + @@ -10436,8 +10633,7 @@ export class AutomationEngine implements IAutomationService { `declare it on the node type's descriptor configSchema instead: the built-in node types were ` + `reconciled so every key their executors read is declared, and a read-but-undeclared key is ` + `exactly the drift that reconciliation removed.`, - ); - } + }; } /** @@ -10469,29 +10665,38 @@ export class AutomationEngine implements IAutomationService { * Hard-fail, like {@link validateNodeConfigKeys}: a flow whose gateway * declares a mode the engine does not have is wrong metadata, and the * caller's per-flow try/catch skips it loudly at boot rather than arming a - * gateway that would run as something the author did not write. + * gateway that would run as something the author did not write. [#23317] + * The refusal is returned, not thrown: `registerFlow` throws it. */ - private validateDecisionModes(flowName: string, flow: FlowParsed): void { + private validateDecisionModes(flowName: string, flow: FlowParsed): FlowRegistrationRefusal | undefined { const failures: string[] = []; + const sites: FlowRegistrationSite[] = []; for (const graph of collectFlowGraphs(flow)) { const at = graph.scope ? `${graph.scope}: ` : ''; - for (const node of graph.nodes) { - if (node.type !== 'decision') continue; + graph.nodes.forEach((node, index) => { + if (node.type !== 'decision') return; const verdict = DecisionConfigSchema.safeParse(node.config ?? {}); - if (verdict.success) continue; + if (verdict.success) return; for (const issue of verdict.error.issues) { if (issue.path[0] !== 'mode') continue; - failures.push(` • ${at}node '${node.id}' (decision) at config.mode: ${issue.message}`); + const detail = `${at}node '${node.id}' (decision) at config.mode: ${issue.message}`; + failures.push(` • ${detail}`); + sites.push({ + path: `${storedGraphMemberPath(graph, 'nodes', index)}.config.mode`, + from: JSON.stringify((node.config as Record | undefined)?.mode ?? null), + detail, + }); } - } + }); } - if (failures.length > 0) { - throw new Error( + if (failures.length === 0) return undefined; + return { + sites, + message: `Flow '${flowName}' rejected: ${failures.length} invalid decision \`mode\` ` + `declaration${failures.length > 1 ? 's' : ''}. The config is metadata, so re-registering ` + `changes nothing; fix the node in the flow definition:\n${failures.join('\n')}`, - ); - } + }; } /** @@ -10514,31 +10719,45 @@ export class AutomationEngine implements IAutomationService { * Hard-fail, like {@link validateNodeConfigKeys}: every `registerFlow` call * site already try/catches per flow, so a refused flow is skipped loudly * at boot, and the `/automation` write doors answer the throw as `400 - * VALIDATION_FAILED`. + * VALIDATION_FAILED`. [#23317] The refusal is returned, not thrown: + * `registerFlow` throws it. */ - private validateApiTriggerSecret(flowName: string, flow: FlowParsed): void { + private validateApiTriggerSecret(flowName: string, flow: FlowParsed): FlowRegistrationRefusal | undefined { const resolved = this.deriveTriggerBinding(flowName, flow); - if (resolved?.triggerType !== 'api') return; + if (resolved?.triggerType !== 'api') return undefined; const config = (resolved.binding.config ?? {}) as Record; - if (typeof config.secret === 'string' && config.secret.trim() !== '') return; + if (typeof config.secret === 'string' && config.secret.trim() !== '') return undefined; // [#20790] …or the write-only credential channel holds it: a flow stored // through the metadata save door keeps no secret in its definition, and // its binding carries the reader instead. Only for the WITHHELD form — // `hookSecretResolver` yields no reader for a start node that writes // `secret: ''` (cleared), so that one is refused below whatever the // channel still holds. - if (resolved.binding.resolveSecret) return; + if (resolved.binding.resolveSecret) return undefined; const asks = [ flow.type === 'api' ? "`type: 'api'`" : undefined, config.triggerType === 'api' ? "start-node `config.triggerType: 'api'`" : undefined, ].filter((s): s is string => s !== undefined); - throw new Error( - `Flow '${flowName}' rejected: it binds the inbound \`api\` trigger (${asks.join(' and ')}) but its ` + + const why = + `it binds the inbound \`api\` trigger (${asks.join(' and ')}) but its ` + `start node declares no \`config.secret\`. An inbound hook is armed only with a per-flow secret that ` + `every post is HMAC-verified against (ADR-0041), so this flow could never be armed. Set a non-blank ` + `\`config.secret\` on the start node. A flow that is only ever started explicitly and never receives ` + - `inbound posts is \`type: 'autolaunched'\`, with no \`triggerType: 'api'\` on its start node.`, - ); + `inbound posts is \`type: 'autolaunched'\`, with no \`triggerType: 'api'\` on its start node.`; + // The site is the start node's `config.secret`, the key the author sets + // (the flow itself when it has no start node). What was found there is + // shown only as a blank string or `null`: a secret that is set clears + // this check, so nothing this prints can be one. + const startIndex = flow.nodes.findIndex((n) => n.type === 'start'); + const found = typeof config.secret === 'string' ? config.secret : null; + return { + message: `Flow '${flowName}' rejected: ${why}`, + sites: [{ + path: startIndex < 0 ? STORED_FLOW_PATH : `${STORED_FLOW_PATH}.nodes[${startIndex}].config.secret`, + from: JSON.stringify(found), + detail: `${startIndex < 0 ? 'the flow' : `node '${flow.nodes[startIndex]!.id}' (start) at config.secret`}: ${why}`, + }], + }; } /** @@ -10562,7 +10781,7 @@ export class AutomationEngine implements IAutomationService { schema: ConfigSchemaNode, value: unknown, path: string, - violations: string[], + violations: UndeclaredConfigKey[], ): void { if (schema.type === 'array' && schema.items) { if (!Array.isArray(value)) return; @@ -10595,13 +10814,16 @@ export class AutomationEngine implements IAutomationService { // a bonus for the cases it does catch. const suggestion = nearestName(key, declared); const guidance = FLOW_NODE_UNKNOWN_KEY_GUIDANCE[node.type]?.[key]; - violations.push( - `node '${node.id}' (${node.type}): unknown config key \`${key}\` at ${path}.${key}` + - (suggestion ? ` — did you mean \`${suggestion}\`?` : '') + - ` It is not declared by this node type's configSchema, so nothing reads it.` + - ` Declared here: ${declared.join(', ')}.` + - (guidance ? ` ${guidance}` : ''), - ); + violations.push({ + key, + at: `${path}.${key}`, + line: + `node '${node.id}' (${node.type}): unknown config key \`${key}\` at ${path}.${key}` + + (suggestion ? ` — did you mean \`${suggestion}\`?` : '') + + ` It is not declared by this node type's configSchema, so nothing reads it.` + + ` Declared here: ${declared.join(', ')}.` + + (guidance ? ` ${guidance}` : ''), + }); } } @@ -10641,10 +10863,26 @@ export class AutomationEngine implements IAutomationService { * in arithmetic) as ADVISORY warnings (logged, never thrown). Flow conditions * bind the record's fields flat, so the schema pass uses `flattened` scope. * The fatal set is unchanged — a resolver can never break a flow that used to - * register cleanly. + * register cleanly. `advisory: false` skips that pass: the stored pass + * previews refusals ({@link flowRegistrationRefusals}), and an advisory + * finding is none. + * + * [#23317] The refusal is returned, not thrown: `registerFlow` throws it. + * Each failure is recorded with its site, so the stored pass lists it + * where the author wrote it. */ - private validateFlowExpressions(flowName: string, flow: FlowParsed): void { + private validateFlowExpressions( + flowName: string, + flow: FlowParsed, + { advisory }: { advisory: boolean }, + ): FlowRegistrationRefusal | undefined { const failures: string[] = []; + const sites: FlowRegistrationSite[] = []; + /** One failure: its line in the thrown message, and its site for the stored pass. */ + const refuse = (path: string, where: string, message: string, source: unknown): void => { + failures.push(` • ${where}: ${message}\n source: \`${source}\``); + sites.push({ path, from: JSON.stringify(String(source)), detail: `${where}: ${message}` }); + }; // Resolve the flow's record-change target object (start node's // `config.objectName`) so `record.*`/bare field refs can be checked @@ -10659,19 +10897,19 @@ export class AutomationEngine implements IAutomationService { })() : undefined; - const check = (where: string, raw: unknown, useSchemaHint = true): void => { + const check = (path: string, where: string, raw: unknown, useSchemaHint = true): void => { if (raw == null) return; // Fatal pass — syntax, brace-in-CEL, unknown-function (ADR-0032 §5). // Unchanged: matches the CLI build's fatal set exactly. const result = validateExpression('predicate', raw as string | { dialect?: string; source?: string }); for (const e of result.errors) { - failures.push(` • ${where}: ${e.message}\n source: \`${e.source}\``); + refuse(path, where, e.message, e.source); } // Advisory schema-aware pass — only when the source is syntactically // valid (else it would just re-report the fatal error). Everything it // finds (field-existence, tier-3 typo, tier-4 type mismatch) is LOGGED, // never thrown, so registration behaviour is strictly additive (#1928). - if (useSchemaHint && result.errors.length === 0 && schemaHint) { + if (advisory && useSchemaHint && result.errors.length === 0 && schemaHint) { const schemaPass = validateExpression('predicate', raw as string | { dialect?: string; source?: string }, schemaHint); for (const issue of [...schemaPass.errors, ...schemaPass.warnings]) { // #6499's separately-argued 14th site — the OPPOSITE cause @@ -10754,19 +10992,19 @@ export class AutomationEngine implements IAutomationService { return { message: verdict.error.issues[0]?.message ?? EVALUATED_EXPRESSION_SOURCE_REQUIRED, source }; }; - const checkStructuralCondition = (where: string, raw: unknown): void => { + const checkStructuralCondition = (path: string, where: string, raw: unknown): void => { if (raw == null) return; const shapeRefusal = structuralConditionRefusal(raw); if (shapeRefusal) { - failures.push(` • ${where}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``); + refuse(path, where, shapeRefusal.message, shapeRefusal.source); return; } const blankRefusal = evaluatedSourceRefusal(raw); if (blankRefusal) { - failures.push(` • ${where}: ${blankRefusal.message}\n source: \`${blankRefusal.source}\``); + refuse(path, where, blankRefusal.message, blankRefusal.source); return; } - check(where, raw); + check(path, where, raw); }; // #4347 — every graph in the flow, not just the top-level arrays. An @@ -10778,10 +11016,11 @@ export class AutomationEngine implements IAutomationService { // region so a nested finding says where it is. for (const graph of collectFlowGraphs(flow)) { const at = graph.scope ? `${graph.scope}: ` : ''; - for (const node of graph.nodes) { + for (const [index, node] of graph.nodes.entries()) { + const nodePath = storedGraphMemberPath(graph, 'nodes', index); const cfg = (node.config ?? {}) as Record; // start-node trigger gate + decision/branch predicates live in config.condition - checkStructuralCondition(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition); + checkStructuralCondition(`${nodePath}.config.condition`, `${at}node '${node.id}' (${node.type}) condition`, cfg.condition); // [#19939] The value slots' OTHER form — a literal — refused where // it still spells the retired `{…}` template dialect (the C half @@ -10794,9 +11033,11 @@ export class AutomationEngine implements IAutomationService { // `assignment` node — so a flow that registers is never refused // there, and vice versa. for (const refusal of flowNodeValueTemplateRefusals(node.type, node.config)) { - failures.push( - ` • ${at}node '${node.id}' (${node.type}) ${refusal.label} at config.${refusal.path}: ` - + `${refusal.message}\n source: \`${refusal.source}\``, + refuse( + `${nodePath}.config.${refusal.path}`, + `${at}node '${node.id}' (${node.type}) ${refusal.label} at config.${refusal.path}`, + refusal.message, + refusal.source, ); } @@ -10809,14 +11050,15 @@ export class AutomationEngine implements IAutomationService { // hole holding logic or an unknown formatter stops the flow here // instead of throwing at the node mid-run. for (const slot of flowNodeTextSlotSources(node.type, node.config)) { + const slotPath = `${nodePath}.config.${slot.path}`; const slotWhere = `${at}node '${node.id}' (${node.type}) ${slot.label} at config.${slot.path}`; const tokenRefusal = textSlotTemplateRefusal(slot.source); if (tokenRefusal !== undefined) { - failures.push(` • ${slotWhere}: ${tokenRefusal}\n source: \`${slot.source}\``); + refuse(slotPath, slotWhere, tokenRefusal, slot.source); continue; } for (const e of validateExpression('template', slot.source).errors) { - failures.push(` • ${slotWhere}: ${e.message}\n source: \`${slot.source}\``); + refuse(slotPath, slotWhere, e.message, slot.source); } } @@ -10830,6 +11072,7 @@ export class AutomationEngine implements IAutomationService { // since #19939 retired the single-brace one with the `loop` / `map` // `collection` it recorded. for (const found of resolveFlowNodeExpressions(node.type, node.config)) { + const slotPath = `${nodePath}.config.${found.path}`; const slotWhere = `${at}node '${node.id}' (${node.type}) ${found.entry.label} at config.${found.path}`; // [#15137] `value` slots — the ruled `assignment.assignments.*` @@ -10847,7 +11090,7 @@ export class AutomationEngine implements IAutomationService { // nothing said at any layer. if (found.entry.role === 'value') { for (const refusal of this.valueEnvelopeRefusals(found.value)) { - failures.push(` • ${slotWhere}: ${refusal.message}\n source: \`${refusal.source}\``); + refuse(slotPath, slotWhere, refusal.message, refusal.source); } continue; } @@ -10875,29 +11118,34 @@ export class AutomationEngine implements IAutomationService { // `edge.condition` before `checkStructuralCondition` does. const shapeRefusal = predicateSlotRefusal(found.value); if (shapeRefusal) { - failures.push(` • ${slotWhere}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``); + refuse(slotPath, slotWhere, shapeRefusal.message, shapeRefusal.source); continue; } // No schema hint: a screen's `visibleWhen` binds the screen's OWN // collected values, not the trigger record's fields, so the // field-existence pass would report every field name as unknown. - check(slotWhere, found.value, false); + check(slotPath, slotWhere, found.value, false); } } - for (const edge of graph.edges) { - checkStructuralCondition(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown); + for (const [index, edge] of graph.edges.entries()) { + checkStructuralCondition( + `${storedGraphMemberPath(graph, 'edges', index)}.condition`, + `${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, + edge.condition as unknown, + ); } } - if (failures.length > 0) { - throw new Error( + if (failures.length === 0) return undefined; + return { + sites, + message: `Flow '${flowName}' has ${failures.length} invalid expression${failures.length > 1 ? 's' : ''} (ADR-0032 §1a). ` + `Predicates — conditions and declared bare-CEL slots such as a screen field's \`visibleWhen\` — ` + `must not wrap references in \`{…}\` template braces, and neither may a value slot (a computed ` + `value is a CEL envelope, the list a \`loop\` or \`map\` iterates included):\n` + `${failures.join('\n')}`, - ); - } + }; } /** From b55d68483189a93ccdc378efed0381607f4cd213 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:06:37 +0000 Subject: [PATCH 2/9] test(service-automation): pin the stored preview's whole-registration listing Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../src/canonicalize-stored-flow.test.ts | 186 +++++++++++++++++- 1 file changed, 185 insertions(+), 1 deletion(-) diff --git a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts index 74ae232508c..b799155b3d5 100644 --- a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts +++ b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts @@ -17,7 +17,9 @@ * canonicalization pass exists to remove, so these pin that it stays out. */ import { describe, expect, it, vi } from 'vitest'; -import { AutomationEngine } from './engine.js'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import { AutomationEngine, FLOW_REGISTRATION_TODO_ID } from './engine.js'; +import type { NodeExecutor } from './engine.js'; const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), @@ -221,3 +223,185 @@ describe('canonicalizeStoredFlow — a site registerFlow refuses comes back as a expect(await engine.getFlow('fd_refused_filter')).not.toBeNull(); }); }); + +/** + * [#23317] `canonicalizeStoredFlow` runs `registerFlow`'s whole validation, + * not one judge: every registration check, through the one list + * `registerFlow` itself throws from, each refused site listed as a TODO under + * `registerFlow`. Before, only the value-slot judge reached the stored pass + * (through the spec's refused-shape detector), so a stored flow refused for + * any other reason after the parse was counted as on protocol. + */ +describe('canonicalizeStoredFlow — every site registerFlow refuses after the parse is a TODO (#23317)', () => { + const STAMP = 'stored_stamp'; + const stampExecutor: NodeExecutor = { + type: STAMP, + descriptor: defineActionDescriptor({ + type: STAMP, version: '1.0.0', name: 'Stored stamp', category: 'custom', paradigms: ['flow'], source: 'plugin', + configSchema: { type: 'object', properties: { count: { type: 'number' } } }, + }), + async execute() { return { success: true }; }, + }; + const engineWithStamp = () => { + const engine = new AutomationEngine(silentLogger); + engine.registerNodeExecutor(stampExecutor); + return engine; + }; + const flowOf = (nodes: unknown[], extra: Record = {}) => ({ + name: 'fd_stored', + label: 'Stored', + type: 'autolaunched', + status: 'active', + nodes: [{ id: 'start', type: 'start', label: 'Start' }, ...nodes, { id: 'end', type: 'end', label: 'End' }], + edges: [], + ...extra, + }); + const notify = (title: string) => ({ + id: 'tell', type: 'notify', label: 'Tell', config: { recipients: ['usr_1'], title, message: 'Done' }, + }); + const inLoop = (node: unknown) => ({ + id: 'each', type: 'loop', label: 'Each', + config: { collection: { dialect: 'cel', source: 'items' }, body: { nodes: [node], edges: [] } }, + }); + + /** + * One flow per registration check, each refused by `registerFlow` after + * `FlowSchema.parse` accepted it: [label, refused flow, the same flow on + * protocol, the site, the check's words]. + */ + const CASES: Array<[string, () => unknown, () => unknown, string, RegExp]> = [ + ['a text slot still on the single-brace dialect', + () => flowOf([notify('Hi {record.name}')]), () => flowOf([notify('Hi {{ record.name }}')]), + 'flows[0].nodes[1].config.title', /notify title at config\.title/], + ['a text slot whose hole holds an unknown formatter', + () => flowOf([notify('Hi {{ record.name | nope }}')]), () => flowOf([notify('Hi {{ record.name }}')]), + 'flows[0].nodes[1].config.title', /notify title at config\.title/], + ['a text slot inside a loop body', + () => flowOf([inLoop(notify('Hi {item.name}'))]), () => flowOf([inLoop(notify('Hi {{ item.name }}'))]), + 'flows[0].nodes[1].config.body.nodes[0].config.title', /loop 'each' body: node 'tell' \(notify\) notify title at config\.title/], + ['a decision mode the engine does not have', + () => flowOf([{ id: 'gate', type: 'decision', label: 'Gate', config: { mode: 'bogus' } }]), + () => flowOf([{ id: 'gate', type: 'decision', label: 'Gate', config: { mode: 'exclusive' } }]), + 'flows[0].nodes[1].config.mode', /node 'gate' \(decision\) at config\.mode/], + ['a node predicate that does not parse', + () => flowOf([{ id: 'gate', type: 'decision', label: 'Gate', config: { condition: 'amount >' } }]), + () => flowOf([{ id: 'gate', type: 'decision', label: 'Gate', config: { condition: 'amount > 1' } }]), + 'flows[0].nodes[1].config.condition', /node 'gate' \(decision\) condition/], + ['an edge predicate in template braces', + () => flowOf([], { edges: [{ id: 'e1', source: 'start', target: 'end', condition: '{record.x} == 1' }] }), + () => flowOf([], { edges: [{ id: 'e1', source: 'start', target: 'end', condition: 'record.x == 1' }] }), + 'flows[0].edges[0].condition', /edge 'e1' \(start→end\) condition/], + ['a plugin node config key its descriptor does not declare', + () => flowOf([{ id: 'w', type: STAMP, label: 'W', config: { count: 1, countz: 2 } }]), + () => flowOf([{ id: 'w', type: STAMP, label: 'W', config: { count: 1 } }]), + 'flows[0].nodes[1].config.countz', /unknown config key `countz` at config\.countz/], + ['an api flow whose start node declares no secret', + () => flowOf([], { type: 'api' }), + () => ({ ...flowOf([], { type: 'api' }), nodes: [{ id: 'start', type: 'start', label: 'Start', config: { secret: 'whsec_test' } }, { id: 'end', type: 'end', label: 'End' }] }), + 'flows[0].nodes[0].config.secret', /binds the inbound `api` trigger/], + ]; + + it.each(CASES)('%s: listed under registerFlow at its site, in the check\'s words, and refused by registerFlow', (_label, refused, _onProtocol, path, words) => { + const engine = engineWithStamp(); + const flow = refused(); + const { notices, todos } = engine.canonicalizeStoredFlow('fd_stored', flow); + + expect(notices).toEqual([]); + expect(todos.map((t) => [t.conversionId, t.path])).toEqual([[FLOW_REGISTRATION_TODO_ID, path]]); + const todo = todos[0]!; + expect(todo.reason).toMatch(/^No conversion rewrites it, and registerFlow refuses it: left as stored, the flow is not registered at boot and never runs\. Flow `fd_stored`, /); + expect(todo.reason).toMatch(words); + + let thrown: unknown; + try { + engineWithStamp().registerFlow('fd_stored', flow); + } catch (e) { + thrown = e; + } + expect(thrown).toBeInstanceOf(Error); + expect((thrown as Error).message).toMatch(words); + }); + + it.each(CASES)('%s, on protocol: no TODO, and the flow registers', async (_label, _refused, onProtocol) => { + const engine = engineWithStamp(); + expect(engine.canonicalizeStoredFlow('fd_stored', onProtocol()).todos).toEqual([]); + engine.registerFlow('fd_stored', onProtocol()); + expect(await engine.getFlow('fd_stored')).not.toBeNull(); + }); + + it('the listing and the door agree in both directions: a flow is listed exactly when registerFlow refuses it', () => { + const corpus = [...CASES.flatMap(([, refused, onProtocol]) => [refused, onProtocol]), legacyFlow]; + for (const make of corpus) { + const listed = engineWithStamp().canonicalizeStoredFlow('fd_stored', make()).todos.length > 0; + let refusedAtRegistration = false; + try { + engineWithStamp().registerFlow('fd_stored', make()); + } catch { + refusedAtRegistration = true; + } + expect(listed).toBe(refusedAtRegistration); + } + }); + + it('the listing rewrites nothing and throws nothing: a refused flow on no other old shape comes back as stored', () => { + const flow = flowOf([notify('Hi {record.name}')]); + const result = engineWithStamp().canonicalizeStoredFlow('fd_stored', flow); + expect(result.storable).toBe(flow); + expect(result.todos).toHaveLength(1); + }); + + it('every site of a flow refused by several checks is listed, in the order registerFlow judges them', () => { + const flow = flowOf([ + { id: 'w', type: STAMP, label: 'W', config: { count: 1, countz: 2 } }, + { id: 'gate', type: 'decision', label: 'Gate', config: { mode: 'bogus' } }, + notify('Hi {record.name}'), + ]); + const { todos } = engineWithStamp().canonicalizeStoredFlow('fd_stored', flow); + expect(todos.map((t) => t.path)).toEqual([ + 'flows[0].nodes[1].config.countz', + 'flows[0].nodes[2].config.mode', + 'flows[0].nodes[3].config.title', + ]); + // registerFlow throws the first check's refusal, as it always has. + expect(() => engineWithStamp().registerFlow('fd_stored', flow)).toThrow(/1 undeclared config key/); + }); + + it('a site the conversion chain already lists under its D3 entry is listed once, under that entry', () => { + const flow = flowOf([ + { id: 'get', type: 'get_record', label: 'Get', config: { objectName: 'lead', filter: { id: '{record.id}' } } }, + notify('Hi {record.name}'), + ]); + const { todos } = engineWithStamp().canonicalizeStoredFlow('fd_stored', flow); + expect(todos.map((t) => [t.conversionId, t.path])).toEqual([ + ['flow-value-slot-template-dialect-refused', 'flows[0].nodes[1].config.filter.id'], + [FLOW_REGISTRATION_TODO_ID, 'flows[0].nodes[2].config.title'], + ]); + }); + + it('a refusal the parse makes is not a TODO: canonicalizeStoredFlow throws it, as registerFlow does', () => { + // A builtin value of the wrong type, written as a `{token}`: the value + // arm of the builtin contracts refuses it inside `FlowSchema.parse`, so + // the stored pass reports the row `failed` with the parse's words. + const flow = flowOf([{ id: 'get', type: 'get_record', label: 'Get', config: { objectName: 'lead', limit: '{page.size}' } }]); + expect(() => engineWithStamp().canonicalizeStoredFlow('fd_stored', flow)).toThrow(/limit/); + expect(() => engineWithStamp().registerFlow('fd_stored', flow)).toThrow(/limit/); + }); + + it('logs nothing for the checks it runs: the schema-aware advisory pass is registration\'s alone', () => { + const warn = vi.fn(); + const engine = new AutomationEngine({ ...silentLogger, warn }); + engine.setObjectSchemaResolver(() => ({ fields: ['amount'], fieldTypes: { amount: 'number' } })); + // `amout` is not a field of the object: an advisory finding, never a refusal. + const flow = { + ...flowOf([]), + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'deal', triggerType: 'record-after-update', condition: 'amout > 1' } }, + { id: 'end', type: 'end', label: 'End' }, + ], + }; + expect(engine.canonicalizeStoredFlow('fd_stored', flow).todos).toEqual([]); + expect(warn).not.toHaveBeenCalled(); + engine.registerFlow('fd_stored', flow); + expect(warn).toHaveBeenCalled(); + }); +}); From c4d8c592c053960cbddbf1266eb468f3220cf9bc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:17:16 +0000 Subject: [PATCH 3/9] test(service-automation): the undeclared-key walk returns its refusal Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../src/builtin/config-unknown-keys.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts b/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts index b6adecc96e9..248a6ab59dc 100644 --- a/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts +++ b/packages/services/service-automation/src/builtin/config-unknown-keys.test.ts @@ -159,7 +159,9 @@ describe('a builtin\'s undeclared key is the spec\'s, met at registration (#4277 it('the descriptor walk stands aside for every type the spec judges, and keeps the rest', () => { const engine = engineWith(recordingLogger().logger); - const walk = (type: string, config: Record) => () => + // [#23317] The walk returns its refusal (`registerFlow` throws it), so the + // stored pass can list the same sites without registering. + const walk = (type: string, config: Record): { message: string } | undefined => (engine as any).validateNodeConfigKeys('f', flowWith(type, config)); // One judge per type: the spec already refused these in `FlowSchema.parse`. for (const [type, config] of [ @@ -169,11 +171,11 @@ describe('a builtin\'s undeclared key is the spec\'s, met at registration (#4277 ['try_catch', { try: { nodes: [], edges: [] }, bogusKey: 1, retry: { maxRetry: 2 } }], ] as const) { expect(builtinNodeConfigKeysJudged(type), type).toBe(true); - expect(walk(type, config), type).not.toThrow(); + expect(walk(type, config), type).toBeUndefined(); } // The walk keeps plugin types. expect(builtinNodeConfigKeysJudged(STAMP)).toBe(false); - expect(walk(STAMP, { count: 1, bogusKey: 1 })).toThrow(/undeclared config key/); + expect(walk(STAMP, { count: 1, bogusKey: 1 })?.message).toMatch(/undeclared config key/); }); }); From 7750c3900719f770e6a36989770e7c79b209028c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:21:12 +0000 Subject: [PATCH 4/9] feat(spec): the stored preview lists a page component that binds the retired view-level API source The view-data-api-provider-retired detector now also walks every stored page's components and lists a component whose own props row refuses its data prop with the retirement's prescription (object-grid, object-map, object-gantt, object-tree), by page, component and position, with its fate. The D3 entry's scope names those components. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../src/conversions/refused-shapes.test.ts | 99 +++++++++++++++++++ .../spec/src/conversions/refused-shapes.ts | 76 ++++++++++++-- .../18.view-data-api-provider-retired.ts | 22 +++-- 3 files changed, 183 insertions(+), 14 deletions(-) diff --git a/packages/spec/src/conversions/refused-shapes.test.ts b/packages/spec/src/conversions/refused-shapes.test.ts index c8281ce237a..e648493ea32 100644 --- a/packages/spec/src/conversions/refused-shapes.test.ts +++ b/packages/spec/src/conversions/refused-shapes.test.ts @@ -5,6 +5,8 @@ import { describe, expect, it } from 'vitest'; import { flowNodeValueTemplateRefusals } from '../automation/flow-value-slot-template.js'; import { ObjectSchema } from '../data/object.zod.js'; import { MIGRATIONS_BY_MAJOR } from '../migrations/registry.js'; +import { ComponentPropsMap } from '../ui/component.zod.js'; +import { PageSchema } from '../ui/page.zod.js'; import { ViewDataSchema } from '../ui/view.zod.js'; import { applyConversions, applyConversionsToFlow } from './apply.js'; import { REFUSED_STORED_SHAPES } from './refused-shapes.js'; @@ -332,3 +334,100 @@ describe('#23269 — a stored flow whose CRUD filter operand is a {…} token is expect(judged).toEqual(['flows[0].nodes[1].config.filter.id']); }); }); + +// ── #23317 ──────────────────────────────────────────────────────────────── + +describe('#23317 — a stored page whose component binds the retired API source in its `data` prop is listed by page and component', () => { + const PRESCRIPTION = /`view\.data\.provider: 'api'` was removed in @objectstack\/spec 18/; + /** The components whose `data` prop is a view data source, as the D3 entry names them. */ + const VIEW_DATA_COMPONENTS = ['object-grid', 'object-map', 'object-gantt', 'object-tree'] as const; + const component = (type: string, data: unknown, id = 'block') => ({ id, type, properties: { objectName: 'crm_case', data } }); + const pageRow = (components: unknown[], extra: Record = {}) => ({ + name: 'partner_page', + label: 'Partner', + type: 'app', + regions: [{ name: 'main', components }], + ...extra, + }); + + it.each(VIEW_DATA_COMPONENTS)('%s: one TODO at the data prop, naming the page and the component, with its fate and the prescription; the row is not rewritten', (type) => { + const row = pageRow([component(type, apiData())]); + const { item, todos } = storedTodos('page', row); + expect(item).toEqual(row); + expect(todos.map((t) => [t.conversionId, t.path])).toEqual([ + ['view-data-api-provider-retired', 'pages[0].regions[0].components[0].properties.data'], + ]); + const todo = todos[0]!; + expect(todo.reason).toContain(`The \`${type}\` component \`block\` of page \`partner_page\`, at pages[0].regions[0].components[0], binds the retired view-level API data source.`); + expect(todo.reason).toMatch(/No mechanical move: the successor is a page component's `dataSource` binding/); + expect(todo.reason).toMatch(/Left as stored, the page still loads and saves, because the page parse does not judge a component's properties/); + expect(todo.reason).toMatch(PRESCRIPTION); + }); + + it('reaches a component nested in another component and a component in a slot', () => { + const row = pageRow( + [{ id: 'tabs', type: 'page:tabs', properties: { items: [{ label: 'Cases', children: [component('object-grid', apiData(), 'inner')] }] } }], + { slots: { details: component('object-tree', apiData(), 'slotted') } }, + ); + const { todos } = storedTodos('page', row); + expect(todos.map((t) => t.path)).toEqual([ + 'pages[0].regions[0].components[0].properties.items[0].children[0].properties.data', + 'pages[0].slots.details.properties.data', + ]); + }); + + it('never prints a credential: the URL and the method are shown, every other request key is withheld', () => { + const { todos } = storedTodos('page', pageRow([component('object-grid', apiData())])); + const todo = todos[0]!; + expect(JSON.parse(todo.from)).toEqual({ + provider: 'api', + read: { url: 'https://partner.example.com/cases', method: 'GET', headers: '[withheld]' }, + write: { url: '/api/v1/cases', method: 'POST' }, + }); + expect(`${todo.from} ${todo.reason} ${todo.message}`).not.toContain('s3cr3t'); + }); + + it.each([ + ['the `object` provider', component('object-grid', { provider: 'object', object: 'crm_case' })], + ['the `value` provider', component('object-map', { provider: 'value', items: [{ name: 'a' }] })], + ['a component whose `data` is a row array, not a view data source', component('object-kanban', apiData())], + ['a type with no props row', component('custom:partner-widget', apiData())], + ['the governed element binding on the component itself', { + id: 'block', type: 'object-grid', properties: { objectName: 'crm_case' }, dataSource: { provider: API, read: { url: '/api/v1/cases' } }, + }], + ])('not listed: %s', (_label, node) => { + const row = pageRow([node]); + const { item, todos } = storedTodos('page', row); + expect(item).toEqual(row); + expect(todos).toEqual([]); + }); + + it('agrees with the refusing door: the listed components are exactly the props rows whose `data` refuses the binding with the prescription', () => { + const listed = Object.keys(ComponentPropsMap).filter((type) => storedTodos('page', pageRow([component(type, apiData())])).todos.length > 0); + expect(listed).toEqual([...VIEW_DATA_COMPONENTS]); + for (const type of Object.keys(ComponentPropsMap)) { + const verdict = (ComponentPropsMap as Record } } }>)[type]! + .safeParse({ objectName: 'crm_case', data: apiData() }); + const refusedWithPrescription = !verdict.success + && verdict.error!.issues.some((i) => i.path[0] === 'data' && PRESCRIPTION.test(i.message)); + expect(refusedWithPrescription, type).toBe((VIEW_DATA_COMPONENTS as readonly string[]).includes(type)); + } + }); + + it('the fate it states holds: the page parse accepts the page, and the component props row refuses the binding', () => { + const row = pageRow(VIEW_DATA_COMPONENTS.map((type, i) => component(type, apiData(), `b${i}`))); + expect(PageSchema.safeParse(row).success).toBe(true); + for (const type of VIEW_DATA_COMPONENTS) { + expect(ComponentPropsMap[type].safeParse({ objectName: 'crm_case', data: apiData() }).success, type).toBe(false); + } + }); + + it('the D3 entry and the detector name the components the detector lists', () => { + const entry = MIGRATIONS_BY_MAJOR[18]?.semantic.find((s) => s.id === 'view-data-api-provider-retired'); + const detector = REFUSED_STORED_SHAPES.find((r) => r.id === 'view-data-api-provider-retired'); + for (const type of VIEW_DATA_COMPONENTS) { + expect(entry?.surface, type).toContain(`\`${type}\``); + expect(detector?.surface, type).toContain(type); + } + }); +}); diff --git a/packages/spec/src/conversions/refused-shapes.ts b/packages/spec/src/conversions/refused-shapes.ts index 9c396b044e3..ca0541b4b80 100644 --- a/packages/spec/src/conversions/refused-shapes.ts +++ b/packages/spec/src/conversions/refused-shapes.ts @@ -2,7 +2,7 @@ /** * Stored shapes protocol 18 refuses, and no conversion rewrites — LISTED, never - * rewritten (#23269, #23270, #23271). + * rewritten (#23269, #23270, #23271, #23317). * * ADR-0087's model is "convert where lossless, a structured TODO otherwise — * never silence". A D2 conversion carries the lossless half. For the shapes @@ -33,10 +33,12 @@ * touched, and the shape stays as stored. * - It never decides. The predicate is the refusing door's own: the * `controlled_by_parent` refinement's - * ({@link controlledByParentMasterRefusal}), `ViewDataSchema`'s parse, and - * the value-slot judge `AutomationEngine.registerFlow` and `objectstack - * validate` run ({@link flowNodeValueTemplateRefusals}). A site is listed - * here exactly when that door refuses it. + * ({@link controlledByParentMasterRefusal}), `ViewDataSchema`'s parse — for + * a page component, through the component's own props row, which the + * component-props gate parses — and the value-slot judge + * `AutomationEngine.registerFlow` and `objectstack validate` run + * ({@link flowNodeValueTemplateRefusals}). A site is listed here exactly + * when that door refuses it. * - It never prints a credential. A view-level API source could carry request * `headers`. The listing names the URL and the method, and withholds the rest. * @@ -49,9 +51,10 @@ import { controlledByParentMasterRefusal, type ControlledByParentMasterRefusal, } from '../data/controlled-by-parent-master.js'; +import { ComponentPropsMap } from '../ui/component.zod.js'; import { ViewDataSchema } from '../ui/view.zod.js'; import type { ConversionTodoDetail } from './types.js'; -import { mapCollection, mapFlowNodeList, mapViewPayloads } from './walk.js'; +import { mapCollection, mapFlowNodeList, mapPageComponents, mapViewPayloads } from './walk.js'; type Dict = Record; @@ -192,6 +195,38 @@ function apiBindingRefusal(data: unknown): string | undefined { return issue?.message; } +/** What the detector needs of a schema: a parse that reports issues. */ +interface SafeParseable { + safeParse(value: unknown): { success: true } | { success: false; error: { issues: ReadonlyArray<{ message: string }> } }; +} + +/** + * The prescription a page component's own props row refuses its `data` with, + * when that refusal is this retirement's — `undefined` otherwise (#23317). + * + * The page parse leaves a component's `properties` an open record; the door + * that judges them is the component-props gate, which parses the bag against + * the type's `ComponentPropsMap` row. So the judge here is that row's own + * `data` member, and the site is listed only when it refuses the binding with + * the prescription `ViewDataSchema` refuses it with. That holds for every + * component whose `data` is a view data source and for no other, without a + * list of types to keep in step: a row whose `data` is a bare array refuses + * the binding for its shape, never with this prescription. + */ +function componentApiBindingRefusal(type: unknown, data: unknown): string | undefined { + const prescription = apiBindingRefusal(data); + if (prescription === undefined || typeof type !== 'string') return undefined; + if (!Object.prototype.hasOwnProperty.call(ComponentPropsMap, type)) return undefined; + const row = (ComponentPropsMap as Readonly>)[type]; + const shape = (row as { shape?: unknown } | undefined)?.shape; + if (!isDict(shape)) return undefined; + const member = shape.data as Partial | undefined; + if (typeof member?.safeParse !== 'function') return undefined; + const verdict = (member as SafeParseable).safeParse(data); + if (verdict.success) return undefined; + return verdict.error.issues.some((issue) => issue.message === prescription) ? prescription : undefined; +} + const VIEW_API_BINDING_FATE = 'No mechanical move: the replacement is a `dataSource` binding on a page component, so a page must exist ' + 'to carry it, and the view stays a view. The one edit a pass could make, deleting the view\'s `data`, ' @@ -199,10 +234,19 @@ const VIEW_API_BINDING_FATE = + 'refuses the view: it registers under `[metadata_spec_invalid]` and its next save is refused with 422. ' + 'Decide which page composes the endpoint, and what the view itself binds now.'; +const COMPONENT_API_BINDING_FATE = + 'No mechanical move: the successor is a page component\'s `dataSource` binding, which takes a same-origin ' + + '`url` and no other request key, so the request cannot be carried over as written; and deleting `data` ' + + 'would make the component query its object instead and change what it shows. Left as stored, the page ' + + 'still loads and saves, because the page parse does not judge a component\'s properties, while ' + + '`os validate`, `os build` and `os lint` refuse the binding in source. Decide what the component binds now, ' + + 'and which same-origin endpoint, if any, replaces the request.'; + const viewDataApiProvider: RefusedStoredShape = { id: 'view-data-api-provider-retired', toMajor: 18, - surface: 'view.data — a list or form view\'s data source, in a view body or an object\'s listViews', + surface: 'view.data — a list or form view\'s data source, in a view body or an object\'s listViews, ' + + 'and the `data` prop of a page component that takes one (object-grid, object-map, object-gantt, object-tree)', detect(stack, report) { const judge = (payload: Dict, path: string, where: string): void => { const prescription = apiBindingRefusal(payload.data); @@ -225,6 +269,24 @@ const viewDataApiProvider: RefusedStoredShape = { } return obj; }); + // [#23317] A page component whose `data` prop is a view data source. + const pages = Array.isArray(stack.pages) ? stack.pages : []; + mapPageComponents(stack, (component, path) => { + const properties = component.properties; + if (!isDict(properties)) return component; + const prescription = componentApiBindingRefusal(component.type, properties.data); + if (prescription === undefined) return component; + const page = pages[Number(/^pages\[(\d+)\]/.exec(path)?.[1])]; + const pageName = isDict(page) && typeof page.name === 'string' ? page.name : path.split('.')[0]!; + const id = typeof component.id === 'string' ? ` \`${component.id}\`` : ''; + report({ + path: `${path}.properties.data`, + from: describeApiBinding(properties.data as Dict), + reason: `The \`${String(component.type)}\` component${id} of page \`${pageName}\`, at ${path}, binds the ` + + `retired view-level API data source. ${COMPONENT_API_BINDING_FATE} ${prescription}`, + }); + return component; + }); }, }; diff --git a/packages/spec/src/migrations/entries/semantic/18.view-data-api-provider-retired.ts b/packages/spec/src/migrations/entries/semantic/18.view-data-api-provider-retired.ts index a39fac8e70d..25cfed93468 100644 --- a/packages/spec/src/migrations/entries/semantic/18.view-data-api-provider-retired.ts +++ b/packages/spec/src/migrations/entries/semantic/18.view-data-api-provider-retired.ts @@ -17,8 +17,11 @@ import type { SemanticMigration } from '../../types.js'; // arm is refused at parse with the prescription instead. export const entry: SemanticMigration = { id: 'view-data-api-provider-retired', - surface: "view.data (`provider: 'api'`, with its `read` / `write` request blocks) — the view-level API data source", - replacement: "A view binds its object (`provider: 'object'`) or static rows (`provider: 'value'`); the API read " + surface: "view.data (`provider: 'api'`, with its `read` / `write` request blocks) — the view-level API data source, " + + "in a view and in the `data` prop of each page component that takes a view data source (`object-grid`, " + + '`object-map`, `object-gantt`, `object-tree`)', + replacement: "A view binds its object (`provider: 'object'`) or static rows (`provider: 'value'`), and so does a " + + "page component's `data` prop; the API read " + "moves onto a page component's `dataSource` binding — `{ provider: 'api', read: { url }, path? }` under a " + "page's `regions[].components[]` — which is same-origin, carries no credentials and is read with the " + "console's signed-in fetch.", @@ -29,12 +32,17 @@ export const entry: SemanticMigration = { + 'stored row alike, and the author decides for each such view which page composes the endpoint it ' + "read and what the view itself should now bind. The arm's `headers` block is the part with no " + "successor at all: a key an author had written there was a credential in metadata, and the page " - + 'element binding refuses the key; the console sends its own signed-in fetch instead.', + + 'element binding refuses the key; the console sends its own signed-in fetch instead. A page component ' + + 'whose `data` prop is a view data source took the same arm through the same schema and has no mechanical ' + + 'move either: deleting its `data` would make it query its object instead.', acceptanceCriteria: "No view in `stack.views[]`, in any object's `listViews` / `formViews` map or in a stored " - + "`sys_metadata` view row carries `data.provider: 'api'`; the parse refuses one with the prescription. " + + "`sys_metadata` view row carries `data.provider: 'api'`, and no page component's `data` prop does; the " + + 'parse refuses one in a view, and the component-props gate of `os validate`, `os build` and `os lint` ' + + 'refuses one in a page component, with the prescription. ' + "Every endpoint such a view read is either a page component's API binding on a same-origin path " + 'with no credential key, or dropped on purpose; and every converted view binds its object or static ' - + 'rows and loads. `os migrate meta --stored` lists each stored view still bound to the API source as a ' - + 'TODO under this entry\'s id, before the upgrade and after it, by row and position, showing the ' - + 'request URL and method and withholding every other request key, and rewrites none of them.', + + 'rows and loads. `os migrate meta --stored` lists each stored view, and each page component of a stored ' + + 'page, still bound to the API source as a TODO under this entry\'s id, before the upgrade and after it, ' + + 'by row and position, showing the request URL and method and withholding every other request key, and ' + + 'rewrites none of them.', }; From f2a9074dc4c0fb35481fb5b414f00b629718cf72 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:33:18 +0000 Subject: [PATCH 5/9] docs: the stored preview's registerFlow TODOs and page-component listing, declared where they are read The conversion TODO's conversionId may name 'registerFlow'; the contract's todos member says it carries registration's whole verdict; the CLI page's --stored table lists both new families; and the engine's TODO stamp is classified beside the conversion pass's own. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- content/docs/deployment/cli.mdx | 3 ++- .../runtime/src/dispatcher-error-vocabulary.ts | 14 ++++++++++++++ packages/spec/src/contracts/automation-service.ts | 9 +++++++-- packages/spec/src/conversions/types.ts | 4 +++- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 3124e4b83e2..9f1b1b6f69b 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1473,7 +1473,8 @@ can produce, because both supply a live one. | Rows that still fail the current schema after conversion | That is a genuine contract violation, not chain-owned history. The write path's rejection is correct; fix the row in Studio | | A flow whose rename the conflict guard refused | The old node-type token is a live name something else owns here. Rewriting would clobber that owner, so the row fails loudly naming the token — never a silent skip | | A site the conversion chain leaves as stored because no lossless rewrite exists — above all a page filter carrying `$and` / `$or` / `$not` | Flattening a combinator changes which rows the page selects, so it is never done. Each site is printed as a `TODO` line under its row — path, block, and what blocks the rewrite — whatever the row's outcome; a row with nothing but TODOs is reported `skipped`. It does not fail the run, since no run of this pass can clear it: rewrite each site by hand | -| A stored shape protocol 18 refuses and no conversion rewrites — a `controlled_by_parent` object without exactly one required `master_detail` (two masters, none, a required `lookup` only, an explicit `required: false`), a view bound to the retired view-level API data source, a flow value slot carrying a `{…}` token (a `get_record` / `update_record` / `delete_record` filter operand among them) | Each would need a choice only its owner can make — which master, which page, what an absent value should do — so it is never rewritten. Each site is printed as a `TODO` line under its row, naming the step-18 entry that refuses it, the object, view or flow node, and the owner's choices; a view's request headers are withheld. Until it is rewritten by hand, the parse or the boot refuses the row: an object registers but derives no master, a view registers under `[metadata_spec_invalid]`, and a flow is not registered | +| A stored shape protocol 18 refuses and no conversion rewrites — a `controlled_by_parent` object without exactly one required `master_detail` (two masters, none, a required `lookup` only, an explicit `required: false`), a view bound to the retired view-level API data source or a page component whose `data` prop binds it (`object-grid`, `object-map`, `object-gantt`, `object-tree`), a flow value slot carrying a `{…}` token (a `get_record` / `update_record` / `delete_record` filter operand among them) | Each would need a choice only its owner can make — which master, which page, what an absent value should do — so it is never rewritten. Each site is printed as a `TODO` line under its row, naming the step-18 entry that refuses it, the object, view, page component or flow node, and the owner's choices; a binding's request headers are withheld. Until it is rewritten by hand, the row is refused: an object registers but derives no master, a view registers under `[metadata_spec_invalid]`, a page still loads and saves while `os validate`, `os build` and `os lint` refuse the binding in source, and a flow is not registered | +| Any other site `registerFlow` refuses in a stored flow the parse accepts — a text slot's template hole, a decision's `mode`, a malformed predicate, a plugin node's undeclared config key, an `api` flow with no secret | The flow's own definition is wrong, and no conversion rewrites it. Each site is printed as a `TODO registerFlow` line under its row, in the words `registerFlow` refuses it with; a site already listed under a step-18 entry is not repeated. Until it is fixed, the flow is not registered at boot and never runs. A flow the parse itself refuses is reported among the rows that could not be rewritten, in the parse's words | **One thing it lists and never writes: decision nodes that changed meaning at protocol 18.** A stored `decision` with no `config.conditions`, no `mode` and two diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index eeac1e06146..e7b8a8c7ac0 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -617,6 +617,20 @@ export const UNREGISTERED_CODE_SITES: readonly UnregisteredCodeSite[] = [ 'pass flattens it onto its report rows, where `os migrate meta --stored` prints it; it ' + 'carries no `code` there. A callback payload, not a thrown error and not a response body.', }, + { + code: 'OS_METADATA_CONVERSION_TODO', + file: 'packages/services/service-automation/src/engine.ts', + shape: 'objlitconst', + door: 'none', + verdict: 'foreign-vocabulary', + why: + 'The same conversion-TODO vocabulary as the apply.ts row above, stamped by the automation ' + + 'engine for a site its flow registration refuses once the parse has accepted the flow: ' + + "`canonicalizeStoredFlow` returns it in `todos` beside the chain's own TODOs, and the " + + 'stored-metadata migration pass flattens it onto its report rows exactly as it does those, ' + + 'with no `code`. A returned record, not a thrown error and not a response body: ' + + '`registerFlow` throws its own plain refusal and never this.', + }, { code: 'ERR_BULK_PER_ROW_HOOK_LIMIT', file: 'packages/spec/src/data/bulk-write-hook-conformance.ts', diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index b42795b3ba0..8ab5cfd7d47 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -802,7 +802,8 @@ export interface IAutomationService { * `condition` envelopes, deliberately WITHOUT schema defaults, so a * written-back row is not frozen on today's default values), plus the * conversions applied, any rewrite the guard refused, and (`todos`) the - * sites left as stored for want of a lossless rewrite. + * sites left as stored for want of a lossless rewrite, and the sites + * {@link registerFlow} refuses once the parse has accepted the flow. * @throws when the definition cannot be canonicalized at all (a strict-schema * violation, a malformed control-flow region) — such a flow cannot be * registered either, so a caller reports it rather than persisting a guess. @@ -817,7 +818,11 @@ export interface IAutomationService { * rewrite exists — a value slot carrying a `{…}` token among them, which * {@link registerFlow} refuses although the parse accepts it — so * `os migrate meta --stored` lists a flow the boot refuses (#23269); - * absent reads as none. + * absent reads as none. [#23317] It also lists every other site + * {@link registerFlow} refuses once the parse has accepted the flow (a + * text slot's template hole, a decision's `mode`, a malformed + * predicate), each under the `conversionId` `'registerFlow'`, so the + * stored pass previews registration's whole verdict, not one judge's. */ todos?: ConversionTodoNotice[]; }; diff --git a/packages/spec/src/conversions/types.ts b/packages/spec/src/conversions/types.ts index b2b1ed45319..2cfd26bac6a 100644 --- a/packages/spec/src/conversions/types.ts +++ b/packages/spec/src/conversions/types.ts @@ -145,7 +145,9 @@ export interface ConversionTodoNotice { * The {@link MetadataConversion.id} whose surface the site is on — or, for a * stored shape no conversion covers (a refused shape, see * {@link CONVERSION_TODO_CODE}), the id of the D3 semantic entry that - * refuses it. + * refuses it. A TODO the automation engine adds for a site its flow + * registration refuses and neither covers names that door instead: + * `'registerFlow'` (`IAutomationService.canonicalizeStoredFlow`). */ conversionId: string; /** Dotted surface the conversion governs. */ From 1cbea715452d08fff26713e0b59d8a8118a0f686 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:34:45 +0000 Subject: [PATCH 6/9] test(metadata-protocol): the stored preview lists a page component bound to the retired API source Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../src/protocol.stored-migration.test.ts | 31 +++++++++++++++++++ .../metadata-protocol/src/stored-migration.ts | 12 +++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.stored-migration.test.ts b/packages/metadata-protocol/src/protocol.stored-migration.test.ts index 890f61e88da..53c4fb6fb04 100644 --- a/packages/metadata-protocol/src/protocol.stored-migration.test.ts +++ b/packages/metadata-protocol/src/protocol.stored-migration.test.ts @@ -1184,6 +1184,37 @@ describe('migrateStoredMetadata — the v18 stored shapes refused with no lossle expect(text).not.toContain('s3cr3t'); }); + it('#23317 — a page whose component binds the retired API source in its data prop is listed by page and component, without its headers', async () => { + const block = (type: string, data: unknown) => ({ id: 'cases', type, properties: { objectName: 'crm_case', data } }); + const page = (name: string, data: unknown) => ({ + type: 'page', + name, + metadata: { name, label: name, type: 'app', regions: [{ name: 'main', components: [block('object-grid', data)] }] }, + }); + const { engine, tables } = makeStubEngine([ + page('partner_page', { provider: API, read: { url: 'https://partner.example.com/cases', headers: { Authorization: 'Bearer s3cr3t' } } }), + page('case_page', { provider: 'object', object: 'crm_case' }), + ]); + const before = JSON.stringify(metaRows(tables)); + const protocol = new ObjectStackProtocolImplementation(engine); + + const report = await protocol.migrateStoredMetadata(); + + expect(report.canonical).toBe(1); + expect(report.rows.map((r) => [r.type, r.name, r.outcome, r.todos.map((t) => [t.conversionId, t.path])])).toEqual([ + ['page', 'partner_page', 'skipped', [['view-data-api-provider-retired', 'pages[0].regions[0].components[0].properties.data']]], + ]); + const text = formatStoredMigrationReport(report).join('\n'); + expect(text).toContain('page/partner_page [env-wide]'); + expect(text).toContain('The `object-grid` component `cases` of page `partner_page`'); + expect(text).not.toContain('s3cr3t'); + // Listed, never written. + expect(storedMigrationClean(report)).toBe(true); + const applied = await protocol.migrateStoredMetadata({ apply: true }); + expect(applied.rewritten).toBe(0); + expect(JSON.stringify(metaRows(tables))).toBe(before); + }); + it('#23269 — a flow whose CRUD filter operand is a {…} token is listed by flow and node; the CEL form is canonical', async () => { const { engine, tables } = makeStubEngine([ crudFlow('fd_refused_filter', '{record.id}'), diff --git a/packages/metadata-protocol/src/stored-migration.ts b/packages/metadata-protocol/src/stored-migration.ts index 50b13d395af..7771d25c452 100644 --- a/packages/metadata-protocol/src/stored-migration.ts +++ b/packages/metadata-protocol/src/stored-migration.ts @@ -61,8 +61,10 @@ export interface StoredFlowCanonicalization { /** * Sites the chain left as stored because no lossless rewrite exists (#23269) * — above all a value slot carrying a `{…}` template token, which - * `registerFlow` refuses although the parse accepts the flow. Listed under - * the row exactly as a non-flow row's TODOs are ({@link StoredMigrationTodo}). + * `registerFlow` refuses although the parse accepts the flow — and every + * other site `registerFlow` refuses once the parse has accepted it (#23317), + * each under `conversionId: 'registerFlow'`. Listed under the row exactly as + * a non-flow row's TODOs are ({@link StoredMigrationTodo}). * It restates the contract's own optional member, * `IAutomationService.canonicalizeStoredFlow(...).todos?` (`@objectstack/spec`): * a canonicalizer that reports none still conforms, and absent reads as none. @@ -119,7 +121,11 @@ export interface StoredMigrationNotice { * this list the row would read as already on protocol. */ export interface StoredMigrationTodo { - /** The `MetadataConversion.id` whose surface the site is on. */ + /** + * The `MetadataConversion.id` whose surface the site is on — or the D3 entry + * that refuses a stored shape no conversion covers, or `registerFlow` for a + * site flow registration refuses. + */ conversionId: string; /** Dotted surface that conversion governs. */ surface: string; From 706df2da0804d129b8a651bddd4fd223b134a3d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:35:36 +0000 Subject: [PATCH 7/9] chore(changeset): the stored preview lists page components on the retired API source and registerFlow's refusals Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .changeset/23317-stored-preview-gaps.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/23317-stored-preview-gaps.md diff --git a/.changeset/23317-stored-preview-gaps.md b/.changeset/23317-stored-preview-gaps.md new file mode 100644 index 00000000000..c2639b85c18 --- /dev/null +++ b/.changeset/23317-stored-preview-gaps.md @@ -0,0 +1,14 @@ +--- +'@objectstack/spec': minor +'@objectstack/service-automation': minor +--- + +fix: `os migrate meta --stored` lists the two families of stored rows it still read as on protocol: a page component whose `data` prop binds the retired view-level API source, and every other site `registerFlow` refuses in a stored flow the parse accepts. + +Clause-②: yes (widening: `ConversionTodoNotice.conversionId` in `@objectstack/spec` may now name `registerFlow`, the refusing door; `IAutomationService.canonicalizeStoredFlow(...).todos?` in `@objectstack/spec` and `AutomationEngine.canonicalizeStoredFlow(...).todos` in `@objectstack/service-automation` now also carry each site flow registration refuses; the `onTodo` sink of `applyConversions` now also receives a page component bound to the retired API source; nothing narrows, and no door accepts or refuses anything it did not before) + +**A page component bound to the retired view-level API source: refuse and list, by page and component (`view-data-api-provider-retired`).** `object-grid`, `object-map`, `object-gantt` and `object-tree` take a view data source in their `data` prop, so the step-18 retirement of the `provider: 'api'` arm reaches them through the same schema. The listing judges each stored page component with its own props row, the one the component-props gate parses, and lists it only when that row refuses the binding with the retirement's prescription; a component whose `data` is a row array, a custom type and a component's own `dataSource` binding are not listed. The line names the page, the component and its position, shows the request URL and method, and withholds every other request key. Nothing is rewritten: deleting `data` would make the component query its object instead. Left as stored, the page still loads and saves, because the page parse does not judge a component's properties, while `os validate`, `os build` and `os lint` refuse the binding in source. The D3 entry `view-data-api-provider-retired` now names these components in its scope. + +**A stored flow refused by `registerFlow` after the parse: refuse and list, by site (`registerFlow`).** The stored pass ran only one of registration's judges, the value-slot one. `canonicalizeStoredFlow` now runs registration's whole validation without registering: every check `registerFlow` runs once the parse has accepted the flow, through the one list `registerFlow` itself throws from. Each refused site is a `TODO registerFlow` line under its row, in the check's own words: a text slot still on the single-brace dialect or with a hole that does not compile, a decision's `mode`, a malformed node or edge predicate, a plugin node's undeclared config key, an `api` flow with no secret. A check added to registration later is listed the same way, with no second change. A site the chain already lists under a step-18 entry is not repeated. What the parse itself refuses, the builtin config contracts' value arm among it, is not a TODO: `canonicalizeStoredFlow` throws it, and the preview reports the row among those that could not be rewritten, in the parse's words. + +`registerFlow` refuses exactly what it refused before, with the same message: its checks now return their refusal instead of throwing it, and `registerFlow` throws the first one. The doors that canonicalize a flow without registering it (the metadata save door, a package duplication) read no `todos`, and the stored pass's run of the checks logs nothing, so they behave as before. From 377dc370bbb9fa8cf8e32f58610b0e1ae5790a98 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 23:54:05 +0000 Subject: [PATCH 8/9] refactor(service-automation): name the registration checks' refusal types apart from the flow registration verdict Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../services/service-automation/src/engine.ts | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 46984b1903d..77e0ec4741a 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -2255,14 +2255,14 @@ const CONVERSIONS_NOT_REPLAYED_AT_REHYDRATION: readonly string[] = [ * One site a registration check refuses (#23317): where it is, the shape * found there, and the check's own words for it. * - * A check does not throw: it returns a {@link FlowRegistrationRefusal}, which + * A check does not throw: it returns a {@link RegistrationCheckRefusal}, which * {@link AutomationEngine.registerFlow} throws and * {@link AutomationEngine.canonicalizeStoredFlow} lists site by site. So the * stored pass (`os migrate meta --stored`) previews exactly what registration * refuses, through the same checks, and a check added to registration is * previewed with no second change. */ -interface FlowRegistrationSite { +interface RegistrationCheckSite { /** * The site's path in the stack the stored pass wraps one flow in — * `flows[0].nodes[2].config.title`, `flows[0].edges[1].condition` — the @@ -2280,9 +2280,9 @@ interface FlowRegistrationSite { * What one registration check refuses in a flow: the error `registerFlow` * throws, byte for byte, and every site it names. */ -interface FlowRegistrationRefusal { +interface RegistrationCheckRefusal { readonly message: string; - readonly sites: readonly FlowRegistrationSite[]; + readonly sites: readonly RegistrationCheckSite[]; } /** @@ -2330,7 +2330,7 @@ function storedGraphMemberPath(graph: FlowGraph, list: 'nodes' | 'edges', index: * check's own words follow, as a refused-shape TODO's do: the site stays as * stored, and the flow does not register. */ -function flowRegistrationTodo(flowName: string, site: FlowRegistrationSite): ConversionTodoNotice { +function flowRegistrationTodo(flowName: string, site: RegistrationCheckSite): ConversionTodoNotice { const reason = 'No conversion rewrites it, and registerFlow refuses it: left as stored, the flow is not ' + `registered at boot and never runs. Flow \`${flowName}\`, ${site.detail}`; return { @@ -4456,7 +4456,7 @@ export class AutomationEngine implements IAutomationService { * * [#23317] `todos` also carries every site {@link registerFlow} refuses * once the parse has accepted the flow: the same registration checks, run - * here without registering ({@link flowRegistrationRefusals}), each site + * here without registering ({@link registrationCheckRefusals}), each site * listed under {@link FLOW_REGISTRATION_TODO_ID}. A text slot's template * hole, a decision's `mode`, a malformed predicate, a plugin node's * undeclared config key and an `api` flow with no secret are among them. @@ -4482,7 +4482,7 @@ export class AutomationEngine implements IAutomationService { // names the retirement; the registration check's line for it is the // same refusal and is not repeated. const listed = new Set(canonical.todos.map((todo) => todo.path)); - for (const refusal of this.flowRegistrationRefusals(name, canonical.parsed)) { + for (const refusal of this.registrationCheckRefusals(name, canonical.parsed)) { for (const site of refusal.sites) { if (listed.has(site.path)) continue; canonical.todos.push(flowRegistrationTodo(name, site)); @@ -10460,7 +10460,7 @@ export class AutomationEngine implements IAutomationService { * [#23317] What {@link registerFlow} judges a canonical flow by after the * parse — every registration check, in the order it runs them. The ONE * list: `registerFlow` throws the first refusal, and - * {@link flowRegistrationRefusals} collects them all for the stored pass, + * {@link registrationCheckRefusals} collects them all for the stored pass, * so a check added here is refused at registration and previewed by * `os migrate meta --stored` alike. * @@ -10470,7 +10470,7 @@ export class AutomationEngine implements IAutomationService { */ private registrationChecks( { advisory }: { advisory: boolean }, - ): ReadonlyArray<(flowName: string, flow: FlowParsed) => FlowRegistrationRefusal | undefined> { + ): ReadonlyArray<(flowName: string, flow: FlowParsed) => RegistrationCheckRefusal | undefined> { return [ // #4277 — config keys the node's descriptor does not declare (the // tightened #4059 warning; a `visibleIf` typo used to register in @@ -10509,10 +10509,10 @@ export class AutomationEngine implements IAutomationService { * is listed as one refusal at the flow itself: the doors that canonicalize * without registering must not start throwing for a check they never ran. */ - private flowRegistrationRefusals(flowName: string, flow: FlowParsed): FlowRegistrationRefusal[] { - const refusals: FlowRegistrationRefusal[] = []; + private registrationCheckRefusals(flowName: string, flow: FlowParsed): RegistrationCheckRefusal[] { + const refusals: RegistrationCheckRefusal[] = []; for (const check of this.registrationChecks({ advisory: false })) { - let refusal: FlowRegistrationRefusal | undefined; + let refusal: RegistrationCheckRefusal | undefined; try { refusal = check(flowName, flow); } catch (e) { @@ -10590,9 +10590,9 @@ export class AutomationEngine implements IAutomationService { * rather than crashing the kernel. [#23317] The refusal is returned, not * thrown: `registerFlow` throws it ({@link registrationChecks}). */ - private validateNodeConfigKeys(flowName: string, flow: FlowParsed): FlowRegistrationRefusal | undefined { + private validateNodeConfigKeys(flowName: string, flow: FlowParsed): RegistrationCheckRefusal | undefined { const violations: string[] = []; - const sites: FlowRegistrationSite[] = []; + const sites: RegistrationCheckSite[] = []; // #4389 — every graph, not just the flow's own. `visibleIf` is the typo // this check exists to catch, and moving the node into a `loop` body // used to restore the silence #4277 closed. No double-reporting from the @@ -10668,9 +10668,9 @@ export class AutomationEngine implements IAutomationService { * gateway that would run as something the author did not write. [#23317] * The refusal is returned, not thrown: `registerFlow` throws it. */ - private validateDecisionModes(flowName: string, flow: FlowParsed): FlowRegistrationRefusal | undefined { + private validateDecisionModes(flowName: string, flow: FlowParsed): RegistrationCheckRefusal | undefined { const failures: string[] = []; - const sites: FlowRegistrationSite[] = []; + const sites: RegistrationCheckSite[] = []; for (const graph of collectFlowGraphs(flow)) { const at = graph.scope ? `${graph.scope}: ` : ''; graph.nodes.forEach((node, index) => { @@ -10722,7 +10722,7 @@ export class AutomationEngine implements IAutomationService { * VALIDATION_FAILED`. [#23317] The refusal is returned, not thrown: * `registerFlow` throws it. */ - private validateApiTriggerSecret(flowName: string, flow: FlowParsed): FlowRegistrationRefusal | undefined { + private validateApiTriggerSecret(flowName: string, flow: FlowParsed): RegistrationCheckRefusal | undefined { const resolved = this.deriveTriggerBinding(flowName, flow); if (resolved?.triggerType !== 'api') return undefined; const config = (resolved.binding.config ?? {}) as Record; @@ -10864,7 +10864,7 @@ export class AutomationEngine implements IAutomationService { * bind the record's fields flat, so the schema pass uses `flattened` scope. * The fatal set is unchanged — a resolver can never break a flow that used to * register cleanly. `advisory: false` skips that pass: the stored pass - * previews refusals ({@link flowRegistrationRefusals}), and an advisory + * previews refusals ({@link registrationCheckRefusals}), and an advisory * finding is none. * * [#23317] The refusal is returned, not thrown: `registerFlow` throws it. @@ -10875,9 +10875,9 @@ export class AutomationEngine implements IAutomationService { flowName: string, flow: FlowParsed, { advisory }: { advisory: boolean }, - ): FlowRegistrationRefusal | undefined { + ): RegistrationCheckRefusal | undefined { const failures: string[] = []; - const sites: FlowRegistrationSite[] = []; + const sites: RegistrationCheckSite[] = []; /** One failure: its line in the thrown message, and its site for the stored pass. */ const refuse = (path: string, where: string, message: string, source: unknown): void => { failures.push(` • ${where}: ${message}\n source: \`${source}\``); From eccc154a24a16470bf3826da415f154403ecc79e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 00:10:45 +0000 Subject: [PATCH 9/9] perf(service-automation): the registration listing runs only for a caller that reads todos The save door, a package duplication and the sync's held-flow comparison read parsed or storable only, so they pay nothing for the stored preview's listing. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H --- .../src/canonicalize-stored-flow.test.ts | 14 ++++++ .../services/service-automation/src/engine.ts | 49 +++++++++++++------ 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts index b799155b3d5..cdfe1c2ca2d 100644 --- a/packages/services/service-automation/src/canonicalize-stored-flow.test.ts +++ b/packages/services/service-automation/src/canonicalize-stored-flow.test.ts @@ -350,6 +350,20 @@ describe('canonicalizeStoredFlow — every site registerFlow refuses after the p expect(result.todos).toHaveLength(1); }); + it('runs the checks only for a caller that reads `todos`, and once however often it is read', () => { + const engine = engineWithStamp(); + const checks = vi.spyOn(engine as any, 'registrationCheckRefusals'); + const result = engine.canonicalizeStoredFlow('fd_stored', flowOf([notify('Hi {record.name}')])); + // What the save door, a duplication and the sync's held-flow comparison read. + void result.parsed; + void result.storable; + void result.conflicts; + expect(checks).not.toHaveBeenCalled(); + expect(result.todos).toHaveLength(1); + expect(result.todos).toBe(result.todos); + expect(checks).toHaveBeenCalledTimes(1); + }); + it('every site of a flow refused by several checks is listed, in the order registerFlow judges them', () => { const flow = flowOf([ { id: 'w', type: STAMP, label: 'W', config: { count: 1, countz: 2 } }, diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 77e0ec4741a..ee7a567e90b 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -4465,9 +4465,10 @@ export class AutomationEngine implements IAutomationService { * stored pass reports that row `failed` with the parse's words. * * None of it changes `parsed` or `storable`, and nothing here throws for - * it, so a caller that reads no `todos` (the save door, a package - * duplication) behaves as before. `registerFlow` reads none either: it - * runs the same checks itself and throws the first refusal. + * it. The checks run on the first read of `todos`, so a caller that reads + * none (the save door, a package duplication, the sync's comparison of a + * held flow) does no more work than before. `registerFlow` reads none + * either: it runs the same checks itself and throws the first refusal. */ canonicalizeStoredFlow(name: string, definition: unknown): { parsed: FlowParsed; @@ -4476,19 +4477,37 @@ export class AutomationEngine implements IAutomationService { conflicts: ConversionConflictNotice[]; todos: ConversionTodoNotice[]; } { - const canonical = this.canonicalizeFlow(name, definition); - // One listing per site: a site a conversion TODO names (the value-slot - // refusal the spec lists under its D3 entry) keeps that TODO, which - // names the retirement; the registration check's line for it is the - // same refusal and is not repeated. - const listed = new Set(canonical.todos.map((todo) => todo.path)); - for (const refusal of this.registrationCheckRefusals(name, canonical.parsed)) { - for (const site of refusal.sites) { - if (listed.has(site.path)) continue; - canonical.todos.push(flowRegistrationTodo(name, site)); + const { parsed, storable, notices, conflicts, todos: chainTodos } = this.canonicalizeFlow(name, definition); + // The registration checks run when `todos` is first READ, once: the + // stored pass reads it, and the doors that only want `parsed` or + // `storable` (the save door, a package duplication, the sync's + // "already held?" comparison) pay nothing for a listing they discard. + const listRegistrationRefusals = (): ConversionTodoNotice[] => { + const todos = [...chainTodos]; + // One listing per site: a site a conversion TODO names (the value-slot + // refusal the spec lists under its D3 entry) keeps that TODO, which + // names the retirement; the registration check's line for it is the + // same refusal and is not repeated. + const listed = new Set(chainTodos.map((todo) => todo.path)); + for (const refusal of this.registrationCheckRefusals(name, parsed)) { + for (const site of refusal.sites) { + if (listed.has(site.path)) continue; + todos.push(flowRegistrationTodo(name, site)); + } } - } - return canonical; + return todos; + }; + let listedTodos: ConversionTodoNotice[] | undefined; + return { + parsed, + storable, + notices, + conflicts, + get todos(): ConversionTodoNotice[] { + listedTodos ??= listRegistrationRefusals(); + return listedTodos; + }, + }; } /**