diff --git a/.changeset/connector-action-declared-effect.md b/.changeset/connector-action-declared-effect.md new file mode 100644 index 0000000000..a11a76a829 --- /dev/null +++ b/.changeset/connector-action-declared-effect.md @@ -0,0 +1,50 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-automation": minor +--- + +feat(integration): 连接器动作可以声明它在上游做了什么，`connector_action` 因此能被计数 (#4395) + +#4354 给每次流程运行加上了 `selected` / `acted` 汇总，断扫告警是 +`selected > 0 AND acted = 0 AND unmeasured = 0`。`connector_action` 当时只能给出三个 +答案里最诚实的那个：`ConnectorActionSchema` 只描述动作的**形状**（`key` / `label` / +`inputSchema` / `outputSchema`），对它究竟读还是写只字未提，所以 `crm.push_opportunity` +和 `crm.lookup_account` 在运行时完全无法区分。`acted: 0` 会低报一次 Salesforce 创建， +让每一条健康的连接器扫描都触发告警，操作员很快学会忽略它；`acted: 1` 会高报一次查询， +让告警永不触发——那正是 #4354 要修的原始 bug 换个楼层重演。于是执行器报 +`metrics: { unmeasuredEffect: true }`，运行汇总记一笔 `unmeasured`。 + +诚实，但也是盲区：**任何走连接器的自动化流程都贡献不出任何信号**——既无法证明自己 +干过活，也无法在停止干活时被标记出来。 + +**现在动作可以自己声明。** `ConnectorActionSchema` 新增可选的 `effect`： + +```ts +actions: [ + { key: 'push_opportunity', label: 'Push Opportunity', effect: 'write' }, + { key: 'lookup_account', label: 'Lookup Account', effect: 'read' }, + { key: 'legacy_action', label: 'Legacy' }, // 不声明 —— 行为完全不变 +] +``` + +`connector_action` 执行器据此计数：声明 `write` 且派发成功 → `acted: 1`；声明 `read` +→ `acted: 0`（这是一个**真实测得的零**，不是耸肩，所以只做查询的流程重新落入断扫告警 +的射程）；不声明 → 维持原样 `unmeasuredEffect`。派发失败时，声明 `write` 的动作回落为 +不可计数而非零——处理器抛错时上游可能已经写成了，这与 `http` 节点对被拒绝的写请求做的 +判断一致；声明 `read` 的动作则仍报 `acted: 0`，它无论如何都不可能改动任何东西。 + +声明是可选的，这是有意为之：**已有的连接器一个字都不用改，报告的内容与之前逐字相同**， +声明它是纯增益而不是一次迁移。`unmeasuredEffect` 的含义和消费者一个都没变，它现在是 +兜底而不是唯一答案。 + +同一个声明也随 `ConnectorActionDescriptor` 一路送到设计器：`GET /api/v1/automation/connectors` +现在会带上 `effect`，作者在流程设计器里挑动作时，"这个会写" 是关于这次选择的事实。 + +`effect` 落在**可作者化的** `ConnectorActionSchema` 上，而不只是描述符接口上，因为那是 +唯一可能的产地：`AutomationEngine.registerConnector` 存的是 `ConnectorSchema.parse(def)` +的结果，描述符是从这份 def 投影出来的。插件注册路径和 ADR-0097 声明式 materialization +路径都经过这一次 parse，所以两条路都能声明；只加在描述符上则永远无法被任何东西填充 +（`ConnectorSchema` 是非 strict 的 `z.object`，改动前作者写下的 `effect` 会被静默丢弃）。 + +bulk 场景的**计数型**效果（一次动作报告它在上游碰了多少条记录）暂不做，等真实需求。 +读/写这一刀才是解开告警的那一刀。 diff --git a/content/docs/references/index.mdx b/content/docs/references/index.mdx index d324715fdc..a8ac7b7839 100644 --- a/content/docs/references/index.mdx +++ b/content/docs/references/index.mdx @@ -1,6 +1,6 @@ --- title: Protocol Reference -description: Every schema published by @objectstack/spec — 1609 schemas across 14 protocol modules +description: Every schema published by @objectstack/spec — 1610 schemas across 14 protocol modules --- {/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} @@ -25,7 +25,7 @@ counts are sums of the rows they head. Regenerate with | [Cloud Protocol](/docs/references/cloud) | 11 | 94 | Environments, packages and versions, marketplace, developer portal, tenancy. | | [Data Protocol](/docs/references/data) | 29 | 164 | Objects, fields, queries, filters, datasources and drivers — the ObjectQL layer. | | [Identity Protocol](/docs/references/identity) | 5 | 28 | Users and accounts, organizations, positions, API keys, SCIM provisioning. | -| [Integration Protocol](/docs/references/integration) | 1 | 26 | The single connector protocol (ADR-0097) — catalog descriptors and provider-bound instances. | +| [Integration Protocol](/docs/references/integration) | 1 | 27 | The single connector protocol (ADR-0097) — catalog descriptors and provider-bound instances. | | [Kernel Protocol](/docs/references/kernel) | 31 | 187 | Plugin lifecycle and manifests, capabilities and security, metadata loading, service registry. | | [Qa Protocol](/docs/references/qa) | 1 | 8 | Declarative test suites — scenarios, steps, actions and assertions. | | [Security Protocol](/docs/references/security) | 5 | 27 | Permission sets, row-level security, sharing rules, tenancy posture. | @@ -33,7 +33,7 @@ counts are sums of the rows they head. Regenerate with | [Studio Protocol](/docs/references/studio) | 3 | 35 | Studio designer metadata — the authoring surfaces for the protocols above. | | [System Protocol](/docs/references/system) | 37 | 295 | The runtime environment — logging, jobs, cache, metrics, notifications, i18n and compliance. | | [UI Protocol](/docs/references/ui) | 17 | 155 | Apps, pages, views, dashboards, reports, actions and themes — the ObjectUI layer. | -| **Total** | **201** | **1609** | 14 protocol modules | +| **Total** | **201** | **1610** | 14 protocol modules | --- @@ -203,13 +203,13 @@ Users and accounts, organizations, positions, API keys, SCIM provisioning. ## Integration Protocol -**Source:** `packages/spec/src/integration/` · **Import:** `@objectstack/spec/integration` · **1 page, 26 schemas** +**Source:** `packages/spec/src/integration/` · **Import:** `@objectstack/spec/integration` · **1 page, 27 schemas** The single connector protocol (ADR-0097) — catalog descriptors and provider-bound instances. | File | Schemas | | :--- | :--- | -| [`connector.zod.ts`](/docs/references/integration/connector) | `CircuitBreakerConfig`, `Connector`, `ConnectorAction`, `ConnectorConflictResolution`, `ConnectorErrorCategory`, `ConnectorFieldMapping`, `ConnectorHealth`, `ConnectorInstanceAPIKeyAuth`, `ConnectorInstanceAuth`, `ConnectorInstanceBasicAuth`, `ConnectorInstanceBearerAuth`, `ConnectorInstanceNoAuth`, `ConnectorRetryStrategy`, `ConnectorStatus`, `ConnectorTrigger`, `ConnectorType`, `DataSyncConfig`, `DeclarativeConnectorEntry`, `ErrorMappingConfig`, `ErrorMappingRule`, `HealthCheckConfig`, `RetryConfig`, `SyncStrategy`, `WebhookConfig`, `WebhookEvent`, `WebhookSignatureAlgorithm` | +| [`connector.zod.ts`](/docs/references/integration/connector) | `CircuitBreakerConfig`, `Connector`, `ConnectorAction`, `ConnectorActionEffect`, `ConnectorConflictResolution`, `ConnectorErrorCategory`, `ConnectorFieldMapping`, `ConnectorHealth`, `ConnectorInstanceAPIKeyAuth`, `ConnectorInstanceAuth`, `ConnectorInstanceBasicAuth`, `ConnectorInstanceBearerAuth`, `ConnectorInstanceNoAuth`, `ConnectorRetryStrategy`, `ConnectorStatus`, `ConnectorTrigger`, `ConnectorType`, `DataSyncConfig`, `DeclarativeConnectorEntry`, `ErrorMappingConfig`, `ErrorMappingRule`, `HealthCheckConfig`, `RetryConfig`, `SyncStrategy`, `WebhookConfig`, `WebhookEvent`, `WebhookSignatureAlgorithm` | --- diff --git a/content/docs/references/integration/connector.mdx b/content/docs/references/integration/connector.mdx index 229a500d9e..9f947505d4 100644 --- a/content/docs/references/integration/connector.mdx +++ b/content/docs/references/integration/connector.mdx @@ -134,8 +134,8 @@ a dead end of the same class in #4738.) ## TypeScript Usage ```typescript -import { CircuitBreakerConfigSchema, ConnectorSchema, ConnectorActionSchema, ConnectorConflictResolutionSchema, ConnectorErrorCategorySchema, ConnectorFieldMappingSchema, ConnectorHealthSchema, ConnectorInstanceAPIKeyAuthSchema, ConnectorInstanceAuthSchema, ConnectorInstanceBasicAuthSchema, ConnectorInstanceBearerAuthSchema, ConnectorInstanceNoAuthSchema, ConnectorRetryStrategySchema, ConnectorStatusSchema, ConnectorTriggerSchema, ConnectorTypeSchema, DataSyncConfigSchema, DeclarativeConnectorEntrySchema, ErrorMappingConfigSchema, ErrorMappingRuleSchema, HealthCheckConfigSchema, RetryConfigSchema, SyncStrategySchema, WebhookConfigSchema, WebhookEventSchema, WebhookSignatureAlgorithmSchema } from '@objectstack/spec/integration'; -import type { CircuitBreakerConfig, Connector, ConnectorConflictResolution, ConnectorErrorCategory, ConnectorFieldMapping, ConnectorHealth, ConnectorInstanceAuth, ConnectorRetryStrategy, ConnectorStatus, ConnectorType, DataSyncConfig, DeclarativeConnectorEntry, ErrorMappingConfig, ErrorMappingRule, HealthCheckConfig, RetryConfig, SyncStrategy, WebhookConfig, WebhookEvent, WebhookSignatureAlgorithm } from '@objectstack/spec/integration'; +import { CircuitBreakerConfigSchema, ConnectorSchema, ConnectorActionSchema, ConnectorActionEffectSchema, ConnectorConflictResolutionSchema, ConnectorErrorCategorySchema, ConnectorFieldMappingSchema, ConnectorHealthSchema, ConnectorInstanceAPIKeyAuthSchema, ConnectorInstanceAuthSchema, ConnectorInstanceBasicAuthSchema, ConnectorInstanceBearerAuthSchema, ConnectorInstanceNoAuthSchema, ConnectorRetryStrategySchema, ConnectorStatusSchema, ConnectorTriggerSchema, ConnectorTypeSchema, DataSyncConfigSchema, DeclarativeConnectorEntrySchema, ErrorMappingConfigSchema, ErrorMappingRuleSchema, HealthCheckConfigSchema, RetryConfigSchema, SyncStrategySchema, WebhookConfigSchema, WebhookEventSchema, WebhookSignatureAlgorithmSchema } from '@objectstack/spec/integration'; +import type { CircuitBreakerConfig, Connector, ConnectorActionEffect, ConnectorConflictResolution, ConnectorErrorCategory, ConnectorFieldMapping, ConnectorHealth, ConnectorInstanceAuth, ConnectorRetryStrategy, ConnectorStatus, ConnectorType, DataSyncConfig, DeclarativeConnectorEntry, ErrorMappingConfig, ErrorMappingRule, HealthCheckConfig, RetryConfig, SyncStrategy, WebhookConfig, WebhookEvent, WebhookSignatureAlgorithm } from '@objectstack/spec/integration'; // Validate data const result = CircuitBreakerConfigSchema.parse(data); @@ -205,6 +205,19 @@ Circuit breaker configuration | **description** | `string` | optional | | | **inputSchema** | `Record` | optional | Input parameters schema (JSON Schema) | | **outputSchema** | `Record` | optional | Output schema (JSON Schema) | +| **effect** | `Enum<'read' \| 'write'>` | optional | What the action does upstream: 'read' never mutates (reports acted:0); 'write' does (a successful dispatch reports acted:1). Omit when the effect is not knowable — the step is then reported as unmeasured, not as zero | + + +--- + +## ConnectorActionEffect + +What the action does upstream: 'read' never mutates (reports acted:0); 'write' does (a successful dispatch reports acted:1). Omit when the effect is not knowable — the step is then reported as unmeasured, not as zero + +### Allowed Values + +* `read` +* `write` --- diff --git a/packages/services/service-automation/src/builtin/connector-nodes.test.ts b/packages/services/service-automation/src/builtin/connector-nodes.test.ts index 5fbc665be6..b71b5462ee 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.test.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.test.ts @@ -223,6 +223,153 @@ describe('connector_action (baseline node)', () => { }); }); +// ─── Declared upstream effect (#4395) ──────────────────────────────── + +/** + * The three answers a `connector_action` step can give, each driven through the + * REAL executor: register a connector whose action declares (or omits) `effect`, + * run a one-node flow, and read #4354's run summary — the actual consumer of + * these metrics, and what the broken-sweep alert + * (`selected > 0 AND acted = 0 AND unmeasured = 0`) queries. + * + * A one-node flow makes the summary fully discriminating: + * declared write, dispatched → acted 1, unmeasured 0 + * declared read → acted 0, unmeasured 0 + * undeclared → acted 0, unmeasured 1 + */ +describe('connector_action declared effect (#4395)', () => { + /** A connector whose single `run` action declares `effect` (or omits it). */ + function connectorDeclaring(effect?: 'read' | 'write'): Connector { + return { + name: 'crm', + label: 'CRM', + type: 'saas', + authentication: { type: 'none' }, + actions: [{ key: 'run', label: 'Run', ...(effect ? { effect } : {}) }], + } as Connector; + } + + /** One-node flow dispatching `crm.run`. */ + function registerCallerFlow(engine: AutomationEngine): void { + engine.registerFlow('caller', { + name: 'caller', + label: 'Caller', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'call', + type: 'connector_action', + label: 'Call', + connectorConfig: { connectorId: 'crm', actionId: 'run', input: {} }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'end' }, + ], + }); + } + + async function runWith( + effect: 'read' | 'write' | undefined, + handler: () => Promise>, + ) { + const engine = new AutomationEngine(createTestLogger()); + registerConnectorNodes(engine, createCtx()); + engine.registerConnector(connectorDeclaring(effect), { run: handler }); + registerCallerFlow(engine); + return engine.execute('caller'); + } + + const ok = async () => ({ id: 'ext_1' }); + const boom = async () => { throw new Error('upstream refused'); }; + + it('declared write + successful dispatch → acted: 1 (the sweep can prove it worked)', async () => { + const result = await runWith('write', ok); + expect(result.success).toBe(true); + expect(result.summary).toMatchObject({ acted: 1, unmeasured: 0 }); + }); + + it('declared read → acted: 0, and a REAL zero (unmeasured stays 0)', async () => { + const result = await runWith('read', ok); + expect(result.success).toBe(true); + // The distinction that matters: `acted: 0` here is a measurement, not a + // shrug — so a flow whose only action is a lookup is correctly eligible + // for the broken-sweep alert instead of hiding behind `unmeasured`. + expect(result.summary).toMatchObject({ acted: 0, unmeasured: 0 }); + }); + + it('undeclared → unchanged pre-#4395 behaviour: unmeasured, never acted: 0', async () => { + const result = await runWith(undefined, ok); + expect(result.success).toBe(true); + expect(result.summary).toMatchObject({ acted: 0, unmeasured: 1 }); + }); + + it('declared write whose dispatch FAILED is uncountable, not zero', async () => { + // The handler threw, but the upstream may already have been reached — + // same call the `http` node makes for a rejected mutating request. + const result = await runWith('write', boom); + expect(result.success).toBe(false); + expect(result.summary).toMatchObject({ acted: 0, unmeasured: 1 }); + }); + + it('declared read that failed still reports acted: 0 — it could not have mutated', async () => { + const result = await runWith('read', boom); + expect(result.success).toBe(false); + expect(result.summary).toMatchObject({ acted: 0, unmeasured: 0 }); + }); + + it('resolves the declaration per ACTION, not per connector', async () => { + const engine = new AutomationEngine(createTestLogger()); + registerConnectorNodes(engine, createCtx()); + engine.registerConnector( + { + name: 'crm', + label: 'CRM', + type: 'saas', + authentication: { type: 'none' }, + actions: [ + { key: 'push', label: 'Push', effect: 'write' }, + { key: 'lookup', label: 'Lookup', effect: 'read' }, + { key: 'legacy', label: 'Legacy' }, + ], + } as Connector, + { push: ok, lookup: ok, legacy: ok }, + ); + expect(engine.resolveConnectorActionEffect('crm', 'push')).toBe('write'); + expect(engine.resolveConnectorActionEffect('crm', 'lookup')).toBe('read'); + expect(engine.resolveConnectorActionEffect('crm', 'legacy')).toBeUndefined(); + // Unknown connector / unknown action are the same undeclared answer, + // never a throw: the executor already refuses those with its own error. + expect(engine.resolveConnectorActionEffect('crm', 'ghost')).toBeUndefined(); + expect(engine.resolveConnectorActionEffect('ghost', 'push')).toBeUndefined(); + }); + + it('serves the declaration to the designer through GET /connectors', async () => { + const engine = new AutomationEngine(createTestLogger()); + engine.registerConnector( + { + name: 'crm', + label: 'CRM', + type: 'saas', + authentication: { type: 'none' }, + actions: [ + { key: 'push', label: 'Push', effect: 'write' }, + { key: 'legacy', label: 'Legacy' }, + ], + } as Connector, + { push: ok, legacy: ok }, + ); + const [descriptor] = engine.getConnectorDescriptors(); + expect(descriptor.actions.map((a) => [a.key, a.effect])).toEqual([ + ['push', 'write'], + ['legacy', undefined], + ]); + }); +}); + // ─── Engine connector registry ─────────────────────────────────────── describe('AutomationEngine connector registry', () => { diff --git a/packages/services/service-automation/src/builtin/connector-nodes.ts b/packages/services/service-automation/src/builtin/connector-nodes.ts index b6350759d4..9c8fc3804a 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.ts @@ -86,25 +86,49 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont logger: ctx.logger, }; + // #4354 / #4395 — what this step reports depends on what the + // connector DECLARED the action does upstream, because nothing on + // this side can observe it. + // + // Undeclared was the only answer #4354 could give: `acted: 0` would + // understate a Salesforce create, `acted: 1` would overstate a + // lookup and make the broken-sweep alert never fire (the original + // bug back again), so it reported the honest third answer — this + // run's `acted` is incomplete. That answer is correct but blind: + // a connector-driven flow contributed NO signal, neither confirming + // work nor being flagged for doing none. + // + // #4395 lets the action declare `effect`, and this is where the + // declaration is spent. Resolved BEFORE dispatch so both the + // success and the failure path read one value. + const effect = engine.resolveConnectorActionEffect(cfg.connectorId, cfg.actionId); + try { const output = await handler((cfg.input ?? {}) as Record, handlerCtx); - // #4354 — the action reached an external system and the platform - // cannot say what it did there: `ConnectorActionDescriptor` - // declares `key` / `label` / `description` / `inputSchema` / - // `outputSchema` and NOTHING about whether the action reads or - // writes. `acted: 0` would understate a Salesforce create; - // `acted: 1` would overstate a lookup and make the broken-sweep - // alert never fire — the original bug back again. Report the - // honest third answer: this run's `acted` is incomplete. - // #4395 proposes declaring the effect kind on the descriptor, - // which would turn this into a real count. - return { success: true, output, metrics: { unmeasuredEffect: true } }; + return { + success: true, + output, + // A declared `write` the upstream ACCEPTED is one effect; a + // declared `read` never mutates, so its zero is real rather + // than a guess. Undeclared stays uncountable — same as the + // `http` node's method-derived split (a GET reports 0, an + // accepted mutation reports 1). + metrics: effect === 'write' + ? { acted: 1 } + : effect === 'read' + ? { acted: 0 } + : { unmeasuredEffect: true }, + }; } catch (err) { return { success: false, error: `connector_action(${cfg.connectorId}.${cfg.actionId}) failed: ${(err as Error).message}`, - // A handler that threw may still have reached the upstream. - metrics: { unmeasuredEffect: true }, + // A handler that threw may still have reached the upstream — + // so a declared WRITE falls back to uncountable here rather + // than to `acted: 0`, exactly as a rejected mutating `http` + // call does. A declared READ is the one case a failure still + // answers: it could not have mutated anything either way. + metrics: effect === 'read' ? { acted: 0 } : { unmeasuredEffect: true }, }; } }, diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 5069c3a281..cd975f0425 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -24,6 +24,7 @@ import { applyConversionsToFlow, type ConversionNotice, type ConversionConflictN import type { FlowRegionParsed } from '@objectstack/spec/automation'; import type { Connector, + ConnectorActionEffect, ConnectorProviderFactory, ConnectorOrigin, ConnectorState, @@ -1817,6 +1818,23 @@ export class AutomationEngine implements IAutomationService { return this.connectors.get(connectorId)?.handlers[actionId]; } + /** + * Resolve what a connector action does upstream (#4395), so the + * `connector_action` node can COUNT its step instead of reporting it as + * uncountable. `undefined` means the connector declared nothing — the + * honest third answer, and still the default. + * + * Read from the registered connector's stored `def` rather than from + * {@link getConnectorDescriptors}: the def is what `registerConnector` + * validated through `ConnectorSchema`, and the descriptor list is a + * projection of it built for HTTP discovery. Both the plugin path and the + * ADR-0097 declarative materialization path store their def here, so one + * lookup covers both origins. + */ + resolveConnectorActionEffect(connectorId: string, actionId: string): ConnectorActionEffect | undefined { + return this.connectors.get(connectorId)?.def.actions?.find((a) => a.key === actionId)?.effect; + } + /** * Wire the engine to the host's named-function registry (#1870). The * automation plugin calls this in `start()` with a resolver backed by @@ -1955,6 +1973,11 @@ export class AutomationEngine implements IAutomationService { description: a.description, inputSchema: a.inputSchema, outputSchema: a.outputSchema, + // #4395 — the declared upstream effect travels to the designer + // too, not only to the executor: `GET /connectors` is where a + // flow author picks the action, and "this one writes" is a fact + // about the pick. + effect: a.effect, })), })); } diff --git a/packages/services/service-automation/src/run-summary.test.ts b/packages/services/service-automation/src/run-summary.test.ts index e699ea2aaa..1b4ed7cb3a 100644 --- a/packages/services/service-automation/src/run-summary.test.ts +++ b/packages/services/service-automation/src/run-summary.test.ts @@ -731,7 +731,7 @@ describe('uncountable effects (#4354 follow-up)', () => { expect(calls).toEqual(['GET', 'POST', 'PUT']); }); - it('a connector_action is unmeasured — its descriptor declares no read/write', async () => { + it('a connector_action that declares no effect is unmeasured (#4395: still the default)', async () => { const logger = makeLogger(); const engine = new AutomationEngine(logger); registerConnectorNodes(engine, { logger, getService: () => undefined } as never); @@ -755,6 +755,40 @@ describe('uncountable effects (#4354 follow-up)', () => { expect(res.summary).toMatchObject({ acted: 0, unmeasured: 1 }); }); + it('a connector_action that DECLARES its effect is counted (#4395)', async () => { + // The same flow as above, with the one difference #4395 introduces: + // the action says what it does upstream, so the run stops reporting + // "cannot count" and reports the count. This is the whole point — a + // connector-driven sweep can now both prove it worked (`acted`) and be + // flagged when it stops (`acted: 0` with `unmeasured: 0` matches the + // broken-sweep query, which a blanket `unmeasuredEffect` never could). + for (const [effect, expected] of [ + ['write', { acted: 1, unmeasured: 0 }], + ['read', { acted: 0, unmeasured: 0 }], + ] as const) { + const logger = makeLogger(); + const engine = new AutomationEngine(logger); + registerConnectorNodes(engine, { logger, getService: () => undefined } as never); + engine.registerConnector( + { name: 'crm', label: 'CRM', type: 'saas', actions: [{ key: 'push', label: 'Push', effect }] } as never, + { push: async () => ({ id: 'ext_1' }) }, + ); + engine.registerFlow('f', { + name: 'f', label: 'f', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'S' }, + { id: 'push', type: 'connector_action', label: 'P', config: { connectorId: 'crm', actionId: 'push', input: {} } }, + { id: 'end', type: 'end', label: 'E' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'push' }, { id: 'e2', source: 'push', target: 'end' }], + } as never); + + const res = await engine.execute('f', {} as AutomationContext); + expect(res.success, `effect ${effect}`).toBe(true); + expect(res.summary, `effect ${effect}`).toMatchObject(expected); + } + }); + it('propagates through a subflow roll-up, so the parent knows its acted is incomplete', async () => { const logger = makeLogger(); const engine = new AutomationEngine(logger); diff --git a/packages/spec/api-surface/integration.json b/packages/spec/api-surface/integration.json index e9bf4091d9..031bd24d38 100644 --- a/packages/spec/api-surface/integration.json +++ b/packages/spec/api-surface/integration.json @@ -8,6 +8,8 @@ "CircuitBreakerConfigSchema (const)", "Connector (type)", "ConnectorActionDescriptor (interface)", + "ConnectorActionEffect (type)", + "ConnectorActionEffectSchema (const)", "ConnectorActionSchema (const)", "ConnectorConflictResolution (type)", "ConnectorConflictResolutionSchema (const)", diff --git a/packages/spec/authorable-surface/integration.json b/packages/spec/authorable-surface/integration.json index 06042c028a..fa287d4e93 100644 --- a/packages/spec/authorable-surface/integration.json +++ b/packages/spec/authorable-surface/integration.json @@ -32,6 +32,7 @@ "integration/Connector:type", "integration/Connector:webhooks", "integration/ConnectorAction:description", + "integration/ConnectorAction:effect", "integration/ConnectorAction:inputSchema", "integration/ConnectorAction:key", "integration/ConnectorAction:label", diff --git a/packages/spec/json-schema.manifest/integration.json b/packages/spec/json-schema.manifest/integration.json index 2166b0a6eb..29685e1189 100644 --- a/packages/spec/json-schema.manifest/integration.json +++ b/packages/spec/json-schema.manifest/integration.json @@ -5,6 +5,7 @@ "integration/CircuitBreakerConfig", "integration/Connector", "integration/ConnectorAction", + "integration/ConnectorActionEffect", "integration/ConnectorConflictResolution", "integration/ConnectorErrorCategory", "integration/ConnectorFieldMapping", diff --git a/packages/spec/src/automation/execution.zod.ts b/packages/spec/src/automation/execution.zod.ts index 793ab890a2..a3a6731863 100644 --- a/packages/spec/src/automation/execution.zod.ts +++ b/packages/spec/src/automation/execution.zod.ts @@ -53,12 +53,21 @@ export type ExecutionStatus = z.infer; * is different from `0` — "read nothing" is a fact, "reads nothing" is a kind. * * And `unmeasuredEffect` is the third answer, which a two-counter model would - * have had to fake: a `connector_action` reaches an external system through a - * descriptor that declares nothing about whether the action reads or writes, so - * `0` understates a write and `1` overstates a read. Both are worse than saying - * so — an understated `0` fires the broken-sweep alert on a healthy run until - * operators learn to ignore it, and an overstated `1` makes the alert never - * fire at all, which is the original bug back again. + * have had to fake: a `connector_action` reaches an external system, and when + * the action declares nothing about whether it reads or writes, `0` understates + * a write and `1` overstates a read. Both are worse than saying so — an + * understated `0` fires the broken-sweep alert on a healthy run until operators + * learn to ignore it, and an overstated `1` makes the alert never fire at all, + * which is the original bug back again. + * + * [#4395] "Declares nothing" is now the fallback rather than the only case: + * `ConnectorActionSchema.effect` lets an action say `read` or `write`, and a + * step dispatching a declared action reports a real `acted` (0 for a read, 1 + * for an accepted write) instead of this flag. `unmeasuredEffect` keeps exactly + * its meaning and its consumers — it is what an UNdeclared action still + * reports, alongside a declared write whose dispatch failed (the upstream may + * have been reached) and a `script` step calling a function declared + * `'writes'` (#4396). */ export const ExecutionStepMetricsSchema = lazySchema(() => z.object({ selected: z.number().int().min(0).optional() diff --git a/packages/spec/src/integration/connector-descriptor.ts b/packages/spec/src/integration/connector-descriptor.ts index b18e109588..bae1cfc02d 100644 --- a/packages/spec/src/integration/connector-descriptor.ts +++ b/packages/spec/src/integration/connector-descriptor.ts @@ -19,6 +19,8 @@ * `type`. The declaration moved here; the engine imports it back. */ +import type { ConnectorActionEffect } from './connector.zod'; + /** * How a registered connector reached the engine (ADR-0097 §4). `plugin` — a * connector plugin called `registerConnector` directly (ADR-0018 §Addendum). @@ -40,8 +42,9 @@ export type ConnectorOrigin = 'plugin' | 'declarative'; export type ConnectorState = 'ready' | 'degraded'; /** - * A designer-facing view of one connector action — identity + its JSON-Schema - * input/output. The runtime handler is intentionally omitted; this is metadata. + * A designer-facing view of one connector action — identity, its JSON-Schema + * input/output, and what it does upstream. The runtime handler is intentionally + * omitted; this is metadata. */ export interface ConnectorActionDescriptor { readonly key: string; @@ -49,6 +52,18 @@ export interface ConnectorActionDescriptor { readonly description?: string; readonly inputSchema?: Record; readonly outputSchema?: Record; + /** + * What the action does upstream (#4395): `read` never mutates; `write` does. + * Absent keeps the pre-#4395 behaviour — the run reports the effect as + * uncountable (`ExecutionStepMetrics.unmeasuredEffect`) rather than as zero. + * + * Projected verbatim from the connector's authored + * `ConnectorActionSchema.effect`, which is where an author or a provider + * factory declares it. This interface is the READ side: it is what + * `GET /api/v1/automation/connectors` serves, so a designer can show that a + * `crm.push_opportunity` writes while a `crm.lookup_account` does not. + */ + readonly effect?: ConnectorActionEffect; } /** diff --git a/packages/spec/src/integration/connector.test.ts b/packages/spec/src/integration/connector.test.ts index 0479eb44c0..73deea43ad 100644 --- a/packages/spec/src/integration/connector.test.ts +++ b/packages/spec/src/integration/connector.test.ts @@ -21,6 +21,10 @@ import { ConnectorTypeSchema, ConnectorStatusSchema, + // Action + its declared upstream effect (#4395) + ConnectorActionSchema, + ConnectorActionEffectSchema, + // Error Mapping ErrorMappingConfigSchema, ErrorMappingRuleSchema, @@ -346,6 +350,51 @@ describe('RetryConfigSchema', () => { }); }); +// ============================================================================ +// Connector Action Effect (#4395) +// ============================================================================ + +describe('ConnectorActionSchema.effect (#4395)', () => { + it('declares exactly read | write — the two countable answers', () => { + expect(ConnectorActionEffectSchema.options).toEqual(['read', 'write']); + }); + + it('is optional, and absent stays absent (the uncountable default)', () => { + const parsed = ConnectorActionSchema.parse({ key: 'push', label: 'Push' }); + expect(parsed.effect).toBeUndefined(); + expect('effect' in parsed).toBe(false); + }); + + it('carries a declared effect through ConnectorSchema.parse, which is the ONLY producer', () => { + // The regression this pins: `ConnectorSchema` is a non-strict `z.object`, + // so before #4395 an authored `effect` was SILENTLY STRIPPED here. The + // engine stores this parsed def and both the `connector_action` executor + // and `GET /connectors` read the declaration back out of it — a descriptor + // field alone could never have been populated by anything. + const parsed = ConnectorSchema.parse({ + name: 'crm', + label: 'CRM', + type: 'saas', + actions: [ + { key: 'push_opportunity', label: 'Push Opportunity', effect: 'write' }, + { key: 'lookup_account', label: 'Lookup Account', effect: 'read' }, + { key: 'legacy', label: 'Legacy' }, + ], + }); + expect(parsed.actions?.map((a) => a.effect)).toEqual(['write', 'read', undefined]); + }); + + it('rejects a value outside the enum instead of coercing it', () => { + // `writes` is the `FlowFunctionEffectSchema` (#4396) spelling — the nearest + // wrong word an author already knows. It must fail loudly here rather than + // parse to something the executor then counts. + expect(ConnectorActionEffectSchema.safeParse('writes').success).toBe(false); + expect(ConnectorActionEffectSchema.safeParse('reads').success).toBe(false); + expect(ConnectorActionEffectSchema.safeParse('pure').success).toBe(false); + expect(() => ConnectorActionSchema.parse({ key: 'k', label: 'L', effect: 'write ' })).toThrow(); + }); +}); + // ============================================================================ // Base Connector Tests // ============================================================================ diff --git a/packages/spec/src/integration/connector.zod.ts b/packages/spec/src/integration/connector.zod.ts index 6c6bf5d75c..f4fb73db48 100644 --- a/packages/spec/src/integration/connector.zod.ts +++ b/packages/spec/src/integration/connector.zod.ts @@ -560,6 +560,45 @@ export const ConnectorStatusSchema = lazySchema(() => z.enum([ export type ConnectorStatus = z.infer; +/** + * What one connector action does **upstream** (#4395). + * + * - `'read'` — the action never mutates the external system (a lookup, a + * search, a fetch). Its step reports a real `acted: 0`. + * - `'write'` — it mutates (a create, an update, a delete, an enqueue). A + * step whose dispatch SUCCEEDED reports `acted: 1`. + * + * Absent is the third answer and stays the default: the platform reports + * `ExecutionStepMetrics.unmeasuredEffect`, i.e. "this may have caused an effect + * I cannot count" — never `acted: 0`, which would claim it did nothing. + * + * ## Why the runtime needs this declared rather than inferred + * + * `connector_action` dispatches to a handler that reaches an external system; + * nothing on this side can see what happened there. #4354's per-run summary + * reports `selected` / `acted` and the broken-sweep alert is + * `selected > 0 AND acted = 0 AND unmeasured = 0`, so the two guesses are both + * wrong in a costly direction: a blanket `acted: 0` trips the alert on every + * healthy connector sweep until operators learn to ignore it, and a blanket + * `acted: 1` makes a read-only sweep look busy forever so the alert never + * fires. `http` gets to skip this question because the HTTP method answers it + * (`GET` reads, anything else mutates); a connector action's key does not. + * + * ## Why the vocabulary differs from `FlowFunctionEffectSchema` + * + * That one (`'pure' | 'writes'`, #4396) classifies a *local* compute step and + * deliberately has no `reads` member, because a `script` node's writes are + * downstream declarative nodes that count themselves. Here BOTH members are + * countable facts about a remote call, which is exactly what makes declaring + * `read` worth writing: it converts an uncountable step into a measured zero. + */ +export const ConnectorActionEffectSchema = lazySchema(() => z.enum([ + 'read', + 'write', +]).describe("What the action does upstream: 'read' never mutates (reports acted:0); 'write' does (a successful dispatch reports acted:1). Omit when the effect is not knowable — the step is then reported as unmeasured, not as zero")); + +export type ConnectorActionEffect = z.infer; + /** * Connector Action Definition */ @@ -569,6 +608,20 @@ export const ConnectorActionSchema = lazySchema(() => z.object({ description: z.string().optional(), inputSchema: z.record(z.string(), z.unknown()).optional().describe('Input parameters schema (JSON Schema)'), outputSchema: z.record(z.string(), z.unknown()).optional().describe('Output schema (JSON Schema)'), + /** + * What this action does upstream (#4395) — see + * {@link ConnectorActionEffectSchema}. Optional on purpose: every connector + * written before this key keeps working and reports exactly what it reported + * before (`unmeasuredEffect`), so declaring it is a strict improvement rather + * than a migration. + * + * This is the ONLY producer of the effect. `AutomationEngine.registerConnector` + * stores `ConnectorSchema.parse(def)`, and the designer-facing + * `ConnectorActionDescriptor` is projected from that stored def — so both the + * hand-registered path and the ADR-0097 declarative materialization path read + * the declaration from right here. + */ + effect: ConnectorActionEffectSchema.optional(), })); /** diff --git a/packages/spec/src/type-alias-convention.pin.test.ts b/packages/spec/src/type-alias-convention.pin.test.ts index 2db5ec4050..f1b1ebf1ab 100644 --- a/packages/spec/src/type-alias-convention.pin.test.ts +++ b/packages/spec/src/type-alias-convention.pin.test.ts @@ -788,6 +788,16 @@ export type Iso381 = Assert, z.infer< typeof M78.ErrorMappingRuleSchema > >>; export type Iso383 = Assert, z.infer< typeof M78.ConnectorTypeSchema > >>; export type Iso384 = Assert, z.infer< typeof M78.ConnectorStatusSchema > >>; +// [#4395] Added after the generated corpus, so its number continues from the +// file's end rather than from its neighbours — the `IsoNNN` label is only a +// unique name (the gate reads the `z.input< typeof Mn.XSchema >` occurrence, +// not the label), and renumbering 300+ following lines to close the gap would +// be a merge-conflict magnet for no reader benefit. Grouped with its siblings +// here because the FILE heading is what a reader navigates by. +// A bare `z.enum`, exactly like `ConnectorType` / `ConnectorStatus` two lines +// up: no default, no transform, so author and parsed states coincide and D5 +// gives it no `XParsed`. +export type Iso718 = Assert, z.infer< typeof M78.ConnectorActionEffectSchema > >>; // kernel/cli-extension.zod.ts export type Iso385 = Assert, z.infer< typeof M79.CLICommandContributionSchema > >>; @@ -1361,7 +1371,7 @@ export type AFamilyParsedIsParseState = Assert< // --------------------------------------------------------------------------- describe('ADR-0122 type-alias convention', () => { - it('still declares all 718 isomorphic pins', () => { + it('still declares all 719 isomorphic pins', () => { // The truth of each pin is proved by tsc, not here — an `Assert>` // that stops holding is a compile error with the alias named. What tsc // cannot notice is a pin that was DELETED: removing the assertion removes @@ -1372,9 +1382,15 @@ describe('ADR-0122 type-alias convention', () => { // Rebalancing this number is normal — it drops by one whenever a schema // gains a shape and its alias gains an `XParsed`. Dropping it without that // corresponding alias is the edit this case exists to stop. + // + // It also RISES, which the count had not yet seen when it was written: a + // NEW isomorphic schema arrives with a bare alias, and `check:spec-parsed-alias` + // sends it here rather than to an `XParsed` (718 -> 719 was + // `ConnectorActionEffectSchema`, #4395 — a bare `z.enum`, like the + // `ConnectorType` / `ConnectorStatus` pins beside it). const self = readFileSync(fileURLToPath(import.meta.url), 'utf8'); const pins = self.match(/^export type Iso\d+ = Assert {