From 7a11b5ecc4abf6c2d5fb4c5bd9e24e056eef39df Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 10:55:14 +0000 Subject: [PATCH] feat(spec,service-automation): a run says when its `acted` count is incomplete, instead of guessing (#4354) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4354 shipped selected/acted counts sourced from the executors that know what they did, but left out the four node types a flow uses to act on anything OUTSIDE the platform. That gap was not cosmetic: a sweep whose whole job runs through a connector reported `acted: 0` and looked exactly like the dead sweep the counter exists to find. A detector that fires on healthy runs is worse than none — operators tune it out, and then it is not watching the flows that really did stop. Closing it needed a third answer, because for two of those nodes the platform genuinely cannot know what happened: - `connector_action` — `ConnectorActionDescriptor` declares nothing about whether an action reads or writes, so `acted: 0` understates a create and `acted: 1` overstates a lookup (and makes the alert never fire, which is the original bug one layer out). Reports `unmeasuredEffect` instead. #4395 proposes declaring the effect kind, which would make it a real count. - `http` — knowable from the method. GET/HEAD/OPTIONS report a real `acted: 0`; an accepted mutating call reports 1; `durable: true` reports 1 (the outbox row is a durable effect this run caused); a rejected or timed-out mutating call reports unmeasured, because a 500 can arrive after the write landed. - `script` — deliberately unchanged. A registered function is contractually pure (data I/O stays on the flow graph), so reporting no record metrics is accurate rather than a guess. Nothing enforces that purity — filed as #4396 rather than papered over, since a blanket `unmeasuredEffect` here would suppress the signal on every flow calling any function to cover one contract violation. The alert gains a clause: `selected > 0 AND acted = 0 AND unmeasured = 0`, with an `unmeasured_count` column to serve it — without the third clause it fires on every healthy connector-driven flow. The log line gains `unmeasured=N` only when non-zero, since its PRESENCE is what a reader must not miss. `unmeasured` propagates through subflow/map roll-ups and `creditChildRun`, so a parent whose child dispatched an uncountable effect knows its own `acted` is incomplete. `FlowRunSummary.unmeasured` is optional and undefined is NOT 0: a run recorded before this existed did not track uncountable effects at all. Verified: service-automation 546/47 (21 new), spec 7193/281 (2 new); all 8 check:generated gates plus the seven pure audits; check:nul-bytes and eslint clean. Branch restarted from main after #4377 merged; #4347's fix confirmed first by running that issue's own repro. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SzJqHp1yXjri1tqKP8WoL4 --- .changeset/run-summary-uncountable-effects.md | 72 ++++++++ content/docs/automation/flows.mdx | 44 ++++- .../docs/references/automation/execution.mdx | 7 +- .../src/builtin/connector-nodes.ts | 14 +- .../src/builtin/http-nodes.ts | 25 ++- .../src/builtin/map-node.ts | 17 +- .../src/builtin/screen-nodes.ts | 9 + .../src/builtin/subflow-node.ts | 10 +- .../services/service-automation/src/engine.ts | 7 + .../src/run-summary.test.ts | 162 ++++++++++++++++++ .../service-automation/src/run-summary.ts | 13 ++ .../src/suspended-run-store.ts | 2 + .../src/sys-automation-run.object.ts | 7 + packages/spec/authorable-surface.json | 3 + .../spec/src/automation/execution.test.ts | 19 ++ packages/spec/src/automation/execution.zod.ts | 24 +++ 16 files changed, 418 insertions(+), 17 deletions(-) create mode 100644 .changeset/run-summary-uncountable-effects.md diff --git a/.changeset/run-summary-uncountable-effects.md b/.changeset/run-summary-uncountable-effects.md new file mode 100644 index 0000000000..3176ee1ddb --- /dev/null +++ b/.changeset/run-summary-uncountable-effects.md @@ -0,0 +1,72 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-automation": minor +--- + +feat(spec,service-automation): a run says when its `acted` count is incomplete, instead of guessing (#4354) + +#4354 shipped `selected` / `acted` counts on every flow run, sourced from the +executors that know what they did. Four node types were left out — and the gap +was not cosmetic: `connector_action`, `http` and `script` are how a flow acts on +anything *outside* the platform, so a sweep whose whole job runs through them +reported `acted: 0` and looked exactly like the dead sweep the counter exists to +find. A detector that fires on healthy runs is worse than no detector: operators +tune it out, and then it is not watching the flows that really did stop. + +Closing it needed a third answer, because for two of those nodes the platform +genuinely cannot know: + +**`connector_action` — unknowable, and now it says so.** +`ConnectorActionDescriptor` declares `key` / `label` / `description` / +`inputSchema` / `outputSchema` and *nothing* about whether the action reads or +writes, so `crm.push_opportunity` and `crm.lookup_account` are the same shape to +the runtime. `acted: 0` understates the create; `acted: 1` overstates the +lookup and makes the alert never fire — #4354's original bug, one layer out. +The executor reports `metrics: { unmeasuredEffect: true }` instead, and the run +carries an `unmeasured` tally. Filed #4395 to let a connector declare its effect +kind, which would turn this into a real count. + +**`http` — knowable, and now counted.** The method says it: +`GET`/`HEAD`/`OPTIONS` report a real `acted: 0` (a read cannot write); a mutating +call the upstream accepted reports `acted: 1`; `durable: true` reports `acted: 1` +because the outbox row is a durable effect this run caused. A mutating call that +was *rejected or timed out* reports `unmeasured` — a 500 can arrive after the +write landed, and claiming zero there would let a run swear it changed nothing +when it had. + +**`script` — deliberately unchanged.** A registered function is contractually +pure ("Data I/O stays on the flow graph — the function itself does no writes"), +so every write it causes is a downstream node counting itself and "reports no +record metrics" is accurate rather than a guess. Nothing *enforces* that purity, +so a function that writes behind the platform's back under-reports its run — +filed as #4396 rather than papered over here, because a blanket +`unmeasuredEffect` on `script` would suppress the signal on every flow that +calls any function in order to accommodate one contract violation. + +**The alert gains a clause.** `selected > 0 AND acted = 0` becomes +`selected > 0 AND acted = 0 AND unmeasured = 0`, and `sys_automation_run` gains +an `unmeasured_count` column to serve it. Without that third clause the alert +fires on every healthy connector-driven flow. The log line gains +`unmeasured=N` — only when non-zero, since its *presence* is what a reader must +not miss: `acted=0` on a line that also says `unmeasured=3` means "cannot tell", +not "did nothing". + +`unmeasured` propagates through `subflow` and `map` roll-ups (and through +`creditChildRun` for a child that paused), so a parent whose child dispatched an +uncountable effect knows its own `acted` is incomplete. N uncountable effects in +a child collapse to one flag on the parent's step — the child keeps the real +count in its own run row, and the question this feeds is boolean. + +`FlowRunSummary.unmeasured` is optional and `undefined` is **not** `0`: a run +recorded before this existed did not track uncountable effects at all, and +defaulting it to zero would tell an operator "fully measured" about a run nobody +measured. Same rule the `null` count columns already follow. + +Additive: new optional fields only, no new exports, no execution behaviour +changes. + +Verified: `@objectstack/service-automation` **546 tests / 47 files** (21 new), +`@objectstack/spec` **7193 / 281** (2 new); all 8 `check:generated` gates plus +the seven pure audits (liveness, empty-state, variant-docs, strictness-ledger, +react-conformance, skill-examples, exported-any); `check:nul-bytes` and eslint +clean. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index eb588da6b0..faede6a16f 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -596,6 +596,7 @@ run in `listRuns` / `getRun`, and in the log: | `selected` | Records **read** by the run's data nodes | | `acted` | Records **created / updated / deleted**, plus effects dispatched (notifications delivered) | | `skipped` | Node executions a **closed gate** prevented — one per loop iteration whose conditional edge evaluated false | +| `unmeasured` | Executions that reached something the platform **cannot count** — see below | | `nodes[]` | Per-node terminal status with `runs` / `failures` / `skipped` and its own selected/acted | | `gates[]` | Which gates closed and how often, most-skipped first | @@ -605,9 +606,32 @@ from the engine guessing at a node's output shape. A node that touches no records (`decision`, `assignment`) reports nothing at all, which is different from reporting zero. +#### When the platform cannot count + +Some nodes reach outside the platform, and for those `acted` has a third +possible answer. A `connector_action` dispatches to an external system through a +descriptor that declares nothing about whether the action reads or writes, so +counting it `0` would understate a Salesforce create and counting it `1` would +overstate a lookup — and the overstatement is the worse one, because it makes +the broken-sweep alert never fire. Those executions increment `unmeasured` +instead: + +| Node | Reported | +| :--- | :--- | +| `http`, `GET`/`HEAD`/`OPTIONS` | `acted: 0` — a read can never write | +| `http`, mutating method, response OK | `acted: 1` | +| `http`, mutating method, rejected / timed out | `unmeasured` — a 500 can arrive after the write landed | +| `http`, `durable: true` | `acted: 1` — the outbox row is a real, durable effect | +| `connector_action` | `unmeasured` | +| `script` | nothing — a registered function is **contractually pure**: data I/O stays on the flow graph, so every write it causes is a downstream node that counts itself | + +`unmeasured` propagates through `subflow` and `map` roll-ups, so a parent whose +child dispatched an uncountable effect knows its own `acted` is incomplete. + The same counts land on `sys_automation_run` as **queryable columns** -(`selected_count`, `acted_count`, `skipped_count`, plus a `summary_json` -breakdown), so a broken sweep is something you can alert on rather than notice: +(`selected_count`, `acted_count`, `skipped_count`, `unmeasured_count`, plus a +`summary_json` breakdown), so a broken sweep is something you can alert on +rather than notice: ```typescript // Runs that selected work and did none of it, newest first. @@ -616,18 +640,22 @@ const suspect = await engine.find('sys_automation_run', { status: 'completed', selected_count: { $gt: 0 }, acted_count: 0, + // Without this clause the alert fires on every healthy connector-driven + // flow: those runs report acted 0 because the count is INCOMPLETE, not zero. + unmeasured_count: 0, started_at: { $gte: since }, }, orderBy: [{ field: 'started_at', order: 'desc' }], }); ``` -`selected > 0 && acted == 0` over several consecutive runs is a near-perfect -broken-sweep detector — the case that is otherwise invisible, because nobody is -watching automation until it has already been dead for a month. A single such -run is not proof of anything: a sweep whose work is all already done reports the -same thing legitimately, which is why the signal is *consecutive* runs, and why -the platform reports the counts rather than raising the alarm itself. +`selected > 0 && acted == 0 && unmeasured == 0` over several consecutive runs is +a near-perfect broken-sweep detector — the case that is otherwise invisible, +because nobody is watching automation until it has already been dead for a +month. A single such run is not proof of anything: a sweep whose work is all +already done reports the same thing legitimately, which is why the signal is +*consecutive* runs, and why the platform reports the counts rather than raising +the alarm itself. Rows written before summaries existed carry `null` counts, not `0` — "not measured" must not read as "measured zero". The log line defaults to `info`; diff --git a/content/docs/references/automation/execution.mdx b/content/docs/references/automation/execution.mdx index 3eb479551c..d1b7f9639a 100644 --- a/content/docs/references/automation/execution.mdx +++ b/content/docs/references/automation/execution.mdx @@ -108,7 +108,7 @@ const result = Checkpoint.parse(data); | **status** | `Enum<'pending' \| 'running' \| 'paused' \| 'completed' \| 'failed' \| 'cancelled' \| 'timed_out' \| 'retrying'>` | ✅ | Current execution status | | **trigger** | `{ type: string; recordId?: string; object?: string; userId?: string; … }` | ✅ | What triggered this execution | | **steps** | `{ nodeId: string; nodeType: string; nodeLabel?: string; status: Enum<'success' \| 'failure' \| 'skipped'>; … }[]` | ✅ | Ordered list of executed steps | -| **summary** | `{ selected: integer; acted: integer; skipped: integer; nodes: { nodeId: string; nodeType: string; nodeLabel?: string; status: Enum<'success' \| 'failure' \| 'skipped'>; … }[]; … }` | optional | Per-run rollup: records selected / acted on, gate skips, per-node status | +| **summary** | `{ selected: integer; acted: integer; skipped: integer; unmeasured?: integer; … }` | optional | Per-run rollup: records selected / acted on, gate skips, per-node status | | **variables** | `Record` | optional | Final state of flow variables | | **startedAt** | `string` | ✅ | Execution start timestamp | | **completedAt** | `string` | optional | Execution completion timestamp | @@ -155,7 +155,7 @@ const result = Checkpoint.parse(data); | **parentNodeId** | `string` | optional | Enclosing structured-region container node ID (loop/parallel/try_catch) | | **iteration** | `integer` | optional | Zero-based loop iteration or parallel branch index of the enclosing region | | **regionKind** | `string` | optional | Region kind the step ran in: loop-body \| parallel-branch \| try \| catch | -| **metrics** | `{ selected?: integer; acted?: integer }` | optional | Records this step selected / acted on, as reported by the node executor | +| **metrics** | `{ selected?: integer; acted?: integer; unmeasuredEffect?: boolean }` | optional | Records this step selected / acted on, as reported by the node executor | | **skippedBy** | `{ nodeId: string; edgeId?: string; label?: string }` | optional | The gate that closed, when `status` is `skipped` | @@ -169,6 +169,7 @@ const result = Checkpoint.parse(data); | :--- | :--- | :--- | :--- | | **selected** | `integer` | optional | Records this node READ or matched (a `get_record` query, a lookup) | | **acted** | `integer` | optional | Records this node WROTE (created / updated / deleted) or effects it dispatched (notifications delivered) | +| **unmeasuredEffect** | `boolean` | optional | This execution may have caused an effect the platform cannot count (an external write through a connector). NOT interchangeable with `acted: 0` — it says the count is unknown, not that it is zero. | --- @@ -216,6 +217,7 @@ const result = Checkpoint.parse(data); | **skipped** | `integer` | ✅ | Times a closed gate kept this node from running at all | | **selected** | `integer` | optional | Records read across every execution — omitted for a node that reads none | | **acted** | `integer` | optional | Records written / effects dispatched across every execution — omitted for a node that writes none | +| **unmeasured** | `integer` | optional | Executions that may have caused an effect the platform cannot count (see ExecutionStepMetrics.unmeasuredEffect) | --- @@ -229,6 +231,7 @@ const result = Checkpoint.parse(data); | **selected** | `integer` | ✅ | Total records read by the run | | **acted** | `integer` | ✅ | Total records written / effects dispatched by the run | | **skipped** | `integer` | ✅ | Total node executions a closed gate prevented | +| **unmeasured** | `integer` | optional | Total executions that may have caused an effect the platform cannot count. Absent = not tracked (an older run), which is not the same as zero. | | **nodes** | `{ nodeId: string; nodeType: string; nodeLabel?: string; status: Enum<'success' \| 'failure' \| 'skipped'>; … }[]` | ✅ | Per-node breakdown, in first-execution order | | **gates** | `{ nodeId: string; targetNodeId: string; edgeId?: string; label?: string; … }[]` | ✅ | Gates that closed during the run, most-skipped first | | **detailOmitted** | `boolean` | optional | Set when persistence dropped `nodes`/`gates` to keep the stored row bounded — the totals are still exact. Declared so empty arrays are never mistaken for "nothing ran". | diff --git a/packages/services/service-automation/src/builtin/connector-nodes.ts b/packages/services/service-automation/src/builtin/connector-nodes.ts index 1fd890b210..b6350759d4 100644 --- a/packages/services/service-automation/src/builtin/connector-nodes.ts +++ b/packages/services/service-automation/src/builtin/connector-nodes.ts @@ -88,11 +88,23 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont try { const output = await handler((cfg.input ?? {}) as Record, handlerCtx); - return { success: true, output }; + // #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 } }; } 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 }, }; } }, diff --git a/packages/services/service-automation/src/builtin/http-nodes.ts b/packages/services/service-automation/src/builtin/http-nodes.ts index b647ab2a4d..a8346ee36c 100644 --- a/packages/services/service-automation/src/builtin/http-nodes.ts +++ b/packages/services/service-automation/src/builtin/http-nodes.ts @@ -131,7 +131,9 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext): timeoutMs, payload: body ?? {}, }); - return { success: true, output: { deliveryId, enqueued: true } }; + // #4354 — the outbox row IS a durable effect this run + // caused, even though the upstream call happens later. + return { success: true, output: { deliveryId, enqueued: true }, metrics: { acted: 1 } }; } catch (err) { return { success: false, error: `http (durable) failed to enqueue: ${(err as Error).message}` }; } @@ -144,6 +146,10 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext): // ── Request/response mode (default; preserves http_request) ─────── const method = cfg.method ?? 'GET'; + // #4354 — unlike a connector action, an HTTP call's effect IS + // knowable: the method says it. A GET reads and can never write, so + // it reports a real `0`; anything else is a mutating call. + const reads = /^(GET|HEAD|OPTIONS)$/i.test(method); const controller = new AbortController(); const timer = timeoutMs ? setTimeout(() => controller.abort(), timeoutMs) : undefined; try { @@ -158,11 +164,26 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext): success: response.ok, output: { response: data, status: response.status }, error: response.ok ? undefined : `HTTP ${response.status}`, + // A mutating call the upstream ACCEPTED is one effect. One it + // rejected is unknown, not zero: a 500 can arrive after the + // write landed, and claiming `0` there would let a run report + // it changed nothing while it had. + metrics: reads + ? { acted: 0 } + : response.ok + ? { acted: 1 } + : { unmeasuredEffect: true }, }; } catch (err) { const e = err as { name?: string; message?: string }; const msg = e?.name === 'AbortError' ? `timeout after ${timeoutMs}ms` : e?.message ?? String(err); - return { success: false, error: `http: ${msg}` }; + // A timed-out or aborted mutating request may well have landed — + // the response is what we lost, not necessarily the write. + return { + success: false, + error: `http: ${msg}`, + metrics: reads ? { acted: 0 } : { unmeasuredEffect: true }, + }; } finally { if (timer) clearTimeout(timer); } diff --git a/packages/services/service-automation/src/builtin/map-node.ts b/packages/services/service-automation/src/builtin/map-node.ts index 62888eb7d1..5cd8eb8e63 100644 --- a/packages/services/service-automation/src/builtin/map-node.ts +++ b/packages/services/service-automation/src/builtin/map-node.ts @@ -138,6 +138,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v // child run bubbles back (AutomationEngine.creditChildRun). let selected = 0; let acted = 0; + let unmeasured = false; // Drive items in order. Synchronous items advance inline; a pausing item // suspends the run and is resumed via re-entry. @@ -175,7 +176,10 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v // Mark this item started and suspend; the engine re-enters on bubble. state.started = idx + 1; variables.set(stateKey, state); - return { success: true, suspend: true, correlation: `map:${child.runId}`, metrics: { selected, acted } }; + return { + success: true, suspend: true, correlation: `map:${child.runId}`, + metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}) }, + }; } if (!child.success) { return { @@ -183,7 +187,11 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v error: `map '${node.id}': item ${idx} (subflow '${flowName}') failed: ${child.error ?? 'unknown error'}`, // Items that already succeeded wrote real rows; a later item's // failure must not erase them from the run's totals. - metrics: { selected: selected + (child.summary?.selected ?? 0), acted: acted + (child.summary?.acted ?? 0) }, + metrics: { + selected: selected + (child.summary?.selected ?? 0), + acted: acted + (child.summary?.acted ?? 0), + ...(unmeasured || child.summary?.unmeasured ? { unmeasuredEffect: true } : {}), + }, }; } // Synchronous completion — record and advance. @@ -191,6 +199,9 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v state.results.push(child.output ?? null); selected += child.summary?.selected ?? 0; acted += child.summary?.acted ?? 0; + // One uncountable effect anywhere in the batch makes the batch's + // `acted` incomplete — the flag rides out with this entry's metrics. + if (child.summary?.unmeasured) unmeasured = true; } // All items done. @@ -199,7 +210,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v return { success: true, output: { results: state.results, count: state.results.length }, - metrics: { selected, acted }, + metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}) }, }; }, }); diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index ac0e2e01aa..e6d572cd00 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.ts @@ -282,6 +282,15 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext // exposes it as a flow variable so a later declarative node persists it // (e.g. `update_record fields: { ai_category: '{aiResult.ai_category}' }`). // Data I/O stays on the flow graph — the function itself does no writes. + // + // #4354 — which is why this node reports NO metrics, deliberately: by + // that contract every write it causes is a downstream `update_record` + // counting itself, so "absent" (this kind of node touches no records) + // is the accurate answer, not a guess. Nothing ENFORCES the purity + // though, so a function that writes behind the platform's back makes + // its run under-report. Filed as #4396 rather than accommodated: a blanket + // `unmeasuredEffect` here would suppress the broken-sweep signal on + // every flow that calls any function, to cover a contract violation. if (outputVariable) variables.set(outputVariable, result); return { success: true, output: { function: target, result } }; } catch (err) { diff --git a/packages/services/service-automation/src/builtin/subflow-node.ts b/packages/services/service-automation/src/builtin/subflow-node.ts index d18c5169ad..4c881e0bc8 100644 --- a/packages/services/service-automation/src/builtin/subflow-node.ts +++ b/packages/services/service-automation/src/builtin/subflow-node.ts @@ -119,7 +119,15 @@ export function registerSubflowNode(engine: AutomationEngine, ctx: PluginContext // broken sweep. The child keeps its own run row with its own summary; // the parent's answers "what did this run cause", subflows included. const rolled = child.summary - ? { metrics: { selected: child.summary.selected, acted: child.summary.acted } } + ? { + metrics: { + selected: child.summary.selected, + acted: child.summary.acted, + // An uncountable effect inside the child is uncountable for the + // parent too — the parent's `acted` cannot be read as complete. + ...(child.summary.unmeasured ? { unmeasuredEffect: true } : {}), + }, + } : {}; if (!child.success) { diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 5f76879809..de1f82485a 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -2372,6 +2372,12 @@ export class AutomationEngine implements IAutomationService { metrics: { selected: (prior.selected ?? 0) + child.selected, acted: (prior.acted ?? 0) + child.acted, + // N uncountable effects in the child collapse to ONE flag on + // the parent's step: this execution dispatched something the + // platform cannot count. The child keeps the real count in + // its own run row, and the question this feeds — "is the + // parent's `acted` complete?" — is boolean either way. + ...(prior.unmeasuredEffect || child.unmeasured ? { unmeasuredEffect: true } : {}), }, }; return; @@ -2873,6 +2879,7 @@ export class AutomationEngine implements IAutomationService { selected: entry.summary.selected, acted: entry.summary.acted, skipped: entry.summary.skipped, + unmeasured: entry.summary.unmeasured, gates: entry.summary.gates, }; if (this.runSummaryLog === 'debug') this.logger.debug(line, meta); diff --git a/packages/services/service-automation/src/run-summary.test.ts b/packages/services/service-automation/src/run-summary.test.ts index e9fa5a4552..7da266420b 100644 --- a/packages/services/service-automation/src/run-summary.test.ts +++ b/packages/services/service-automation/src/run-summary.test.ts @@ -18,6 +18,8 @@ import { registerLogicNodes } from './builtin/logic-nodes.js'; import { registerNotifyNode } from './builtin/notify-node.js'; import { registerSubflowNode } from './builtin/subflow-node.js'; import { registerMapNode } from './builtin/map-node.js'; +import { registerHttpNodes } from './builtin/http-nodes.js'; +import { registerConnectorNodes } from './builtin/connector-nodes.js'; import type { AutomationContext } from '@objectstack/spec/contracts'; const AT = '2026-07-31T00:00:00.000Z'; @@ -623,6 +625,166 @@ describe('a child run that PAUSED still counts toward its parent', () => { }); }); +// ── The third answer: effects the platform cannot count ───────────────────── + +describe('uncountable effects (#4354 follow-up)', () => { + // `acted: 0` and "we cannot tell" are different facts. Collapsing them + // either way breaks the detector: understating fires it on healthy runs + // until operators tune it out; overstating makes it never fire at all. + + it('counts an unmeasured execution without touching acted', () => { + const s = summarizeRun([ + step({ nodeId: 'q', nodeType: 'get_record', metrics: { selected: 5 } }), + step({ nodeId: 'push', nodeType: 'connector_action', metrics: { unmeasuredEffect: true } }), + ]); + expect(s).toMatchObject({ selected: 5, acted: 0, unmeasured: 1 }); + expect(s.nodes.find((n) => n.nodeId === 'push')).toMatchObject({ runs: 1, unmeasured: 1 }); + }); + + it('counts once per EXECUTION, so a connector call in a 30-item loop shows 30', () => { + const s = summarizeRun( + Array.from({ length: 30 }, (_, i) => + step({ nodeId: 'push', nodeType: 'connector_action', metrics: { unmeasuredEffect: true }, parentNodeId: 'each', iteration: i }), + ), + ); + expect(s.unmeasured).toBe(30); + expect(s.nodes).toHaveLength(1); + }); + + it('leaves unmeasured at 0 for a run with nothing uncountable', () => { + expect(summarizeRun([step({ nodeId: 'w', nodeType: 'create_record', metrics: { acted: 1 } })]).unmeasured).toBe(0); + }); + + it('names itself on the log line ONLY when non-zero', () => { + const clean = formatRunSummaryLine( + { flowName: 'f', runId: 'r', status: 'completed' }, + summarizeRun([step({ nodeId: 'w', nodeType: 'create_record', metrics: { acted: 1 } })]), + ); + expect(clean).not.toContain('unmeasured'); + + const murky = formatRunSummaryLine( + { flowName: 'f', runId: 'r', status: 'completed' }, + summarizeRun([ + step({ nodeId: 'q', nodeType: 'get_record', metrics: { selected: 9 } }), + step({ nodeId: 'push', nodeType: 'connector_action', metrics: { unmeasuredEffect: true } }), + ]), + ); + // The line an operator greps for `acted=0` must carry the qualifier that + // says the zero is incomplete. + expect(murky).toContain('selected=9 acted=0 skipped=0 unmeasured=1'); + }); + + it('an http node reports by METHOD — the one case the platform CAN count', async () => { + const calls: string[] = []; + const realFetch = globalThis.fetch; + globalThis.fetch = (async (_url: string, init: any) => { + calls.push(init.method); + return { ok: init.method !== 'PUT', status: init.method === 'PUT' ? 500 : 200, async json() { return {}; }, async text() { return ''; } }; + }) as never; + try { + for (const [method, expected] of [ + ['GET', { acted: 0, unmeasured: 0 }], + ['POST', { acted: 1, unmeasured: 0 }], + ['PUT', { acted: 0, unmeasured: 1 }], // rejected → unknown, not zero + ] as const) { + const logger = makeLogger(); + const engine = new AutomationEngine(logger); + registerHttpNodes(engine, { logger, getService: () => undefined } as never); + engine.registerFlow('f', { + name: 'f', label: 'f', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'S' }, + { id: 'call', type: 'http', label: 'C', config: { url: 'https://example.test/x', method } }, + { id: 'end', type: 'end', label: 'E' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'call' }, { id: 'e2', source: 'call', target: 'end' }], + } as never); + const res = await engine.execute('f', {} as AutomationContext); + expect(res.summary, `method ${method}`).toMatchObject(expected); + } + } finally { + globalThis.fetch = realFetch; + } + expect(calls).toEqual(['GET', 'POST', 'PUT']); + }); + + it('a connector_action is unmeasured — its descriptor declares no read/write', async () => { + 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' }] } 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).toBe(true); + // NOT acted:1 — the platform cannot know whether `push` wrote anything. + expect(res.summary).toMatchObject({ acted: 0, unmeasured: 1 }); + }); + + it('propagates through a subflow roll-up, so the parent knows its acted is incomplete', async () => { + const logger = makeLogger(); + const engine = new AutomationEngine(logger); + const ctx: any = { logger, getService: () => undefined }; + registerConnectorNodes(engine, ctx); + registerSubflowNode(engine, ctx); + engine.registerConnector( + { name: 'crm', label: 'CRM', type: 'saas', actions: [{ key: 'push', label: 'Push' }] } as never, + { push: async () => ({ id: 'ext_1' }) }, + ); + engine.registerFlow('child', { + name: 'child', label: 'child', 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); + engine.registerFlow('parent', { + name: 'parent', label: 'parent', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'S' }, + { id: 'sub', type: 'subflow', label: 'Sub', config: { flowName: 'child' } }, + { id: 'end', type: 'end', label: 'E' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'sub' }, { id: 'e2', source: 'sub', target: 'end' }], + } as never); + + const res = await engine.execute('parent', {} as AutomationContext); + expect(res.summary).toMatchObject({ acted: 0, unmeasured: 1 }); + }); + + it('persists as a queryable column, null when never tracked', async () => { + const rows: any[] = []; + const engine: any = { + async find() { return []; }, + async insert(_o: string, row: any) { rows.push(row); return row; }, + async update() { return 1; }, + async delete() { return 1; }, + }; + const store = new ObjectStoreSuspendedRunStore(engine); + await store.recordTerminal({ + runId: 'r1', flowName: 'f', status: 'completed', startedAt: AT, + summary: { selected: 9, acted: 0, skipped: 0, unmeasured: 3, nodes: [], gates: [] }, + }); + await store.recordTerminal({ runId: 'r2', flowName: 'f', status: 'completed', startedAt: AT }); + + expect(rows[0].unmeasured_count).toBe(3); + expect(rows[1].unmeasured_count).toBeNull(); // not measured ≠ measured zero + }); +}); + // ── The engine's own contract with executors ──────────────────────────────── describe('engine ↔ executor metrics plumbing', () => { diff --git a/packages/services/service-automation/src/run-summary.ts b/packages/services/service-automation/src/run-summary.ts index c19f910495..cc6afe3f7a 100644 --- a/packages/services/service-automation/src/run-summary.ts +++ b/packages/services/service-automation/src/run-summary.ts @@ -35,6 +35,7 @@ export function summarizeRun(steps: readonly StepLogEntry[]): FlowRunSummary { let selected = 0; let acted = 0; let skipped = 0; + let unmeasured = 0; for (const step of steps) { let node = nodes.get(step.nodeId); @@ -90,6 +91,13 @@ export function summarizeRun(steps: readonly StepLogEntry[]): FlowRunSummary { node.acted = (node.acted ?? 0) + metrics.acted; acted += metrics.acted; } + if (metrics?.unmeasuredEffect) { + // Counted per EXECUTION, like `runs` — a connector call in a 30-item + // loop leaves 30 unmeasured effects, and the alert has to see that + // the run's `acted` covers none of them. + node.unmeasured = (node.unmeasured ?? 0) + 1; + unmeasured += 1; + } } for (const node of nodes.values()) { @@ -103,6 +111,7 @@ export function summarizeRun(steps: readonly StepLogEntry[]): FlowRunSummary { selected, acted, skipped, + unmeasured, nodes: [...nodes.values()], gates: [...gates.values()].sort((a, b) => b.skipped - a.skipped), }; @@ -140,6 +149,10 @@ export function formatRunSummaryLine( `acted=${summary.acted}`, `skipped=${summary.skipped}`, ); + // Only when non-zero, like `gate=`: its absence is the common case and its + // PRESENCE is the thing a reader must not miss — `acted=0` on a line that + // also says `unmeasured=3` means "cannot tell", not "did nothing". + if (summary.unmeasured) parts.push(`unmeasured=${summary.unmeasured}`); const topGate = summary.gates[0]; if (topGate) { parts.push(`gate=${topGate.nodeId}->${topGate.targetNodeId}:${topGate.skipped}`); diff --git a/packages/services/service-automation/src/suspended-run-store.ts b/packages/services/service-automation/src/suspended-run-store.ts index 5ea1b62d4e..9af410cbb9 100644 --- a/packages/services/service-automation/src/suspended-run-store.ts +++ b/packages/services/service-automation/src/suspended-run-store.ts @@ -268,6 +268,7 @@ export class ObjectStoreSuspendedRunStore implements SuspendedRunStore { selected_count: record.summary?.selected ?? null, acted_count: record.summary?.acted ?? null, skipped_count: record.summary?.skipped ?? null, + unmeasured_count: record.summary?.unmeasured ?? null, summary_json: record.summary ? serializeSummaryBounded(record.summary) : null, }; const existing = await this.engine.find(TABLE, { @@ -446,6 +447,7 @@ function serializeSummaryBounded(summary: NonNullable): st selected: summary.selected, acted: summary.acted, skipped: summary.skipped, + unmeasured: summary.unmeasured, nodes: [], gates: [], detailOmitted: true, diff --git a/packages/services/service-automation/src/sys-automation-run.object.ts b/packages/services/service-automation/src/sys-automation-run.object.ts index c243b14e4c..cf9d490184 100644 --- a/packages/services/service-automation/src/sys-automation-run.object.ts +++ b/packages/services/service-automation/src/sys-automation-run.object.ts @@ -212,6 +212,13 @@ export const SysAutomationRun = ObjectSchema.create({ group: 'Outcome', }), + unmeasured_count: Field.number({ + label: 'Uncountable Effects', + required: false, + description: 'Executions that reached something the platform cannot count (a `connector_action`, a mutating `http` call whose response was lost). The qualifier `acted_count` needs to be trusted: the broken-sweep alert is `selected_count > 0 AND acted_count = 0 AND unmeasured_count = 0`, because a run with uncountable effects has an INCOMPLETE acted count, not a zero one. Null on rows written before this was tracked.', + group: 'Outcome', + }), + summary_json: Field.textarea({ label: 'Run Summary', required: false, diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index cde48c6e1a..dd90656437 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -2375,6 +2375,7 @@ "automation/ExecutionStepLog:status", "automation/ExecutionStepMetrics:acted", "automation/ExecutionStepMetrics:selected", + "automation/ExecutionStepMetrics:unmeasuredEffect", "automation/ExecutionStepSkipReason:edgeId", "automation/ExecutionStepSkipReason:label", "automation/ExecutionStepSkipReason:nodeId", @@ -2435,12 +2436,14 @@ "automation/FlowRunNodeSummary:selected", "automation/FlowRunNodeSummary:skipped", "automation/FlowRunNodeSummary:status", + "automation/FlowRunNodeSummary:unmeasured", "automation/FlowRunSummary:acted", "automation/FlowRunSummary:detailOmitted", "automation/FlowRunSummary:gates", "automation/FlowRunSummary:nodes", "automation/FlowRunSummary:selected", "automation/FlowRunSummary:skipped", + "automation/FlowRunSummary:unmeasured", "automation/FlowVariable:isInput", "automation/FlowVariable:isOutput", "automation/FlowVariable:name", diff --git a/packages/spec/src/automation/execution.test.ts b/packages/spec/src/automation/execution.test.ts index 261662a7ca..c24d8cecf5 100644 --- a/packages/spec/src/automation/execution.test.ts +++ b/packages/spec/src/automation/execution.test.ts @@ -207,6 +207,25 @@ describe('FlowRunSummarySchema', () => { expect(() => FlowRunSummarySchema.parse({ ...brokenSweep, acted: -1 })).toThrow(); }); + it('carries an uncountable-effect tally distinct from acted', () => { + // A connector-driven run: `acted: 0` is INCOMPLETE, not zero, and the + // broken-sweep query has to be able to tell. + const summary = FlowRunSummarySchema.parse({ + selected: 9, acted: 0, skipped: 0, unmeasured: 3, + nodes: [{ nodeId: 'push', nodeType: 'connector_action', status: 'success' as const, runs: 3, failures: 0, skipped: 0, unmeasured: 3 }], + gates: [], + }); + expect(summary.unmeasured).toBe(3); + expect(summary.nodes[0].unmeasured).toBe(3); + }); + + it('leaves `unmeasured` absent on a run that never tracked it — absent is not zero', () => { + // A row written before the field existed did not measure uncountable + // effects at all; defaulting it to 0 would tell an operator "fully + // measured" about a run nobody measured. + expect(FlowRunSummarySchema.parse(brokenSweep).unmeasured).toBeUndefined(); + }); + it('rides on an execution log', () => { const log = ExecutionLogSchema.parse({ id: 'exec_004', diff --git a/packages/spec/src/automation/execution.zod.ts b/packages/spec/src/automation/execution.zod.ts index 0daa5c989a..793ab890a2 100644 --- a/packages/spec/src/automation/execution.zod.ts +++ b/packages/spec/src/automation/execution.zod.ts @@ -51,12 +51,22 @@ export type ExecutionStatus = z.infer; * * Absent ⇒ the node touched no records (a `decision`, an `assignment`), which * 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. */ export const ExecutionStepMetricsSchema = lazySchema(() => z.object({ selected: z.number().int().min(0).optional() .describe('Records this node READ or matched (a `get_record` query, a lookup)'), acted: z.number().int().min(0).optional() .describe('Records this node WROTE (created / updated / deleted) or effects it dispatched (notifications delivered)'), + unmeasuredEffect: z.boolean().optional() + .describe('This execution may have caused an effect the platform cannot count (an external write through a connector). NOT interchangeable with `acted: 0` — it says the count is unknown, not that it is zero.'), })); export type ExecutionStepMetrics = z.infer; @@ -130,6 +140,7 @@ export const FlowRunNodeSummarySchema = lazySchema(() => z.object({ skipped: z.number().int().min(0).describe('Times a closed gate kept this node from running at all'), selected: z.number().int().min(0).optional().describe('Records read across every execution — omitted for a node that reads none'), acted: z.number().int().min(0).optional().describe('Records written / effects dispatched across every execution — omitted for a node that writes none'), + unmeasured: z.number().int().min(0).optional().describe('Executions that may have caused an effect the platform cannot count (see ExecutionStepMetrics.unmeasuredEffect)'), })); export type FlowRunNodeSummary = z.infer; @@ -162,6 +173,19 @@ export const FlowRunSummarySchema = lazySchema(() => z.object({ selected: z.number().int().min(0).describe('Total records read by the run'), acted: z.number().int().min(0).describe('Total records written / effects dispatched by the run'), skipped: z.number().int().min(0).describe('Total node executions a closed gate prevented'), + /** + * The qualifier `acted` needs to be trusted. A run that dispatched an + * uncountable effect (a `connector_action`) can report `acted: 0` while + * having done plenty, so the broken-sweep query is + * `selected > 0 AND acted = 0 AND unmeasured = 0` — the third clause is what + * keeps the alert off healthy connector-driven flows. + * + * Optional, and `undefined` is NOT `0`: a run recorded before this field + * existed did not track uncountable effects at all, and defaulting it to zero + * would tell an operator "fully measured" about a run nobody measured. + */ + unmeasured: z.number().int().min(0).optional() + .describe('Total executions that may have caused an effect the platform cannot count. Absent = not tracked (an older run), which is not the same as zero.'), nodes: z.array(FlowRunNodeSummarySchema).describe('Per-node breakdown, in first-execution order'), gates: z.array(FlowRunGateSummarySchema).describe('Gates that closed during the run, most-skipped first'), detailOmitted: z.boolean().optional()