Skip to content

docs(service-messaging): mark the sys_notification_subscription expansion not-yet-wired and align principal with the resolver - #10027

Merged
os-warren merged 4 commits into
mainfrom
claude/issue-9807-subscription-expansion-not-wired
Aug 19, 2026
Merged

docs(service-messaging): mark the sys_notification_subscription expansion not-yet-wired and align principal with the resolver#10027
os-warren merged 4 commits into
mainfrom
claude/issue-9807-subscription-expansion-not-wired

Conversation

@os-warren

@os-warren os-warren commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Fixes #9807

Implements the maintainer ruling recorded in comment 5339441800 — Option B, adopted verbatim. Option A (build the Layer-3 'subscribers' expansion) is not implemented here and nothing in this diff moves toward it.

What changed

  1. Object header — the sentence claiming that a producer emitting audience: 'subscribers' causes the resolver to expand a topic's subscriptions is replaced with a marker saying the expansion is NOT WIRED in this repo, naming the three facts a reader can re-derive it from, and stating that delivery today comes from the explicit audience passed to emit(). The genuinely-true content (what a subscription is; the contrast with sys_notification_preference) is kept.
  2. principal field description — widened from four forms to the six RecipientResolver.resolveOne() really accepts for a string spec.
  3. en translation bundle — regenerated (see Declared surface widening below).
  4. Changeset — a real patch, not skip-changeset (see Changeset decision).

Plus a merge of today's main (see CI investigation). The three content commits are unchanged since first review.

Premise re-verification (measured, not recalled)

  • The aspirational sentence stood at notification-subscription.object.ts:9-13, wrapping across five lines.
  • git grep "'subscribers'" -- '*.ts'exactly one hit repo-wide, line 11 of that same header. Counter-checked against a term known present: audience hits 6 files in the package, so the search is live rather than silently matching nothing.
  • AudienceSpec (messaging-service.ts:50-52) is string | { ownerOf: { object, id } } — no 'subscribers' member; EmitInput.audience is required.
  • grep sys_notification_subscription across the repo: outside service-messaging/src only ADR/design prose, a driver test, changelogs, platform-object-names.ts, and a comment in object.zod.ts. Inside the package: the declaration, its own tests, generated translations, and the Setup nav entry. No runtime read.

The principal forms were verified by reading recipient-resolver.ts:129-146 directly rather than trusting the card: prefix branches user: / role: / team: / owner_of:<object>:<id>, then looksLikeEmail()resolveEmail() (matched against sys_user, kept verbatim when no user matches), then bare user id as the fallback for anything unmatched.

Confidence gap — what I measured and what I could not

  • objectstack — zero runtime consumers (above).
  • objectui — measured, confirmed zero. git grep for sys_notification_subscription, notification_subscription, notificationSubscription, notification-subscription all return nothing. Counter-checked live: sys_inbox_message returns 20+ files including the real inbox path, and reading useHomeInbox.ts, sharedUserFeeds.ts and InboxPopover.tsx shows the inbox UI reads exactly sys_inbox_message, sys_notification_receipt and sys_activity. So the inbox renders the materialized delivery, never subscription rows.
  • cloud⚠️ NOT measured; not in this session's repo scope. I make no claim about it either way.

Declared narrowing: the wording is deliberately scoped — "NOT WIRED in this repo", "nothing here reads these rows at runtime" — so it stays true even if cloud turns out to read them.

Declared surface widening — ONE file beyond the dispatched surface

The dispatched surface was notification-subscription.object.ts alone. packages/services/service-messaging/src/translations/en.objects.generated.ts was added because check:i18n failed on the annotation commit:

  services/service-messaging     DRIFTED (1)
  • services/service-messaging: 1 bundle(s) drifted from the schema

