Skip to content

Commit d25a0ec

Browse files
os-zhuangclaude
andauthored
feat(spec,service-automation): a run says when its acted count is incomplete, instead of guessing (#4354) (#4397)
#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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 5c13368 commit d25a0ec

16 files changed

Lines changed: 418 additions & 17 deletions

File tree

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-automation": minor
4+
---
5+
6+
feat(spec,service-automation): a run says when its `acted` count is incomplete, instead of guessing (#4354)
7+
8+
#4354 shipped `selected` / `acted` counts on every flow run, sourced from the
9+
executors that know what they did. Four node types were left out — and the gap
10+
was not cosmetic: `connector_action`, `http` and `script` are how a flow acts on
11+
anything *outside* the platform, so a sweep whose whole job runs through them
12+
reported `acted: 0` and looked exactly like the dead sweep the counter exists to
13+
find. A detector that fires on healthy runs is worse than no detector: operators
14+
tune it out, and then it is not watching the flows that really did stop.
15+
16+
Closing it needed a third answer, because for two of those nodes the platform
17+
genuinely cannot know:
18+
19+
**`connector_action` — unknowable, and now it says so.**
20+
`ConnectorActionDescriptor` declares `key` / `label` / `description` /
21+
`inputSchema` / `outputSchema` and *nothing* about whether the action reads or
22+
writes, so `crm.push_opportunity` and `crm.lookup_account` are the same shape to
23+
the runtime. `acted: 0` understates the create; `acted: 1` overstates the
24+
lookup and makes the alert never fire — #4354's original bug, one layer out.
25+
The executor reports `metrics: { unmeasuredEffect: true }` instead, and the run
26+
carries an `unmeasured` tally. Filed #4395 to let a connector declare its effect
27+
kind, which would turn this into a real count.
28+
29+
**`http` — knowable, and now counted.** The method says it:
30+
`GET`/`HEAD`/`OPTIONS` report a real `acted: 0` (a read cannot write); a mutating
31+
call the upstream accepted reports `acted: 1`; `durable: true` reports `acted: 1`
32+
because the outbox row is a durable effect this run caused. A mutating call that
33+
was *rejected or timed out* reports `unmeasured` — a 500 can arrive after the
34+
write landed, and claiming zero there would let a run swear it changed nothing
35+
when it had.
36+
37+
**`script` — deliberately unchanged.** A registered function is contractually
38+
pure ("Data I/O stays on the flow graph — the function itself does no writes"),
39+
so every write it causes is a downstream node counting itself and "reports no
40+
record metrics" is accurate rather than a guess. Nothing *enforces* that purity,
41+
so a function that writes behind the platform's back under-reports its run —
42+
filed as #4396 rather than papered over here, because a blanket
43+
`unmeasuredEffect` on `script` would suppress the signal on every flow that
44+
calls any function in order to accommodate one contract violation.
45+
46+
**The alert gains a clause.** `selected > 0 AND acted = 0` becomes
47+
`selected > 0 AND acted = 0 AND unmeasured = 0`, and `sys_automation_run` gains
48+
an `unmeasured_count` column to serve it. Without that third clause the alert
49+
fires on every healthy connector-driven flow. The log line gains
50+
`unmeasured=N` — only when non-zero, since its *presence* is what a reader must
51+
not miss: `acted=0` on a line that also says `unmeasured=3` means "cannot tell",
52+
not "did nothing".
53+
54+
`unmeasured` propagates through `subflow` and `map` roll-ups (and through
55+
`creditChildRun` for a child that paused), so a parent whose child dispatched an
56+
uncountable effect knows its own `acted` is incomplete. N uncountable effects in
57+
a child collapse to one flag on the parent's step — the child keeps the real
58+
count in its own run row, and the question this feeds is boolean.
59+
60+
`FlowRunSummary.unmeasured` is optional and `undefined` is **not** `0`: a run
61+
recorded before this existed did not track uncountable effects at all, and
62+
defaulting it to zero would tell an operator "fully measured" about a run nobody
63+
measured. Same rule the `null` count columns already follow.
64+
65+
Additive: new optional fields only, no new exports, no execution behaviour
66+
changes.
67+
68+
Verified: `@objectstack/service-automation` **546 tests / 47 files** (21 new),
69+
`@objectstack/spec` **7193 / 281** (2 new); all 8 `check:generated` gates plus
70+
the seven pure audits (liveness, empty-state, variant-docs, strictness-ledger,
71+
react-conformance, skill-examples, exported-any); `check:nul-bytes` and eslint
72+
clean.

content/docs/automation/flows.mdx

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,7 @@ run in `listRuns` / `getRun`, and in the log:
596596
| `selected` | Records **read** by the run's data nodes |
597597
| `acted` | Records **created / updated / deleted**, plus effects dispatched (notifications delivered) |
598598
| `skipped` | Node executions a **closed gate** prevented — one per loop iteration whose conditional edge evaluated false |
599+
| `unmeasured` | Executions that reached something the platform **cannot count** — see below |
599600
| `nodes[]` | Per-node terminal status with `runs` / `failures` / `skipped` and its own selected/acted |
600601
| `gates[]` | Which gates closed and how often, most-skipped first |
601602

@@ -605,9 +606,32 @@ from the engine guessing at a node's output shape. A node that touches no
605606
records (`decision`, `assignment`) reports nothing at all, which is different
606607
from reporting zero.
607608

609+
#### When the platform cannot count
610+
611+
Some nodes reach outside the platform, and for those `acted` has a third
612+
possible answer. A `connector_action` dispatches to an external system through a
613+
descriptor that declares nothing about whether the action reads or writes, so
614+
counting it `0` would understate a Salesforce create and counting it `1` would
615+
overstate a lookup — and the overstatement is the worse one, because it makes
616+
the broken-sweep alert never fire. Those executions increment `unmeasured`
617+
instead:
618+
619+
| Node | Reported |
620+
| :--- | :--- |
621+
| `http`, `GET`/`HEAD`/`OPTIONS` | `acted: 0` — a read can never write |
622+
| `http`, mutating method, response OK | `acted: 1` |
623+
| `http`, mutating method, rejected / timed out | `unmeasured` — a 500 can arrive after the write landed |
624+
| `http`, `durable: true` | `acted: 1` — the outbox row is a real, durable effect |
625+
| `connector_action` | `unmeasured` |
626+
| `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 |
627+
628+
`unmeasured` propagates through `subflow` and `map` roll-ups, so a parent whose
629+
child dispatched an uncountable effect knows its own `acted` is incomplete.
630+
608631
The same counts land on `sys_automation_run` as **queryable columns**
609-
(`selected_count`, `acted_count`, `skipped_count`, plus a `summary_json`
610-
breakdown), so a broken sweep is something you can alert on rather than notice:
632+
(`selected_count`, `acted_count`, `skipped_count`, `unmeasured_count`, plus a
633+
`summary_json` breakdown), so a broken sweep is something you can alert on
634+
rather than notice:
611635

612636
```typescript
613637
// Runs that selected work and did none of it, newest first.
@@ -616,18 +640,22 @@ const suspect = await engine.find('sys_automation_run', {
616640
status: 'completed',
617641
selected_count: { $gt: 0 },
618642
acted_count: 0,
643+
// Without this clause the alert fires on every healthy connector-driven
644+
// flow: those runs report acted 0 because the count is INCOMPLETE, not zero.
645+
unmeasured_count: 0,
619646
started_at: { $gte: since },
620647
},
621648
orderBy: [{ field: 'started_at', order: 'desc' }],
622649
});
623650
```
624651

625-
`selected > 0 && acted == 0` over several consecutive runs is a near-perfect
626-
broken-sweep detector — the case that is otherwise invisible, because nobody is
627-
watching automation until it has already been dead for a month. A single such
628-
run is not proof of anything: a sweep whose work is all already done reports the
629-
same thing legitimately, which is why the signal is *consecutive* runs, and why
630-
the platform reports the counts rather than raising the alarm itself.
652+
`selected > 0 && acted == 0 && unmeasured == 0` over several consecutive runs is
653+
a near-perfect broken-sweep detector — the case that is otherwise invisible,
654+
because nobody is watching automation until it has already been dead for a
655+
month. A single such run is not proof of anything: a sweep whose work is all
656+
already done reports the same thing legitimately, which is why the signal is
657+
*consecutive* runs, and why the platform reports the counts rather than raising
658+
the alarm itself.
631659

632660
Rows written before summaries existed carry `null` counts, not `0` — "not
633661
measured" must not read as "measured zero". The log line defaults to `info`;

content/docs/references/automation/execution.mdx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ const result = Checkpoint.parse(data);
108108
| **status** | `Enum<'pending' \| 'running' \| 'paused' \| 'completed' \| 'failed' \| 'cancelled' \| 'timed_out' \| 'retrying'>` || Current execution status |
109109
| **trigger** | `{ type: string; recordId?: string; object?: string; userId?: string; … }` || What triggered this execution |
110110
| **steps** | `{ nodeId: string; nodeType: string; nodeLabel?: string; status: Enum<'success' \| 'failure' \| 'skipped'>; … }[]` || Ordered list of executed steps |
111-
| **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 |
111+
| **summary** | `{ selected: integer; acted: integer; skipped: integer; unmeasured?: integer; … }` | optional | Per-run rollup: records selected / acted on, gate skips, per-node status |
112112
| **variables** | `Record<string, any>` | optional | Final state of flow variables |
113113
| **startedAt** | `string` || Execution start timestamp |
114114
| **completedAt** | `string` | optional | Execution completion timestamp |
@@ -155,7 +155,7 @@ const result = Checkpoint.parse(data);
155155
| **parentNodeId** | `string` | optional | Enclosing structured-region container node ID (loop/parallel/try_catch) |
156156
| **iteration** | `integer` | optional | Zero-based loop iteration or parallel branch index of the enclosing region |
157157
| **regionKind** | `string` | optional | Region kind the step ran in: loop-body \| parallel-branch \| try \| catch |
158-
| **metrics** | `{ selected?: integer; acted?: integer }` | optional | Records this step selected / acted on, as reported by the node executor |
158+
| **metrics** | `{ selected?: integer; acted?: integer; unmeasuredEffect?: boolean }` | optional | Records this step selected / acted on, as reported by the node executor |
159159
| **skippedBy** | `{ nodeId: string; edgeId?: string; label?: string }` | optional | The gate that closed, when `status` is `skipped` |
160160

161161

@@ -169,6 +169,7 @@ const result = Checkpoint.parse(data);
169169
| :--- | :--- | :--- | :--- |
170170
| **selected** | `integer` | optional | Records this node READ or matched (a `get_record` query, a lookup) |
171171
| **acted** | `integer` | optional | Records this node WROTE (created / updated / deleted) or effects it dispatched (notifications delivered) |
172+
| **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. |
172173

173174

174175
---
@@ -216,6 +217,7 @@ const result = Checkpoint.parse(data);
216217
| **skipped** | `integer` || Times a closed gate kept this node from running at all |
217218
| **selected** | `integer` | optional | Records read across every execution — omitted for a node that reads none |
218219
| **acted** | `integer` | optional | Records written / effects dispatched across every execution — omitted for a node that writes none |
220+
| **unmeasured** | `integer` | optional | Executions that may have caused an effect the platform cannot count (see ExecutionStepMetrics.unmeasuredEffect) |
219221

220222

221223
---
@@ -229,6 +231,7 @@ const result = Checkpoint.parse(data);
229231
| **selected** | `integer` || Total records read by the run |
230232
| **acted** | `integer` || Total records written / effects dispatched by the run |
231233
| **skipped** | `integer` || Total node executions a closed gate prevented |
234+
| **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. |
232235
| **nodes** | `{ nodeId: string; nodeType: string; nodeLabel?: string; status: Enum<'success' \| 'failure' \| 'skipped'>; … }[]` || Per-node breakdown, in first-execution order |
233236
| **gates** | `{ nodeId: string; targetNodeId: string; edgeId?: string; label?: string; … }[]` || Gates that closed during the run, most-skipped first |
234237
| **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". |

packages/services/service-automation/src/builtin/connector-nodes.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,23 @@ export function registerConnectorNodes(engine: AutomationEngine, ctx: PluginCont
8888

8989
try {
9090
const output = await handler((cfg.input ?? {}) as Record<string, unknown>, handlerCtx);
91-
return { success: true, output };
91+
// #4354 — the action reached an external system and the platform
92+
// cannot say what it did there: `ConnectorActionDescriptor`
93+
// declares `key` / `label` / `description` / `inputSchema` /
94+
// `outputSchema` and NOTHING about whether the action reads or
95+
// writes. `acted: 0` would understate a Salesforce create;
96+
// `acted: 1` would overstate a lookup and make the broken-sweep
97+
// alert never fire — the original bug back again. Report the
98+
// honest third answer: this run's `acted` is incomplete.
99+
// #4395 proposes declaring the effect kind on the descriptor,
100+
// which would turn this into a real count.
101+
return { success: true, output, metrics: { unmeasuredEffect: true } };
92102
} catch (err) {
93103
return {
94104
success: false,
95105
error: `connector_action(${cfg.connectorId}.${cfg.actionId}) failed: ${(err as Error).message}`,
106+
// A handler that threw may still have reached the upstream.
107+
metrics: { unmeasuredEffect: true },
96108
};
97109
}
98110
},

packages/services/service-automation/src/builtin/http-nodes.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext):
131131
timeoutMs,
132132
payload: body ?? {},
133133
});
134-
return { success: true, output: { deliveryId, enqueued: true } };
134+
// #4354 — the outbox row IS a durable effect this run
135+
// caused, even though the upstream call happens later.
136+
return { success: true, output: { deliveryId, enqueued: true }, metrics: { acted: 1 } };
135137
} catch (err) {
136138
return { success: false, error: `http (durable) failed to enqueue: ${(err as Error).message}` };
137139
}
@@ -144,6 +146,10 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext):
144146

145147
// ── Request/response mode (default; preserves http_request) ───────
146148
const method = cfg.method ?? 'GET';
149+
// #4354 — unlike a connector action, an HTTP call's effect IS
150+
// knowable: the method says it. A GET reads and can never write, so
151+
// it reports a real `0`; anything else is a mutating call.
152+
const reads = /^(GET|HEAD|OPTIONS)$/i.test(method);
147153
const controller = new AbortController();
148154
const timer = timeoutMs ? setTimeout(() => controller.abort(), timeoutMs) : undefined;
149155
try {
@@ -158,11 +164,26 @@ export function registerHttpNodes(engine: AutomationEngine, ctx: PluginContext):
158164
success: response.ok,
159165
output: { response: data, status: response.status },
160166
error: response.ok ? undefined : `HTTP ${response.status}`,
167+
// A mutating call the upstream ACCEPTED is one effect. One it
168+
// rejected is unknown, not zero: a 500 can arrive after the
169+
// write landed, and claiming `0` there would let a run report
170+
// it changed nothing while it had.
171+
metrics: reads
172+
? { acted: 0 }
173+
: response.ok
174+
? { acted: 1 }
175+
: { unmeasuredEffect: true },
161176
};
162177
} catch (err) {
163178
const e = err as { name?: string; message?: string };
164179
const msg = e?.name === 'AbortError' ? `timeout after ${timeoutMs}ms` : e?.message ?? String(err);
165-
return { success: false, error: `http: ${msg}` };
180+
// A timed-out or aborted mutating request may well have landed —
181+
// the response is what we lost, not necessarily the write.
182+
return {
183+
success: false,
184+
error: `http: ${msg}`,
185+
metrics: reads ? { acted: 0 } : { unmeasuredEffect: true },
186+
};
166187
} finally {
167188
if (timer) clearTimeout(timer);
168189
}

packages/services/service-automation/src/builtin/map-node.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v
138138
// child run bubbles back (AutomationEngine.creditChildRun).
139139
let selected = 0;
140140
let acted = 0;
141+
let unmeasured = false;
141142

142143
// Drive items in order. Synchronous items advance inline; a pausing item
143144
// suspends the run and is resumed via re-entry.
@@ -175,22 +176,32 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v
175176
// Mark this item started and suspend; the engine re-enters on bubble.
176177
state.started = idx + 1;
177178
variables.set(stateKey, state);
178-
return { success: true, suspend: true, correlation: `map:${child.runId}`, metrics: { selected, acted } };
179+
return {
180+
success: true, suspend: true, correlation: `map:${child.runId}`,
181+
metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}) },
182+
};
179183
}
180184
if (!child.success) {
181185
return {
182186
success: false,
183187
error: `map '${node.id}': item ${idx} (subflow '${flowName}') failed: ${child.error ?? 'unknown error'}`,
184188
// Items that already succeeded wrote real rows; a later item's
185189
// failure must not erase them from the run's totals.
186-
metrics: { selected: selected + (child.summary?.selected ?? 0), acted: acted + (child.summary?.acted ?? 0) },
190+
metrics: {
191+
selected: selected + (child.summary?.selected ?? 0),
192+
acted: acted + (child.summary?.acted ?? 0),
193+
...(unmeasured || child.summary?.unmeasured ? { unmeasuredEffect: true } : {}),
194+
},
187195
};
188196
}
189197
// Synchronous completion — record and advance.
190198
state.started = idx + 1;
191199
state.results.push(child.output ?? null);
192200
selected += child.summary?.selected ?? 0;
193201
acted += child.summary?.acted ?? 0;
202+
// One uncountable effect anywhere in the batch makes the batch's
203+
// `acted` incomplete — the flag rides out with this entry's metrics.
204+
if (child.summary?.unmeasured) unmeasured = true;
194205
}
195206

196207
// All items done.
@@ -199,7 +210,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v
199210
return {
200211
success: true,
201212
output: { results: state.results, count: state.results.length },
202-
metrics: { selected, acted },
213+
metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}) },
203214
};
204215
},
205216
});

0 commit comments

Comments
 (0)