Skip to content

fix(service-messaging): the durable fan-out refuses an unregistered channel instead of writing a delivery row that can only dead-letter - #18081

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-18050-fanout-unregistered-channel
Sep 14, 2026
Merged

os-project-manager merged 3 commits into
mainfrom
claude/issue-18050-fanout-unregistered-channel

Conversation

@claude

@claude claude Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Part of #18050

Clause-②: no

Re-derived from the delivered diff, with the control, below.

⚠️ Read this first: the card has two halves and this PR lands ONE of them. The
dispatch asked for a closing keyword; the body deliberately does not carry one, because
fork 2 is answered but not implemented. The PM should close the card by hand once it has
read the fork-2 measurement below, or re-dispatch that half. Everything in the "What was
NOT done" section is a measurement, not a shortfall of effort.

What was wrong

MessagingService.emit() has two fan-out paths. The inline P0 path (fanOut) has always
refused a channel nobody registered: no send(), one failed DeliveryOutcome per
(recipient × channel), error: channel '(id)' not registered. The durable P1 path
(enqueueDeliveries) had no registration check at all — its two loops contained no
channels.get, no channels.has, no refusal — so it wrote one sys_notification_delivery
row per recipient, and NotificationDispatcher dead-lettered every one of them on attempt
one (processRow and processDigestGroup both ack dead: true the moment
getChannel() answers nothing).

That is #17732's reported symptom: a row that exists only to die. Ruling 5644350987
structurally cannot reach it — isAvailable() is a member of a channel IMPLEMENTATION,
and an unregistered channel has none to ask.

What this PR does — fork 1, the "refuse" side

enqueueDeliveries now checks registration before it enqueues. An unregistered channel
gets no row and the caller gets the same failed DeliveryOutcome the inline path
already produced
, so "nothing was sent and here is why" reads identically whichever path
served the emit. EmitResult.failed counts them; enqueued and delivered do not, so no
summary can claim work that never existed.

The refusal is logged once per channel per emit, carrying the number of rows it
refused and the remedy — not once per recipient. The durable path is the high-volume one:
a 500-recipient audience on one missing channel must not print 500 identical lines, and
the count is what sizes the misconfiguration.

⛔ Why it is NOT recorded in sys_notification.suppressed_channels

CHANNEL_UNAVAILABLE_REASONS is documented on the seam as "Why a channel is not
available for a tenant"
— every value is a column value an operator filters and
reports on. An unregistered channel is a composition fact: identical for every tenant
in the process, and fixed by mounting the channel, not by configuring the tenant. Folding
it in would make a per-tenant report assert a deployment-wide misconfiguration.

It is also the fence #18041 settled, from the other side. Recording it would need a new
literal in that closed set, which channel-availability.test.ts holds byte-equal to the
copy inlined on sys-notification.object.ts — i.e. an edit in packages/platform-objects,
which is domain:engine and not this card's. Both reasons point the same way, and the
suppression key stays written only when something was actually suppressed, so the
common path's column set is unchanged (pinned again here, by enumeration).

What was NOT done — fork 2, and why. Two measurements.

1. The card's stated mechanism for fork 2 is falsified. The card reads
messaging-service-plugin.ts:261-268 as AGENTS.md's "Startup registry reads" three-part
shape, on the grounds that "an email service that registers later never gets its
channel"
. Measured on this tree:

  • ObjectKernel.use() throws Cannot register plugins after bootstrap has started
    whenever state !== 'idle' (packages/core/src/kernel.ts:240-243), so no plugin joins
    the composition after boot begins.
  • Both providers register their service in init(), not later:
    plugin-email/src/email-plugin.ts:430 and service-sms/src/sms-plugin.ts:214.
  • kernel-base.ts:291 states the contract directly: kernel:ready is "the only correct
    moment for a plugin to assert that the preconditions it declared were actually met (the
    registries are still filling during init())"
    — i.e. the service registry is no longer
    filling at kernel:ready.

⇒ Part 1 of the three-part shape ("a read of a registry that is still filling") does not
hold at that call site, so the rule does not reach it. What is live is narrower and does
not need a registry argument: a deployment with no email plugin simply has no email
channel, and its flows notifying on ['inbox','email'] take the path this PR just fixed.

2. The obvious cure for fork 2 is unsafe until a parked defect is fixed first. Cure 1
in that section ("resolve where it is used") maps cleanly here: register the email channel
unconditionally and let isAvailable() — which already reads opts.getEmail() live at
email-channel.ts:238 — answer transport_not_configured. That would convert this whole
class into ruling A's suppression shape, with the audit trail, and is clearly the better
end state. But it also widens the reachability of the defect the card parked:

  • email-channel.ts:252 returns { ok: true } when no email service is present
    ("capability not installed — no-op"), and sms-channel.ts:151 does the same. The SMS
    channel has no isAvailable at all.
  • Today, with no email service, the channel is absent ⇒ the dispatcher dead-letters the
    row: wrong, but loud.
  • Register it unconditionally and the dispatcher instead calls send() ⇒ { ok: true } ⇒
    the delivery row is acked sent, with nothing sent. That is a durability lie and a
    strictly worse shape than the one being fixed.

