Skip to content

Commit bf74258

Browse files
committed
fix(spec,service-automation): ADR-0087 — wire conversions into the runtime flow-load seam + conflict guard
Close two third-party-facing risks in the P1 conversion layer. Risk 1 (data-loss regression) — retiring the executor's `filters` fallback while the conversion only ran on the build/validate seam meant a stored flow rehydrated from the DB (engine.registerFlow → FlowSchema.parse, which bypassed the conversion) kept carrying `config.filters`; with the fallback gone the executor saw an empty `filter` and a `delete_record`/`update_record` widened to the whole table. Fix: run the D2 conversion in registerFlow BEFORE parse (new applyConversionsToFlow), the runtime load seam ADR-0087 D2 calls for. Stored old-shape flows (`filters`, `webhook`/`http_request` callouts) are canonicalized on rehydration; the executor only ever sees the canonical shape. This is the gap config-aliases.ts already described (graph-lint can't reach never-republished prod flows) — now closed for `filters` by the conversion, not a fallback. Risk 2 (silent third-party clobber) — `flow.node.type` is an OPEN namespace (ADR-0018 removed the enum gate), so a retired official name could be re-registered by a third party. The node-type rename is now conflict-aware: the runtime seam passes its live executor registry as reservedNodeTypes; if a retired alias (`http_request`/`http_call`/`webhook`) is a live custom node, the rewrite is REFUSED and a loud, structured OS_METADATA_CONVERSION_CONFLICT diagnostic (node path, conversion id, rename hint) is emitted instead of a silent clobber (ADR-0078). The pure build/validate seam has no registry, so the historical alias converts as before. Mechanism/policy split kept clean: spec provides the conflict-aware conversion (reservedNodeTypes context on apply); service-automation supplies the data. The crud-config-aliases test is rewritten to prove the stored-`filters` flow now keeps its filter (the risk-1 fix); a new flow-load-conversion test covers the rehydration rename and the conflict guard. New exports are additive. Refs #2643, #2645 (ADR-0087 D2), ADR-0078. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YA9vjVpDrKLuaK6nbm1s37
1 parent 40b329e commit bf74258

11 files changed

Lines changed: 313 additions & 40 deletions

File tree

.changeset/adr-0087-p1-conversion-layer.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
'@objectstack/spec': minor
3-
'@objectstack/service-automation': patch
3+
'@objectstack/service-automation': minor
44
---
55

66
ADR-0087 P1:元数据转换层(conversion layer,D2)——大多数破坏性变更对使用方零操作。
@@ -13,6 +13,10 @@ ADR-0087 P1:元数据转换层(conversion layer,D2)——大多数破坏性变
1313
- `page-kind-jsx-to-html`:页面 `kind: 'jsx'``'html'`(ADR-0080 规范拼写)。
1414
- `flow-node-crud-filter-alias`:CRUD 流程节点 `config.filters``config.filter`
1515

16+
**运行时加载 seam(存量流程零回归的关键)。** 转换不仅接在构建/校验入口,也接到运行时 `AutomationEngine.registerFlow`(在 `FlowSchema.parse` 之前跑,新增 `applyConversionsToFlow`)。这样从数据库 rehydrate 的**存量流程**也会被规范化——否则删掉 `filters` 执行器兜底会让存量 `delete_record` / `update_record` 的过滤条件被静默清空(退化成作用于全表)。这才真正兑现 D2 “applied at load, the same seam”。
17+
18+
**开放命名空间的冲突守卫(第三方零静默误伤)。** `flow.node.type` 是开放命名空间(ADR-0018 移除了 enum gate),退役的官方名可能被第三方复用为自定义节点。转换层新增“保留名冲突”感知:运行时 seam 传入本环境已注册的执行器类型,若某退役别名(`http_request`/`http_call`/`webhook`)正被活的自定义执行器占用,则**拒绝改写并发出响亮的结构化告警 `OS_METADATA_CONVERSION_CONFLICT`**(带节点位置、conversion id、“请改名”的处置建议),而不是静默把它改成 `http` 破坏第三方节点。构建/校验入口无注册表上下文,历史别名照常转换。
19+
1620
并落实 PD #12 退役路径示范:`filters``filter` 别名从 `service-automation` 执行器的 `readAliasedConfig` 兜底中删除,提升为上面这条声明式转换条目;执行器改为直接读取规范键 `cfg.filter`
1721

