You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #4327, filed per Prime Directive #10 (out-of-scope finding, named rather than silently absorbed).
Context
#4327 shipped os migrate meta --stored: it walks sys_metadata (active + draft, all orgs), replays the ADR-0087 conversion chain, and re-saves each changed body through saveMetaItem, so a deployment that runs it stops carrying pre-protocol dialects at rest.
It covers every metadata type except flow, and reports each flow row as skipped with the reason rather than counting it done:
⚠ 1 row(s) are outside this pass — they keep reading through the chain:
• flow/purge_flow [env-wide] — flows canonicalize at AutomationEngine.registerFlow —
the node-type conflict guard needs the live executor registry this layer does not have
That is the same carve-out applyConversionsToStoredItem and ObjectStackProtocolImplementation.convertStoredItem already make, and for the same reason: flow-node conversions carry ADR-0078's open-namespace conflict guard, which needs reservedNodeTypes — the engine's live executor registry plus action descriptors plus the structural node types.
Why it matters
Flows are exactly the type most likely to carry legacy shapes: the graduated conversions flow-node-crud-filter-alias, flow-node-crud-object-alias, flow-node-notify-config-aliases and flow-node-script-config-aliases are all flow-node entries (PD #12, protocol 17). So the type with the most stored dialect is the one type the new pass cannot end.
For a deployment with stored flows: migrateStoredMetadata reports skipped however many times it is run, those rows keep re-lowering at registerFlow on every boot, and storedMigrationClean is true while they are still legacy — deliberately, since a documented carve-out is not unfinished work, but it does mean "my metadata is on protocol N" is narrower than it reads.
Finding 1 — "canonical flow" is three steps, and writing back the third would fossilize defaults
registerFlow canonicalizes in three steps, so "expose reservedNodeTypes, run applyConversionsToFlow, write it back" is not sufficient:
constconverted=applyConversionsToFlow(definition,{ reservedNodeTypes,includeRetired: true, … });constflowShell=FlowSchema.parse(converted);// a Zod transform: it CHANGES the bodyvalidateControlFlow(flowShell);constparsed=normalizeControlFlowRegions(flowShell);// #4347: canonicalizes INSIDE regions
Measured, not assumed. Driving a pre-17 flow (legacy filters at top level and inside a loop body, bare-string edge conditions in both places) through all three steps:
result
keys removed by parse+normalize
none
keys added
version, runAs, and per-edge type / isDefault — all schema defaults
edge condition
both sites lowered to { dialect: 'cel', source } (step 1 leaves both as bare strings)
Two consequences, and the first corrects an earlier version of this issue:
The graftNormalizedOperators precedent does not transfer. That helper exists because parsed.data on the view surface strips Studio-only auxiliary keys (isPinned, isDefault, sortOrder) that intentionally ride along. FlowSchema is strict — since 未知键静默剥离仍是全仓默认:把 #3405 的 strict 收紧从一个 schema 推广到整个可授权面(ADR-0078 完整性闸门) #4001 an unrecognized key is a hard parse error, not a silent drop (verified: a flow carrying _uiPosition throws unrecognized_keys rather than losing it). So on flows there is nothing for a graft to protect, and "wholesale write-back loses the author's extras" is simply not a risk here.
The real hazard is the opposite direction: fossilized defaults. Writing back the step-3 body persists version, runAs, type and isDefault values the author never wrote. If any of those defaults later changes, every row this migration touched is frozen on the old value while untouched rows follow the new one — two populations with different behaviour, which is precisely the split this migration exists to remove. A migration must not become a source of the drift it cleans up.
So the write-back shape should be conversion output plus the region/edge condition envelopes, and nothing else — deliberately excluding schema defaults. That is still a selective graft, but the thing being excluded is defaults, not author extras.
(Also worth knowing: step 1 already reaches inside loop bodies — the filters → filter rewrite fired on the nested crud node too. Only the condition envelope needs steps 2–3.)
Finding 2 — the CLI cannot host this as-is
AutomationServicePlugin looks cheap to boot (dependencies: [], registers automation in init()), so adding it to bootSchemaStack's extraPlugins is the tempting route. It is not safe:registerFlow calls activateFlowTrigger(name), and the plugin's start() pulls every flow from the artifact / ObjectQL registry and registers it. Booting it for a one-shot migration would therefore arm record triggers and scheduled jobs, and start() also materializes declarative connectors (which can open MCP connections and child processes).
Two ways out:
Give the plugin an inert mode — e.g. AutomationServicePluginOptions.registerFlows: false (sibling of the existing registerRoutes: false convention), so the engine is present with its executor registry populated but no flow registered and no trigger armed. The migration needs only reservedNodeTypes, not running flows.
The second is less code; the first makes os migrate meta --stored whole. Not exclusive.
Sketch
AutomationEngine.canonicalizeStoredFlow(definition) — one public entry point running the same policy registerFlow uses, returning { flow, notices, conflicts }; registerFlow calls it so there is one source, not two copies. It returns the Finding-1 shape (conversions + condition envelopes, no defaults).
A refused rewrite (the conflict guard firing because a third party owns a since-retired node type) must surface as its own outcome — a loud failed row naming the token and its owner, never a silent skip and never a clobber. That is the whole reason the guard exists.
Adjacent, found while probing: because FlowSchema is strict, a stored flow row carrying any unrecognized key cannot be registered at all — registerFlow throws rather than degrading. Such a row is already broken at runtime, and the stored pass would report it as failed with the parse error. Worth confirming whether any real deployment has one before assuming the population is empty.
Not urgent
The read path remains the guarantee: a stored flow is canonicalized at registerFlow before parse and execution, so nothing is broken and no behaviour depends on this. Same hygiene argument #4327 makes — cleaner rows, quieter boots, and a verdict that covers everything rather than almost everything.
Refs #4327, #3903, #4001, #4347, #4453, PR #4317, ADR-0087 addenda "stored metadata replays the chain" + "the stored chain gets a finish line", ADR-0078.
Follow-up to #4327, filed per Prime Directive #10 (out-of-scope finding, named rather than silently absorbed).
Context
#4327 shipped
os migrate meta --stored: it walkssys_metadata(active + draft, all orgs), replays the ADR-0087 conversion chain, and re-saves each changed body throughsaveMetaItem, so a deployment that runs it stops carrying pre-protocol dialects at rest.It covers every metadata type except
flow, and reports each flow row asskippedwith the reason rather than counting it done:That is the same carve-out
applyConversionsToStoredItemandObjectStackProtocolImplementation.convertStoredItemalready make, and for the same reason: flow-node conversions carry ADR-0078's open-namespace conflict guard, which needsreservedNodeTypes— the engine's live executor registry plus action descriptors plus the structural node types.Why it matters
Flows are exactly the type most likely to carry legacy shapes: the graduated conversions
flow-node-crud-filter-alias,flow-node-crud-object-alias,flow-node-notify-config-aliasesandflow-node-script-config-aliasesare all flow-node entries (PD #12, protocol 17). So the type with the most stored dialect is the one type the new pass cannot end.For a deployment with stored flows:
migrateStoredMetadatareportsskippedhowever many times it is run, those rows keep re-lowering atregisterFlowon every boot, andstoredMigrationCleanistruewhile they are still legacy — deliberately, since a documented carve-out is not unfinished work, but it does mean "my metadata is on protocol N" is narrower than it reads.Finding 1 — "canonical flow" is three steps, and writing back the third would fossilize defaults
registerFlowcanonicalizes in three steps, so "exposereservedNodeTypes, runapplyConversionsToFlow, write it back" is not sufficient:Measured, not assumed. Driving a pre-17 flow (legacy
filtersat top level and inside aloopbody, bare-string edge conditions in both places) through all three steps:version,runAs, and per-edgetype/isDefault— all schema defaultscondition{ dialect: 'cel', source }(step 1 leaves both as bare strings)Two consequences, and the first corrects an earlier version of this issue:
graftNormalizedOperatorsprecedent does not transfer. That helper exists becauseparsed.dataon the view surface strips Studio-only auxiliary keys (isPinned,isDefault,sortOrder) that intentionally ride along.FlowSchemais strict — since 未知键静默剥离仍是全仓默认:把 #3405 的 strict 收紧从一个 schema 推广到整个可授权面(ADR-0078 完整性闸门) #4001 an unrecognized key is a hard parse error, not a silent drop (verified: a flow carrying_uiPositionthrowsunrecognized_keysrather than losing it). So on flows there is nothing for a graft to protect, and "wholesale write-back loses the author's extras" is simply not a risk here.version,runAs,typeandisDefaultvalues the author never wrote. If any of those defaults later changes, every row this migration touched is frozen on the old value while untouched rows follow the new one — two populations with different behaviour, which is precisely the split this migration exists to remove. A migration must not become a source of the drift it cleans up.So the write-back shape should be conversion output plus the region/edge
conditionenvelopes, and nothing else — deliberately excluding schema defaults. That is still a selective graft, but the thing being excluded is defaults, not author extras.(Also worth knowing: step 1 already reaches inside
loopbodies — thefilters → filterrewrite fired on the nested crud node too. Only theconditionenvelope needs steps 2–3.)Finding 2 — the CLI cannot host this as-is
AutomationServicePluginlooks cheap to boot (dependencies: [], registersautomationininit()), so adding it tobootSchemaStack'sextraPluginsis the tempting route. It is not safe:registerFlowcallsactivateFlowTrigger(name), and the plugin'sstart()pulls every flow from the artifact / ObjectQL registry and registers it. Booting it for a one-shot migration would therefore arm record triggers and scheduled jobs, andstart()also materializes declarative connectors (which can open MCP connections and child processes).Two ways out:
AutomationServicePluginOptions.registerFlows: false(sibling of the existingregisterRoutes: falseconvention), so the engine is present with its executor registry populated but no flow registered and no trigger armed. The migration needs onlyreservedNodeTypes, not running flows.migrateStoredMetadataalready returns a structured report a route can render as-is. This is the "or an admin route" half of [P3] os migrate meta --stored: rewrite sys_metadata rows in place so the read-path chain has a finish line #4327 that was left undone, and flows are the reason to do it.The second is less code; the first makes
os migrate meta --storedwhole. Not exclusive.Sketch
AutomationEngine.canonicalizeStoredFlow(definition)— one public entry point running the same policyregisterFlowuses, returning{ flow, notices, conflicts };registerFlowcalls it so there is one source, not two copies. It returns the Finding-1 shape (conversions + condition envelopes, no defaults).migrateStoredMetadata—canonicalizeFlow?: (def) => …, used forflowrows when supplied, falling back to today'sskippedwhen not. Deliberately not added in [P3] os migrate meta --stored: rewrite sys_metadata rows in place so the read-path chain has a finish line #4327: nothing could pass it, and an unused parameter advertises a capability the runtime does not deliver.A refused rewrite (the conflict guard firing because a third party owns a since-retired node type) must surface as its own outcome — a loud
failedrow naming the token and its owner, never a silent skip and never a clobber. That is the whole reason the guard exists.Adjacent, found while probing: because
FlowSchemais strict, a stored flow row carrying any unrecognized key cannot be registered at all —registerFlowthrows rather than degrading. Such a row is already broken at runtime, and the stored pass would report it asfailedwith the parse error. Worth confirming whether any real deployment has one before assuming the population is empty.Not urgent
The read path remains the guarantee: a stored flow is canonicalized at
registerFlowbefore parse and execution, so nothing is broken and no behaviour depends on this. Same hygiene argument #4327 makes — cleaner rows, quieter boots, and a verdict that covers everything rather than almost everything.Refs #4327, #3903, #4001, #4347, #4453, PR #4317, ADR-0087 addenda "stored metadata replays the chain" + "the stored chain gets a finish line", ADR-0078.