Skip to content

fix(plugin-approvals): an approval notification names the record by its title, as the recipient may read it - #26389

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-tlbu2c
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-tlbu2c

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #26181

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:审批通知(铃铛和邮件正文)不再用记录 id 指代记录,改用记录标题,而且按每个收件人各自能看到的字段来定:收件人能读标题字段就显示标题,读不到、被掩码或标题为空就退回 id。一条通知发给多人且各人看到的不一样时,按名称拆成几条发,谁也拿不到别人才能看的标题。
  • 为什么改:「您的合同 C8Mp7TjRYta8mOWx 已获批准」收件人看不出是哪份合同;维护者已按 A′ 重新裁决(plugin-approvals: approval notifications name the record by its id — "Your Contract C8Mp7TjRYta8mOWx was approved." #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 security(approvals): the inbox's subject record_title is resolved under the system context and served to every approver without their field-level security, against payload-redaction's serve-time contract and #7497's ruling C #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.

Package suite and typecheck (under the verify lock)
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
Red on main, green on the fix
# approval-service.ts + approval-notification-text.ts at 6390fe52, 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 fbf728e7 == HEAD
# at HEAD
Tests  4 passed (4)
Ablation: the field-level security narrowing
ablation-replace: anchor   "const title = ApprovalService.titleAsServed(record, displayField, readable);"  x1 -> x0
ablation-replace: ok mutation landed: anchor 1 -> 0, blob fbf728e71664 -> 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 (fbf728e71664) and `git diff HEAD` is empty
Derived gates and lint
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.

Overlap with open PRs

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 e4513376 exits 0 (no conflict). #26180 has no open PR yet.


Generated by Claude Code

…ts title, as the recipient may read it

Each approval notification is delivered to one named recipient, so its body
now names the record by its display title (ADR-0079 nameField) as that
recipient may read it, narrowed by their field-level security, and by the
record id when the title is empty or unreadable to them. notify() resolves the
name per recipient and splits a notice whose recipients read different names.
The token-bearing action-link pages keep naming the record by its id.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kfr6fhXWwYppMd27Z6fPM8
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 37 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/plugins/plugin-approvals/src/action-link-pages.ts, packages/plugins/plugin-approvals/src/translations/messages.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

9 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via approvals.getRequest (sdk, the route ledger binds it to GET /api/v1/approvals/requests/:id, selected by route anchor /approvals/requests/:id), getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/api/plugin-endpoints.mdx (via /approvals/requests/:id (route, bridged from symbol sendBack — its route source's handler names it))
  • content/docs/automation/approvals.mdx (via sys_approval_request (literal, a string literal in notify), getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id), /approvals/requests/:id (route, bridged from symbol sendBack — its route source's handler names it))
  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class), sys_approval_request (literal, a string literal in notify), /approvals/requests/:id (route, bridged from symbol sendBack — its route source's handler names it))
  • content/docs/automation/workflows.mdx (via sys_approval_request (literal, a string literal in notify))
  • content/docs/kernel/services-checklist.mdx (via /approvals/requests/:id (route, bridged from symbol sendBack — its route source's handler names it))
  • content/docs/permissions/system-context.mdx (via sendBack (symbol, a method of class ApprovalService))
  • content/docs/plugins/packages.mdx (via sys_approval_request (literal, a string literal in notify))
  • content/docs/ui/translations.mdx (via sys_approval_request (literal, a string literal in notify))

⛔ 4 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via sys_approval_request (literal, a string literal in notify), getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/releases/v17/17-0.mdx (via getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class), sys_approval_request (literal, a string literal in notify))
  • content/docs/releases/v18/18-1.mdx (via ApprovalService (symbol, a top-level class), /approvals/requests/:id (route, bridged from symbol sendBack — its route source's handler names it))

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
  • 2 changed file(s) yielded no anchor (packages/plugins/plugin-approvals/src/action-link-pages.ts, packages/plugins/plugin-approvals/src/translations/messages.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, 43 pages)
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 55 of 207 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 — 6 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 abb9046057b6713b78712dfe53a329deefa534ba → packageMentionDocs.

Which tree this was computed on

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

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

⚠️ 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 abb9046057b6713b78712dfe53a329deefa534ba → 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

席位核对 · ACCEPT · domain:server seat 1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC · 2026-10-27T02:09Z

Head 6cc01b7264, report 6431754453 (on #26181), claim 6430894336. Clause-②: no, not governed, no packages/spec/**: no contract review is owed.

item reading
Shape Draft, base main, Fixes #26181, patch changeset for @objectstack/plugin-approvals. Not governed (0 of 11 paths), 770 changed lines.
Fix Ruling 6429362190 letter A′. Every approval notification that names a record now goes through notify({ about, text }), and subjectNamesFor resolves the name per recipient. It reads the record under the system context, as the inbox does (#7497 ruling C), then picks the title as served to that recipient (servedFields over an envelope built by resolveUserAuthzGrants and vetOrganizationClaim in the request's organization). An empty title, an unreadable or masked title field, an unknown e-mail, or any failure gives the id. Recipients who read the same name share one emit, so no recipient gets another's title. The action-link pages keep label and id, with the dated #26181 note beside the #23594 comment. The templates keep {{object}} {{record}}. A sent-back note in the approver's own words still names no record.
Evidence 38 check runs succeeded and 10 were expected skips on this head, 0 failed; mergeable clean. Tests (a)–(d) in notification-subject-title-per-recipient.integration.test.ts cover the ruling's four cases: a readable title, an empty title, an FLS-hidden or masked title, and two recipients with different FLS on one request. They are red on main for the title cases, and the FLS ablation is red. Package suite 1284/1284, derived gates 98 of 98 exit 0.
Carried Claim surface, recorded here: content/docs/automation/approvals.mdx gains the per-recipient naming paragraph, because its Deal D_123 example was made false by this change. Overlap: #26337 merge-tree clean; #26180 (same plugin) has no PR yet and re-measures against this one.
Landing 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay, once every check on this head is green or an expected skip.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 27, 2026 02:10
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 27, 2026 02:10
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit c51aed4 Oct 27, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-tlbu2c branch October 27, 2026 02:31
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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-approvals: approval notifications name the record by its id — "Your Contract C8Mp7TjRYta8mOWx was approved."

2 participants