Skip to content

Commit ae6dcf6

Browse files
os-teslaclaude
andauthored
fix(service-automation): a notify node reports the recipients it addressed, so a zero-delivery run stops reading like a successful one (#17339)
* fix(service-automation): notify reports the recipients it addressed A notify node whose delivery came back zero contributed `acted: 0` and nothing else, so a run that notified nobody folded to `selected: 0, acted: 0, unmeasured: 0` -- the same triple a run with no notify node at all reports, and one the broken-sweep filter cannot match because its first clause is `selected > 0`. Report `selected` (the recipient entries addressed) on every path that reaches a recipient list. `acted` / `unmeasuredEffect` keep their rules, so a delivering run's reading is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): differential control for notify zero-delivery Same flow, same recipients, both trigger families (the cron tick context ScheduleTrigger really builds, and the REST trigger's session context), on both drivers (memory and better-sqlite3), over the real MessagingService + outbox + inbox channel. Three-way comparison: the zero-delivery run, the delivering run, and a run that genuinely had nothing to notify about -- the first two of which used to be told apart only by a token on the OTHER row, and the first and third of which were the same triple. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): migrate the second backend off the frozen memory driver `pnpm check:driver-memory-census` refuses a new binding to @objectstack/driver-memory and says in as many words that adding a ledger entry to silence it is not the author's call: the consumer set is a maintainer ruling. Take the migrate route instead -- the matrix is now sqlite-wasm x sqlite-native, two real storage implementations. Admitting a memory arm needs that ruling; noted on the PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): take the second arm without a maintainer-only widening check:type-source-resolution refuses @objectstack/driver-sqlite-wasm and @objectstack/platform-objects as new dist-resolved type imports, says widening its shrink-only registry is not the fix, and names `paths` as the measured-wrong tool for a package whose rootDir is `src` (TS6059). Its own remedy for that case is to not take the dependency. So: object fixtures are declared locally, and the non-SQL arm is an in-process IDataEngine rather than the frozen mingo driver. Declared as a deviation in the file header; the real memory arm needs a ruling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * chore: drop the lockfile residue from the reverted dev dependencies Two devDependencies were added and then taken back out (the frozen memory driver, then the wasm one); the manifest returned to its original state but the lockfile kept the entry. Restored to origin/main and re-verified with a full `pnpm install`, which rewrote nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): pin the published summary lines verbatim The three rows the PR publishes are now assertions rather than prose, so the table is a measurement anyone can re-run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): route the in-process engine double's findOne through the producer predicate The zero-delivery differential control's second arm is an in-process IDataEngine standing in for the frozen mingo driver. Its findOne read an absent filter as "match everything", which is looser than ObjectQL.findOne — the exact shape check:engine-double-contract pins, and the shape that turns a green suite into no suite at all. Open it with assertEngineFindOnePredicate(object, query), imported from @objectstack/metadata-core — the predicate's home, and already a declared dependency of this package, so no new dependency edge and no objectql reverse edge. The RETAINED ledger learns the new row through the gate's own --write; the shrink-only baseline is untouched, byte for byte. All nine cases of the differential control still pass unchanged: no assertion was loosened to accommodate the predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-automation): hold the caller's bound and refuse the combinators this double does not implement Two further gates were RED on this same in-process engine double before this branch's last head, and neither had run in CI: the lint job executes its gates sequentially under `bash -e`, so check:engine-double-contract's exit 1 halted the job and masked every step behind it. Both are the same defect class as the finding that halted it — a test double looser than the engine it stands in for — in the same literal, and both gates state their baseline never grows, so the mechanical fix each prints is the only route. check:where-matcher — `matches` read a combinator as a FIELD NAME. No row carries a column called `$or`, so such a clause silently drops every row and this arm would report "nobody was reached" for a reason that is not the one under test, in the very file written to make that distinction visible. The store answers scalar equality, so it now refuses a combinator loudly rather than answering wrongly. check:objectql-double-limit — `find` applied the caller's bound by truthiness, so `limit: 0` returned every row: the one call that asked for none. Applied by presence now, after the filter. Neither baseline gained a file ("no files added", both gates). No assertion and no case was touched: the diff contains zero `expect(`/`it(`/`describe(` lines, and all nine cases of the differential control still pass on both arms, CONTROLs included — the in-process arm still genuinely delivers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 54b3d1d commit ae6dcf6