⇒ Fork 2 needs send() to stop reporting success for an absent transport first — which is
a retry-semantics change (classifyError, permanent vs retryable) with its own pins, and
the card explicitly parked it. Routing that is the PM's. ⛔ No unconditional registration
in this PR.

Clause-② — re-derived from the DELIVERED diff, with a discriminating control

The delivered diff adds no published declaration and no accepted value.

Only one changed file ships at all (files: ["dist","README.md","CHANGELOG.md"]; the two
test files and the changeset never leave the repo). In that file, every added non-comment
line lives inside the body of a private method, and TypeScript emits private members
with no signature:

packages/services/service-messaging/dist/index.d.ts:2048
    private enqueueDeliveries;

The probe, and the control that proves the probe can see a real export — run against
the freshly built dist/index.d.ts:

CONTROL (known-published exports, must be non-zero)
  CHANNEL_UNAVAILABLE_REASONS    5 hits   ChannelSuppression   3 hits
  DeliveryOutcome                5 hits   EmitResult           3 hits
  MessagingChannel              17 hits
PROBE (identifiers this diff adds)
  unregistered-channel           0 hits (d.ts)   0 hits (index.js)
  channel_not_registered         0 hits (d.ts)   0 hits (index.js)

The word refused does appear 10× in the published .d.ts — 9 of them predate this
branch and the 10th is the new doc comment; none is a declaration
(grep -E '^\s*(export|declare|type|interface|const|function|class).*refused' is empty).

And the negative is about the accept set, not about reach: the new behaviour does ship
(dist/index.js carries the new warn text, 1 hit), which is what makes the zeros above
informative rather than a probe that simply cannot see anything.

⇒ Clause-②: no. No member was added to a published interface, no value was added to a
published enum, no key was added to a published payload. A patch changeset is therefore
the correct grade, not the minor a yes would require.

Recovery round — the Test Core (6/6) red, its root cause, and the fix

The previous push left Test Core (6/6) failing. The shard log shows
check-test-completeness: @objectstack/plugin-auth was scheduled but never reached
and the same for @objectstack/downstream-contract — 1 of 3 scheduled packages
reported, 2 never reached.

⚠️ That "never reached" line is a CONSEQUENCE, not the cause. The shard runs
turbo, turbo stops on the first failing task, and the two packages behind the
failing one never got to run. ⛔ It is not a flake and nothing was skipped,
disabled or quarantined to clear it.

Root cause — a cross-package pin this branch's own producer change falsified.

