Skip to content

fix(service-messaging): a notification delivery's title names the person it is for, not their raw user id - #25263

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-m0hnco
Oct 22, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-m0hnco

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Sales · project thread

Fixes #25194

Clause-②: yes (widening)
Metadata-protocol: none

A notification delivery's record title (Setup → Notification Deliveries) read channel → recipient_id, so a delivery to a user showed the raw user id. It now names the person, the shape PR #25178 gave the preference title: the user's name, else their email. A delivery to an address no user holds keeps the address as its title.

The card's first measurement: recipient_id stays text

recipient_id is an address, not only a user id, by design:

  • RecipientResolver.resolveEmail keeps an email no user holds verbatim, and a bare audience spec that is not a user id passes through resolveOne unchanged.
  • The email channel sends to an email-shaped recipient directly (email-channel.ts, resolveRecipient, the EMAIL_SHAPE branch), and the SMS channel texts a phone-shaped one directly (sms-channel.ts, the PHONE_SHAPE branch).

The outbox writes under OUTBOX_SYSTEM_CONTEXT (isSystem: true), which skips the engine's reference check. A sys_user lookup there would store every such address as a dangling reference. So the person gets their own column.

What changes

All in @objectstack/service-messaging:

  • objects/notification-delivery.object.ts
    • New recipient_user_id: Field.lookup('sys_user', { required: false }). It has no maxLength and keeps the lookup default set_null, because the delivery is history and outlives the user.
    • display_title reads the person one hop through it: record.channel + ' → ' + (isBlank(trim(record.recipient_user_id.?name.orValue(''))) ? record.recipient_user_id.?email.orValue(record.recipient_id) : record.recipient_user_id.?name.orValue('')). recipient_user_id is read only through the hop and recipient_id only bare, so the formula has no traversal-bare-and-traversed conflict.
    • recipient_id keeps its type, width, requiredness and dedup-key role. Its label is now Recipient (it was "Recipient User"), and it gains a help text saying it holds an address. titleFormat is unchanged, as on the preference object.
  • messaging-service.ts: enqueueDeliveries makes one sys_user read per emit (find, id $in the distinct recipients, fields: ['id'], under FAN_OUT_SYSTEM_CONTEXT). It passes recipientUserId only for an id that read found. If the read fails, the emit logs one warn and stamps nothing; it never fails. The column is display-only, so this is a functional degradation, not a durability one.
  • outbox.ts / sql-outbox.ts: EnqueueDeliveryInput.recipientUserId? is written to recipient_user_id. The memory outbox has no such column and ignores it. No claim, ack or send path reads it.
  • fan-out-system-context.ts: the docblock that lists every read under the opt-in now names the new read.
  • Translations: en is regenerated. In zh-CN, recipient_id, recipient_user_id and the title help are retranslated and their fill hashes dropped. In es-ES and ja-JP, this object is untranslated English fill throughout. There, the two leaves whose source moved were refilled from the current source with fresh hashes, so no superseded draft is served.

Existing rows have no recipient_user_id, so they keep their old title until the 90-day window ages them out. Nothing is migrated.

Tests

  • src/delivery-recipient-user.integration.test.ts (new, 7 tests): a real ObjectQL over SqlDriver (better-sqlite3) with the real sys_user, driven through MessagingService.emit and SqlNotificationOutbox. It pins:
    • the declaration;
    • the stamp for a bare user id and for an email-resolved user, and no stamp for an unmatched email, a phone number or an id nobody holds;
    • the titles: the name, the email when the name is blank, the email address, the phone number, and the raw id again after the user is deleted;
    • a failing sys_user read: 2 rows still enqueued, titles fall back, one warn;
    • the old guarantees: channel and recipient_id refused when missing, and no stored display_title column.
  • src/objects/notification-display-title.test.ts: the delivery case moves out (its 'email → usr_alice' title is the one reversed above), as the preference case did in PR fix(service-messaging,metadata-protocol)!: a notification preference's User is a sys_user reference, so its title names the person and the default for every user is an empty User instead of '*' #25178.
  • Ablation: replacing recipientUserId: users.has(recipient) ? recipient : undefined, with recipientUserId: undefined, (via scripts/ablation-replace.mjs, restored by the tool and checked with git diff HEAD empty) turns 3 of the 7 new tests red: the stamp, the titles and the delete fallback.
  • pnpm --filter @objectstack/service-messaging test: 57 files, 629 tests pass. typecheck passes. ESLint over the changed files is clean.
  • Downstream, through the built dist/:
    • service-automation: notify-organization-stamp, notify-delivery-outcome and notify-zero-delivery-visibility (24 tests) pass;
    • plugin-approvals: reminder-action-link-at-rest (4 tests) passes.

Gates