18-
新增导出(纯增量,无破坏):`applyConversions``collectConversionNotices``ALL_CONVERSIONS``CONVERSIONS_BY_MAJOR``CONVERSION_NOTICE_CODE`,以及类型 `MetadataConversion``ConversionNotice``ConversionApplication``ConversionFixture``ApplyConversionsOptions``NormalizeStackInputOptions``normalizeStackInput` 现接受可选第二参 `{ onConversionNotice }`(向后兼容)。
22+
新增导出(纯增量,无破坏):`applyConversions``applyConversionsToFlow``collectConversionNotices``ALL_CONVERSIONS``CONVERSIONS_BY_MAJOR``CONVERSION_NOTICE_CODE``CONVERSION_CONFLICT_CODE`,以及类型 `MetadataConversion``ConversionNotice``ConversionApplication``ConversionFixture``ConversionContext``ConversionConflictNotice``ConversionConflictDetail``ApplyConversionsOptions``NormalizeStackInputOptions``normalizeStackInput` 现接受可选第二参 `{ onConversionNotice, convert }`(向后兼容)。

packages/services/service-automation/src/builtin/config-aliases.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,12 @@
2828
* The `filters` → `filter` alias has since been **retired from this shim** and
2929
* promoted into the ADR-0087 D2 conversion layer (`@objectstack/spec` conversion
3030
* `flow-node-crud-filter-alias`): it is rewritten to the canonical key **at
31-
* load**, so the CRUD executors read `cfg.filter` directly. That is the PD #12
32-
* retirement path the ADR prescribes — a scattered consumer-side fallback
33-
* replaced by one declared, loud, tested, expiring conversion entry. The
31+
* load** — including the runtime rehydration seam (`AutomationEngine.registerFlow`
32+
* runs the conversion before parse), which is exactly the stored-prod-flow gap
33+
* this shim describes above, now closed for `filters` by the conversion instead
34+
* of an executor fallback. So the CRUD executors read `cfg.filter` directly. That
35+
* is the PD #12 retirement path the ADR prescribes — a scattered consumer-side
36+
* fallback replaced by one declared, loud, tested, expiring conversion entry. The
3437
* remaining `object` → `objectName` alias is the next candidate to graduate.
3538
*
3639
* @see crud-nodes.ts for the remaining call site (`objectName`).

packages/services/service-automation/src/builtin/crud-config-aliases.test.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,30 +90,28 @@ describe('CRUD config-key aliases: object→objectName (executor shim) + filters
9090
expect(warns.length).toBe(before);
9191
});
9292