5 files changed

Lines changed: 629 additions & 3 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
`notify` now reports the recipients it addressed, so a run that notified nobody stops reading like a run that had nobody to notify
6+
7+
A `notify` node whose delivery count came back zero contributed `acted: 0` and nothing else to the run summary. A flow whose only effect-bearing node is that one then folded to `selected: 0, acted: 0, unmeasured: 0` — byte for byte the summary of a run that had nothing to notify about, and of a run whose `notify` node never executed. The run read healthy, and the only trace was a log line.
8+
9+
`emit()` returns `delivered: 0, enqueued: 0` on several paths, each after logging and nothing else: an audience that resolved to no recipient, a preference filter that suppressed every (recipient × channel) pair, a dedup hit, every enqueue failing. A stack with no messaging service installed lands in the same place. All of them were silent in the summary, so this is not one cause being fixed — it is the whole class becoming visible.
10+
11+
The node now reports `selected` — the recipient entries it addressed — on every path that reaches a recipient list, alongside the `acted` / `unmeasuredEffect` rules it already had. Those two are unchanged, so a delivering run keeps its existing `acted` (inline) or `unmeasured` (outbox) reading and stays outside the broken-sweep filter; a zero-delivery run now reports `selected: N, acted: 0` with no `unmeasured`, which is the platform's declared "matched N, acted on none, and that zero is trustworthy" signature and puts the run **inside** `selected > 0 AND acted = 0 AND unmeasured = 0` — the filter that exists for exactly this, and whose first clause the old reading could never satisfy.
12+
13+
The zero is deliberately NOT reported as `unmeasuredEffect`. That flag means the count is unknown; this count is known and it is zero, and claiming otherwise would take the run out of the very filter it belongs in.
14+
15+
`selected` counts audience entries, not resolved users: the entry (`role:manager`, a bare id) is what the node has, since expansion happens inside the messaging service and is not reported back.

‎content/docs/automation/flows.mdx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,7 @@ instead:
10601060
| `connector_action` | `unmeasured` |
10611061
| `script`, function declared pure (the default) | 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 |
10621062
| `script`, function declared `effect: 'writes'` | `unmeasured` — the function said it writes where the platform cannot see, so the run says the count is incomplete |
1063+
| `notify` | `selected`: the recipient entries the node addressed, always. Then `acted: <delivered>` when the messaging stack delivered inline and knows the outcome, or `unmeasured` when it handed the deliveries to the outbox and the dispatcher decides later. A notify that reached **nobody** — an audience that resolved to no recipient, a preference filter that suppressed every pair, a dedup hit, no messaging service installed — reports `selected: N, acted: 0` and no `unmeasured`, which is what puts it inside the broken-sweep filter instead of leaving it silent |
10631064

10641065
The `script` row is a contract, not a measurement: nothing stops a registered
10651066
function from writing, so an **undeclared** writer still makes its run report
@@ -1071,6 +1072,17 @@ every flow that calls any function, to cover the few that break the rule.
10711072
`unmeasured` propagates through `subflow` and `map` roll-ups, so a parent whose
10721073
child dispatched an uncountable effect knows its own `acted` is incomplete.
10731074