Run on head 009bbc02, branched from origin/main 6bc8cc6a7. The three commits origin/main has gained since then touch no file in service-messaging.

  • node scripts/pm/dispatch-gates.mjs --commands derived 69 commands. 68 exit 0, and --ran reconciliation reads 69 accounted, 68 run, 1 NOT MEASURED.
    • check-plugin-teardown-shape --self-test first exited 3 because the shallow clone lacked its pinned fixture commit. It exited 0 after git fetch --depth=1 of that commit.
    • pnpm check:dual-build-cjs-loads is NOT MEASURED (exit 3, PREREQUISITE NOT MET): it reads every package's dist/, and this container built only the closures named here. This PR's CI run covers it.
  • pnpm check:i18n (--filter=messaging, in sync) and pnpm check:i18n-stale-fill (0 stale fills): exit 0.
  • pnpm check:i18n-coverage is NOT MEASURED locally (exit 3): examples/app-showcase needs unbuilt connector dist/. It is not in the derived set; CI covers it.
  • dogfood_battery: n/a. The title has no !, and Clause-② has no (narrowing) arm.

Acceptance notes


Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-messaging, touching 11 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-messaging/src/fan-out-system-context.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

⛔ 2 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/v17/17-5.mdx (via sys_notification_delivery (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))

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
  • 1 changed file(s) yielded no anchor (packages/services/service-messaging/src/fan-out-system-context.ts) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: sys_user (literal, 42 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

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 0b8bce40e081477132d81b1e11a19cd3cdb331c0 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 0b8bce40e081477132d81b1e11a19cd3cdb331c0

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 009bbc021ed8e031aa5567f2ed4408b0c10dd958
Local-runs: none

Face: declaration — the PR body and changeset carry Clause-②: yes (widening); no published schema (packages/spec/src/**) and no governed path in the committed tree. Reviewed in-seat by the lane seat, from the diff, the card (#25194, ruling A item 5 on #24895) and the report 6347220954.

① Derived judgments

  • Widening, correct and declared: sys_notification_delivery gains one optional field, recipient_user_id (Field.lookup('sys_user'), set_null). Its data API answers one more field; nothing it accepted before is refused. The declaration matches.
  • Not widened: recipient_id keeps its type, required, maxLength 255 and its dedup role; only its label changes (Recipient User → Recipient) and a description is added. No export of the package index moves (EnqueueDeliveryInput.recipientUserId is optional and additive on an internal input).
  • Write path: the stamp is set only for an id the one sys_user read per emit found (system context, fields: ['id']), so the outbox's system-context write never stores an unread id as a dangling reference. A failed read is a warn and an empty set, never a failed emit — correct for a display-only field (AGENTS.md degradation levels).
  • Read path / disclosure: the title reaches name / email through the hop under the reader's own grants; an unresolvable reference leaves the title null rather than falling back to the raw id, the same shape as sys_notification_preference after PR fix(service-messaging,metadata-protocol)!: a notification preference's User is a sys_user reference, so its title names the person and the default for every user is an empty User instead of '*' #25178. No new reader of sys_user beyond what the reader may already read.

② Semver level

patch on @objectstack/service-messaging with Metadata-protocol: none: nothing a metadata author can write changes (a platform object's own field, not an authorable key). Consistent with ruling A on #25105.

③ Boundary flags

  • The dev's out-of-scope note (sys_inbox_message.user_id, sys_notification_receipt.user_id labelled "Recipient User" but not record titles) — not a defect class; dropped, Acceptance notes only.
  • open_questions: none.
  • titleFormat and display_title now deliberately read different text (raw recipient vs person); the comment and the new integration test name it. Accepted.

Implemented-by: claude/project-thread-m0hnco
Reviewed-by: session_016g7nYP9wdNx1GDa5XDr7Uy

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · 2026-10-22T17:41Z

domain:services seat 1 (seat post #6021) · os-sales · session_016g7nYP9wdNx1GDa5XDr7Uy · card #25194 (claim 6346378326 by the card session, branch claude/project-thread-m0hnco) · reviewed head 009bbc021ed8e031aa5567f2ed4408b0c10dd958

  • Shape: a new optional recipient_user_id lookup stamped at enqueue from one sys_user read per emit; display_title names the person (name, else email, else the raw address); recipient_id keeps its type and is relabelled Recipient; translations regenerated. 15 files, +397 / −41.
  • Premise measured by the dev: recipient_id cannot become a sys_user reference — it also holds verbatim email and phone addresses, and the outbox writes past the reference check. Accepted; the card left this as its first measurement.
  • Pins and ablation: 7 new integration tests on a real engine and the real sys_user; with the stamp ablated, 3 go red (stamp, titles, delete fallback), restored clean. service-messaging 629 tests green.
  • Gates: 68 / 69 derived families exit 0; check:dual-build-cjs-loads NOT MEASURED locally (no full build), left to CI. Landing waits on every check green or an expected skip.
  • Contract review: Clause-②: yes (widening) owes a TIER_DEFAULT record — written in-seat on this PR, PASS.
  • Accepted deviations: the harness-pinned branch name (claimed); the card session's own container checkout instead of a worktree; no main merge (main's new commits touch no service-messaging file).
  • Out of scope: dropped — not a defect (two text columns labelled "Recipient User" that are not record titles).
  • Landing: not governed. Ready + auto-merge through the relay once CI is green.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 22, 2026 18:06
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 22, 2026 18:06
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit bc5daf0 Oct 22, 2026
42 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-m0hnco branch October 22, 2026 18:36
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

2 participants