Skip to content

fix(messaging): mount the email and SMS channels per lookup, not once at kernel:ready - #18272

Merged
claude[bot] merged 5 commits into
mainfrom
claude/issue-18050-outbox-fanout-unregistered-channel
Sep 15, 2026
Merged

claude[bot] merged 5 commits into
mainfrom
claude/issue-18050-outbox-fanout-unregistered-channel

Conversation

@os-project-manager

@os-project-manager os-project-manager commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Part of #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 #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#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" (#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 #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 #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 #35, on #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

… at kernel:ready

The plugin registered both channels behind a guard that ran exactly once, at
kernel:ready, so a transport service registering later in the same boot never
got its channel and every notify naming it was refused for the life of the
process. MessagingService.registerChannelProvider resolves a mount on every
lookup; an absent transport keeps today's refusal, writes no delivery row and
records no suppression.

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

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/webhooks.mdx (via MessagingServicePlugin (symbol, a top-level class))

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

  • content/docs/releases/v14.mdx (via MessagingServicePlugin (symbol, a top-level class))
  • 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 e9091487f5adeb74b580b927921ee83e32550a84packageMentionDocs.

Which tree this was computed on

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

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

⚠️ 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 e9091487f5adeb74b580b927921ee83e32550a84 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

The diff adds MessagingService.registerChannelProvider to an already-published
class. A purely additive widening of a published surface takes at least minor
(maintainer ruling 2026-09-04, decision batch #35), and the PR declares clause
two yes, so patch was a self-contradiction inside one PR.

Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj
Co-authored-by: Claude <noreply@anthropic.com>
The #18050 plugin tests wrote `const messaging: any = ctx.getService(...)`,
which is the erasure shape slot-lookup bans; the file is grandfathered for its
existing sites only, so the ratchet grew 2 to 4. Typed at the source with the
spelling this file already uses; the two grandfathered sites are left to the
sweep batch that owns them and the baseline is untouched.

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

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 6166e303cec1416f0f9ab3e772b0fabbafd8a85f

Default-tier, in-seat, per contract-review.md: 「逐车道:spec 席达档审契约增量;余席默认档自审加门禁、挂标;豁免仅独立性件与保险丝。」 ⚠️ No independence case applies here — open_questions is empty, the dev raised no flag, the surface is packages/services/service-messaging alone, and there is no contract fork. ⛔ This seat had earlier told the dev an at-tier session would be commissioned; that was this seat's error, carried over from the spec lane's rule, and is corrected here.

① Derived judgments

  1. A new callable member on an ALREADY-EXPORTED class. MessagingService.registerChannelProvider(id, resolve) is added. Reachability re-derived in-seat, ⛔ not adopted from the report: src/index.ts line 22 names MessagingService in a CLOSED export list, the package's exports carries only "." and files ships dist, and there is no export * in the barrel (count 0, against a control export that counts 1). ⇒ the published surface grows by one member. CORRECT, and it is the mechanical floor's first trigger.

  2. Zero added export lines — and that is the trap, not the answer. The whole diff adds none, and src/index.ts is untouched. A judgement made by scanning the diff for export reads no here and is WRONG. The dev's derivation used the published entry with positive controls (an already-published member) and three negative controls (a plugin-local helper, an unexported type, a nonexistent name). CORRECT.

  3. The parameter type is NOT published. ChannelProvider appears 0 times in the barrel while the control type appears once. ⇒ an out-of-package caller can pass an inline function but cannot ANNOTATE a variable with the type this member takes. Judged acceptable, and named rather than waved through: it narrows ergonomics, not capability, and ⛔ nothing in this card asked for a new exported type. Successor: whoever first needs to name it.

  4. Behavioural widening on two already-published members. getChannel now asks the provider and getRegisteredChannels lists only what resolves NOW, so the latter's answer can differ between two calls in one process. That is the fix's whole point (a transport registering later in the same boot is picked up without a restart) and it is a change in what a published method answers, ⛔ not a new key. CORRECT, and correctly reflected in ②.

  5. The refused-vs-suppressed boundary is HELD, not rewritten. An unmounted channel is still REFUSED — no delivery row, no suppressed_channels entry — so feat(service-messaging,platform-objects): fan-out consults per-tenant channel availability and writes no delivery row for a channel with no transport (#17732) #18041's settlement and its channel-availability.test.ts pin stand untouched and green. CORRECT, and it is the judgement this card was most exposed to getting wrong.

② Semver level

.changeset/lazy-messaging-channel-mounts.md grades @objectstack/service-messaging minor, read at this head. That agrees with the Clause-②: yes declaration on the PR body, under the maintainer ruling of 2026-09-04 (decision batch #35, on #15294): a purely additive widening of a published package's public surface takes at least minor.

⚠️ It graded patch at the previous head, which is the self-contradiction Check Changeset refused. The dev regraded and named the lockstep cost rather than dodging it. CONSISTENT.

③ Boundary flags

open_questions is empty; five items are reported noted, not filed, and each carries a successor. Reviewed one by one, ⛔ none is escalated and ⛔ none is left without an owner:

  • email/sms channels answering { ok: true } with no transport — already on this card's own body as explicitly out of scope, and this change NARROWS its reach rather than widening it. Stays on the card.
  • sms-channel.ts implements no isAvailable — same surface as the above, not reachable as a defect while the mount gates the transport-absent case.
  • startup-registry-verdict.baseline.json is empty and its gate is green before AND after, so this defect was live in a shape that gate cannot see. A detector widening with its own blast radius; ⛔ correctly not taken here.
  • the two grandfathered slot-lookup erasure sites in messaging-service-plugin.test.ts — left untouched. ⭐ That is the right call: scripts/slot-lookup-baseline.json is a shrink-only ledger owned by the The service-lookup any rule misses getService<any>(...) — 80 sites erase the slot contract, 3 of them inside the rule's own scope #4251 sweep, and a one-off sweep from a card that does not own it moves a shared ledger sideways.
  • engine-double-contract.pinned.json untouched, gate exit 0 — this card adds no engine double.

⭐ One thing recorded in the delivery's favour, because it is the opposite of the failure mode this review exists to catch: the Slot-lookup ratchet red was a REAL finding about the diff — two tests the dev added used ctx.getService('messaging') typed as any, the erasure shape that ratchet bans, growing the file's count 2 to 4. It was fixed AT THE SOURCE by typing the lookups, with the baseline and the grandfathered sites left alone. ⛔ Re-baselining to clear one's own red is precisely what that gate exists to prevent, and it was not done.

Implemented-by: claude/issue-18050-outbox-fanout-unregistered-channel
Reviewed-by: session_01URLHobLUJB9K1ABV6ofdjj

VERDICT: PASS


Generated by Claude Code

@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Provenance for the carrier strip — domain:services execution PM seat.

The dual needs:contract-review carrier is stripped from BOTH sides in one stroke: this PR and card #18050 (label-write.mjs, read-back MATCHES on each).

  • Record of record: comment 5675644498 on this PR.
  • Head judged: 6166e303cec1416f0f9ab3e772b0fabbafd8a85f — the current head, ⛔ not an earlier one.
  • Verdict: PASS, default tier, in-seat, per contract-review.md's per-lane rule (the spec seat reviews contract increments at tier; ⛔ the remaining seats review at default tier in-seat). No independence case applies: empty open_questions, no dev flag, no cross-lane surface, no contract fork.
  • Independence pair: Implemented-by names the dev's branch, Reviewed-by names this seat's session — different identities, so ⛔ no self-review.

⚠️ Landing still waits on the pre-checks: all checks green on this head (Lint & Repo Gates was still running when this was written), the post-strip --pair 18272 exit, and check-governed-merges. ⛔ A stripped carrier is not a landing.


Generated by Claude Code

@claude
claude Bot marked this pull request as ready for review September 15, 2026 06:24
@claude
claude Bot enabled auto-merge September 15, 2026 06:24
@claude
claude Bot added this pull request to the merge queue Sep 15, 2026
Merged via the queue into main with commit e07eecf Sep 15, 2026
46 checks passed
@claude
claude Bot deleted the claude/issue-18050-outbox-fanout-unregistered-channel branch September 15, 2026 06:50
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.

2 participants