93-
it('no longer honors a raw `filters` alias in the executor (retired to the D2 conversion layer)', async () => {
93+
it('a stored `filters` flow is canonicalized at registerFlow, so the filter is NOT dropped', async () => {
94+
// Risk-1 regression guard (ADR-0087 D2 runtime load seam). Before the seam
95+
// was wired, retiring the executor's `filters` fallback meant a stored
96+
// `delete_record`/`get_record` flow carrying `filters` reached the executor
97+
// with an empty `filter` — silently widening the query to the whole table.
98+
// Now `registerFlow` runs the D2 conversion, so `filters`→`filter` happens
99+
// on rehydration and the stored filter survives.
94100
const engine = new AutomationEngine(silentLogger());
95101
const { data, calls } = fakeData();
96-
const warns: string[] = [];
97-
registerCrudNodes(engine, ctxWith(data, collectingLogger(warns)));
102+
registerCrudNodes(engine, ctxWith(data, silentLogger()));
98103

99-
// A flow that reached the executor WITHOUT going through the load conversion
100-
// still carries `filters`. The executor now reads only the canonical `filter`,
101-
// so `filters` is ignored and no `filters`→`filter` warning is emitted here.
102104
engine.registerFlow('gr', getRecordFlow({ objectName: 'crm_lead', filters: { id: 'L1' } }));
103105
await engine.execute('gr');
104106

105-
expect(calls[0].opts.where).toEqual({}); // `filters` no longer honored by the executor
106-
const filterWarn = warns.find((w) => w.includes("'filters'"));
107-
expect(filterWarn).toBeFalsy();
107+
expect(calls[0].opts.where).toEqual({ id: 'L1' }); // filter preserved, not dropped
108108
});
109109

110-
it('the D2 conversion at load rewrites `filters` → `filter` so the flow works end-to-end', async () => {
110+
it('the same conversion runs on the build/validate seam too (normalizeStackInput)', async () => {
111111
const engine = new AutomationEngine(silentLogger());
112112
const { data, calls } = fakeData();
113113
registerCrudNodes(engine, ctxWith(data, silentLogger()));
114114

115-
// Author with the deprecated `filters` key, then run it through the same load
116-
// seam a real stack load uses. The conversion canonicalizes it to `filter`.
117115
const raw = getRecordFlow({ objectName: 'crm_lead', filters: { id: 'L1' }, outputVariable: 'lead' });
118116
const converted = (normalizeStackInput({ flows: [raw] }).flows as any[])[0];
119117
engine.registerFlow('gr', converted);

packages/services/service-automation/src/engine.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { ExecutionLog, ActionDescriptor } from '@objectstack/spec/automatio
55
import type { AutomationContext, AutomationResult, ResumeSignal, IAutomationService, ScreenSpec } from '@objectstack/spec/contracts';
66
import type { Logger } from '@objectstack/spec/contracts';
77
import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, findRegionEntry, defineActionDescriptor } from '@objectstack/spec/automation';
8+
import { applyConversionsToFlow } from '@objectstack/spec';
89
import type { FlowRegionParsed } from '@objectstack/spec/automation';
910
import type { Connector } from '@objectstack/spec/integration';
1011
import { ConnectorSchema } from '@objectstack/spec/integration';
@@ -884,7 +885,26 @@ export class AutomationEngine implements IAutomationService {
884885
// ── IAutomationService Contract Implementation ────────
885886

886887
registerFlow(name: string, definition: unknown): void {
887-
const parsed = FlowSchema.parse(definition);
888+
// ADR-0087 D2 — the runtime load seam. A stored flow authored against an
889+
// old shape (a `webhook`/`http_request` callout node, a `delete_record`
890+
// with `config.filters`) is canonicalized on rehydration, BEFORE parse +
891+
// execution, so the executor only ever sees the canonical shape and a
892+
// dropped-alias upgrade never silently changes behavior (e.g. an empty
893+
// `filter` deleting a whole table). `reservedNodeTypes` is this engine's
894+
// live executor registry: an open-namespace node-type rename over a type
895+
// a custom executor owns becomes a loud, refused conflict — never a
896+
// silent clobber of the third party's node.
897+
const reservedNodeTypes = new Set<string>([
898+
...FLOW_STRUCTURAL_NODE_TYPES,
899+
...this.nodeExecutors.keys(),
900+
...this.actionDescriptors.keys(),
901+
]);
902+
const converted = applyConversionsToFlow(definition, {
903+
reservedNodeTypes,
904+
onNotice: (n) => this.logger.warn(`[flow '${name}'] ${n.code}: ${n.message}`),
905+
onConflict: (c) => this.logger.warn(`[flow '${name}'] ${c.code}: ${c.message}`),
906+
});
907+
const parsed = FlowSchema.parse(converted);
888908

889909
// DAG cycle detection
890910
this.detectCycles(parsed);
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ADR-0087 D2 runtime load seam: `AutomationEngine.registerFlow` canonicalizes a
5+
* stored flow's shape on rehydration, BEFORE parse + execution — so a stored
6+
* flow authored against an old protocol shape keeps running with zero action,
7+
* and dropping a deprecated executor alias never silently changes behavior.
8+
*
9+
* Open-namespace node-type renames (`webhook`/`http_request`/`http_call` → `http`)
10+
* are conflict-aware: if a live custom executor owns the retired name, the rename
11+
* is refused and a loud diagnostic is logged instead of clobbering the node.
12+
*/
13+
import { describe, it, expect } from 'vitest';
14+
import { AutomationEngine } from './engine.js';
15+
16+
function collectingLogger(warns: string[]): any {
17+
const l: any = { info() {}, warn(m: string) { warns.push(m); }, error() {}, debug() {} };
18+
l.child = () => l;
19+
return l;
20+
}
21+
22+
function flowWith(node: Record<string, unknown>) {
23+
return {
24+
name: 'f', label: 'F', type: 'autolaunched',
25+
nodes: [
26+
{ id: 'start', type: 'start', label: 'Start' },
27+
{ id: 'n', label: 'N', ...node },
28+
{ id: 'end', type: 'end', label: 'End' },
29+
],
30+
edges: [
31+
{ id: 'e1', source: 'start', target: 'n' },
32+
{ id: 'e2', source: 'n', target: 'end' },
33+
],
34+
};
35+
}
36+
37+
describe('registerFlow canonicalizes stored flows (ADR-0087 D2 runtime seam)', () => {
38+
it('rewrites a retired callout node type to `http` on rehydration', async () => {
39+
const warns: string[] = [];
40+
const engine = new AutomationEngine(collectingLogger(warns));
41+
engine.registerFlow('f', flowWith({ id: 'n', type: 'http_request', config: { url: 'https://x' } }));
42+
43+
const stored = await engine.getFlow('f');
44+
expect(stored?.nodes.find((n) => n.id === 'n')?.type).toBe('http');
45+
expect(warns.some((w) => w.includes("'http_request' → 'http'"))).toBe(true);
46+
});
47+
48+
it('leaves a canonical flow untouched (no notice)', async () => {
49+
const warns: string[] = [];
50+
const engine = new AutomationEngine(collectingLogger(warns));
51+
engine.registerFlow('f', flowWith({ id: 'n', type: 'http', config: { url: 'https://x' } }));
52+
53+
const stored = await engine.getFlow('f');
54+
expect(stored?.nodes.find((n) => n.id === 'n')?.type).toBe('http');
55+
expect(warns.some((w) => w.includes('OS_METADATA_CONVERTED') || w.includes('→'))).toBe(false);
56+
});
57+
58+
it('refuses to rewrite a retired name a live custom executor owns, logging a conflict', async () => {
59+
const warns: string[] = [];
60+
const engine = new AutomationEngine(collectingLogger(warns));
61+
62+
// A third party registers a custom node under the retired official name.
63+
let ran = false;
64+
engine.registerNodeExecutor({
65+
type: 'webhook',
66+
async execute() { ran = true; return { success: true, output: {} }; },
67+
});
68+
69+
engine.registerFlow('f', flowWith({ id: 'n', type: 'webhook', config: {} }));
70+
71+
const stored = await engine.getFlow('f');
72+
// NOT clobbered to `http` — the custom node survives.
73+
expect(stored?.nodes.find((n) => n.id === 'n')?.type).toBe('webhook');
74+
expect(warns.some((w) => w.includes('OS_METADATA_CONVERSION_CONFLICT'))).toBe(true);
75+
76+
await engine.execute('f');
77+
expect(ran).toBe(true); // the custom executor actually ran
78+
});
79+
});

packages/spec/api-surface.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@
1313
"BUILTIN_IDENTITY_PLATFORM_ADMIN (const)",
1414
"BuiltinIdentityName (type)",
1515
"CONVERSIONS_BY_MAJOR (const)",
16+
"CONVERSION_CONFLICT_CODE (const)",
1617
"CONVERSION_NOTICE_CODE (const)",
1718
"ComposeStacksOptions (type)",
1819
"ComposeStacksOptionsSchema (const)",
1920
"ConflictStrategy (type)",
2021
"ConflictStrategySchema (const)",
2122
"ConversionApplication (interface)",
23+
"ConversionConflictDetail (interface)",
24+
"ConversionConflictNotice (interface)",
25+
"ConversionContext (interface)",
2226
"ConversionFixture (interface)",
2327
"ConversionNotice (interface)",
2428
"CronExpressionInputSchema (const)",
@@ -91,6 +95,7 @@
9195
"Tool (type)",
9296
"ViewKeyCollision (interface)",
9397
"applyConversions (function)",
98+
"applyConversionsToFlow (function)",
9499
"applyMetaMigrations (function)",
95100
"cel (function)",
96101
"collectConversionNotices (function)",

packages/spec/src/conversions/apply.ts

Lines changed: 75 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,13 @@
1212
*/
1313

1414
import { ALL_CONVERSIONS } from './registry.js';
15-
import { CONVERSION_NOTICE_CODE, type ConversionNotice } from './types.js';
15+
import {
16+
CONVERSION_CONFLICT_CODE,
17+
CONVERSION_NOTICE_CODE,
18+
type ConversionConflictNotice,
19+
type ConversionContext,
20+
type ConversionNotice,
21+
} from './types.js';
1622

1723
export interface ApplyConversionsOptions {
1824
/**
@@ -21,6 +27,18 @@ export interface ApplyConversionsOptions {
2127
* choice. `objectstack validate` passes a sink that prints them.
2228
*/
2329
onNotice?: (notice: ConversionNotice) => void;
30+
/**
31+
* Sink for each structured **conflict** — a rename refused because its old
32+
* token is a live name in this environment (see {@link ConversionContext}).
33+
* Populated by the runtime load seam; absent on the build/validate seam.
34+
*/
35+
onConflict?: (notice: ConversionConflictNotice) => void;
36+
/**
37+
* Node types that are live in this environment. Supplied by the runtime load
38+
* seam so open-namespace renames can detect a collision with a live owner
39+
* rather than silently clobber it.
40+
*/
41+
reservedNodeTypes?: ReadonlySet<string>;
2442
}
2543

2644
/**
@@ -35,34 +53,71 @@ export function applyConversions(
3553
stack: Record<string, unknown>,
3654
options: ApplyConversionsOptions = {},
3755
): Record<string, unknown> {
38-
const { onNotice } = options;
56+
const { onNotice, onConflict, reservedNodeTypes } = options;
3957
let current = stack;
4058

4159
for (const conversion of ALL_CONVERSIONS) {
4260
const retiresIn = conversion.toMajor + 1;
43-
current = conversion.apply(current, (detail) => {
44-
if (!onNotice) return;
45-
onNotice({
46-
code: CONVERSION_NOTICE_CODE,
47-
conversionId: conversion.id,
48-
surface: conversion.surface,
49-
toMajor: conversion.toMajor,
50-
retiresIn,
51-
from: detail.from,
52-
to: detail.to,
53-
path: detail.path,
54-
message:
55-
`[protocol] converted ${conversion.surface} at ${detail.path}: ` +
56-
`'${detail.from}' → '${detail.to}' (deprecated; ADR-0087 conversion ` +
57-
`'${conversion.id}', retires from the load path in protocol ${retiresIn}). ` +
58-
`Update the source to '${detail.to}'.`,
59-
});
60-
});
61+
const context: ConversionContext = {
62+
reservedNodeTypes,
63+
reportConflict: onConflict
64+
? (detail) =>
65+
onConflict({
66+
code: CONVERSION_CONFLICT_CODE,
67+
conversionId: conversion.id,
68+
surface: conversion.surface,
69+
token: detail.token,
70+
path: detail.path,
71+
message: `[protocol] ${detail.reason} (ADR-0087 conversion '${conversion.id}').`,
72+
})
73+
: undefined,
74+
};
75+
current = conversion.apply(
76+
current,
77+
(detail) => {
78+
if (!onNotice) return;
79+
onNotice({
80+
code: CONVERSION_NOTICE_CODE,
81+
conversionId: conversion.id,
82+
surface: conversion.surface,
83+
toMajor: conversion.toMajor,
84+
retiresIn,
85+
from: detail.from,
86+
to: detail.to,
87+
path: detail.path,
88+
message:
89+
`[protocol] converted ${conversion.surface} at ${detail.path}: ` +
90+
`'${detail.from}' → '${detail.to}' (deprecated; ADR-0087 conversion ` +
91+
`'${conversion.id}', retires from the load path in protocol ${retiresIn}). ` +
92+
`Update the source to '${detail.to}'.`,
93+
});
94+
},
95+
context,
96+
);
6197
}
6298

6399
return current;
64100
}
65101

102+
/**
103+
* Apply the conversion pass to a **single flow definition** — the shape the
104+
* runtime automation engine loads one at a time via `registerFlow`.
105+
*
106+
* This is the runtime load seam ADR-0087 D2 calls for: a stored flow authored
107+
* against an old shape (e.g. a `delete_record` node with `config.filters`, or a
108+
* `webhook` callout node) is canonicalized on rehydration, so the executor only
109+
* ever sees the canonical shape. Callers pass `reservedNodeTypes` (their live
110+
* executor registry) so an open-namespace rename over a live custom node becomes
111+
* a reported conflict, not a silent clobber. Non-object input is returned
112+
* unchanged.
113+
*/
114+
export function applyConversionsToFlow<T>(flow: T, options: ApplyConversionsOptions = {}): T {
115+
if (flow == null || typeof flow !== 'object' || Array.isArray(flow)) return flow;
116+
const converted = applyConversions({ flows: [flow as Record<string, unknown>] }, options);
117+
const flows = converted.flows;
118+
return Array.isArray(flows) && flows.length > 0 ? (flows[0] as T) : flow;
119+
}
120+
66121
/**
67122
* Collect the notices a stack would emit without needing an external sink —
68123
* convenience for `validate` / `lint` / the future MCP `spec_deprecations` tool.

packages/spec/src/conversions/conversions.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,36 @@ describe('conversion layer (ADR-0087 D2)', () => {
7878
});
7979
});
8080

81+
describe('flow-node-http-callout-rename — reserved-name conflict guard', () => {
82+
it('refuses to rewrite an alias a live executor owns, reporting a conflict', () => {
83+
const notices: ConversionNotice[] = [];
84+
const conflicts: { token: string; path: string; conversionId: string }[] = [];
85+
const out = applyConversions(
86+
{ flows: [{ name: 'f', nodes: [{ id: 'a', type: 'webhook' }] }] },
87+
{
88+
onNotice: (n) => notices.push(n),
89+
onConflict: (c) => conflicts.push({ token: c.token, path: c.path, conversionId: c.conversionId }),
90+
reservedNodeTypes: new Set(['webhook']), // a third-party custom node owns this name
91+
},
92+
);
93+
// Not rewritten — the custom node is preserved.
94+
expect((out.flows as any[])[0].nodes[0].type).toBe('webhook');
95+
expect(notices).toHaveLength(0);
96+
expect(conflicts).toEqual([
97+
{ token: 'webhook', path: 'flows[0].nodes[0].type', conversionId: 'flow-node-http-callout-rename' },
98+
]);
99+
});
100+
101+
it('converts normally when the alias is not a live type (build/validate seam)', () => {
102+
// No reservedNodeTypes → the historical alias converts as usual.
103+
const { stack, notices } = collectConversionNotices({
104+
flows: [{ name: 'f', nodes: [{ id: 'a', type: 'webhook' }] }],
105+
});
106+
expect((stack.flows as any[])[0].nodes[0].type).toBe('http');
107+
expect(notices).toHaveLength(1);
108+
});
109+
});
110+
81111
describe('flow-node-crud-filter-alias (PD #12 retirement)', () => {
82112
it('renames config.filters → config.filter only for CRUD node types', () => {
83113
const { stack, notices } = collectConversionNotices({

0 commit comments

Comments
 (0)