fix(plugin-approvals): approval notifications reach their recipient with their text, sent as body, the field messaging reads - #21888
Conversation
…nd at the inbox door Two new pins, committed ahead of the fix so their red reading is taken on the unfixed service: - plugin-approvals: every call site that notifies, driven through the real ApprovalService on ObjectQL over SqlDriver; a comment, a request-info question and a send-back note are their recipient's body, and every payload carries a non-empty body and no undeclared field. - dogfood: a comment and a request-info question read back from the recipient's GET /api/v1/notifications on a booted app with the real approvals + messaging chain. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
…y, the field messaging reads Every notification the approvals service sent put its text in payload.message. The messaging service builds the delivered notification from payload.title and payload.body only (the inline fan-out, the outbox snapshot and the dispatcher alike), so GET /api/v1/notifications served each one as a title over an empty body. - The twelve call sites now send the text as body. The texts themselves are byte-identical; who is notified, and when, is unchanged. - notify() declares its payload (title, body, actionUrl, and a reminder's actions), so a call site that spells the text any other way does not compile. - No alias in service-messaging: one field, as EmitInput documents. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e6e00a4b5a0ed7bf3007405e0b77d7d2f671ed95 && git checkout e6e00a4b5a0ed7bf3007405e0b77d7d2f671ed95
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e864db56dffc2dec3290e5f0700f9d1606a1c830 7ca65ce74c1c49489492d687ad96f603257b502d && git checkout -B drift-repro e864db56dffc2dec3290e5f0700f9d1606a1c830 && git merge --no-ff 7ca65ce74c1c49489492d687ad96f603257b502d
node scripts/docs-audit/affected-docs.mjs --json e864db56dffc2dec3290e5f0700f9d1606a1c830
|
Fixes #21847
Clause-②: no
What changes
Every notification the approvals service sends reached its recipient as a title over an empty body. That covers a comment, a request-info question, a send-back note, a reassignment, a reminder, an escalation, an SLA breach and an out-of-office substitution. The service put the text in
payload.message. The messaging service builds the delivered notification frompayload.titleandpayload.bodyonly: the inline fan-out and the outbox snapshot inmessaging-service.ts, andprocessRowindispatcher.ts. SoGET /api/v1/notificationsansweredbody: "", and the inbox row'sbody_mdwas empty.packages/plugins/plugin-approvals/src/approval-service.ts: the twelvenotify()call sites send their text asbody. The texts are byte-identical: each removedmessage:line equals its addedbody:line once the key is stripped, indentation included (12 of 12). Who is notified, and when, is unchanged.notify()now declares its payload as a local, non-exported typeApprovalNotificationPayload(title,body,actionUrl, and a reminder'sactions). It used to take a free-form string-keyed record. A call site that spells the text any other way is now a compile error (Leg B below). No package entry gains an export.service-messagingorpackages/spec, and nomessagealias anywhere. There is one field, asEmitInputdocuments, per the triage direction on the card..changeset/21847-approval-notification-body.md: apatchfor@objectstack/plugin-approvals.Measured before the change (
origin/maine864db5)payload.message(approval-service.ts:3076 :3087 :4209 :4252 :4512 :4571 :4594 :4788 :4835 :5880 :5898 :5911). Messaging reads onlypayload.body(messaging-service.ts:1101 and :1221,dispatcher.ts:341). The red reading is under Tests.notify(): the onlymessaging.emitin the package is the helper's own. The helper forwards the payload each call site builds, so the one fix inside the helper is the declared payload type. The renames happen at the call sites.payload.messagefrom an approval notification.git grepforpayload.message,payload?.messageand{{ message }}answered 5 hits. None reads a notification: an SMS transport's HTTP error, CLI and runtime test fixtures, and a PM script..objectui-shapin0abd4f9f: 7 hits, all HTTP error-response readers. For example,apps/console/src/services/approvalsApi.ts:200reads a failed request's JSON. Control term/notificationsanswered 58 lines at the same tree.template-renderer.tsspreads the payload into the template context, so a tenant-authoredsys_notification_templaterow for anapproval.*topic could have written{{ message }}. This repo seeds no such row. The changeset names the{{ body }}spelling.content/docs/**sentence is made false. The notification passages incontent/docs/automation/approvals.mdxdescribe topics and deep links, not the field.Tests
Both pins read plugin-approvals source. The unit file imports
./approval-service.js. The dogfoodisolatedproject aliases@objectstack/plugin-approvalstosrc/index.ts. Leg A proves this:dist/held the fix while the door pin went red.plugin-approvals/src/approval-notification-body.integration.test.ts(new) drives the realApprovalServiceonObjectQLoverSqlDriver(in-memory better-sqlite3). It pins three things:body;bodyand no field outside the declared type.packages/qa/dogfood/test/approval-notification-body.dogfood.test.ts(new) boots an app with real approvals, messaging and REST, and an email-authored user approver. A submitter's comment and an approver's request-info question are read back from each recipient'sGET /api/v1/notifications. Each is the one row of its topic and has a title, so the old defect reads as a body that lost its text, not as a missing notification.Red, at 68baad4 (the tests only, unfixed service):
Tests 2 failed | 1 passed (3). The comment readexpected [ [ 'u_alice' ], undefined ]. There were 22 offenders,body is undefinedplusundeclared payload field 'message', at each of the 11 reached sites. The coverage pin was green.AssertionError: the comment is the notification's body: expected '' to be 'The signed contract is attached to th…'. The row count of 1 and the non-empty title both held.Green, at 7ca65ce:
pnpm --filter @objectstack/plugin-approvals typecheck: exit 0. The test layer held 8 files, 324 errors and 27 pinned signatures, and the new file is clean.pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2:Test Files 61 passed (61),Tests 898 passed (898).pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2 --project isolated test/approval-notification-body.dogfood.test.ts:Tests 1 passed (1).pnpm --filter @objectstack/dogfood typecheck: exit 0.pnpm turbo run build --filter='@objectstack/dogfood^...' --filter='@objectstack/plugin-approvals^...' --concurrency=1ran 63 of 63 tasks, 48 cached. plugin-approvals was rebuilt at 7ca65ce.Ablation, at 7ca65ce (
scripts/ablation-replace.mjs, wrap mode, plus a trap restore)messageput back in the helper. The anchorseverity: 'info', ...event, payload, audience,went from 1 hit to 0. It was replaced by a call that sends{ ...rest, message: body }, and the replacement went from 0 hits to 1. The blob moved from 9a1822845a2f to d066658afd62.bodyundefined; 22 offenders).expected '' to be 'The signed contract…'.git diff HEADis empty. The trap's restore leg repeated that proof at exit.message. At the request-info site,body: input.comment.trim(),becamemessage: input.comment.trim(),.tsc --noEmitexited 1 withsrc/approval-service.ts(4810,11): error TS2353: Object literal may only specify known properties, and 'message' does not exist in type 'ApprovalNotificationPayload'.Restore: blob equals HEAD.Gates, at 7ca65ce
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(no paths) derived 70 commands, and all 70 ran.pnpm check:dual-build-cjs-loadsexited 3 (PREREQUISITE NOT MET: 8 packages had nodist/).--ranverdict:70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN.pnpm exec eslint --no-inline-config --format jsonon the three touched.tsfiles. The JSON reports 3 files linted, with 0 errors and 0 warnings.eslint.config.mjs'spackages/**/*.{ts,…}blocks match all three, and none is ignored.parserOptions.project), so this diff cannot change the verdict on any untouched file.pnpm lint, the full dogfood suite and the full turbo test are CI's.Acceptance notes
position:P-style pending entries. Its audience holds only slot literals, andnotify()drops every audience entry containing a colon, so it returns 0 before emitting. Its payload is renamed with the rest and held by the compiler, and the producer battery covers the other eleven call sites.Generated by Claude Code