1075+
<Callout type="warn">
1076+
A `notify` node reports `selected` for the recipients it addressed **whether or
1077+
not any of them were reached**, and that is deliberate: it is the only thing
1078+
that separates a run which notified nobody from a run that had nobody to
1079+
notify. Both used to fold to `selected: 0, acted: 0, unmeasured: 0` — the same
1080+
triple a run with no `notify` node at all reports — so a flow that quietly
1081+
stopped delivering read exactly like a healthy quiet day, and the broken-sweep
1082+
filter could not match it because its first clause is `selected > 0`. The zero
1083+
is reported as a **measured** zero, never as `unmeasured`: the count is known.
1084+
</Callout>
1085+
10741086
The same counts land on `sys_automation_run` as **queryable columns**
10751087
(`selected_count`, `acted_count`, `skipped_count`, `unmeasured_count`, plus a
10761088
`summary_json` breakdown), so a broken sweep is something you can query for

‎packages/services/service-automation/src/builtin/notify-node.ts‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,13 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
321321
// #4354 — nothing was delivered, and the run summary must say
322322
// so: a nudge sweep whose messaging service is absent is
323323
// precisely the "green but inert" case this counter exists for.
324-
metrics: { acted: 0 },
324+
//
325+
// `selected` is what makes that `acted: 0` READABLE — see the
326+
// block above `metrics` on the emit path below. Without it a
327+
// run whose notify reached nobody folds to
328+
// `selected: 0, acted: 0, unmeasured: 0`, which is the same
329+
// triple a run with no notify node at all reports.
330+
metrics: { selected: recipients.length, acted: 0 },
325331
};
326332
}
327333

@@ -439,9 +445,48 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
439445
//
440446
// Waiting for the real outcome is not on the table: a notify
441447
// node must not block a flow on a downstream channel.
448+
//
449+
// ── `selected`: what makes a ZERO dispatch readable (#17123) ──
450+
//
451+
// `acted` and `unmeasuredEffect` above answer "what did this
452+
// node cause". Neither can answer "this node tried to notify
453+
// somebody and reached NOBODY", and that answer is the one an
454+
// operator needs: `emit()` has several paths that return
455+
// `delivered: 0, enqueued: 0` after logging a line and nothing
456+
// else — an audience that resolved to no recipient, a
457+
// preference filter that suppressed every (recipient x
458+
// channel) pair, a dedup hit, every enqueue failing. Each of
459+
// them lands here as `{ acted: 0 }`, and a run whose only
460+
// effect-bearing node is this one then folds to
461+
// `selected: 0, acted: 0, unmeasured: 0` — byte for byte the
462+
// summary of a run that had nothing to notify about, and of a
463+
// run whose notify node never executed at all.
464+
//
465+
// ⛔ The fix is NOT to report the zero as `unmeasuredEffect`.
466+
// That flag means "the count is unknown", and this count is
467+
// known and it is zero; claiming otherwise would take the run
468+
// OUT of the broken-sweep filter
469+
// (`selected > 0 AND acted = 0 AND unmeasured = 0`) — the
470+
// platform's own alarm for a green-but-inert sweep — on
471+
// precisely the run that should be inside it.
472+
//
473+
// So the node declares the other half of the pair instead, in
474+
// the key that already means it: `selected` is "records this
475+
// node READ or matched", and the recipients it addressed are
476+
// exactly that. Reporting it costs a delivering run nothing
477+
// (`acted`/`unmeasuredEffect` keep it out of the filter) and
478+
// buys the zero-delivery run its place inside it, which is
479+
// what makes the two runs read differently at all.
480+
//
481+
// It counts audience ENTRIES the node addressed, not resolved
482+
// users: the entry (`role:manager`, a bare id) is what this
483+
// node has: expansion happens inside the messaging service and
484+
// is not reported back. `selected` and `acted` are not
485+
// required to be commensurate anywhere else either — a
486+
// `get_record` selects ten and an update acts on three.
442487
metrics: enqueued > 0
443-
? { ...(delivered > 0 ? { acted: delivered } : {}), unmeasuredEffect: true }
444-
: { acted: delivered },
488+
? { selected: recipients.length, ...(delivered > 0 ? { acted: delivered } : {}), unmeasuredEffect: true }
489+
: { selected: recipients.length, acted: delivered },
445490
};
446491
} catch (err) {
447492
return { success: false, error: `notify failed: ${(err as Error).message}` };

0 commit comments

Comments
 (0)