Repository navigation
Commit c51aed4
fix(plugin-approvals): an approval notification names the record by its title, as the recipient may read it (#26389)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XMBYHj1ZHBx7grUypasmFts)_
Fixes #26181
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么**:审批通知(铃铛和邮件正文)不再用记录 id
指代记录,改用记录标题,而且按每个收件人各自能看到的字段来定:收件人能读标题字段就显示标题,读不到、被掩码或标题为空就退回
id。一条通知发给多人且各人看到的不一样时,按名称拆成几条发,谁也拿不到别人才能看的标题。
- **为什么改**:「您的合同 C8Mp7TjRYta8mOWx 已获批准」收件人看不出是哪份合同;维护者已按 A′ 重新裁决(#26181
裁决 6429362190)。
- **风险与回滚**:风险在字段级安全的收窄上,所有失败路径都退回
id,不会泄露标题;每个收件人多一次权限解析(只在装了安全插件且记录有标题时)。回滚就是回退这个 PR。邮件里一键审批的令牌页面不变,仍显示
id。
- **先看**:`approval-service.ts` 的 `notify()` 和 `subjectNamesFor()`;新测试
`notification-subject-title-per-recipient.integration.test.ts`;`action-link-pages.ts`
新加的说明。
- **两分钟验证**:`pnpm --filter @objectstack/plugin-approvals exec vitest run
--maxWorkers=2
src/notification-subject-title-per-recipient.integration.test.ts`,预期 4
个用例全绿。
- **需要你做的**:无,席位落地。
## What changes
- **`notify()` names the record per recipient.** A notice about a record
now hands `notify()` the record (`about`) and a builder for its sentence
(`text`), instead of a text built once. `notify()` asks
`subjectNamesFor()` for the name each recipient reads, groups the
recipients by name, and emits once per name, so a recipient never
receives a text built around another recipient's title. A dedup key on a
split notice is suffixed with each part's first recipient so messaging
does not fold one part into another (no current call site sends a
multi-recipient notice with a dedup key that reaches messaging; the
reminder to slot literals is dropped before emit).
- **`subjectNamesFor()`** is the inbox's own title rule
(`resolveSubjectDisplays`) asked as the recipient: the record is read
once under the system context, and the title (ADR-0079 `nameField` via
`resolveDisplayField`) is picked from it as served to the recipient,
`titleAsServed()` over their `servedFields()` answer. The id stands when
the record has no title, when the recipient may not read the title field
or is served it masked, and when any read fails (at `warn`).
`titleAsServed()` is extracted from `titlesAsServed()` so the inbox and
the notifications share one spelling.
- **`recipientContext()`** builds the recipient's envelope the way the
`@mention` recipient gate in `plugin-audit` does:
`resolveUserAuthzGrants` in the request's organization, the organization
kept while `vetOrganizationClaim` backs it under the posture in force.
An email address is looked up to its account; one no account carries
gets the id.
- **Cost.** One record read per notice; per recipient, one envelope
resolution plus one field-visibility ask, and only when a
field-visibility source is composed and the record has a title. With no
security plugin the seam passes everything through, so no envelope is
resolved. Most call sites notify one recipient; the slate-wide ones (SLA
breach, escalation) pay per approver, the same resolution the
administrators' slate already pays per member. Per-recipient is the rule
itself (A′), so there is nothing to batch across recipients.
- **Unchanged:** the message templates keep `{{object}} {{record}}` (no
title-plus-id suffix); the literal-body notices (comment, request for
information, send-back note) name no record; the token-bearing
action-link pages keep the id, with a dated note beside the #23594
comment in `action-link-pages.ts`.
- **Docs:** `content/docs/automation/approvals.mdx` said the outcome
notification names the record as `Deal D_123`; it now says title as the
recipient may read it, id as the fallback, id on the action pages.
## Tests
- New `notification-subject-title-per-recipient.integration.test.ts`
(real `ApprovalService` on `ObjectQL` + `SqlDriver`, a field-visibility
double keyed by user): (a) a reader of the title field reads it in the
literal body and the keyed `record` param, on `approval.requested` and
`approval.approved`; (b) an empty title gives the id; (c) a recipient
who may not read the title field, or is served it masked, gets the id
and no trace of the title; (d) two recipients with different field-level
security on one request get different bodies, both on per-approver emits
(the opening) and on one emit naming both (the SLA breach, split into
two emits).
- Red first on `main`: with `approval-service.ts` and
`approval-notification-text.ts` checked out at the base `6390fe52`, (a)
and (d) fail (`Received: "A decision on Contract C1 is waiting on
you."`); (b) and (c) pass there, as the fallback controls they are.
- Ablation: `titleAsServed(record, displayField, readable)` mutated to
ignore `readable` (via `scripts/ablation-replace.mjs`, mutation proven
on disk, restored to the HEAD blob) turns (c) and (d) red.
- Updated, because they pinned ruling B's id in bodies whose fixtures
carry a title: `approval-requested-notification` (the titled-record case
now pins the live title, never the snapshot's),
`approval-decision-outcome-notification`, `approval-notification-body`
(the record name per call site), `approval-notification-keyed-text`
(localized bodies and the end-to-end messaging render).
## Acceptance notes
- Two copies of "the envelope a named user reads with" now exist:
`plugin-audit`'s `contextForMentionedUser` and approvals'
`recipientContext`. Lifting one into `@objectstack/core` is outside this
card's file surface. Noted, not filed.
- A cross-organization approver (ADR-0105 D9) with no membership in the
request's organization never reaches a slate
(`filterApproversWhoCanRead`), so the organization claim
`recipientContext` vets holds for every approver that can be notified;
one that does not hold resolves with no active organization and fails
closed to the id.
## Verification
All at head `6cc01b7264`, base `6390fe52`.
<details><summary>Package suite and typecheck (under the verify
lock)</summary>
```
pnpm --filter @objectstack/plugin-approvals test && pnpm --filter @objectstack/plugin-approvals typecheck
Test Files 100 passed (100)
Tests 1284 passed (1284)
check:test-typecheck: OK — @objectstack/plugin-approvals's test layer compiles under packages/plugins/plugin-approvals/tsconfig.test.json
os-verify-lock: VERDICT command-exit 0
```
</details>
<details><summary>Red on main, green on the fix</summary>
```
# approval-service.ts + approval-notification-text.ts at 6390fe5, the new test file at HEAD
× (a) a recipient who may read the title reads it, literal and keyed alike
× (d) two recipients with different field-level security on one request read different bodies
Expected: "A decision on Contract Acme MSA 2026 is waiting on you."
Received: "A decision on Contract C1 is waiting on you."
Tests 2 failed | 2 passed (4)
# restored: git diff HEAD empty, approval-service.ts blob fbf728e == HEAD
# at HEAD
Tests 4 passed (4)
```
</details>
<details><summary>Ablation: the field-level security narrowing</summary>
```
ablation-replace: anchor "const title = ApprovalService.titleAsServed(record, displayField, readable);" x1 -> x0
ablation-replace: ok mutation landed: anchor 1 -> 0, blob fbf728e -> 05af8607f28b
× (c) a recipient who may not read the title field, or is served it masked, is named the id
× (d) two recipients with different field-level security on one request read different bodies
Tests 2 failed | 2 passed (4)
ablation-replace: ok restored: blob == HEAD (fbf728e) and `git diff HEAD` is empty
```
</details>
<details><summary>Derived gates and lint</summary>
```
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack # 98 commands, each run with its exit captured before any pipe
✓ dispatch-gates --ran: 98 derived famil(ies) accounted for — 98 run, 0 NOT-MEASURED (a DERIVED zero — all 98 recorded an exit code and none of them is 3).
```
Four exited 3 (prerequisite not met) on the first pass and were re-run
green after the prerequisite: `check-plugin-teardown-shape --self-test`
after fetching its pinned positive control `621a4876`; `check:i18n`
after the build closure it names (`check-i18n-bundles: OK (10 package(s)
— all bundles in sync, no undeclared authoring keys)`);
`check:skill-examples` (264 prose examples type-check);
`check:dual-build-cjs-loads`.
`pnpm exec eslint --no-inline-config
packages/plugins/plugin-approvals/src` exit 0. The repo-wide `pnpm lint`
and `test:repo` are CI's.
</details>
<details><summary>Overlap with open PRs</summary>
Open PRs listed and each one's files read: only #26337 touches the same
files (`approval-service.ts`, `approvals.mdx`). `git merge-tree
--write-tree HEAD e451337` exits 0 (no conflict). #26180 has no open PR
yet.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Kfr6fhXWwYppMd27Z6fPM8)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 48a9d9d commit c51aed4
11 files changed
Lines changed: 594 additions & 176 deletions
File tree
- .changeset
- content/docs/automation
- packages/plugins/plugin-approvals/src
- translations
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
565 | 565 | | |
566 | 566 | | |
567 | 567 | | |
568 | | - | |
569 | | - | |
570 | | - | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
571 | 575 | | |
572 | 576 | | |
573 | 577 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
60 | | - | |
61 | | - | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
62 | 67 | | |
63 | 68 | | |
64 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
234 | 234 | | |
235 | 235 | | |
236 | 236 | | |
237 | | - | |
| 237 | + | |
238 | 238 | | |
239 | 239 | | |
240 | 240 | | |
| |||
251 | 251 | | |
252 | 252 | | |
253 | 253 | | |
254 | | - | |
| 254 | + | |
255 | 255 | | |
256 | 256 | | |
257 | 257 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
43 | | - | |
44 | | - | |
| 42 | + | |
| 43 | + | |
45 | 44 | | |
46 | 45 | | |
47 | 46 | | |
48 | 47 | | |
49 | 48 | | |
50 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
51 | 53 | | |
52 | 54 | | |
53 | 55 | | |
| |||
343 | 345 | | |
344 | 346 | | |
345 | 347 | | |
346 | | - | |
| 348 | + | |
347 | 349 | | |
348 | 350 | | |
349 | | - | |
350 | | - | |
351 | | - | |
352 | | - | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
353 | 356 | | |
354 | | - | |
355 | | - | |
356 | | - | |
357 | | - | |
358 | | - | |
359 | | - | |
360 | | - | |
361 | | - | |
362 | | - | |
363 | | - | |
364 | | - | |
365 | | - | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
366 | 369 | | |
367 | | - | |
368 | | - | |
| 370 | + | |
| 371 | + | |
369 | 372 | | |
370 | 373 | | |
371 | 374 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
277 | 277 | | |
278 | 278 | | |
279 | 279 | | |
280 | | - | |
281 | | - | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
282 | 284 | | |
283 | 285 | | |
284 | 286 | | |
| |||
344 | 346 | | |
345 | 347 | | |
346 | 348 | | |
347 | | - | |
| 349 | + | |
348 | 350 | | |
349 | 351 | | |
350 | | - | |
| 352 | + | |
351 | 353 | | |
352 | 354 | | |
353 | | - | |
| 355 | + | |
354 | 356 | | |
355 | 357 | | |
356 | 358 | | |
357 | 359 | | |
358 | | - | |
| 360 | + | |
359 | 361 | | |
360 | 362 | | |
361 | 363 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
31 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
32 | 35 | | |
33 | 36 | | |
34 | 37 | | |
| |||
75 | 78 | | |
76 | 79 | | |
77 | 80 | | |
78 | | - | |
79 | | - | |
80 | | - | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
81 | 85 | | |
82 | 86 | | |
83 | 87 | | |
84 | 88 | | |
85 | | - | |
| 89 | + | |
86 | 90 | | |
87 | 91 | | |
88 | 92 | | |
89 | | - | |
| 93 | + | |
90 | 94 | | |
91 | 95 | | |
92 | 96 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
25 | | - | |
26 | | - | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
27 | 29 | | |
28 | 30 | | |
29 | 31 | | |
| |||
192 | 194 | | |
193 | 195 | | |
194 | 196 | | |
195 | | - | |
196 | | - | |
| 197 | + | |
| 198 | + | |
197 | 199 | | |
198 | 200 | | |
199 | 201 | | |
200 | 202 | | |
201 | | - | |
| 203 | + | |
202 | 204 | | |
203 | 205 | | |
204 | 206 | | |
| |||
207 | 209 | | |
208 | 210 | | |
209 | 211 | | |
210 | | - | |
| 212 | + | |
211 | 213 | | |
212 | | - | |
213 | | - | |
214 | | - | |
215 | | - | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
216 | 218 | | |
217 | 219 | | |
218 | 220 | | |
| |||
221 | 223 | | |
222 | 224 | | |
223 | 225 | | |
224 | | - | |
225 | | - | |
| 226 | + | |
| 227 | + | |
226 | 228 | | |
227 | 229 | | |
228 | 230 | | |
229 | 231 | | |
230 | 232 | | |
231 | 233 | | |
232 | | - | |
233 | | - | |
| 234 | + | |
234 | 235 | | |
235 | 236 | | |
236 | 237 | | |
| |||
0 commit comments