The en bundle is a copy of the source, rewritten on every run (#8543); the field description reaches it as the principal.help leaf. The file is generator outputnode scripts/check-i18n-bundles.mjs --write, one line changed, no hand edits — same gate family, so it adds no verification surface.

The patch round added no second widening. The Test Core investigation below produced no code change: no test was edited, skipped, quarantined or loosened, and the only new commit is the merge of main.

Changeset decision — on evidence

Not a pure comment diff, so skip-changeset would be wrong. Measured after building the package:

  • grep -rl "owner_of:object:id" packages/services/service-messaging/dist/dist/index.js, dist/index.cjs (+ both .maps). The description is a runtime string in the shipped artifact.
  • It also ships as the field's help text via the en translation bundle, i.e. what an admin reads in the Setup "Notification Subscriptions" grid.

So: a real patch changeset for @objectstack/service-messaging.

CI investigation — the red Test Core (2/3) on 980dec44e

That shard reported Failed: @objectstack/example-showcase#test. Investigated as ours. It did not reproduce, and no defect attributable to this diff was found.

CI captured no assertion to act on. I pulled the complete failing job log (5083 lines) and searched it: example-showcase appears exactly twice, both in the error summary. No Test Files line, no Tests line, no FAIL, no AssertionError, and none of the showcase's test names appear anywhere. The task that failed produced zero output, while other packages on the same shard streamed theirs into the same log. turbo.json sets no output suppression on test.

The guard that was supposed to disambiguate could not. ci.yml states that a red suite plus a green completeness check means real test failures; the run printed check-test-completeness: OK (11 package(s), 3617 test(s) declared and all 3617 accounted for). But check-test-completeness.mjs regexes summary lines present in the log, so a package that printed nothing contributes no row and is invisible to it. Its green never covered the showcase.

What I measured instead:

measurement result
showcase suite at the failing head 980dec44e, dependency closure built Test Files 21 passed (21) / Tests 337 passed (337)
showcase suite at that tree merged with today's main Test Files 21 passed (21) / Tests 337 passed (337)
does any showcase test read messaging metadata? no — the strings this PR moves appear nowhere in examples/app-showcase
is the showcase in this PR's affected set? yes — it depends directly on @objectstack/service-messaging; reproduced turbo ls --affected + partition-test-shards.mjs locally
CI at the merged head a0954815f all three Test Core shards green, aggregate Test Core green

⚠️ One thing that would otherwise mislead re-review: once the merge base advanced, the showcase moved from shard 2/3 to shard 3/3 (shard 2 now carries service-messaging). A green shard 2 alone would prove nothing about it; shard 3 is the one that carries it, and it is green.

⚠️ My own first local run reproduced a false red of the same shape — every failure was Cannot find package '@objectstack/spec/data'-class, i.e. an unbuilt dependency closure in a freshly recreated worktree, not an assertion. Recorded because it reads exactly like "this change broke imports". All numbers above are from runs with the closure built.

The underlying event was neither reproduced nor explained, and I did not manufacture a fix for it. The diagnostic hole it exposed is filed as #10032.

Verification — re-run at the merge head a0954815f

Gate union re-derived with node scripts/pm/dispatch-gates.mjs (no path arguments) against the new merge base 899052acc — still the same 3 paths and the same gate set.

gate verdict line
check:slot-lookup ✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new. · baseline key set verified against 899052a: no files added.re-quoted: the ratchet baseline moved with the merge base
check:i18n check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
check:test-source-alias check-test-source-alias OK — 72 packages with tests scanned; …
check:type-source-resolution check-type-source-resolution OK — 76 packages with a tsconfig.json scanned; …
check:changeset-gate-self-tests all three self-tests
check:objectui-changeset ✓ objectui-range --self-test: all checks passed
check-adr-0087-registration.mjs ✓ … this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
check-changeset-no-major.mjs ✓ This diff introduces no 'major' bump.
check-empty-changeset.mjs ✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
check-affected-docs.mjs ✓ affected-docs self-test: 262 cases pass. (exit 0)

Tests / typecheck at the merge headpnpm --filter @objectstack/service-messaging testTest Files 22 passed (22) / Tests 242 passed (242); typecheck → exit 0 with tsc --noEmit echoed. Counts unchanged from main (measured both sides in the same worktree at first review: base also 22/242).

Ablation: NOT APPLICABLE. Nothing executable changed — a header comment, one description string, one generated bundle leaf, one changeset. There is no guard to remove and no behaviour to mutate, so an ablation here would be fabricated rather than informative.

All heavy commands ran through scripts/pm/os-verify-lock.sh.

Out-of-scope findings filed

Not addressed here

#9807 covers only the annotation. The Layer-3 expansion itself remains unbuilt and unclaimed — see the report's open_questions for the two judgement calls I did not want to make silently.

Generated by Claude Code

claude added 3 commits August 19, 2026 15:28
…t-yet-wired (#9807)

The object header described a live `audience: 'subscribers'` expansion that
does not exist: `AudienceSpec` has no such member, `EmitInput.audience` is
required, and no `RecipientResolver` branch expands a topic's subscriptions.
Mark it NOT WIRED so the Setup grid is not read as a live routing control, and
align the `principal` description with the forms the resolver really accepts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
`check:i18n` verdict on the annotation commit: `services/service-messaging
DRIFTED (1)`. The default (en) bundle is a copy of the source, rewritten from
it on every run (#8543), so the `principal` description edit has to land here
too. Generator output — `node scripts/check-i18n-bundles.mjs --write`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
The `principal` description is a shipped user-visible string (it reaches
dist/index.js and the en translation bundle as Setup help text), so this is a
real patch changeset rather than a `skip-changeset` comment-only diff.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 4 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 f2920e1fea0be91d86700705bc89728ebd27042fpackageMentionDocs.

Which tree this was computed on

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

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

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

@os-warren
os-warren marked this pull request as ready for review August 19, 2026 17:57
@os-warren
os-warren added this pull request to the merge queue Aug 19, 2026
Merged via the queue into main with commit 44738f7 Aug 19, 2026
29 checks passed
@os-warren
os-warren deleted the claude/issue-9807-subscription-expansion-not-wired branch August 19, 2026 18:12
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/s tooling

Projects

None yet

2 participants