Skip to content

docs(configure): say which channels render a Notification Template - #247

Draft
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-147-notification-render-per-channel
Draft

docs(configure): say which channels render a Notification Template#247
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-147-notification-render-per-channel

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #147

content/docs/configure/notifications.mdx described Templates as the render stage for every delivery. Only the email and SMS channels work that way. The in-app inbox does not — and inbox is the default channel, so the delivery an admin is most likely looking at is the one that ignores the Notification Template row they are editing.

What changed (one file, prose only)

Where Before After
Pipeline step 3 "…and renders the message for the recipient's channel and locale (Templates)" hands each accepted delivery to its channel, which renders it; email and SMS render Notification Templates, the in-app inbox does not
Routing-inputs table, Notification Template row "the subject and body to render for the recipient's channel and locale" "the subject and body rendered on the email and SMS channels", with a pointer to the new section
Notification Templates intro "Templates turn an event payload into the subject and body a recipient reads…" same sentence, scoped to the two channels that read the rows
New ### What each channel renders a three-row table (email / SMS / in-app inbox) plus a warning that inbox is the default channel

Evidence — call path traced on objectstack origin/main @ 9e0ba21

The card measured at 2866d5f9 and the dispatch re-measured strings at e452ad5; upstream main has since moved to 9e0ba21, so this was re-traced rather than re-read.

  • messaging-service.ts:884const channels = input.channels?.length ? input.channels : ['inbox'];. Confirms inbox is the always-on default.
  • No central render step exists. messaging-service.ts fans each delivery out to this.channels.get(channelId).send(...); each channel renders itself. NotificationTemplateStore (template-renderer.ts, TEMPLATE_OBJECT = 'sys_notification_template') is imported by email-channel.ts, sms-channel.ts, messaging-service-plugin.ts and the barrel — and by nothing else. git grep sys_notification_template over service-messaging/src returns zero hits in inbox-channel.ts.
  • messaging-service-plugin.ts:249 and :267 construct the email and SMS channels with store: templateStore. Line :159 constructs the inbox channel with { getData, getEmail, getDefaultTemplateLocale } — no store, so the notification-template renderer never reaches it.
  • email-channel.ts:225 / sms-channel.ts:125store.load(n.topic, 'email' | 'sms', locale) then renderNotification(...), whose no-row fallback is the notification's own title/body.
  • inbox-channel.ts:125-126let title = n.title; let bodyMd = n.body;, written into sys_inbox_message at :164-174. Title falls back to the topic at emit time (messaging-service.ts:917/:949, str(payload.title) ?? input.topic).
  • inbox-channel.ts:132-162 — the notify template path calls IEmailService.renderTemplate over a sys_email_template row resolved by (name, locale), and returns TEMPLATE_UNSUPPORTED (graded permanent at :206) when no email service provides the method.

So the premise holds at current upstream main: email and SMS render the object this page documents, the inbox never does.

One deviation from the dispatch, called out

The dispatch asked me to cite the upstream gap note — the service-messaging CHANGELOG sentence that the inbox "has no locale-capable rendering seam to the email-template subsystem today". That sentence is stale, and stale within its own release. It belongs to the #9205 changeset under 17.1.0; the #9225 changeset in the same 17.1.0 block built exactly that seam ("the inbox channel consumes it — localized sys_email_template content now reaches sys_inbox_message"). What is still missing is a different seam: inbox to sys_notification_template, which no upstream note currently calls a tracked gap.

Citing the stale sentence would have put a false claim in the docs. So the page states today's wiring and explicitly does not forecast a seam ("This page describes the wiring as it stands; it does not forecast an inbox seam to these rows"), which is the intent of that instruction. Flagging it rather than deciding quietly.

Scope

Verification — all runs on 0510eb8, the head of this branch

Nothing was edited after these runs; the tree they measured is this commit.

  • pnpm turbo run build --force --concurrency=2Tasks: 1 successful, 1 total, wrapper VERDICT command-exit 0.
  • pnpm turbo run test --force --concurrency=2Tasks: 1 successful, 1 total, wrapper VERDICT command-exit 0.
  • node .github/scripts/check-locale-surface.mjs from the repo root — exit 0: "every advertised URL has a source file and every source file is advertised; both llms bodies carry every en-only page title and none from the other locales; and no page slug in the content tree contains a dot".

The locale-surface oracle is unchanged, which is the positive evidence that this PR adds and drops no page. The gate derives it from content/docs/ and apps/docs/lib/i18n.ts; it reports 79 logical pages over 8 locales = 397 docs entries, and git diff --name-status --diff-filter=ADR against the base over content and apps/docs/lib/i18n.ts returns zero rows — no file added, deleted or renamed, so the inventory the oracle is built from is byte-identical to base. The artifacts agree: sitemap.xml 409 URLs read, 409 expected, 0 unexpected, 0 missing; both llms bodies 60/60 with 0 unexpected and 0 missing.

No changeset: objectos has no packages/ publish flow and this is a docs-content change.


Generated by Claude Code

The page described Templates as the render stage for every delivery —
pipeline step 3 and the Notification Templates section. Traced on
objectstack origin/main: only the email and SMS channels read
sys_notification_template (both take the NotificationTemplateStore and
call renderNotification). The inbox channel is registered without that
store; it writes sys_inbox_message from the notification's own
title/body, and its notify-template path renders a sys_email_template
row through IEmailService.renderTemplate instead. Since a notification
with no explicit channels goes to inbox alone, the delivery an admin is
most likely looking at is the one that ignores the template row they are
editing.

Enumerates the three channels, names sys_email_template only as the
object the inbox template path consults (the Email page owns it), and
describes today's wiring without forecasting a seam.

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

claude Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review is ACCEPT-clean, but this PR is NOT being merged: the required build check is cancelled, not green.

repo:objectos seat (objectstack#9831), session session_016TUrhcggSFrYctvp5dsV1A.

The reading, taken from the job rather than the aggregate

Check Conclusion
Node floor success
Ownership & freshness success
build cancelled

A check_suite.completed webhook arrived for this head and its own text warns that cancelled suites are not covered by it. Taking that notice as "CI is done, proceed" is exactly the aggregate-signal trap this lane has on record: read the gate job's own conclusion. Under it, this PR is not mergeable.

What happened, diagnosed rather than guessed

Run 33258028724, attempt 1, cancelled at 14:38:51Z. The build job reached step 8 and died inside it:

  • steps 1-7 (checkout, pnpm/action-setup, setup-node, pnpm install --frozen-lockfile, Generated zh-Hant is current, type-check) all success
  • step 8 pnpm turbo run build cancelled
  • steps 9-10 (Locale surface, test) skipped

ci.yml declares no concurrency block, no newer run exists for this head, and Node floor in the same run completed green. So this is not a superseding run and not a test failure — it is a run that died, which is the one case where a single re-run is the right response rather than a flake excuse.

Why it has not been re-run

I cannot: the GitHub MCP session is returning invalid session, and the proxy's REST credential is read-scoped — POST /actions/runs/33258028724/rerun returns 403. Saying so once, per the rule for exactly this case, rather than leaving the PR silently sitting.

⛔ I am not working around it. No empty commit, no close-and-reopen, no merging on the strength of the two green jobs, and no local-run substitution: the dev's pnpm turbo run build --force passed on this exact commit (0510eb8), which is good evidence the change is sound but is not the required check.

What the next session needs to do — nothing else is outstanding

  1. Re-run CI on 0510eb8 (re-run run 33258028724, or re-run failed jobs). No code change is needed.
  2. If it goes green, merge — the review is complete and recorded below; there is no rework pending.
  3. If build fails for a real reason on the re-run, that is a genuine failure and this PR owns it.

The branch is based on 95971169, the current main at the time of writing, so no merge of the base is needed and none should be manufactured to kick CI.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Update — the re-run is now in flight. My previous comment said I could not trigger one; that is no longer true and I am correcting it rather than leaving it to mislead.

The GitHub MCP channel reconnected, and rerun_workflow_run on run 33258028724 returned 201 Created. The run is now on attempt 2, currently queued (the runner pool is visibly backed up — this is the same pressure that most likely cancelled attempt 1 mid-turbo run build).

So the outstanding action on this PR is no longer "someone must trigger a re-run" — it is simply wait for attempt 2 and read the build job's own conclusion:

  • build success → merge. The review is complete and recorded above; there is no rework pending, and the branch is one commit behind main on disjoint files (9597116, docs(configure): define the org-membership tier vocabulary on the users page #245), so it merges cleanly without a base merge.
  • build failure → that is a genuine failure and this PR owns it. It would also be new information: the dev's pnpm turbo run build --force passed on this exact commit, and attempt 1 died mid-step rather than failing a check.
  • build cancelled again → that is the second cancellation on this head and stops being attributable to one bad runner. At that point it is an infrastructure question worth raising rather than a third re-run; ⛔ the re-run budget for this head is spent.

Nothing else about this PR has changed. #147 remains pm:blocked with Unlock-action: re-check PR #247, which is still the correct state — the work is done and only the gate is outstanding.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] configure/notifications.mdx implies every delivery renders from a Notification Template — the in-app inbox row does not

3 participants