From a941d0711da0ea946fc879a62542bbfdad361c35 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:46:49 +0000 Subject: [PATCH 1/5] test(plugin-approvals): pin every approval notification body to the ruled subject name Extends the notification-body battery: the reminder, SLA breach (both recipients), escalation, hand-off and both send-back sentences must name the record by the object label and the id, as approval.requested does, with a control for an object whose schema has no label. Red against the object_name/record_id bodies on main. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- ...oval-notification-body.integration.test.ts | 77 ++++++++++++++++++- 1 file changed, 73 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts index 5fa0a6c0df4..a87053b53c5 100644 --- a/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts @@ -37,6 +37,17 @@ * `GET /api/v1/notifications` through the real messaging service — is the * dogfood pin `packages/qa/dogfood/test/approval-notification-body.dogfood.test.ts`. * + * ## And how it names the record (#23978) + * + * Ruling B of #23556 / #23594 names the record an approval notification is + * about by its object's label and its id (`Deal D_123`), with no title, through + * `nameSubjectRecord`. `approval.requested` used it; every other body that + * names the record built `/`, so an approver read the + * API name and a slash in the reminder, the SLA breach, the escalation, the + * hand-off and both send-back sentences. The same battery pins each of those + * bodies, and a control: a record of an object whose schema has no label is + * named by the object's API name and the id, the helper's own fallback. + * * ## And whose notification it is (#23284) * * The same battery pins the organization every call site hands messaging: the @@ -102,6 +113,14 @@ const deal = { title: { name: 'title', label: 'Title', type: 'text' as const }, }, }; +/** [#23978] The control: an object whose schema declares no label. */ +const memo = { + name: 'crm_memo', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + }, +}; /** The flow the requests belong to: an approval node with a revise window. */ const FLOW = { @@ -142,10 +161,10 @@ describe('every approval notification carries its text in body', () => { const emits: Emitted[] = []; const ids: Record = {}; - const open = async (key: string, approvers: any[], extra: Record = {}) => { - await engine.insert('crm_deal', { id: `D_${key}`, title: key, organization_id: orgOfDeal(key) }, { context: SYSTEM } as any); + const open = async (key: string, approvers: any[], extra: Record = {}, object: string = deal.name) => { + await engine.insert(object, { id: `D_${key}`, title: key, organization_id: orgOfDeal(key) }, { context: SYSTEM } as any); const row: any = await svc.openNodeRequest({ - object: 'crm_deal', recordId: `D_${key}`, runId: `run_${key}`, nodeId: 'review', flowName: FLOW.name, + object, recordId: `D_${key}`, runId: `run_${key}`, nodeId: 'review', flowName: FLOW.name, config: { approvers, behavior: 'first_response', ...extra } as any, submitterId: 'u_sub', record: { id: `D_${key}`, title: key }, }, asUser('u_sub')); @@ -167,7 +186,7 @@ describe('every approval notification carries its text in body', () => { client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, }), true); await engine.init(); - for (const def of [sysUser, deal, ...(await packageObjects())]) { + for (const def of [sysUser, deal, memo, ...(await packageObjects())]) { engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); } await engine.syncSchemas(); @@ -202,6 +221,13 @@ describe('every approval notification carries its text in body', () => { await svc.sendBack(ids.revise, { actorId: 'u_alice', comment: SEND_BACK } as any, asUser('u_alice')); await open('budget', [{ type: 'user', value: 'u_alice' }], { maxRevisions: 0 }); await svc.sendBack(ids.budget, { actorId: 'u_alice' } as any, asUser('u_alice')); + // [#23978] Within budget and with no note, the body is the service's own sentence. + await open('rework', [{ type: 'user', value: 'u_alice' }]); + await svc.sendBack(ids.rework, { actorId: 'u_alice' } as any, asUser('u_alice')); + + // ── [#23978] The control: a reminder about a record of an object with no label ── + await open('bare', [{ type: 'user', value: 'u_alice' }], {}, memo.name); + await svc.remind(ids.bare, { actorId: 'u_sub' }, asUser('u_sub')); // ── Out of office: the routing substitutes a delegate ── await engine.insert('sys_approval_delegation', { @@ -235,12 +261,16 @@ describe('every approval notification carries its text in body', () => { 'approval.requested@budget', 'approval.requested@sla_notify', 'approval.requested@sla_reassign', + 'approval.requested@rework', + 'approval.requested@bare', 'approval.ooo_substituted@ooo', 'approval.ooo_skipped@ooo', 'approval.returned@budget', 'approval.returned@revise', + 'approval.returned@rework', 'approval.reassigned@thread', 'approval.reminder@thread+links', + 'approval.reminder@bare+links', 'approval.request_info@thread', 'approval.comment@thread', 'approval.escalated@sla_reassign', @@ -263,6 +293,45 @@ describe('every approval notification carries its text in body', () => { expect([sentBack.audience, sentBack.payload?.body]).toEqual([['u_sub'], SEND_BACK]); }); + it('[#23978] every body that names the record names it by its object label and its id, as approval.requested does', () => { + /** The one body a site sent; a site that sent two different bodies is a failure of its own. */ + const bodyAt = (site: string): unknown => { + const bodies = [...new Set(emits.filter((e) => siteOf(e) === site).map((e) => e.payload?.body))]; + expect(bodies, site).toHaveLength(1); + return bodies[0]; + }; + // Ruling B of #23556 / #23594: label and id, no title — one form for every + // body, the one `approval.requested` already sends. + expect(bodyAt('approval.requested@thread')).toBe('A decision on Deal D_thread is waiting on you.'); + expect({ + reminder: bodyAt('approval.reminder@thread+links'), + slaBreached: bodyAt('approval.sla_breached@sla_notify'), + slaBreachedToSubmitter: bodyAt('approval.sla_breached@sla_notify>submitter'), + escalated: bodyAt('approval.escalated@sla_reassign'), + reassigned: bodyAt('approval.reassigned@thread'), + revisionLimit: bodyAt('approval.returned@budget'), + sentBackWithoutNote: bodyAt('approval.returned@rework'), + }).toEqual({ + reminder: 'A decision on Deal D_thread is still waiting on you.', + slaBreached: 'A decision on Deal D_sla_notify is overdue.', + slaBreachedToSubmitter: "Deal D_sla_notify: escalation action 'notify' was taken.", + escalated: 'An overdue approval on Deal D_sla_reassign was escalated to you.', + reassigned: 'You are now an approver on Deal D_thread.', + revisionLimit: 'Your Deal D_budget exceeded the revision limit (0) and was rejected.', + sentBackWithoutNote: 'Your Deal D_rework needs rework before it can be approved.', + }); + + // The control: the object's schema has no label, so the record is named by + // the object's API name and the id — never a title, never the slash form. + expect(bodyAt('approval.reminder@bare+links')).toBe('A decision on crm_memo D_bare is still waiting on you.'); + + // And no body anywhere in the battery joins the API name and the id with a slash. + const slashed = emits + .map((e) => `${siteOf(e)}: ${String(e.payload?.body)}`) + .filter((line) => /\bcrm_(deal|memo)\/D_/.test(line)); + expect(slashed).toEqual([]); + }); + it('every notification carries a non-empty body, and no field the payload does not declare', () => { const offenders: string[] = []; for (const e of emits) { From 60815ef6a1d4779153e19774a50156651a8d94fa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:51:24 +0000 Subject: [PATCH 2/5] fix(plugin-approvals): name the record by object label and id in every approval notification body The reminder, SLA-breach, escalation, hand-off and send-back bodies built object_name/record_id. Each now names the record through nameSubjectRecord, the label-and-id form (no title) that ruling B of #23556 / #23594 set and approval.requested already used. The object's API name stays the fallback when its schema has no label. Wording is otherwise unchanged. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- .../plugin-approvals/src/approval-service.ts | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index f08a7e74dbf..12c649d4081 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -4747,6 +4747,8 @@ export class ApprovalService implements IApprovalService { const now = this.clock.now().toISOString(); const maxRevisions = typeof (config as any).maxRevisions === 'number' ? (config as any).maxRevisions : 3; + // [#23978] Both sentences below name the record the way every approval notification does. + const subject = this.nameSubjectRecord(raw.object_name, raw.record_id); let priorSendBacks = 0; if (runId && nodeId) { const siblings = await this.engine.find('sys_approval_request', { @@ -4806,7 +4808,7 @@ export class ApprovalService implements IApprovalService { source: { object: 'sys_approval_request', id: requestId }, payload: { title: 'Approval auto-rejected', - body: `Your ${raw.object_name}/${raw.record_id} exceeded the revision limit (${maxRevisions}) and was rejected.`, + body: `Your ${subject} exceeded the revision limit (${maxRevisions}) and was rejected.`, actionUrl: '/system/approvals', }, }); @@ -4850,7 +4852,7 @@ export class ApprovalService implements IApprovalService { source: { object: 'sys_approval_request', id: requestId }, payload: { title: 'Sent back for revision', - body: input.comment?.trim() || `Your ${raw.object_name}/${raw.record_id} needs rework before it can be approved.`, + body: input.comment?.trim() || `Your ${subject} needs rework before it can be approved.`, actionUrl: '/system/approvals', }, }); @@ -5112,7 +5114,7 @@ export class ApprovalService implements IApprovalService { dedupKey: `approval-reassign-${requestId}-${to}`, payload: { title: 'Approval handed to you', - body: `You are now an approver on ${raw.object_name}/${raw.record_id}.`, + body: `You are now an approver on ${this.nameSubjectRecord(raw.object_name, raw.record_id)}.`, actionUrl: '/system/approvals', }, }); @@ -5157,6 +5159,8 @@ export class ApprovalService implements IApprovalService { // Per-approver fan-out: concrete identities (user ids / emails) each get // their OWN one-tap approve/reject links (ADR-0043); `position:*`-style // literals can't carry a personal token and fall back to a plain nudge. + // [#23978] One name for every recipient, the form `approval.requested` uses. + const subject = this.nameSubjectRecord(raw.object_name, raw.record_id); let notified = 0; const concrete = pending.filter(a => a && !a.includes(':')); const literals = pending.filter(a => a && a.includes(':')); @@ -5172,7 +5176,7 @@ export class ApprovalService implements IApprovalService { dedupKey: `approval-remind-${requestId}-${nowIso}-${approver}`, payload: { title: 'Approval reminder', - body: `A decision on ${raw.object_name}/${raw.record_id} is still waiting on you.`, + body: `A decision on ${subject} is still waiting on you.`, actionUrl: '/system/approvals', actions: [ { label: 'Approve', url: this.actionLinkUrl(tokens.approve) }, @@ -5196,7 +5200,7 @@ export class ApprovalService implements IApprovalService { dedupKey: `approval-remind-${requestId}-${nowIso}`, payload: { title: 'Approval reminder', - body: `A decision on ${raw.object_name}/${raw.record_id} is still waiting on you.`, + body: `A decision on ${subject} is still waiting on you.`, actionUrl: '/system/approvals', }, }); @@ -6499,6 +6503,8 @@ export class ApprovalService implements IApprovalService { typeof esc.escalateTo === 'string' && esc.escalateTo.trim() ? esc.escalateTo.trim() : undefined; const now = this.clock.now().toISOString(); const pending = csvSplit(raw.pending_approvers); + // [#23978] Every escalation sentence names the record the way every approval notification does. + const subject = this.nameSubjectRecord(raw.object_name, raw.record_id); // `escalateTo` is a position machine name or a user id (same contract as // the `position` ApproverType, ADR-0090 D3). Position holders win; an @@ -6539,7 +6545,7 @@ export class ApprovalService implements IApprovalService { source: { object: 'sys_approval_request', id: raw.id }, payload: { title: 'Approval escalated to you', - body: `An overdue approval on ${raw.object_name}/${raw.record_id} was escalated to you.`, + body: `An overdue approval on ${subject} was escalated to you.`, actionUrl: '/system/approvals', }, }); @@ -6558,7 +6564,7 @@ export class ApprovalService implements IApprovalService { source: { object: 'sys_approval_request', id: raw.id }, payload: { title: 'Approval SLA breached', - body: `A decision on ${raw.object_name}/${raw.record_id} is overdue.`, + body: `A decision on ${subject} is overdue.`, actionUrl: '/system/approvals', }, }); @@ -6572,7 +6578,7 @@ export class ApprovalService implements IApprovalService { source: { object: 'sys_approval_request', id: raw.id }, payload: { title: 'Your approval request breached its SLA', - body: `${raw.object_name}/${raw.record_id}: escalation action '${action}' was taken.`, + body: `${subject}: escalation action '${action}' was taken.`, actionUrl: '/system/approvals', }, }); From c5af626aa8cce26b0226a25de0631ca6113892e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:54:18 +0000 Subject: [PATCH 3/5] test(plugin-approvals): pin the record's name in each approval body, not the sentence The pin now asserts the subject each body names (object label and id, or the API name and id for an object with no label) rather than freezing each sentence's wording, and covers both out-of-office bodies, which the battery's slash sweep found still building object/record. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- ...oval-notification-body.integration.test.ts | 57 +++++++++---------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts index a87053b53c5..ac93c487ddc 100644 --- a/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts @@ -44,9 +44,10 @@ * `nameSubjectRecord`. `approval.requested` used it; every other body that * names the record built `/`, so an approver read the * API name and a slash in the reminder, the SLA breach, the escalation, the - * hand-off and both send-back sentences. The same battery pins each of those - * bodies, and a control: a record of an object whose schema has no label is - * named by the object's API name and the id, the helper's own fallback. + * hand-off, both send-back sentences and both out-of-office sentences. The same + * battery pins the record's name in each of those bodies, and a control: a + * record of an object whose schema has no label is named by the object's API + * name and the id, the helper's own fallback. * * ## And whose notification it is (#23284) * @@ -294,36 +295,30 @@ describe('every approval notification carries its text in body', () => { }); it('[#23978] every body that names the record names it by its object label and its id, as approval.requested does', () => { - /** The one body a site sent; a site that sent two different bodies is a failure of its own. */ - const bodyAt = (site: string): unknown => { - const bodies = [...new Set(emits.filter((e) => siteOf(e) === site).map((e) => e.payload?.body))]; - expect(bodies, site).toHaveLength(1); - return bodies[0]; - }; // Ruling B of #23556 / #23594: label and id, no title — one form for every - // body, the one `approval.requested` already sends. - expect(bodyAt('approval.requested@thread')).toBe('A decision on Deal D_thread is waiting on you.'); - expect({ - reminder: bodyAt('approval.reminder@thread+links'), - slaBreached: bodyAt('approval.sla_breached@sla_notify'), - slaBreachedToSubmitter: bodyAt('approval.sla_breached@sla_notify>submitter'), - escalated: bodyAt('approval.escalated@sla_reassign'), - reassigned: bodyAt('approval.reassigned@thread'), - revisionLimit: bodyAt('approval.returned@budget'), - sentBackWithoutNote: bodyAt('approval.returned@rework'), - }).toEqual({ - reminder: 'A decision on Deal D_thread is still waiting on you.', - slaBreached: 'A decision on Deal D_sla_notify is overdue.', - slaBreachedToSubmitter: "Deal D_sla_notify: escalation action 'notify' was taken.", - escalated: 'An overdue approval on Deal D_sla_reassign was escalated to you.', - reassigned: 'You are now an approver on Deal D_thread.', - revisionLimit: 'Your Deal D_budget exceeded the revision limit (0) and was rejected.', - sentBackWithoutNote: 'Your Deal D_rework needs rework before it can be approved.', + // body, the one `approval.requested` already sends. The pin is the record's + // name in each body, not the rest of the sentence around it. + const NAMED: Record = { + 'approval.requested@thread': 'Deal D_thread', + 'approval.reminder@thread+links': 'Deal D_thread', + 'approval.sla_breached@sla_notify': 'Deal D_sla_notify', + 'approval.sla_breached@sla_notify>submitter': 'Deal D_sla_notify', + 'approval.escalated@sla_reassign': 'Deal D_sla_reassign', + 'approval.reassigned@thread': 'Deal D_thread', + 'approval.returned@budget': 'Deal D_budget', + 'approval.returned@rework': 'Deal D_rework', + 'approval.ooo_substituted@ooo': 'Deal D_ooo', + 'approval.ooo_skipped@ooo': 'Deal D_ooo', + // The control: the object's schema has no label, so the record is named + // by the object's API name and the id. + 'approval.reminder@bare+links': 'crm_memo D_bare', + }; + const offenders = Object.entries(NAMED).flatMap(([site, name]) => { + const bodies = emits.filter((e) => siteOf(e) === site).map((e) => String(e.payload?.body)); + if (!bodies.length) return [`${site}: sent nothing`]; + return bodies.filter((body) => !body.includes(name)).map((body) => `${site}: ${body}`); }); - - // The control: the object's schema has no label, so the record is named by - // the object's API name and the id — never a title, never the slash form. - expect(bodyAt('approval.reminder@bare+links')).toBe('A decision on crm_memo D_bare is still waiting on you.'); + expect(offenders).toEqual([]); // And no body anywhere in the battery joins the API name and the id with a slash. const slashed = emits From a42511d0ac9daf5116c973d4fbb7f96ed4f11a9e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:54:18 +0000 Subject: [PATCH 4/5] fix(plugin-approvals): name the record by object label and id in the out-of-office bodies approval.ooo_substituted and approval.ooo_skipped built input.object/input.recordId, the same slash form as the other approval bodies under a different variable spelling. Both now name the record through nameSubjectRecord, as approval.requested does in the same method. Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- packages/plugins/plugin-approvals/src/approval-service.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 12c649d4081..c01b0f0749d 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -3603,7 +3603,8 @@ export class ApprovalService implements IApprovalService { // OOO substitution audit + notification (#1322 M4). Each hop that rerouted // an approver away from an out-of-office user is recorded on the request's // audit trail (a system action, no human actor) and notified to both the - // delegate — who now owns the slot — and the skipped approver. + // delegate — who now owns the slot — and the skipped approver. [#23978] + // Both bodies name the record the way every approval notification does. for (const sub of substitutions) { await this.engine.insert('sys_approval_action', await this.actionRow(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, @@ -3620,7 +3621,7 @@ export class ApprovalService implements IApprovalService { dedupKey: `approval-ooo-${id}-${sub.to}`, payload: { title: 'Approval routed to you (out-of-office cover)', - body: `You are covering an approval on ${input.object}/${input.recordId} while ${sub.from} is out of office.`, + body: `You are covering an approval on ${this.nameSubjectRecord(input.object, input.recordId)} while ${sub.from} is out of office.`, actionUrl: '/system/approvals', }, }); @@ -3632,7 +3633,7 @@ export class ApprovalService implements IApprovalService { dedupKey: `approval-ooo-skip-${id}-${sub.from}`, payload: { title: 'Approval routed to your delegate', - body: `An approval on ${input.object}/${input.recordId} was routed to ${sub.to} while you are out of office.`, + body: `An approval on ${this.nameSubjectRecord(input.object, input.recordId)} was routed to ${sub.to} while you are out of office.`, actionUrl: '/system/approvals', }, }); From 1eae118e8c6fc50f9844eada217c7fa7a3ee06b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 21:59:24 +0000 Subject: [PATCH 5/5] chore(changeset): patch for plugin-approvals approval notification subject name Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP Co-authored-by: Claude --- .../23978-approval-notification-subject-name.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/23978-approval-notification-subject-name.md diff --git a/.changeset/23978-approval-notification-subject-name.md b/.changeset/23978-approval-notification-subject-name.md new file mode 100644 index 00000000000..adad29dd302 --- /dev/null +++ b/.changeset/23978-approval-notification-subject-name.md @@ -0,0 +1,11 @@ +--- +"@objectstack/plugin-approvals": patch +--- + +Clause-②: no + +fix(plugin-approvals): every approval notification body names its record by the object's label and the record id + +- **What changed.** The reminder, SLA-breach (to approvers and to the submitter), escalation, hand-off, revision-limit, send-back (with no note) and both out-of-office notification bodies named the record as `/`, for example `crm_deal/D_123`. They now name it the way `approval.requested` already did: the object's label and the record id, for example `Deal D_123`. An object whose schema declares no label is named by its API name and the id, for example `crm_memo D_123`. +- **No title.** No body carries the record's title, from the live record or from the submit snapshot. The body is stored and may be e-mailed, and it has no single reader whose field access it could be narrowed for. +- **Unchanged.** Each sentence's other wording, every topic, title, `actionUrl`, action link and audience, and the `DUPLICATE_REQUEST` refusal text.