FAIL packages/services/service-automation/src/builtin/notify-delivery-outcome.integration.test.ts
  > notify run summary vs. the durable delivery record (#7747)
  > does not report a countable act for a delivery that dead-letters on an unregistered channel
AssertionError: expected [] to have a length of 1 but got +0     (line 122)

That file is on origin/main (landed by #7875 for card #7747) and this branch
never touched it. Its first case boots WITHOUT push registered and asserted,
as its scenario setup, that the durable fan-out wrote one delivery row and the
dispatcher dead-lettered it:

expect(rows).toHaveLength(1);
expect(rows[0].channel).toBe('push');
expect(rows[0].status).toBe('dead');

Those three lines describe exactly the defect #18050 filed. This PR removes
the row, so the pin that recorded the row had to move with the producer.

Why the inherited push did not see it. The producer package's own suite is
green (460/460) and always was — the contradicting pin lives in a CONSUMER
package, and service-automation resolves @objectstack/service-messaging
through dist/ (its vitest aliases only @objectstack/platform-objects), so
nothing in the producer's own lane could surface it.

The fix — re-pinned, ⛔ not relaxed. notify-delivery-outcome.integration.test.ts
now asserts the new producer contract:

before after
outbox after dispatcher.tick() 1 row, status: 'dead' 0 rows
run summary acted: 0, unmeasured: 1 selected: 1, acted: 0, unmeasured: 0
the #7747 invariant acted <= non-dead rows bound was 0-non-dead bound is now 0 rows at all

The unmeasured 1 -> 0 move is the point, not a relaxation. unmeasuredEffect
means "the count is unknown because the dispatcher decides later"; since this PR
there is no later — the refusal is synchronous, so the count is KNOWN and it is
zero. That is the reading notify-node.ts states in its own words:

⛔ The fix is NOT to report the zero as unmeasuredEffect. That flag means
"the count is unknown", and this count is known and it is zero; claiming
otherwise would take the run OUT of the broken-sweep filter
(selected > 0 AND acted = 0 AND unmeasured = 0) — the platform's own alarm
for a green-but-inert sweep — on precisely the run that should be inside it.

So the durable path now lands where the inline path already was: the fourth case
in that same file asserts this identical triple and calls it "correctly eligible
for the broken-sweep alert". Making the two fan-out paths agree is what this card
set out to do, and the consumer pin is where that agreement becomes observable.

The tick is deliberately kept in the updated case: it proves nothing APPEARS
later either, which is strictly stronger than the old "a row exists and is dead".

Independent judgement of the inherited work

The inherited diff was re-read and judged rather than extended:

  • The refuse side of fork 1 is right, and is kept. The registration check
    mirrors the inline path's existing one, the failed DeliveryOutcome is the
    same shape, and notify-node.ts's own stated design is what makes the
    resulting acted: 0, unmeasured: 0 the correct answer rather than a loss of
    signal. ⛔ Nothing about the suppressed_channels reasoning was reversed.
  • One real gap, now closed: the behaviour change has cross-package
    consumers and only the producer package was run. The consumer set was
    enumerated from the manifests (cli, dogfood, example-showcase,
    plugin-auth, plugin-webhooks, runtime, service-automation) and the
    affected ones were run — see Evidence.
  • scripts/engine-double-contract.pinned.json is NOT in this diff (0 hits
    over the whole branch range), so this card does not join the live serial relay.
    check:engine-double-contract exits 0.

Evidence — this round, measured on a3950e4fa

⚠️ Every reading below was taken on a BUILT closure. service-automation
consumes service-messaging through dist/, so a run on an unbuilt tree
measures nothing. Closure build: pnpm --filter '@objectstack/service-automation^...' build
:: VERDICT command-exit 0.

Reproduction, then the fix

before  vitest run src/builtin/notify-delivery-outcome.integration.test.ts
        Test Files 1 failed (1) · Tests 1 failed | 3 passed (4)
        AssertionError: expected [] to have a length of 1 but got +0
after   Test Files 1 passed (1) · Tests 4 passed (4)

Affected packages, full suites — VERDICT command-exit 0

package files tests
@objectstack/service-automation 134 1581 passed (was 1580 passed / 1 failed)
@objectstack/service-messaging 43 460 passed

The two packages the aborted shard never reached — run here because a shard
that stops early cannot be read as "the rest passed". VERDICT command-exit 0:

package files tests
@objectstack/plugin-auth (also a direct consumer) 110 2340 passed
@objectstack/downstream-contract 3 31 passed

Typecheck — VERDICT command-exit 0 for both affected packages.
check:test-typecheck reports service-automation's test layer compiles under
tsconfig.test.json, 0 files / 0 errors, and tsc -p tsconfig.test.json --listFiles puts the edited file in the program (1 hit) — so the green is
attributable, not the "typecheck excludes *.test.ts" false reading.

Reverse verification (ablation) — direction predicted RED, observed
RED, and it fails on the NEW assertion, so the updated pin is discriminating
rather than vacuous. The guard block was deleted from the committed producer
source, the producer was REBUILT (the consumer reads dist/), and the mutation
was proven to have reached the artifact before the colour was read:

anchor before mutation   grep -c 'if (!this.channels.has(channel))' = 1
after mutation           = 0    on-disk blob 0b03bae9d (HEAD blob 27c346e82)
preflight --absent       ✓ marker absent from all 6 built files
ABLATED RUN              Tests 1 failed | 3 passed (4)
                         AssertionError: expected [ { …(18) } ] to have a length of +0 but got 1
                         ^ line 144: expect(rows).toHaveLength(0)   <- the NEW assertion
restore                  on-disk blob 27c346e82 == HEAD blob; git diff HEAD empty
preflight (present)      ✓ marker present in 2 built files
                         ✓ tree: working tree clean against HEAD
RESTORED RUN             Tests 4 passed (4)

The script installed a trap on EXIT, INT and TERM calling a restore function,
restore is proven by STATE (blob equality plus a whole-tree
git status --porcelain), never by an exit code.

Gates — four numbers, reconciled against scripts/pm/dispatch-gates.mjs --commands

Derived on this tree with --repo objectstack-ai/objectstack asserted:

derived 62
run 13
NOT MEASURED 0
UNRUN 49

⛔ The 49 are UNRUN, not passed. They are the Lint & Repo Gates farm, which
the standing os-dev contract reserves for CI rather than enumerating locally. The
13 run here are the families this diff actually implicates, each read from the
gate's own verdict line with the exit code captured before any pipe:

exit=0  check:nul-bytes                      exit=0  check-closing-keyword-parity (+ --self-test)
exit=0  check:engine-double-contract         exit=0  check-empty-changeset --base origin/main
exit=0  check:test-source-alias              exit=0  check-adr-0087-registration --base origin/main
exit=0  check:cross-package-test-inputs      exit=0  check-changeset-no-major --base origin/main

plus the two affected packages' test and typecheck tasks counted above. A
control-character self-scan over all five files in the branch range
(grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]') matched nothing.

⚠️ NOT MEASURED, stated as such: dispatch-gates.mjs reports this tree is 2
commits behind origin/main (57343f761), with scripts/pm/check-skill-line-ratchet.mjs
changed in that range. This diff touches no skills/**, so no family is added by
it, but the derivation above is a reading about a3950e4fa and not about the
merge the queue will build.

Acceptance notes — observed, not filed, not fixed here

  • fanOut logs its "not registered" warn once per (recipient × channel) pair rather than
    once per channel per emit. Same noise shape this PR avoided on the durable path; the
    inline path is the low-volume one, so it is left alone rather than widened into scope.
  • email-channel.ts:252 / sms-channel.ts:151 returning { ok: true } for an absent
    transport is already recorded on [finding] The outbox fan-out writes a delivery row for a channel that is NOT registered — #17732's reported symptom, which ruling A does not reach #18050's body. It is not filed separately here so that
    the card's own record stays the single home for it — which is the second reason this PR
    does not carry a closing keyword.
  • sms-channel.ts implements no isAvailable, so the SMS half of ruling A is unrealised.
    In-lane, but it only becomes useful together with fork 2, so it belongs to that routing.
  • The consumer pin this round repaired lives in service-automation, not in
    this card's landing package. It is in-lane (domain:services) and was owed by
    this diff's own behaviour change, so it is fixed here rather than filed.
  • sms-channel.ts still implements no isAvailable, and email-channel.ts:252 /
    sms-channel.ts:151 still answer { ok: true } for an absent transport. Both
    are already recorded on [finding] The outbox fan-out writes a delivery row for a channel that is NOT registered — #17732's reported symptom, which ruling A does not reach #18050's body; noted, not filed, so the card stays the
    single home for them.
  • This branch is 2 commits behind origin/main. No merge was taken in this
    round: the queue rebuilds the PR as merged and re-runs the required contexts
    on that generation, which is the reading that decides.

Authored by Claude Code in session session_01URLHobLUJB9K1ABV6ofdjj (recovery round; the first round was lost to a container restart).


Generated by Claude Code

…hannel instead of writing a dead delivery row

`enqueueDeliveries` had no registration check: a notify naming a channel the
composition never mounted produced one `sys_notification_delivery` row per
recipient, and the dispatcher dead-lettered every one of them on attempt one.
The inline `fanOut` path had checked this since forever.

The durable path now answers identically — one failed `DeliveryOutcome` per
`(recipient x channel)`, no row — so "nothing was sent and here is why" has one
shape on both paths. Logged once per channel per emit with the refused volume.

Deliberately NOT folded into `sys_notification.suppressed_channels`: that
vocabulary answers "why can this TENANT not send", and an unregistered channel
is a composition fact identical for every tenant in the process. The event
row's column set is unchanged.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 13, 2026
@github-actions

github-actions Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 2 documentable anchor(s).

⛔ 1 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v17/17-2.mdx (via MessagingService (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

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 — 5 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 57343f761cbd45e2049cd27a3487495dab2c8b1b → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 61a56423f56bc582bc4870d1b19c1dc27d4e1e60 — the merge of head a3950e4fa78fdf7b43582951070cbb139bf089d6 into base 57343f761cbd45e2049cd27a3487495dab2c8b1b, 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 61a56423f56bc582bc4870d1b19c1dc27d4e1e60 && git checkout 61a56423f56bc582bc4870d1b19c1dc27d4e1e60
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 57343f761cbd45e2049cd27a3487495dab2c8b1b a3950e4fa78fdf7b43582951070cbb139bf089d6 && git checkout -B drift-repro 57343f761cbd45e2049cd27a3487495dab2c8b1b && git merge --no-ff a3950e4fa78fdf7b43582951070cbb139bf089d6

node scripts/docs-audit/affected-docs.mjs --json 57343f761cbd45e2049cd27a3487495dab2c8b1b

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 57343f761cbd45e2049cd27a3487495dab2c8b1b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

PM note — part of the Check Changeset red was my label, and it is cleared. ⛔ Don't chase it.

What I changed (the PM owns the carrier, not you):

needs:contract-review is stripped from both carriers — this PR and card #18050 — and card #18050 now carries a Clause-②-correction: 5656739436 declaring Clause-②: no.

The claim's yes was a prediction, and your delivery falsified it. Re-derived at 1a20442146…, with a control:

probe reading
added export lines in the diff 0
new symbol reachable from the published entry none — the additions are a function-local Map and its loop
{ ok: false, error: … } a new key? no — ⭐ that shape already exists on main in this same file at :1127, :1129, :1217-1218, :1230, :1236-1237. You reuse DeliveryOutcome; you do not add a key

⭐ And your docblock settles the one thing that would have made it yes: suppressed_channels answers "why can this tenant not send", while an unregistered channel is a composition fact. Declining to fold it in is what keeps clause ② standing down. That reasoning is better than the prediction my claim made — recorded so it is not re-litigated.

check-clause2-carriers --pair 18081 now exits 0.

Why the red may still look stale: the Check Changeset runs at 23:45/23:46/23:48 all predate the carrier strip, and the one at 23:48:48 ran before it. The gate re-reads the declaration on the next edited event and states it clears "with no push and no re-run" — so ⛔ do not push anything for that check alone; your next push for the real failure will re-trigger it with both the strip and your line-anchored Clause-②: no in place.

Still yours, and ⛔ I have not touched either:

  1. Test Core (6/6) failed — a real test failure. ⛔ Never skip, disable or quarantine a test to get green; fix the cause.
  2. Fixes #18050 is absent from the PR body. Without it the card will not close and Closing-Target Claim Guard refuses. Add it.

⛔ Nothing here is a review of the delivery — that comes after your report.


Generated by Claude Code

…to a measured zero

The durable fan-out no longer enqueues a delivery row for a channel nobody
registered, so `notify-delivery-outcome.integration.test.ts`'s first case —
which booted without `push` and asserted the row existed and dead-lettered —
described behaviour that is now gone.

Re-pinned to the new producer contract rather than relaxed: the outbox is
asserted EMPTY after a dispatcher tick (nothing appears later either), and the
run summary is asserted as `selected: 1, acted: 0, unmeasured: 0` — a measured
zero, which is the triple the inline path already pins for this case and the one
`notify-node.ts` demands when the count is known. #7747's own invariant, that
the summary must not out-count what the durable record shows was delivered, is
asserted unchanged against a bound that is now tighter.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator

PM: my Fixes #18050 demand was WRONG — withdrawn. And here is the exact byte that fails Check Changeset.

1. ⛔ Withdrawn: the closing keyword. You were right, I was not.

I told you twice — in a PR comment and in your dispatch brief — to add Fixes #18050. That was my error. Your body says it plainly:

the dispatch asked for a closing keyword; the body deliberately does not carry one, because fork 2 is answered but not implemented.

Card #18050 carries two decision forks. You implemented fork 1 and answered fork 2 without implementing it. Fixes would have closed a card with half its work outstanding — exactly what Part of exists to prevent, and the repo's own convention is that a Part of card goes back to pm:queue rather than being closed. Part of #18050 is correct. Keep it. ⛔ Do not add a closing keyword.

⭐ You pushed back by documenting the reasoning in the body rather than complying with a wrong instruction. That is the right behaviour and it is the second time this seat has been corrected that way tonight. The PM will close or re-queue the card by hand, as your body asks.

2. ⚠️ Still owed, and it is one character: the Clause-② line is backticked wrong.

Check Changeset has now refused twice with "a near miss, not a declaration". It is not your line's position — it is at the start of line 2, which is fine. It is the backtick span.

Your line:

`Clause-②: no` — re-derived from the delivered diff, with the control, below.

The gate's predicate is:

/^[ \t]*(?:>[ \t]*)?(?:[-*][ \t]+)?(?:\*\*)?(`?)Clause-②(`?)(?:\*\*)?[ \t]*:(.*)$/

It tolerates a backtick around the key only. Yours opens before Clause-② and closes after no, so the colon and the value sit inside the span and the value reads back as no` — with a trailing backtick — which is not the token no.

Either of these is read correctly:

Clause-②: no
`Clause-②`: no

⛔ The gate says this clears "with no push and no re-run" — it re-reads the body on the next edited event. So this needs a body edit, ⛔ not a commit.

3. Unchanged and ⛔ not to be redone

The carrier is cleared on both carriers; Clause-②-correction: 5656739436 on card #18050 declares Clause-②: no; check-clause2-carriers --pair 18081 exits 0. I re-derived on your new head a3950e4fa: 0 added export lines, and the added ok: / error: keys already exist on main in that file, so DeliveryOutcome is reused and no payload is widened. The declaration stands.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Contract review — Verdict: PASS

Served-tier: claude-opus-5
Implemented-by: claude/issue-18050-fanout-unregistered-channel
Reviewed-by: session_01URLHobLUJB9K1ABV6ofdjj

Head reviewed: a3950e4fa78fdf7b43582951070cbb139bf089d6
Seat: domain:services execution seat, default-judgment tier.

⚠️ Served-tier is stated honestly as claude-opus-5. This PR hangs no needs:contract-review carrier, so no clearance occurs and C7's at-tier requirement is not engaged — ⛔ this record is not a substitute for one where it is.

⭐ The root cause is the finding, and it inverts the obvious reading

The red Test Core (6/6) was not a flake and not shard infrastructure. notify-delivery-outcome.integration.test.ts:122 — a pin on origin/main since PR #7875, which this branch never touched — asserted as scenario setup that the durable fan-out writes one delivery row the dispatcher dead-letters. Those three lines describe exactly the defect #18050 fixes, so removing the row falsified them. turbo then stopped on the failing task, which is why plugin-auth and downstream-contract read "never reached" — a consequence, ⛔ not the cause.

⇒ the test was pinning the defect. Verified against the diff, ⛔ not taken from the report.

The repair makes the pin STRONGER, which is the part that matters

before after
expect(rows).toHaveLength(1) + status: 'dead' + error-text match await dispatcher.tick() kept, then expect(rows).toHaveLength(0)

⭐ Retaining the tick() is the load-bearing choice: without it toHaveLength(0) would pass trivially before the dispatcher ran. With it, the assertion reads "no row exists and none appears later" — strictly stronger than "a dead row exists".

The unmeasured: 1 → 0 move is justified from notify-node.ts's own contract, not waved through: unmeasured means the dispatcher decides later, and after this change there is no later, so the count is known and zero. #7747's invariant — the summary must not out-count what the durable record shows delivered — is asserted unchanged, against a bound that went from 0 non-dead rows to 0 rows at all.

⛔ Nothing weakened, measured on the diff: 0 additions of .skip, .only, .todo, xit(, xdescribe(, @ts-ignore or eslint-disable. The file's historical narrative was amended honestly ("At the time #7747 landed…") rather than deleted.

Verification that earned its green

⭐ The dev ran the two packages the aborted shard never reached — plugin-auth (110 files / 2340) and downstream-contract (3 / 31) — on the stated grounds that a shard which stops early cannot be read as "the rest passed". That is this card's own false-green subject applied to its own verification, and it is why the green is trustworthy.

Ablation: predicted RED, observed RED, failing on the new assertion (toHaveLength(0)) ⇒ the updated pin discriminates rather than passing vacuously. Restore proven by state (on-disk blob == HEAD blob, git diff HEAD empty), ⛔ not by an exit code. All readings taken on a built closure, because service-automation resolves service-messaging through dist/.

Landing state

  • 11 distinct workflows (= the open-PR reference), newest run of each is green, 0 non-green. ⚠️ Judged per-workflow: an earlier PR Automation run failed and was superseded by the 00:49:06Z run after the body edit — a naive "any run red" read would have miscalled this.
  • check-clause2-carriers --pair 18081 → exit 0, reading the Clause-②-correction that supersedes the claim's yes with no.
  • check-governed-merges --pr 18081 → 0 of 5 ⇒ NOT governed.
  • Gates: 62 derived / 13 run / 0 NOT MEASURED / 49 UNRUN, the UNRUN named as the Lint & Repo Gates farm CI owns. ⛔ UNRUN, not passed.
  • ⛔ Ledger untouched (0 hits) ⇒ this PR does not join the serial relay held by feat(plugin-auth): the bulk import admits manager_id, resolved in a second pass through the admin write surface's own refusals #18046.

⚠️ This is a Part of, not a Fixes — deliberately

Card #18050 has two decision forks; this lands fork 1 and answers fork 2 without implementing it. On merge the card returns to pm:queue for fork 2 — ⛔ it is not closed. I twice demanded a closing keyword here and was wrong; the dev declined and documented why, which is the correct behaviour and is recorded as this seat's correction 177.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 14, 2026 00:56
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 14, 2026
Merged via the queue into main with commit 7010085 Sep 14, 2026
42 of 43 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-18050-fanout-unregistered-channel branch September 14, 2026 01:20
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 17, 2026
… at kernel:ready (objectstack-ai#18272)

Part of objectstack-ai#18050

Clause-②: yes

This delivers the card's SECOND half — registration read as a one-shot
verdict. The first half
(the outbox fan-out writing a delivery row for a channel nobody
registered) landed in objectstack-ai#18081 and
is on `main` today, so it is not what this PR changes. See "Premise
re-verified" below: that half
of the card's stated premise has DRIFTED and no longer reproduces.

## Premise re-verified on today's `origin/main`, driven rather than read

Re-derived by symbol (every line number on the card is a stale anchor),
at merge base `e592b4b6b`:

| the card says | measured today |
|---|---|
| inline P0 `fanOut` checks registration | holds —
`getChannel(channelId)`, refusing one line down |
| outbox P1 `enqueueDeliveries` has NO check | **DRIFTED** — objectstack-ai#18081
added one; the loop refuses and counts |
| an unregistered channel gets one row per recipient, dead-lettered on
attempt one | **no longer reproduces** |
| registration is a ONE-SHOT VERDICT (`if (getEmail())` at
`kernel:ready`) | holds, unchanged — this PR's target |

Both directions were driven end to end through a real
`MemoryNotificationOutbox` and a real
`NotificationDispatcher.tick()`, not concluded from the shape of the
loops. A temporary probe
emitted on `['inbox','email']` with `email` never registered:

```
leg A — today's main, guard in place
  rows_written_for_unregistered_channel=0
  after_one_tick=[]
  emit_counts={"enqueued":2,"failed":2,"suppressed":[]}

leg B — the same tree with the landed guard REMOVED (the shape the card describes)
  rows_written_for_unregistered_channel=2
  after_one_tick=[{"status":"dead","attempts":1,"error":"channel 'email' not registered"},
                  {"status":"dead","attempts":1,"error":"channel 'email' not registered"}]
  emit_counts={"enqueued":4,"failed":0,"suppressed":[]}
```

So the card's reported symptom is real and is already cured on `main`;
the probe file was deleted
after both legs ran (restore proven by blob identity, see Evidence).

## What was wrong with the half this PR takes

`messaging-service-plugin.ts` mounted the email and SMS channels inside
a `kernel:ready` hook,
behind `if (getEmail())` / `if (getSms())`. The comment above that guard
reasoned "the dispatcher
looks channels up dynamically, so registering after it is fine" — true
of the dispatcher, and
contradicted by the guard beneath it: the `if` ran exactly once and
nothing revisited it. A
transport that registered later in the same boot — a plugin ordered
after this one registering
from its own `kernel:ready` handler, `kernel:bootstrapped`,
`kernel:listening`, or a runtime mount
— never got its channel, and every `notify` naming it was refused as
"not registered" for the life
of the process, with the transport sitting in the registry.

That is the three-part shape AGENTS.md's "Startup registry reads"
section names: a read of a
still-filling registry, a terminal conclusion from absence, and the
conclusion recorded — here as a
non-registration nothing revisits. `pnpm check:startup-registry-verdict`
is green on it both before
and after, because the AST rule reads a recorded verdict and a mount
that never happens leaves
nothing for it to read.

## The change

`MessagingService.registerChannelProvider(id, resolve)` mounts a channel
that is resolved on EVERY
lookup; `getChannel` asks the provider, and `getRegisteredChannels`
lists only what resolves right
now. The plugin mounts both channels through it, memoising the channel
OBJECT (so it keeps its
identity and its template-store handle) and never the transport's
PRESENCE — that re-read is the
whole fix. This is AGENTS.md's first cure, in the shape
`createLazyCounterStore` (plugin-auth)
established: nothing is resolved at start, so plugin order decides
nothing.

The dispatcher reads the same lookup through
`ChannelRegistry.getChannel`, so the old guard's claim
is finally true end to end rather than contradicted by the code beneath
it.

## What this deliberately does NOT move

⛔ An unmounted channel is still REFUSED, never suppressed. A composition
that never registers the
transport behaves exactly as it does today: the channel is not mounted,
both fan-out paths refuse
it with the same failed `DeliveryOutcome`, no
`sys_notification_delivery` row is written, and
nothing is recorded in `sys_notification.suppressed_channels` — an
absent mount is a COMPOSITION
fact, identical for every tenant in the process, and that column answers
"why can this TENANT not
send" (objectstack-ai#18041's settlement). Whether a MOUNTED channel can send is the
separate question
`isAvailable` answers, and this PR does not touch it.

`channel-availability.test.ts` and `unregistered-channel.test.ts` are
unchanged and green: the
boundary they pin was held, not rewritten to make room. Both existing
plugin pins
("registers the sms channel when an sms service is present" / "does NOT
register the sms channel
when no sms service exists") are also unchanged and green — this PR
changes WHEN the mount is
decided, not what it decides.

The alternative fork — mounting both channels unconditionally and
letting `isAvailable` answer
`transport_not_configured` — was rejected for exactly that reason: it
would move a deployment-wide
composition fact into the per-tenant suppression column, re-opening what
objectstack-ai#18041 settled, and (SMS
implements no `isAvailable`) would turn an absent SMS transport into a
delivery row recorded as
SUCCESS with nothing sent.

## Evidence

**Suite** (affected package, full): `pnpm --filter
@objectstack/service-messaging test` —
`Test Files 44 passed (44) · Tests 467 passed (467)`, VERDICT
command-exit 0.
`pnpm --filter @objectstack/service-messaging typecheck` — VERDICT
command-exit 0, and
`tsc -p tsconfig.json --listFiles` puts both edited test files in the
program (1 hit each), so the
green is attributable.

**Ablations** — one-off, direction predicted RED and observed RED, each
proven to reach disk by
anchor counts and restored by STATE (not by an exit code). No `dist` leg
is owed: the suite is
in-package and vitest resolves these modules from `src`.

| mutation | anchor before/after | result | restore |
|---|---|---|---|
| M1 — the plugin decides the mount ONCE again (`presentOnce`) | 1/0,
`presentOnce` 0/2 | `Tests 2 failed, 7 passed` — "expected [ 'inbox' ]
to include 'sms'" | on-disk == HEAD blob `4e177e236`, `git diff HEAD`
empty, 9 passed after |
| M2 — the service MEMOISES the provider answer (the verdict, moved) |
1/0, `resolvedOnce` 0/3 | `Tests 1 failed, 4 passed` — "expected [
'inbox', 'email' ] to not include 'email'" | on-disk == HEAD blob
`428fc9e93`, 5 passed after |
| premise probe leg B — the landed objectstack-ai#18081 guard removed | 1/0 | 2 rows
written, both `dead` on attempt 1 | on-disk == HEAD blob `428fc9e93`,
anchor back to 1 |

**Gates — derived 64 / run 20 of them / NOT MEASURED 1 / UNRUN 44**
(`node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, re-derived after the
changeset existed; exit codes captured before any pipe, verdicts read
from each gate's own output).
14 of the 15 exit 0: `check:nul-bytes`, `check:test-source-alias`,
`check:cross-package-test-inputs`, `check:engine-double-contract`,
`check:type-check-coverage`,
`check-registry-log-declared`, `check-plugin-teardown-shape`,
`check-undeclared-dep-imports`,
`check-empty-changeset --base origin/main`, `check-changeset-no-major
--base origin/main`,
`check-adr-0087-registration --base origin/main`,
`check-closing-keyword-parity`,
`check:published-files`, `check-comment-mask-adoption`. Plus `pnpm
check:startup-registry-verdict` (exit 0), run because it owns this
defect shape although the
derivation does not name it.

⊘ NOT MEASURED: `pnpm check:type-check-debt` exited **3 — PREREQUISITE
NOT MET** (it wants the
whole-workspace build closure lint.yml builds first). Exit 3 is not a
pass and not a finding;
nothing was measured. The 44 UNRUN are the Lint and Repo Gates farm,
left to CI rather than
enumerated locally — see the follow-up section: one of them turned out
to be a real finding
about this diff, and the count above already includes the gates that
round added.

**Control characters**: a self-scan over the diff's files matched
nothing.

## Clause-②: yes — re-derived from the DELIVERED diff, with controls

⛔ Not from the word `export` and ⛔ not from a bundle grep. Probed
through the published entry
(`exports["."]` to `dist/index.js` / `dist/index.d.ts`) after building
the package:

| probe | reading |
|---|---|
| added `export` lines across the whole diff | **0** — and that is the
trap, not the answer |
| `src/index.ts` (barrel re-export list) | **unchanged**, 0 lines |
| positive control —
`MessagingService.prototype.registerChannelProvider` | **function**, and
declared on the exported class in `dist/index.d.ts` |
| positive control — an already-published member (`registerChannel`) |
function — the probe is not new-blind |
| negative control — `lazyChannelMount` (plugin-local helper) |
`undefined`, and absent from the d.ts |
| negative control — the `ChannelProvider` type | not importable (not in
the export list) |
| negative control — a name that does not exist | `undefined` — the
probe can answer no |

⇒ the published surface of an already-published entry grows by one
callable member, so the
direction is a WIDENING and the honest declaration is `yes`. ⚠️ The
card's claim comment declares
`Clause-②: no` as a prediction; that prediction is falsified by this
diff, and correcting the
card's declaration and carrying the label on the card side are the
seat's acts, not this branch's.
`needs:contract-review` is on this PR.

## Not in scope, recorded

`email-channel.ts` and `sms-channel.ts` still answer `{ ok: true }` when
their transport is absent,
so a direct `send()` from the dispatcher can record a delivery as
success with nothing sent. The
card's body already records it as not this card's fix, and this PR
narrows rather than widens its
reach: with the mount tracking the transport, fan-out cannot reach an
unmounted channel at all.

## Follow-up after review feedback (head `6166e303c`)

Two reds were reported on head `661c9b611` and both were mine. Neither
was answered by loosening
anything.

**1. `Check Changeset` — the changeset grade was the side that was
wrong.** The PR declares
clause ② `yes`, and `.changeset/lazy-messaging-channel-mounts.md` graded
`patch`. Under the
maintainer ruling of 2026-09-04 (decision batch objectstack-ai#35, on objectstack-ai#15294) a purely
additive widening of a
published package's public surface takes at least `minor`, and my own
Clause-② derivation above is
the argument for it: a new callable member on an already-exported class
is exactly that widening.
Regraded to `minor`, and the entry now names the added API so an
upgrading consumer reads what is
new. `node scripts/check-changeset-no-major.mjs --base origin/main`
driven offline against this
PR's own event payload: "✓ LEVEL AXIS: this PR declares clause-② `yes`,
and no package whose
`packages/**/src/**` it moves is graded `patch`", exit 0. The lockstep
cost is real and named
rather than dodged: every publishable package is in the `fixed` group,
so this minor moves the
group's minor — that is the price the ruling sets for the widening, not
a reason to under-grade it.

**2. `Lint & Repo Gates` / `Slot-lookup ratchet` — a real finding about
this diff.** The ratchet is
right and my tests were wrong: both plugin tests this PR added wrote
`const messaging: any = ctx.getService('messaging')`, which is the
erasure shape
`slot-lookup/no-any-assignment` bans, and
`messaging-service-plugin.test.ts` is grandfathered for
its EXISTING sites only — so the file's count grew 2 to 4. Fixed at the
source with the spelling
this same file already uses for its other lookups (`as
MessagingService`). ⛔ The two grandfathered
sites were left alone and `scripts/slot-lookup-baseline.json` is
untouched: sweeping them is the
batch work that owns that ledger, and re-baselining to clear my own red
would be the one move this
gate exists to prevent. Verdict line after the fix: "✓ slot-lookup
ratchet holds: 106 unswept
site(s) in 25 file(s), none new… baseline key set verified against
e592b4b: no files added",
exit 0 (captured before any pipe).

⚠️ Worth recording against my own gate report: this red came out of the
49 I had left to CI. The
reading was contract-legal, but a diff that adds test code doing service
lookups owes
`check:slot-lookup` locally, and I did not run it.

**Re-verified at `6166e303c`**, after both remedies: closure build,
`pnpm --filter
@objectstack/service-messaging test` -> `Test Files 44 passed (44) ·
Tests 467 passed (467)`, and
`typecheck` — one chained run, VERDICT command-exit 0. Both ablations
re-run at this head because
the ratchet fix touched the very file M1's red is read from: M1 still
`Tests 2 failed | 7 passed`
("expected [ 'inbox' ] to include 'sms'"), M2 still `Tests 1 failed | 4
passed`, each restored to
its HEAD blob with `git diff HEAD` empty.

Authored by Claude Code, session `session_01URLHobLUJB9K1ABV6ofdjj`.

---
_Generated by [Claude Code](https://claude.ai/code)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants