Skip to content

fix(service-automation): a notify node reports the recipients it addressed, so a zero-delivery run stops reading like a successful one - #17339

Merged
os-zhuang merged 9 commits into
mainfrom
claude/issue-17123-zero-delivery-distinguishable-r2
Sep 10, 2026
Merged

fix(service-automation): a notify node reports the recipients it addressed, so a zero-delivery run stops reading like a successful one#17339
os-zhuang merged 9 commits into
mainfrom
claude/issue-17123-zero-delivery-distinguishable-r2

Conversation

@os-tesla

Copy link
Copy Markdown
Collaborator

Fixes #17337

Clause-②: no — no new exported symbol, no packages/spec/** edit, no error-code-ledger row. Four files: a changeset, content/docs/automation/flows.mdx, notify-node.ts (+51/−3) and a new integration test.

⚠️ This pull request is a re-creation, and its branch is a re-push. The original (#17324, branch claude/issue-17123-zero-delivery-distinguishable) and its card (#17123) were removed when the os-trump account was suspended. That PR is unreachable (404) yet still holds its branch, so GitHub refuses a second PR on it — hence the -r2 name. The card has been re-filed as #17337.

The commits are byte-identical: this branch points at 44a30c57d791a9eb99b1995d5986009200a74317, the original head, pushed unchanged from the local object store. ⛔ No rebase, no amend, no squash.

The original body and the os-dev-report were lost with the account; what follows is reconstructed from the delivering seat's report. Every number below was measured at 44a30c57d before the suspension and has NOT been re-run against today's main — CI on this PR is the current reading.

⭐ The finding that shaped the fix: the obvious repair would have made it worse

The card asked for a zero-delivery run to become distinguishable, and the obvious place to put that is unmeasured. Measuring its contract first says otherwise.

ExecutionStepMetrics.unmeasuredEffect, verbatim (packages/spec/src/automation/execution.zod.ts):

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.

and the same docblock documents the broken-sweep first filterselected > 0 AND acted = 0 AND unmeasured = 0 — noting that an understated 0 puts a run that DID act inside it while an overstated 1 keeps a run that acted on nothing outside it, "which is the original bug back again."

⇒ Writing a measured zero into unmeasured would have taken the zero-delivery run out of the very filter it belongs in. ⛔ A new lie in place of the old silence.

The actual fix

The notify node had been reporting only half of a pair. It now reports selected — the recipient entries it addressed — on every path that reaches a recipient list, with the acted / unmeasuredEffect rules byte-unchanged.

A zero-delivery run therefore reads selected=N acted=0, with no unmeasured, and lands inside the broken-sweep filter — whose first clause it could never satisfy while the node reported no selected at all.

⭐ This closes the class, not one cause: emit() returning delivered:0/enqueued:0 from an empty audience, an all-suppressing preference filter, a dedup hit, an all-failing enqueue, and a stack with no messaging service all land in the same place.

The card's fence, discharged as a measurement rather than a promise

The card forbids closing this because #16659 landed. The differential control therefore drives the schedule arm in both shapes — today's org-less cron tick and the org-carrying one #16659 gives it — and the zero-delivery arm is the post-#16659 one. ⇒ "#16659 is unrelated" is a reading here, not an assurance.

Evidence (measured at 44a30c57d, pre-suspension)

Step 1 — the contract, before any code. unmeasured has exactly one writer (run-summary.ts's summarizeRun, fed by ExecutionStepMetrics.unmeasuredEffect from six executors plus the child-run rollup). Its readers: the [automation] run line (which prints the token only when non-zero), sys_automation_run.unmeasured_count + summary_json, the REST run summary, the map/subflow parent rollups, and one cross-package pin in qa/dogfood. The CLI does not render the counters.

Suites — service-automation 129 files / 1522 tests + typecheck green; spec 470 / 13211; runtime (the 7 files of the run-summary population) 362 tests. ⚠️ The first runtime attempt read 173 failed FILES / 1 failed test and was discarded as NOT MEASURED: every one was Failed to resolve entry for package @objectstack/service-datasource — an unbuilt closure, which reads exactly like a diff that broke every import. qa/dogfood's flow-function-effect.dogfood.test.ts: 2 tests green against a freshly built service-automation dist — the positive control of the cross-package sweep, since that pin resolves this package through dist/.

Differential controlnotify-zero-delivery-visibility.integration.test.ts, 9 tests, 2 trigger families × 2 data layers, with a real MessagingService, a real RecipientResolver, a real outbox and a real inbox channel. ⛔ Neither family is hand-rolled: the schedule arm gets the literal context production's ScheduleTrigger builds; the API arm the identity shape buildAutomationContext copies off a session (pinned against drift in-file). The three published summary lines are asserted verbatim, so the table below is re-runnable rather than recalled.

Ablation — prediction written first (turns RED, 8 of 9; the ninth is the context-shape pin that does not touch the node), one shell under trap restore EXIT INT TERM, mutation observed on disk by anchor count (selected: recipients.length 3 → 0), mutated blob 0ea7f5e9 ≠ HEAD blob 2afe90db, with a loud refusal wired for an empty-or-equal hash. Measured: Tests 8 failed | 1 passed (9) — failing on expected 0 to be greater than 0 and on expected 'selected=0 acted=0 unmeasured=0' not to be 'selected=0 acted=0 unmeasured=0', i.e. rows 1 and 3 becoming the same value again — the finding reproduced. Restored with git checkout HEAD -- <abs> and proven by state: empty git diff HEAD, restored blob == HEAD blob, clean git status --porcelain. ⛔ Never by exit code.

Gates — re-derived at the final head (the first derivation warned STALE TREE, so origin/main was merged and it was derived again — identical family set); --ran reconciles 88 derived, 88 run, 0 UNRUN. ⛔ Its 0 NOT-MEASURED is the runner's own claim and is wrong; per-command exits were captured, so annotated by hand: check:dual-build-cjs-loads exit 3 and check:type-check-debt exit 3 are PREREQUISITE NOT MET = NOT MEASURED, never a pass; check:skill-examples exit 1 is the same thing spelled with the wrong code (it declines because client-react/dist holds no declarations and says a verdict now would be a false green) = NOT MEASURED, not a red about this diff. Three gates were genuinely this diff and are now 0: check:driver-memory-census, check:test-source-alias, check:type-source-resolution. Other 82 green.

Carrierscheck-clause2-carriers.mjs on the original PR: exit 0, "readable in the fixed spelling and both carriers agree". check-partof-closing-keyword with the real PR_BODY + PR_COMMITS_FILE: exit 0, "7 commit message(s) carry no card-relation trailer".

⚠️ One declared deviation from the card's literal acceptance

The card requires the differential control on both drivers, memory and sqlite. The sqlite arm is real (ObjectQL + driver-sql on better-sqlite3). The memory arm is not the mingo InMemoryDriver — it is an in-process IDataEngine, declared as a deviation in the test header.

Two gates each refused a route and each says the author must not decide it: check:driver-memory-census refused binding a third consumer, stating that "is this consumer legitimate?" is a maintainer ruling (#5704 Q2, #6664 A) and the gate is bookkeeping under it, never a second opinion; and check:type-source-resolution refused widening this package's shrink-only type-source registry, calling paths measured-wrong here (the TS6059 shape, PR #12570).

⇒ Neither gate was routed around. The thing actually under test — notify metrics → summarizeRun → the folded payload — never touches a driver, so the storage axis is a robustness check; the reading itself is the trigger-family axis, and that is fully driven on both arms. Ruling a third consumer into the census (option A) is a small maintainer decision that would restore the card's exact wording, if wanted.


Generated by Claude Code

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
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
…emory 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
…ly 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
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
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
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d29fdcfa97bdcef52e8d8312adb4d07870d46d06packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4a937f906db5e201a2f12b1c4db2997e32e90a7f — the merge of head eca44d361489196ca1bf9cefc24123019245bd3f into base d29fdcfa97bdcef52e8d8312adb4d07870d46d06, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4a937f906db5e201a2f12b1c4db2997e32e90a7f && git checkout 4a937f906db5e201a2f12b1c4db2997e32e90a7f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d29fdcfa97bdcef52e8d8312adb4d07870d46d06 eca44d361489196ca1bf9cefc24123019245bd3f && git checkout -B drift-repro d29fdcfa97bdcef52e8d8312adb4d07870d46d06 && git merge --no-ff eca44d361489196ca1bf9cefc24123019245bd3f

node scripts/docs-audit/affected-docs.mjs --json d29fdcfa97bdcef52e8d8312adb4d07870d46d06

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

…e 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
…inators 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

Copy link
Copy Markdown
Collaborator Author

Patch round on top of 44a30c57d — the lint finding, and the two it was masking

Lint & Repo Gates failed at check:engine-double-contract. That is fixed, and so are two further reds on the same double that the same job never reached: the lint job runs its gates as sequential steps under bash -e, so the first non-zero exit halts it and every step behind it goes unrun. check:where-matcher and check:objectql-double-limit sit at later steps in that job and were already RED on 44a30c57d.

All three are one defect class — a test double looser than the engine it stands in for — in one literal, and neither of the two extra gates offers a baseline route ("The baseline never grows"), so the mechanical fix each prints is the only one available.

Control measured in a detached worktree at 44a30c57d with deps installed, against the final head:

gate at 44a30c57d at eca44d361
check:engine-double-contract exit 1 — PINNED [findOne], line 235 exit 0 — OK — 852 pinned, 133 in the DEBT ledger, 3 exempt
check:where-matcher exit 1 — line 232 matches: no combinator branch exit 0 — 398 matcher(s) discovered, 398 answer the combinator battery correctly or refuse it loudly (261 refuse); no files added
check:objectql-double-limit exit 1 — line 241: truthiness, not presence exit 0 — none new; no files added

1 · findOne (the named finding). The in-process IDataEngine arm read an absent filter as "match everything", which ObjectQL.findOne refuses (#4419). It now opens with assertEngineFindOnePredicate(object, query) from @objectstack/metadata-core — the predicate's home since #5619, and already a declared dependency of this package, so no new dependency edge and no objectql reverse edge. The RETAINED ledger learned its row through the gate's own --write (+1 row, 0 lost). scripts/engine-double-contract.baseline.json is untouched byte for byte: f4bfac2a3ee8c3ad31c5204885c9f9503ac3d095 before and after.

2 · 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 instead of answering wrongly (the route the gate sanctions; 261 of 398 matchers in the repo conform that way).

3 · 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.

Nothing was loosened to accommodate any of it. This round's diff contains zero expect( / it( / describe( lines, and all nine cases of the differential control still pass on both arms with their CONTROLs — the in-process arm still genuinely delivers rather than passing quietly:

✓ [driver=in-process] DIFFERENTIAL CONTROL · zero-delivery row · notify node row · CONTROL: cron tick TODAY still delivers
✓ [driver=sqlite]     DIFFERENTIAL CONTROL · zero-delivery row · notify node row · CONTROL: cron tick TODAY still delivers
✓ apiTriggerMatchesProductionBuilder
Test Files  1 passed (1)   ·   Tests  9 passed (9)

At eca44d361: pnpm --filter @objectstack/service-automation typecheck && … testcheck:test-typecheck: OK, 129 passed (129) files, 1522 passed (1522) tests. The derived gate union for this change set (scripts/pm/dispatch-gates.mjs, 96 families) is green except check:dual-build-cjs-loads and check:type-check-debt, which each exit 3 = PREREQUISITE NOT MET = NOT MEASURED locally (both need a whole-repo build) — recorded as unmeasured, never as a pass.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A notify node that enqueued NOTHING reports the same as one that delivered: a zero-delivery run is indistinguishable from a successful one

3 participants