From f05f81b91bf54c4ae1f7f689ddc677d7a34c5ee3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 15:10:45 +0000 Subject: [PATCH 01/17] feat(plugin-approvals): engine-written display labels for the approval lists (WIP) Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../plugin-approvals/src/approval-service.ts | 159 +++++++++++++----- .../src/sys-approval-action.object.ts | 64 ++++++- .../src/sys-approval-request.object.ts | 66 +++++++- 3 files changed, 240 insertions(+), 49 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 0e12be2ed24..030dc0a0017 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -1164,9 +1164,11 @@ function prettifyMachineName(raw: string | null | undefined): string | undefined } function rowFromRequest(row: any): ApprovalRequestRow { - // Authored display labels ride the node-config snapshot (`__flowLabel` / - // `__nodeLabel`) so they survive without a schema migration; fall back to a - // prettified machine name for rows written before labels were captured. + // Authored display labels are the row's engine-written columns + // (`process_label` / `current_step_label`, #23889). A row opened before those + // columns carries the same labels on the node-config snapshot (`__flowLabel` + // / `__nodeLabel`); one written before labels were captured at all falls + // back to a prettified machine name. const cfg = parseJson(row.node_config_json, undefined); return { id: String(row.id), @@ -1192,8 +1194,8 @@ function rowFromRequest(row: any): ApprovalRequestRow { updated_at: row.updated_at ?? undefined, // The row is created at submission time; expose the stable inbox-facing name. submitted_at: row.created_at ?? undefined, - process_label: cfg?.__flowLabel ?? prettifyMachineName(row.process_name), - step_label: cfg?.__nodeLabel ?? prettifyMachineName(row.current_step), + process_label: row.process_label ?? cfg?.__flowLabel ?? prettifyMachineName(row.process_name), + step_label: row.current_step_label ?? cfg?.__nodeLabel ?? prettifyMachineName(row.current_step), sla_due_at: slaDueAt(row.created_at, cfg), // ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1). round: typeof cfg?.__round === 'number' ? cfg.__round : undefined, @@ -3509,7 +3511,8 @@ export class ApprovalService implements IApprovalService { const now = nowDate.toISOString(); const id = uid('areq'); const processName = `flow:${input.flowName ?? input.nodeId}`; - // Display labels ride the config snapshot (no schema migration needed); + // Display labels also ride the config snapshot, where rows opened before + // the `process_label` / `current_step_label` columns (#23889) carry them; // `rowFromRequest` surfaces them as `process_label` / `step_label`. const configSnapshot: any = { ...input.config }; if (input.flowLabel) configSnapshot.__flowLabel = input.flowLabel; @@ -3548,9 +3551,16 @@ export class ApprovalService implements IApprovalService { record_id: input.recordId, submitter_id: input.submitterId ?? context.userId ?? null, status: 'pending', + // [#23889] The display snapshots the approval lists show: the flow's and + // the node's authored labels as the flow engine handed them over, and + // whom the request waits on. Engine-written here and never re-derived by + // a reader (`sys-approval-request.object.ts` documents each column). + process_label: input.flowLabel ?? null, current_step: input.nodeId, + current_step_label: input.nodeLabel ?? null, current_step_index: 0, pending_approvers: approvers.join(','), + pending_approver_labels: await this.slateLabels(approvers), payload_json: input.record != null ? JSON.stringify(this.snapshotRecord(input.object, input.record)) : null, flow_run_id: input.runId, flow_node_id: input.nodeId, @@ -3561,11 +3571,11 @@ export class ApprovalService implements IApprovalService { }; await this.engine.insert('sys_approval_request', row, { context: SYSTEM_CTX }); await this.syncApproverIndex(id, approvers, requestOrg, now); - await this.engine.insert('sys_approval_action', { + await this.appendAction(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, step_name: input.nodeId, step_index: 0, action: 'submit', actor_id: input.submitterId ?? context.userId ?? null, comment: null, created_at: now, - }, { context: SYSTEM_CTX }); + }); // [#22824] The request's display says why it is with the administrators: // one system note on its timeline (no person took it, so `actor_id` is @@ -3573,13 +3583,13 @@ export class ApprovalService implements IApprovalService { // approver entries resolved to no one. The same reason rides // `__resolvedFrom` for the audit snapshot. if (routedToAdministrators) { - await this.engine.insert('sys_approval_action', { + await this.appendAction(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, step_name: input.nodeId, step_index: 0, action: 'comment', actor_id: null, comment: `Routed to this organization's administrators because the approver slate named no one: ${routedToAdministrators}.`, created_at: now, - }, { context: SYSTEM_CTX }); + }); } // OOO substitution audit + notification (#1322 M4). Each hop that rerouted @@ -3587,13 +3597,13 @@ export class ApprovalService implements IApprovalService { // audit trail (a system action, no human actor) and notified to both the // delegate — who now owns the slot — and the skipped approver. for (const sub of substitutions) { - await this.engine.insert('sys_approval_action', { + await this.appendAction(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, step_name: input.nodeId, step_index: 0, action: 'ooo_substitute', actor_id: null, comment: `${sub.from} → ${sub.to}${sub.reason ? ` — ${sub.reason}` : ''}`, created_at: now, - }, { context: SYSTEM_CTX }); + }); await this.notify({ topic: 'approval.ooo_substituted', organizationId: requestOrg, @@ -3851,7 +3861,7 @@ export class ApprovalService implements IApprovalService { await this.assertRunResumable(runId, requestId); // Audit the decision first so the quorum/per_group tally below sees it. - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: input.decision, // Two facts (#21411): the PERSON who decided, and the slot the decision @@ -3865,7 +3875,7 @@ export class ApprovalService implements IApprovalService { via_override: viaOverride, attachments: input.attachments?.length ? input.attachments : null, created_at: now, - }, { context: SYSTEM_CTX }); + }); // Multi-approver aggregation on approve (#3266). A rejection always // finalizes the node (one veto), so only the approve path can hold it open. @@ -3907,6 +3917,7 @@ export class ApprovalService implements IApprovalService { const stillPending = original.filter(a => !approved.has(a)); await this.engine.update('sys_approval_request', { id: requestId, pending_approvers: stillPending.join(','), updated_at: now, + pending_approver_labels: await this.slateLabels(stillPending), // #3447 P2: a mid-tally approval may carry outputs too (unanimous / // per_group co-sign, each approver contributing their declared keys) // — accumulate them on the snapshot so the FINALIZING decision hands @@ -3933,7 +3944,7 @@ export class ApprovalService implements IApprovalService { ? { ...((config as any).__decisionOutputs ?? {}), ...(acceptedOutputs ?? {}) } : undefined; await this.engine.update('sys_approval_request', { - id: requestId, status: finalStatus, pending_approvers: null, completed_at: now, updated_at: now, + id: requestId, status: finalStatus, pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, ...(mergedOutputs ? { node_config_json: JSON.stringify({ ...config, __decisionOutputs: mergedOutputs }), } : {}), @@ -4411,14 +4422,14 @@ export class ApprovalService implements IApprovalService { const runId: string | null = raw.flow_run_id ?? null; const now = this.clock.now().toISOString(); - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'recall', actor_id: recordedActor(context), comment: input.comment ?? null, created_at: now, - }, { context: SYSTEM_CTX }); + }); await this.engine.update('sys_approval_request', { - id: requestId, status: 'recalled', pending_approvers: null, completed_at: now, updated_at: now, + id: requestId, status: 'recalled', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, }, { context: SYSTEM_CTX }); await this.syncApproverIndex(requestId, [], org, now); if (config.approvalStatusField) { @@ -4641,19 +4652,19 @@ export class ApprovalService implements IApprovalService { // `actor_id` is null on purpose and is the point of the `cancel` kind: // no person did this. Attributing it to a submitter (`recall`) or an // approver (`reject`) would file a decision nobody made. - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'cancel', actor_id: null, comment: `Auto-cancelled: the ${object} record '${record}' this request is about was deleted`, created_at: now, - }, { context: SYSTEM_CTX }); + }); await this.engine.update('sys_approval_request', { id: requestId, status: 'cancelled', cancel_reason: 'record_deleted', - pending_approvers: null, + pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, }, { context: SYSTEM_CTX }); @@ -4739,23 +4750,23 @@ export class ApprovalService implements IApprovalService { // Audit the revise intent first (audit-first, like decideNode) — on the // auto-reject path the trail then reads `revise → reject`, preserving // what the approver actually asked for. - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'revise', actor_id: recordedActor(context), acted_as: slot ?? null, comment: input.comment ?? null, created_at: now, - }, { context: SYSTEM_CTX }); + }); if (priorSendBacks >= maxRevisions) { // Revision budget exhausted — auto-reject (ADR-0044 loop guard). - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'reject', actor_id: recordedActor(context), acted_as: slot ?? null, comment: `Auto-rejected: revision limit (${maxRevisions}) exceeded`, created_at: now, - }, { context: SYSTEM_CTX }); + }); await this.engine.update('sys_approval_request', { - id: requestId, status: 'rejected', pending_approvers: null, completed_at: now, updated_at: now, + id: requestId, status: 'rejected', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, }, { context: SYSTEM_CTX }); await this.syncApproverIndex(requestId, [], org, now); if (config.approvalStatusField) { @@ -4797,7 +4808,7 @@ export class ApprovalService implements IApprovalService { } await this.engine.update('sys_approval_request', { - id: requestId, status: 'returned', pending_approvers: null, completed_at: now, updated_at: now, + id: requestId, status: 'returned', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, }, { context: SYSTEM_CTX }); await this.syncApproverIndex(requestId, [], org, now); if (config.approvalStatusField) { @@ -4892,11 +4903,11 @@ export class ApprovalService implements IApprovalService { // never open (#4420). await this.assertRunResumable(runId, requestId); - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'resubmit', actor_id: recordedActor(context), comment: input.comment ?? null, created_at: now, - }, { context: SYSTEM_CTX }); + }); let resumed = false; let resumeError: string | undefined; @@ -5048,7 +5059,7 @@ export class ApprovalService implements IApprovalService { const now = this.clock.now().toISOString(); // Audit first, then mutate — mirrors decideNode(), so a failed audit // write can never leave a moved slot without a trail. - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'reassign', // The hand-off parties are STRUCTURED fields (#4365) — the old default @@ -5064,7 +5075,7 @@ export class ApprovalService implements IApprovalService { reassign_from: from, reassign_to: to, via_override: viaOverride, comment: input.comment ?? null, created_at: now, - }, { context: SYSTEM_CTX }); + }); // per_group / quorum (#3266): carry the delegated slot's group membership to // the new approver in the snapshot, so their approval still counts for the // original group. @@ -5080,6 +5091,7 @@ export class ApprovalService implements IApprovalService { } catch { /* snapshot left untouched on parse failure */ } await this.engine.update('sys_approval_request', { id: requestId, pending_approvers: next.join(','), updated_at: now, ...configPatch, + pending_approver_labels: await this.slateLabels(next), }, { context: SYSTEM_CTX }); await this.syncApproverIndex(requestId, next, raw.organization_id ?? null, now); @@ -5128,11 +5140,11 @@ export class ApprovalService implements IApprovalService { const pending = csvSplit(raw.pending_approvers); const nowIso = now.toISOString(); - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'remind', actor_id: recordedActor(context), comment: input.comment ?? null, created_at: nowIso, - }, { context: SYSTEM_CTX }); + }); // Per-approver fan-out: concrete identities (user ids / emails) each get // their OWN one-tap approve/reject links (ADR-0043); `position:*`-style @@ -5408,12 +5420,12 @@ export class ApprovalService implements IApprovalService { } const now = this.clock.now().toISOString(); - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'request_info', actor_id: recordedActor(context), acted_as: slot ?? null, comment: input.comment.trim(), created_at: now, - }, { context: SYSTEM_CTX }); + }); if (raw.submitter_id) { await this.notify({ @@ -5453,14 +5465,14 @@ export class ApprovalService implements IApprovalService { } const now = this.clock.now().toISOString(); - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'comment', actor_id: recordedActor(context), acted_as: slot ?? null, comment: input.comment.trim(), attachments: input.attachments?.length ? input.attachments : null, created_at: now, - }, { context: SYSTEM_CTX }); + }); // Notify the other side of the thread. const audience = isSubmitter ? pending : [String(raw.submitter_id ?? '')].filter(Boolean); @@ -6439,7 +6451,7 @@ export class ApprovalService implements IApprovalService { const nodeId: string | null = raw.flow_node_id ?? raw.current_step ?? null; const now = this.clock.now().toISOString(); - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: raw.id, organization_id: org, step_name: nodeId, step_index: 0, action: 'recall', // ADR-0118 D1: no person did this, so `actor_id` is null — never a @@ -6449,10 +6461,10 @@ export class ApprovalService implements IApprovalService { actor_id: null, comment: `owning flow run ${runId} is ${runStatus} — request abandoned and record lock released`, created_at: now, - }, { context: SYSTEM_CTX }); + }); await this.engine.update('sys_approval_request', { - id: raw.id, status: 'recalled', pending_approvers: null, completed_at: now, updated_at: now, + id: raw.id, status: 'recalled', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, }, { context: SYSTEM_CTX }); await this.syncApproverIndex(raw.id, [], org, now); @@ -6498,17 +6510,18 @@ export class ApprovalService implements IApprovalService { // action is in the comment. The notifications below forward no actor for // the same reason, and the auto-decision runs under SYSTEM_CTX, which // vouches for nobody, so it records nobody either. - await this.engine.insert('sys_approval_action', { + await this.appendAction(raw, { id: uid('aact'), request_id: raw.id, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'escalate', actor_id: null, comment: `${action}${escalateTo ? ` → ${escalateTo}` : ''}`, created_at: now, - }, { context: SYSTEM_CTX }); + }); if (action === 'reassign' && escalatees.length) { await this.engine.update('sys_approval_request', { id: raw.id, pending_approvers: escalatees.join(','), updated_at: now, + pending_approver_labels: await this.slateLabels(escalatees), }, { context: SYSTEM_CTX }); await this.syncApproverIndex(raw.id, escalatees, raw.organization_id ?? null, now); await this.notify({ @@ -6654,6 +6667,68 @@ export class ApprovalService implements IApprovalService { return names; } + /** + * [#23889] The display label of each slot address, in order: the account + * name {@link resolveUserNames} gives a slot that names a person (a user id, + * or an email an account carries), else the address as stored — a + * `position:` / `team:` literal, or an address no account answers to. + * + * The same resolver, under the same system read, that the approvals door + * names `pending_approver_names` and the timeline's hand-off parties with, + * and the same person/literal split (`type:value` literals are not looked + * up). So a label written from here tells a reader of the row nothing the + * approvals door does not already serve that reader for the same request; + * the generic data door serves the row to the same readers + * (`request-read-gate.ts`). + */ + private async slotLabels(slots: readonly string[]): Promise { + const names = await this.resolveUserNames(slots.filter((slot) => !!slot && !slot.includes(':'))); + return slots.map((slot) => names.get(slot) ?? slot); + } + + /** + * [#23889] `sys_approval_request.pending_approver_labels` for a slate: each + * slot's {@link slotLabels} label in slate order, joined with ", "; null for + * an empty slate, as the slate itself is cleared. Written beside + * `pending_approvers` on every write that sets it, so the two describe the + * same slate. + */ + private async slateLabels(slate: readonly string[]): Promise { + if (!slate.length) return null; + return (await this.slotLabels(slate)).join(', '); + } + + /** + * [#23889] Append one `sys_approval_action` row for `request` — the ONE + * writer of the decision log. Beside the caller's columns it copies, from the + * request the action belongs to, what an action list shows without a join: + * + * - `step_label` — the request's `current_step_label`, the node label the + * engine snapshotted at open (a row opened before that column carries the + * same label in its config snapshot, which is read instead); + * - `object_name` + `record_id` — the request's pointer pair; + * - `acted_as_label` — `acted_as` labelled by {@link slotLabels}; null + * exactly when `acted_as` is. + * + * Every value is a column of the request the row's readers already read + * (the decision log is served only where its request is, + * `bindRequestChildReadGates`), or a name the approvals door already serves + * them for it. + */ + private async appendAction(request: any, action: Record): Promise { + const actedAs = typeof action.acted_as === 'string' && action.acted_as ? action.acted_as : null; + const stepLabel = request?.current_step_label + ?? parseJson(request?.node_config_json, undefined)?.__nodeLabel + ?? null; + await this.engine.insert('sys_approval_action', { + ...action, + step_label: stepLabel, + object_name: request?.object_name ?? null, + record_id: request?.record_id ?? null, + acted_as_label: actedAs ? (await this.slotLabels([actedAs]))[0] : null, + }, { context: SYSTEM_CTX }); + } + /** * Reference-typed fields (key + TARGET OBJECT) of an object's schema. * diff --git a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts index 04b55a0e9a2..9424c3b93b0 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts @@ -43,13 +43,18 @@ export const SysApprovalAction = ObjectSchema.create({ // parent request, exactly as `listActions` applies it), so it answers. fileAccessDelegate: 'approvals', + // The display columns (`step_label`, `acted_as_label`) and the record pointer + // (`object_name` + `record_id`) are written by `ApprovalService` with the row, + // copied from the request it belongs to (`appendAction`). The row is + // append-only, so each is the engine's fact at the moment of the action; a + // list shows them as stored and never re-derives them. listViews: { recent: { type: 'grid', name: 'recent', label: 'Recent', data: { provider: 'object', object: 'sys_approval_action' }, - columns: ['created_at', 'request_id', 'step_name', 'action', 'actor_id', 'acted_as', 'via_override', 'comment'], + columns: ['created_at', 'object_name', 'record_id', 'request_id', 'step_label', 'action', 'actor_id', 'acted_as_label', 'via_override', 'comment'], sort: [{ field: 'created_at', order: 'desc' }], pagination: { pageSize: 50 }, emptyState: { title: 'No approval actions yet', message: 'Actions are logged automatically when approvals progress.' }, @@ -59,7 +64,7 @@ export const SysApprovalAction = ObjectSchema.create({ name: 'by_actor', label: 'By Actor', data: { provider: 'object', object: 'sys_approval_action' }, - columns: ['actor_id', 'created_at', 'request_id', 'step_name', 'action'], + columns: ['actor_id', 'created_at', 'request_id', 'step_label', 'action'], sort: [{ field: 'actor_id', order: 'asc' }, { field: 'created_at', order: 'desc' }], grouping: { fields: [{ field: 'actor_id', order: 'asc', collapsed: false }] }, pagination: { pageSize: 100 }, @@ -69,7 +74,7 @@ export const SysApprovalAction = ObjectSchema.create({ name: 'all_actions', label: 'All', data: { provider: 'object', object: 'sys_approval_action' }, - columns: ['created_at', 'request_id', 'step_name', 'action', 'actor_id', 'acted_as', 'via_override', 'comment'], + columns: ['created_at', 'object_name', 'record_id', 'request_id', 'step_label', 'action', 'actor_id', 'acted_as_label', 'via_override', 'comment'], sort: [{ field: 'created_at', order: 'desc' }], pagination: { pageSize: 100 }, }, @@ -110,6 +115,44 @@ export const SysApprovalAction = ObjectSchema.create({ group: 'Target', }), + // The step's authored label at the time of the action, copied from the + // request's `current_step_label` (the node label the engine snapshotted + // when the request opened). Null on rows written before this column. + step_label: Field.text({ + label: 'Approval Step', + required: false, + maxLength: 255, + readonly: true, + description: 'Label of the step at the time of the action', + group: 'Target', + }), + + // The record the request is about — the request's own pointer pair + // (ADR-0052 §5), copied onto the action when it is written so an action + // list can name the record without a join. Null on rows written before + // these columns: their record is still reachable through `request_id`. + object_name: Field.text({ + label: 'Object', + required: false, + maxLength: 100, + readonly: true, + description: 'Object of the record the request is about', + group: 'Target', + }), + + record_id: Field.text({ + label: 'Record', + required: false, + maxLength: 100, + readonly: true, + // The id half of the pointer pair; `object_name` names its object. Both + // are copied from the request in one write, so a stored id is always a + // record id of the object the sibling names. + referenceVia: 'object_name', + description: 'The record the request is about', + group: 'Target', + }), + step_index: Field.number({ label: 'Step Index', required: false, @@ -180,6 +223,21 @@ export const SysApprovalAction = ObjectSchema.create({ + 'actions, system actions and admin overrides.', }), + // `acted_as` for display: the name of the account the slot names (a user + // id, or an email an account carries), resolved when the action is written + // by `resolveUserNames` — the resolver `listActions` names the hand-off + // parties with — or the stored address when the slot names no person (a + // `position:` literal). Null exactly when `acted_as` is null. A snapshot: + // a later rename does not rewrite an append-only row. + acted_as_label: Field.text({ + label: 'Acting As', + required: false, + maxLength: 255, + readonly: true, + description: 'The slot this action was taken as, for display: the name of the person it names, or its address when it names no person', + group: 'Action', + }), + comment: Field.textarea({ label: 'Comment', required: false, group: 'Action' }), // #4466 — the one bit of "who really decided this" that was still dropped. diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index f3a1f863742..3fae6e024f3 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -68,13 +68,20 @@ export const SysApprovalRequest = ObjectSchema.create({ // the caller's own rows. A caller-scoped list view is never an object's // first; an entry that wants one names it. The order decides which view // opens, never which rows a caller may read. + // + // The display columns (`process_label`, `current_step_label`, + // `pending_approver_labels`) are ENGINE-WRITTEN snapshots, set by + // `ApprovalService` on the writes that change the facts they describe. A list + // reads them as stored and never re-derives a label from flow metadata or + // resolves an approver address itself; `process_name`, `current_step` and + // `pending_approvers` keep the machine values every query and decision reads. listViews: { all_requests: { type: 'grid', name: 'all_requests', label: 'All', data: { provider: 'object', object: 'sys_approval_request' }, - columns: ['process_name', 'object_name', 'record_id', 'status', 'current_step', 'submitter_id', 'updated_at'], + columns: ['process_label', 'object_name', 'record_id', 'status', 'current_step_label', 'pending_approver_labels', 'submitter_id', 'updated_at'], sort: [{ field: 'updated_at', order: 'desc' }], pagination: { pageSize: 50 }, }, @@ -83,7 +90,7 @@ export const SysApprovalRequest = ObjectSchema.create({ name: 'my_pending', label: 'My Pending', data: { provider: 'object', object: 'sys_approval_request' }, - columns: ['process_name', 'object_name', 'record_id', 'current_step', 'submitter_id', 'updated_at'], + columns: ['process_label', 'object_name', 'record_id', 'current_step_label', 'pending_approver_labels', 'submitter_id', 'updated_at'], filter: [ { field: 'status', operator: 'equals', value: 'pending' }, { field: 'pending_approvers', operator: 'contains', value: '{current_user_id}' }, @@ -97,7 +104,7 @@ export const SysApprovalRequest = ObjectSchema.create({ name: 'submitted_by_me', label: 'I Submitted', data: { provider: 'object', object: 'sys_approval_request' }, - columns: ['process_name', 'object_name', 'record_id', 'status', 'current_step', 'updated_at'], + columns: ['process_label', 'object_name', 'record_id', 'status', 'current_step_label', 'updated_at'], filter: [{ field: 'submitter_id', operator: 'equals', value: '{current_user_id}' }], sort: [{ field: 'updated_at', order: 'desc' }], pagination: { pageSize: 25 }, @@ -107,7 +114,7 @@ export const SysApprovalRequest = ObjectSchema.create({ name: 'completed', label: 'Completed', data: { provider: 'object', object: 'sys_approval_request' }, - columns: ['process_name', 'object_name', 'record_id', 'status', 'submitter_id', 'completed_at'], + columns: ['process_label', 'object_name', 'record_id', 'status', 'submitter_id', 'completed_at'], // Every terminal state, `cancelled` included (#13568) — a platform-voided // request is kept as audit evidence, and a terminal state absent from the // only curated terminal view is evidence nobody can find. @@ -175,6 +182,21 @@ export const SysApprovalRequest = ObjectSchema.create({ group: 'Target', }), + // The flow's authored label when the request was opened — a snapshot the + // engine writes once, in `openNodeRequest`, from the label the flow engine + // seeds as `$flowLabel` (`FlowSchema.label`, required). A rename of the flow + // later does not rewrite it: the row records what the approver was asked + // under. Null on rows opened before this column, and on a request opened + // by a direct caller that named no flow label. + process_label: Field.text({ + label: 'Flow', + required: false, + maxLength: 255, + readonly: true, + description: 'Label of the flow that opened this request, as it stood when the request was opened', + group: 'Target', + }), + object_name: Field.text({ label: 'Object', required: true, @@ -284,6 +306,19 @@ export const SysApprovalRequest = ObjectSchema.create({ group: 'State', }), + // The Approval node's authored label (`FlowNodeSchema.label`, required), + // written with `current_step` when the request opens. The engine's fact at + // that moment: the console shows it as stored and never reads the flow to + // re-derive it. Null on rows opened before this column. + current_step_label: Field.text({ + label: 'Approval Step', + required: false, + maxLength: 255, + readonly: true, + description: 'Label of the step awaiting approval, as it stood when the request reached it', + group: 'State', + }), + current_step_index: Field.number({ label: 'Current Step Index', required: false, @@ -298,6 +333,29 @@ export const SysApprovalRequest = ObjectSchema.create({ group: 'State', }), + // Whom a pending request waits on, for display: one label per slot of + // `pending_approvers`, in slate order, joined with ", ". The engine writes + // it on every write that sets the slate (open, a partial tally, reassign, + // SLA reassign) and clears it with the slate on every terminal write, so it + // never outlives the slate it describes. A slot that names a person (a user + // id, or an email an account carries) is labelled with that account's name + // by `resolveUserNames`, the resolver the approvals door serves + // `pending_approver_names` with; any other slot (a `position:` / `team:` + // literal, an address no account answers to) shows as its stored address. + // + // A snapshot, not a live read: a user renamed while the request is pending + // keeps the old name here until the slate is next written. Display only — + // no decision, filter or index reads it; `pending_approvers` stays the + // authority. Null on rows written before this column until their next + // slate write. + pending_approver_labels: Field.textarea({ + label: 'Waiting On', + required: false, + readonly: true, + description: 'Who the request is waiting on: the name of each pending approver, or the slot address when it names no person', + group: 'State', + }), + // The module docstring above used to justify this column with "used by // notifications so they can render before the record is locked or // changed". That consumer does not exist, and the claim was cited as the From 9e371fdac3990c59fefd01b4a7c918f3f9ede46c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 15:23:52 +0000 Subject: [PATCH 02/17] feat(plugin-approvals): translate the new approval-list display columns Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../src/translations/en.objects.generated.ts | 28 +++++++++++++++++++ .../translations/es-ES.objects.generated.ts | 28 +++++++++++++++++++ .../translations/ja-JP.objects.generated.ts | 28 +++++++++++++++++++ .../translations/zh-CN.objects.generated.ts | 28 +++++++++++++++++++ 4 files changed, 112 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts index d344c13b01c..226ff32f7a5 100644 --- a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts @@ -35,6 +35,10 @@ export const enObjects: NonNullable = { label: "Source", help: "Origin of the request — `flow:` for node-driven approvals" }, + process_label: { + label: "Flow", + help: "Label of the flow that opened this request, as it stood when the request was opened" + }, object_name: { label: "Object" }, @@ -70,6 +74,10 @@ export const enObjects: NonNullable = { label: "Current Step", help: "Machine name of the step awaiting approval" }, + current_step_label: { + label: "Approval Step", + help: "Label of the step awaiting approval, as it stood when the request reached it" + }, current_step_index: { label: "Current Step Index" }, @@ -77,6 +85,10 @@ export const enObjects: NonNullable = { label: "Pending Approvers", help: "Comma-separated user ids who can act on the current step" }, + pending_approver_labels: { + label: "Waiting On", + help: "Who the request is waiting on: the name of each pending approver, or the slot address when it names no person" + }, payload_json: { label: "Snapshot", help: "Record snapshot at submission time" @@ -244,6 +256,18 @@ export const enObjects: NonNullable = { label: "Step", help: "Machine name of the step at the time of the action" }, + step_label: { + label: "Approval Step", + help: "Label of the step at the time of the action" + }, + object_name: { + label: "Object", + help: "Object of the record the request is about" + }, + record_id: { + label: "Record", + help: "The record the request is about" + }, step_index: { label: "Step Index" }, @@ -273,6 +297,10 @@ export const enObjects: NonNullable = { label: "Acted As", help: "The pending-approver slot this action was taken as, in the slot’s stored spelling: a user id, an email, or a position address. Empty when no slot admitted the action, such as the submitter’s own actions, system actions and admin overrides." }, + acted_as_label: { + label: "Acting As", + help: "The slot this action was taken as, for display: the name of the person it names, or its address when it names no person" + }, comment: { label: "Comment" }, diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index 5f0caa492f2..1a5aa879cad 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -35,6 +35,10 @@ export const esESObjects: NonNullable = { label: "Origen", help: "Origen de la solicitud — `flow:` para aprobaciones por nodo" }, + process_label: { + label: "Flujo", + help: "Etiqueta del flujo que abrió esta solicitud, tal como estaba cuando se abrió" + }, object_name: { label: "Objeto" }, @@ -70,6 +74,10 @@ export const esESObjects: NonNullable = { label: "Paso actual", help: "Nombre técnico del paso pendiente de aprobación." }, + current_step_label: { + label: "Paso de aprobación", + help: "Etiqueta del paso pendiente de aprobación, tal como estaba cuando la solicitud llegó a él" + }, current_step_index: { label: "Índice del paso actual" }, @@ -77,6 +85,10 @@ export const esESObjects: NonNullable = { label: "Aprobadores pendientes", help: "ID de usuario separados por comas que pueden actuar en el paso actual." }, + pending_approver_labels: { + label: "En espera de", + help: "A quién espera la solicitud: el nombre de cada aprobador pendiente, o la dirección del puesto cuando no designa a ninguna persona" + }, payload_json: { label: "Instantánea", help: "Instantánea del registro en el momento del envío." @@ -244,6 +256,18 @@ export const esESObjects: NonNullable = { label: "Paso", help: "Nombre técnico del paso en el momento de la acción." }, + step_label: { + label: "Paso de aprobación", + help: "Etiqueta del paso en el momento de la acción" + }, + object_name: { + label: "Objeto", + help: "Objeto del registro al que se refiere la solicitud" + }, + record_id: { + label: "Registro", + help: "El registro al que se refiere la solicitud" + }, step_index: { label: "Índice del paso" }, @@ -273,6 +297,10 @@ export const esESObjects: NonNullable = { label: "Actuó como", help: "El turno de aprobación pendiente en calidad del cual se realizó esta acción, con la grafía almacenada del turno: un ID de usuario, un correo electrónico o una dirección de puesto. Vacío cuando ningún turno admitió la acción, como las acciones propias del solicitante, las acciones del sistema y las anulaciones de administrador." }, + acted_as_label: { + label: "En nombre de", + help: "El puesto con el que se realizó esta acción, para mostrar: el nombre de la persona que designa, o su dirección cuando no designa a ninguna persona" + }, comment: { label: "Comentario" }, diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts index 1e4841771a4..2f66b8a59b7 100644 --- a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts @@ -35,6 +35,10 @@ export const jaJPObjects: NonNullable = { label: "ソース", help: "リクエストの発生元 — ノード駆動の承認では `flow:`" }, + process_label: { + label: "フロー", + help: "このリクエストを開いたフローのラベル（リクエストを開いた時点の値）" + }, object_name: { label: "オブジェクト" }, @@ -70,6 +74,10 @@ export const jaJPObjects: NonNullable = { label: "現在のステップ", help: "承認待ちのステップの機械名" }, + current_step_label: { + label: "承認ステップ", + help: "承認待ちのステップのラベル（リクエストがそのステップに到達した時点の値）" + }, current_step_index: { label: "現在のステップ番号" }, @@ -77,6 +85,10 @@ export const jaJPObjects: NonNullable = { label: "承認待ち承認者", help: "現在のステップを処理できるユーザー ID のカンマ区切りリスト" }, + pending_approver_labels: { + label: "承認待ちの相手", + help: "リクエストが待っている相手：各承認待ち承認者の名前。スロットが特定の人を指さない場合はそのアドレス" + }, payload_json: { label: "スナップショット", help: "送信時のレコードスナップショット" @@ -244,6 +256,18 @@ export const jaJPObjects: NonNullable = { label: "ステップ", help: "アクション時点のステップの機械名" }, + step_label: { + label: "承認ステップ", + help: "アクション実行時のステップのラベル" + }, + object_name: { + label: "オブジェクト", + help: "リクエストの対象レコードのオブジェクト" + }, + record_id: { + label: "レコード", + help: "リクエストの対象レコード" + }, step_index: { label: "ステップ番号" }, @@ -273,6 +297,10 @@ export const jaJPObjects: NonNullable = { label: "代表スロット", help: "この操作が代表した承認待ちスロット。スロットに保存された表記（ユーザー ID、メールアドレス、またはポジションアドレス）で記録されます。申請者自身の操作、システム操作、管理者オーバーライドなど、スロットを経由せずに許可された操作では空になります。" }, + acted_as_label: { + label: "代理元", + help: "このアクションを実行したスロット（表示用）：人を指す場合はその名前、指さない場合はアドレス" + }, comment: { label: "コメント" }, diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts index 6bc54fe2dc3..718db470207 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -35,6 +35,10 @@ export const zhCNObjects: NonNullable = { label: "来源", help: "请求来源 —— 节点驱动的审批为 `flow:`" }, + process_label: { + label: "流程", + help: "打开此请求的流程标签，记录的是请求打开时的值" + }, object_name: { label: "对象" }, @@ -70,6 +74,10 @@ export const zhCNObjects: NonNullable = { label: "当前步骤", help: "当前等待审批的步骤机器名称" }, + current_step_label: { + label: "审批步骤", + help: "当前等待审批的步骤标签，记录的是请求进入该步骤时的值" + }, current_step_index: { label: "当前步骤索引" }, @@ -77,6 +85,10 @@ export const zhCNObjects: NonNullable = { label: "待审批人", help: "可在当前步骤执行操作的用户 ID，逗号分隔" }, + pending_approver_labels: { + label: "等待审批人", + help: "请求正在等待的对象：每位待审批人的姓名；槽位不指向具体人员时显示其地址" + }, payload_json: { label: "快照", help: "提交时的记录快照" @@ -244,6 +256,18 @@ export const zhCNObjects: NonNullable = { label: "步骤", help: "执行该动作时对应步骤的机器名称" }, + step_label: { + label: "审批步骤", + help: "执行该动作时对应步骤的标签" + }, + object_name: { + label: "对象", + help: "请求所涉及记录的对象" + }, + record_id: { + label: "记录", + help: "请求所涉及的记录" + }, step_index: { label: "步骤索引" }, @@ -273,6 +297,10 @@ export const zhCNObjects: NonNullable = { label: "代表槽位", help: "执行此操作时所代表的待审批槽位，按该槽位存储的写法记录：用户 ID、邮箱或岗位地址。若操作并非经由槽位放行（如提交人自己的操作、系统操作和管理员越权操作），则为空。" }, + acted_as_label: { + label: "代表", + help: "执行此操作时所代表的槽位（用于显示）：指向人员时为其姓名，否则为槽位地址" + }, comment: { label: "评论" }, From 56513b61095d585e89c4bc5471bb147dbc852da1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 15:25:23 +0000 Subject: [PATCH 03/17] test(plugin-approvals): pin the engine-written approval-list display columns on a real engine Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- ...al-list-display-labels.integration.test.ts | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 packages/plugins/plugin-approvals/src/approval-list-display-labels.integration.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-list-display-labels.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-list-display-labels.integration.test.ts new file mode 100644 index 00000000000..33afa6e03eb --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-list-display-labels.integration.test.ts @@ -0,0 +1,296 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23889] The approval lists show engine-written display values: the flow's + * and the step's labels, whom a pending request waits on, the record an action + * is about, and the slot an action was taken as. + * + * ## The contract + * + * The lists are generic list views over `sys_approval_request` and + * `sys_approval_action`: they show stored columns and nothing else. So every + * display value is a column the approval engine writes on the write that + * changes the fact it describes, and a list never re-derives a label from flow + * metadata or resolves an approver address itself: + * + * - `process_label` / `current_step_label` — the flow's and the Approval + * node's authored labels, as the flow engine hands them to + * `openNodeRequest`, snapshotted at open; + * - `pending_approver_labels` — one label per slot of `pending_approvers`, + * written with the slate on every write that sets it and cleared with it; + * - on every action row, from its request: `step_label`, the pointer pair + * `object_name` + `record_id`, and `acted_as_label`. + * + * A slot that names a person (a user id, or an email an account carries) is + * labelled with the account's name by the resolver the approvals door already + * names `pending_approver_names` with; any other slot shows as its address. + * + * ## What this pins + * + * Driven through the real `ApprovalService` on a real `ObjectQL` over + * `SqlDriver` (better-sqlite3, in-memory, real DDL), reading the stored rows + * back as the system: + * + * (a) open writes both labels and the waiting-on column, and the submit + * action carries the step label and the record pointer; + * (b) a partial tally rewrites the waiting-on column, each decision records + * the label of the slot it took (a person's name, a position's address), + * and the finalizing decision clears the column with the slate; + * (c) a reassign and the SLA sweep's reassign rewrite the waiting-on column; + * (d) an action on a request opened before the label columns copies the step + * label from that request's config snapshot; + * (e) read discipline: each name the waiting-on column holds is one the + * approvals door serves the same reader for the same request; + * (f) the declaration: the list views show the display columns in place of + * the machine ones, every column they name is a declared field, and the + * action's `record_id` is the id half of a pointer pair. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { EngineQueryOptions } from '@objectstack/spec/data'; +import { ApprovalService } from './approval-service.js'; +import { ApprovalsServicePlugin } from './approvals-plugin.js'; +import { SysApprovalRequest } from './sys-approval-request.object.js'; +import { SysApprovalAction } from './sys-approval-action.object.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] } as any; +const asUser = (userId: string, positions: string[] = []) => + ({ userId, positions, permissions: [] }) as any; + +const USERS = [ + { id: 'u_sub', name: 'Sam Submitter', email: 'sam@example.com' }, + { id: 'u_legal', name: 'Lee Legal', email: 'lee@example.com' }, + { id: 'u_mail', name: 'Mia Mail', email: 'mail@example.com' }, + { id: 'u_alice', name: 'Alice', email: 'alice@example.com' }, + { id: 'u_bob', name: 'Bob', email: 'bob@example.com' }, + { id: 'u_boss', name: 'Boss', email: 'boss@example.com' }, +]; +const POSITION = 'position:legal'; +const EMAIL = 'mail@example.com'; +const FLOW_LABEL = 'Deal Review'; +const NODE_LABEL = 'Legal sign-off'; + +const sysUser = { + name: 'sys_user', + label: 'User', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + email: { name: 'email', label: 'Email', type: 'text' as const }, + }, +}; +const deal = { + name: 'crm_deal', + label: 'Deal', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + }, +}; +const FLOW = { name: 'deal_review', label: FLOW_LABEL, nodes: [{ id: 'review', type: 'approval', label: NODE_LABEL }], edges: [] }; + +async function packageObjects(): Promise { + let manifest: any; + const ctx: any = { + getService: (name: string) => { + if (name === 'manifest') return { register: (m: any) => { manifest = m; } }; + throw new Error(`no service '${name}'`); + }, + logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }, + }; + await new ApprovalsServicePlugin().init(ctx); + return manifest.objects; +} + +describe('#23889 — the approval lists read engine-written display columns', () => { + let engine: ObjectQL; + let svc: ApprovalService; + let now = Date.parse('2026-10-01T09:00:00.000Z'); + + const requestRow = async (id: string) => + ((await engine.find('sys_approval_request', { where: { id }, limit: 1, context: SYSTEM } satisfies EngineQueryOptions)) as any[])[0]; + const actionsOf = async (requestId: string) => + ((await engine.find('sys_approval_action', { + where: { request_id: requestId }, orderBy: [{ field: 'created_at', order: 'asc' }], context: SYSTEM, + } satisfies EngineQueryOptions)) as any[]); + const open = async (key: string, approvers: any[], extra: Record = {}) => { + await engine.insert('crm_deal', { id: `D_${key}`, title: 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, flowLabel: FLOW_LABEL, nodeLabel: NODE_LABEL, + config: { approvers, behavior: 'first_response', ...extra } as any, + submitterId: 'u_sub', record: { id: `D_${key}`, title: key }, + }, asUser('u_sub')); + return row.id as string; + }; + + beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + }), true); + await engine.init(); + for (const def of [sysUser, deal, ...(await packageObjects())]) { + engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); + } + await engine.syncSchemas(); + for (const u of USERS) await engine.insert('sys_user', u, { context: SYSTEM } as any); + + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(now += 1000) }, + automation: { + resume: async () => ({ status: 'completed' }), + cancelRun: async () => undefined, + getRun: async (runId: string) => ({ id: runId, status: 'paused' }), + getFlow: async () => FLOW, + } as any, + logger: { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} } as any, + }); + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('(a) open writes the flow and step labels and whom the request waits on; the submit action carries the step label and the record pointer', async () => { + const id = await open('a', [ + { type: 'position', value: 'legal' }, { type: 'user', value: 'u_alice' }, { type: 'user', value: EMAIL }, + ], { behavior: 'unanimous' }); + + const row = await requestRow(id); + expect(row.process_name).toBe('flow:deal_review'); + expect(row.process_label).toBe(FLOW_LABEL); + expect(row.current_step).toBe('review'); + expect(row.current_step_label).toBe(NODE_LABEL); + expect(row.pending_approvers).toBe(`${POSITION},u_alice,${EMAIL}`); + // A position nobody holds stays its address; a user id and an email an + // account carries are labelled with the account's name. + expect(row.pending_approver_labels).toBe(`${POSITION}, Alice, Mia Mail`); + + const [submit] = await actionsOf(id); + expect(submit).toMatchObject({ + action: 'submit', step_name: 'review', step_label: NODE_LABEL, + object_name: 'crm_deal', record_id: 'D_a', acted_as: null, acted_as_label: null, + }); + }); + + it('(b) a partial tally rewrites whom the request waits on, each decision labels the slot it took, and the final decision clears the column', async () => { + const id = await open('b', [ + { type: 'position', value: 'legal' }, { type: 'user', value: 'u_alice' }, { type: 'user', value: EMAIL }, + ], { behavior: 'unanimous' }); + + await svc.decideNode(id, { decision: 'approve' } as any, asUser('u_alice')); + expect((await requestRow(id)).pending_approver_labels).toBe(`${POSITION}, Mia Mail`); + + await svc.decideNode(id, { decision: 'approve', actorId: EMAIL } as any, asUser('u_mail')); + expect((await requestRow(id)).pending_approver_labels).toBe(POSITION); + + await svc.decideNode(id, { decision: 'approve', actorId: POSITION } as any, asUser('u_legal', ['legal'])); + const done = await requestRow(id); + expect(done.status).toBe('approved'); + expect(done.pending_approvers).toBeNull(); + expect(done.pending_approver_labels).toBeNull(); + + const decisions = (await actionsOf(id)).filter((a) => a.action === 'approve'); + expect(decisions.map((a) => [a.acted_as, a.acted_as_label])).toEqual([ + ['u_alice', 'Alice'], + [EMAIL, 'Mia Mail'], + [POSITION, POSITION], + ]); + for (const a of decisions) { + expect(a).toMatchObject({ step_label: NODE_LABEL, object_name: 'crm_deal', record_id: 'D_b' }); + } + }); + + it('(c) a reassign and the SLA sweep\'s reassign rewrite whom the request waits on', async () => { + const id = await open('c', [{ type: 'user', value: 'u_alice' }]); + await svc.reassign(id, { actorId: 'u_alice', to: 'u_bob' }, asUser('u_alice')); + expect((await requestRow(id)).pending_approver_labels).toBe('Bob'); + const reassign = (await actionsOf(id)).find((a) => a.action === 'reassign'); + expect(reassign).toMatchObject({ step_label: NODE_LABEL, object_name: 'crm_deal', record_id: 'D_c' }); + + const sla = await open('c_sla', [{ type: 'user', value: 'u_alice' }], + { escalation: { timeoutHours: 1, action: 'reassign', escalateTo: 'u_boss', notifySubmitter: false } }); + now += 2 * 3600_000; + await svc.runEscalations(); + const escalated = await requestRow(sla); + expect(escalated.pending_approvers).toBe('u_boss'); + expect(escalated.pending_approver_labels).toBe('Boss'); + expect((await actionsOf(sla)).find((a) => a.action === 'escalate')).toMatchObject({ + step_label: NODE_LABEL, object_name: 'crm_deal', record_id: 'D_c_sla', acted_as_label: null, + }); + }); + + it('(d) an action on a request opened before the label columns copies the step label from its config snapshot', async () => { + await engine.insert('crm_deal', { id: 'D_d', title: 'd' }, { context: SYSTEM } as any); + const stamp = new Date(now += 1000).toISOString(); + // The row as an earlier writer left it: labels only on the snapshot. + await engine.insert('sys_approval_request', { + id: 'areq_legacy', process_name: 'flow:deal_review', object_name: 'crm_deal', record_id: 'D_d', + submitter_id: 'u_sub', status: 'pending', current_step: 'review', current_step_index: 0, + pending_approvers: 'u_alice', flow_run_id: 'run_d', flow_node_id: 'review', + node_config_json: JSON.stringify({ + approvers: [{ type: 'user', value: 'u_alice' }], behavior: 'first_response', + __flowLabel: FLOW_LABEL, __nodeLabel: 'Old label', + }), + created_at: stamp, updated_at: stamp, + }, { context: SYSTEM } as any); + + await svc.comment('areq_legacy', { actorId: 'u_alice', comment: 'looking' }, asUser('u_alice')); + const comment = (await actionsOf('areq_legacy')).find((a) => a.action === 'comment'); + expect(comment).toMatchObject({ + step_label: 'Old label', object_name: 'crm_deal', record_id: 'D_d', acted_as: 'u_alice', acted_as_label: 'Alice', + }); + // The row itself is not rewritten by a thread entry: it gains its waiting-on + // column on its next slate write. + expect((await requestRow('areq_legacy')).pending_approver_labels).toBeNull(); + }); + + it('(e) every name the waiting-on column holds is one the approvals door serves the same reader for the same request', async () => { + const id = await open('e', [ + { type: 'position', value: 'legal' }, { type: 'user', value: 'u_alice' }, { type: 'user', value: EMAIL }, + ], { behavior: 'unanimous' }); + const reader = asUser('u_alice'); + const served: any = await svc.getRequest(id, reader); + expect(served).not.toBeNull(); + const servedNames = new Set(Object.values(served.pending_approver_names ?? {})); + const slots = new Set(served.pending_approvers); + const labels = String((await requestRow(id)).pending_approver_labels).split(', '); + expect(labels).toHaveLength(slots.size); + for (const label of labels) { + expect(servedNames.has(label) || slots.has(label)).toBe(true); + } + }); + + it('(f) the list views show the display columns in place of the machine ones, and every column is a declared field', () => { + const req: any = SysApprovalRequest; + const act: any = SysApprovalAction; + + for (const [name, view] of Object.entries(req.listViews)) { + expect(view.columns, name).toContain('process_label'); + expect(view.columns, name).not.toContain('process_name'); + expect(view.columns, name).not.toContain('current_step'); + for (const col of view.columns) expect(req.fields[col], `${name}.${col}`).toBeDefined(); + } + for (const name of ['all_requests', 'my_pending', 'submitted_by_me']) { + expect(req.listViews[name].columns, name).toContain('current_step_label'); + } + for (const name of ['all_requests', 'my_pending']) { + expect(req.listViews[name].columns, name).toContain('pending_approver_labels'); + } + + for (const [name, view] of Object.entries(act.listViews)) { + expect(view.columns, name).toContain('step_label'); + expect(view.columns, name).not.toContain('step_name'); + for (const col of view.columns) expect(act.fields[col], `${name}.${col}`).toBeDefined(); + } + for (const name of ['recent', 'all_actions']) { + expect(act.listViews[name].columns, name).toEqual(expect.arrayContaining(['object_name', 'record_id', 'acted_as_label'])); + expect(act.listViews[name].columns, name).not.toContain('acted_as'); + } + expect(act.fields.record_id.referenceVia).toBe('object_name'); + }); +}); From 790b37936a5e83dc10747be857e2a44af8852cb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 15:28:03 +0000 Subject: [PATCH 04/17] chore(changeset): plugin-approvals approval-list display labels Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../23889-approval-list-display-labels.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .changeset/23889-approval-list-display-labels.md diff --git a/.changeset/23889-approval-list-display-labels.md b/.changeset/23889-approval-list-display-labels.md new file mode 100644 index 00000000000..b5dfb53614f --- /dev/null +++ b/.changeset/23889-approval-list-display-labels.md @@ -0,0 +1,28 @@ +--- +'@objectstack/plugin-approvals': minor +--- + +Clause-②: yes (widening) + +feat(plugin-approvals): the approval lists show engine-written display labels, which are the step and flow labels, whom a pending request waits on, the record an action is about, and the slot it was taken as (#23889) + +The Requests and Action History lists are generic list views. They show stored columns only, so admins used to read node ids (`manager_review`), `flow:` api names, slot addresses and raw user ids there. The approval engine now writes display columns beside the machine ones, and the list views show those columns. Nothing is removed or renamed. The machine columns (`process_name`, `current_step`, `pending_approvers`, `step_name`, `acted_as`) keep their values, and every query and decision still reads them. + +**`sys_approval_request`**, written by `ApprovalService`: + +- `process_label`: the flow's label, and `current_step_label`: the Approval node's label. Both are snapshots taken when the request opens, from the labels the flow engine already hands to `openNodeRequest`. Renaming the flow later does not rewrite them. +- `pending_approver_labels`: whom the request waits on, one label per pending slot, joined with `, `. A slot that names a person (a user id, or an email an account carries) gets that account's name. Any other slot (a `position:` or `team:` literal) shows its address. The column is rewritten whenever the slate is written (open, a partial tally, reassign, SLA reassign) and cleared with the slate on every terminal write. +- The list views: `process_label` replaces `process_name` in every view. `current_step_label` replaces `current_step` in `all_requests`, `my_pending` and `submitted_by_me`. `pending_approver_labels` is added to `all_requests` and `my_pending`. + +**`sys_approval_action`**: each new row copies these columns from its request: + +- `step_label`: the request's step label. +- `object_name` + `record_id`: the request's record pointer (`record_id` declares `referenceVia: 'object_name'`), so a list can name the record. +- `acted_as_label`: `acted_as` labelled the same way as the waiting-on column. It is null exactly when `acted_as` is null. +- The list views: `step_label` replaces `step_name` in `recent`, `all_actions` and `by_actor`. `recent` and `all_actions` add the record pointer and show `acted_as_label` in place of `acted_as`. + +The inbox door's `process_label` / `step_label` now read the new columns first. They fall back to the config snapshot for rows opened earlier, so what they serve is unchanged. + +**Read security.** Every new value is either a column of the request the row's readers can already read, or a name the approvals door already serves those readers for that request: `pending_approver_names`, and the timeline's names. The names come from the same resolver, under the same system read. The generic data door serves both objects only to the callers the approvals door would serve. + +**Rows written before this release** carry null in the new columns. A pending request gets its waiting-on column at its next slate write. Its actions copy the step label from the request's config snapshot. Older action rows have no record pointer, but their record can still be reached through `request_id`. No backfill runs. From c907710372b5cdd0eaf245de21fd1873fdd94958 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 15:41:30 +0000 Subject: [PATCH 05/17] refactor(plugin-approvals): keep each decision-log insert at its call site; a helper builds the row The tenant-audit census certifies every write call site, so the display columns are added by a row builder (actionRow) rather than by funnelling the fifteen inserts through one writer. Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../plugin-approvals/src/approval-service.ts | 72 +++++++++---------- .../src/sys-approval-action.object.ts | 2 +- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 030dc0a0017..55ada4ce937 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -3571,11 +3571,11 @@ export class ApprovalService implements IApprovalService { }; await this.engine.insert('sys_approval_request', row, { context: SYSTEM_CTX }); await this.syncApproverIndex(id, approvers, requestOrg, now); - await this.appendAction(row, { + await this.engine.insert('sys_approval_action', await this.actionRow(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, step_name: input.nodeId, step_index: 0, action: 'submit', actor_id: input.submitterId ?? context.userId ?? null, comment: null, created_at: now, - }); + }), { context: SYSTEM_CTX }); // [#22824] The request's display says why it is with the administrators: // one system note on its timeline (no person took it, so `actor_id` is @@ -3583,13 +3583,13 @@ export class ApprovalService implements IApprovalService { // approver entries resolved to no one. The same reason rides // `__resolvedFrom` for the audit snapshot. if (routedToAdministrators) { - await this.appendAction(row, { + await this.engine.insert('sys_approval_action', await this.actionRow(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, step_name: input.nodeId, step_index: 0, action: 'comment', actor_id: null, comment: `Routed to this organization's administrators because the approver slate named no one: ${routedToAdministrators}.`, created_at: now, - }); + }), { context: SYSTEM_CTX }); } // OOO substitution audit + notification (#1322 M4). Each hop that rerouted @@ -3597,13 +3597,13 @@ export class ApprovalService implements IApprovalService { // audit trail (a system action, no human actor) and notified to both the // delegate — who now owns the slot — and the skipped approver. for (const sub of substitutions) { - await this.appendAction(row, { + await this.engine.insert('sys_approval_action', await this.actionRow(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, step_name: input.nodeId, step_index: 0, action: 'ooo_substitute', actor_id: null, comment: `${sub.from} → ${sub.to}${sub.reason ? ` — ${sub.reason}` : ''}`, created_at: now, - }); + }), { context: SYSTEM_CTX }); await this.notify({ topic: 'approval.ooo_substituted', organizationId: requestOrg, @@ -3861,7 +3861,7 @@ export class ApprovalService implements IApprovalService { await this.assertRunResumable(runId, requestId); // Audit the decision first so the quorum/per_group tally below sees it. - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: input.decision, // Two facts (#21411): the PERSON who decided, and the slot the decision @@ -3875,7 +3875,7 @@ export class ApprovalService implements IApprovalService { via_override: viaOverride, attachments: input.attachments?.length ? input.attachments : null, created_at: now, - }); + }), { context: SYSTEM_CTX }); // Multi-approver aggregation on approve (#3266). A rejection always // finalizes the node (one veto), so only the approve path can hold it open. @@ -4422,11 +4422,11 @@ export class ApprovalService implements IApprovalService { const runId: string | null = raw.flow_run_id ?? null; const now = this.clock.now().toISOString(); - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'recall', actor_id: recordedActor(context), comment: input.comment ?? null, created_at: now, - }); + }), { context: SYSTEM_CTX }); await this.engine.update('sys_approval_request', { id: requestId, status: 'recalled', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, @@ -4652,13 +4652,13 @@ export class ApprovalService implements IApprovalService { // `actor_id` is null on purpose and is the point of the `cancel` kind: // no person did this. Attributing it to a submitter (`recall`) or an // approver (`reject`) would file a decision nobody made. - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'cancel', actor_id: null, comment: `Auto-cancelled: the ${object} record '${record}' this request is about was deleted`, created_at: now, - }); + }), { context: SYSTEM_CTX }); await this.engine.update('sys_approval_request', { id: requestId, @@ -4750,21 +4750,21 @@ export class ApprovalService implements IApprovalService { // Audit the revise intent first (audit-first, like decideNode) — on the // auto-reject path the trail then reads `revise → reject`, preserving // what the approver actually asked for. - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'revise', actor_id: recordedActor(context), acted_as: slot ?? null, comment: input.comment ?? null, created_at: now, - }); + }), { context: SYSTEM_CTX }); if (priorSendBacks >= maxRevisions) { // Revision budget exhausted — auto-reject (ADR-0044 loop guard). - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'reject', actor_id: recordedActor(context), acted_as: slot ?? null, comment: `Auto-rejected: revision limit (${maxRevisions}) exceeded`, created_at: now, - }); + }), { context: SYSTEM_CTX }); await this.engine.update('sys_approval_request', { id: requestId, status: 'rejected', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, }, { context: SYSTEM_CTX }); @@ -4903,11 +4903,11 @@ export class ApprovalService implements IApprovalService { // never open (#4420). await this.assertRunResumable(runId, requestId); - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: org, step_name: nodeId, step_index: 0, action: 'resubmit', actor_id: recordedActor(context), comment: input.comment ?? null, created_at: now, - }); + }), { context: SYSTEM_CTX }); let resumed = false; let resumeError: string | undefined; @@ -5059,7 +5059,7 @@ export class ApprovalService implements IApprovalService { const now = this.clock.now().toISOString(); // Audit first, then mutate — mirrors decideNode(), so a failed audit // write can never leave a moved slot without a trail. - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'reassign', // The hand-off parties are STRUCTURED fields (#4365) — the old default @@ -5075,7 +5075,7 @@ export class ApprovalService implements IApprovalService { reassign_from: from, reassign_to: to, via_override: viaOverride, comment: input.comment ?? null, created_at: now, - }); + }), { context: SYSTEM_CTX }); // per_group / quorum (#3266): carry the delegated slot's group membership to // the new approver in the snapshot, so their approval still counts for the // original group. @@ -5140,11 +5140,11 @@ export class ApprovalService implements IApprovalService { const pending = csvSplit(raw.pending_approvers); const nowIso = now.toISOString(); - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'remind', actor_id: recordedActor(context), comment: input.comment ?? null, created_at: nowIso, - }); + }), { context: SYSTEM_CTX }); // Per-approver fan-out: concrete identities (user ids / emails) each get // their OWN one-tap approve/reject links (ADR-0043); `position:*`-style @@ -5420,12 +5420,12 @@ export class ApprovalService implements IApprovalService { } const now = this.clock.now().toISOString(); - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'request_info', actor_id: recordedActor(context), acted_as: slot ?? null, comment: input.comment.trim(), created_at: now, - }); + }), { context: SYSTEM_CTX }); if (raw.submitter_id) { await this.notify({ @@ -5465,14 +5465,14 @@ export class ApprovalService implements IApprovalService { } const now = this.clock.now().toISOString(); - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: requestId, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'comment', actor_id: recordedActor(context), acted_as: slot ?? null, comment: input.comment.trim(), attachments: input.attachments?.length ? input.attachments : null, created_at: now, - }); + }), { context: SYSTEM_CTX }); // Notify the other side of the thread. const audience = isSubmitter ? pending : [String(raw.submitter_id ?? '')].filter(Boolean); @@ -6451,7 +6451,7 @@ export class ApprovalService implements IApprovalService { const nodeId: string | null = raw.flow_node_id ?? raw.current_step ?? null; const now = this.clock.now().toISOString(); - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: raw.id, organization_id: org, step_name: nodeId, step_index: 0, action: 'recall', // ADR-0118 D1: no person did this, so `actor_id` is null — never a @@ -6461,7 +6461,7 @@ export class ApprovalService implements IApprovalService { actor_id: null, comment: `owning flow run ${runId} is ${runStatus} — request abandoned and record lock released`, created_at: now, - }); + }), { context: SYSTEM_CTX }); await this.engine.update('sys_approval_request', { id: raw.id, status: 'recalled', pending_approvers: null, pending_approver_labels: null, completed_at: now, updated_at: now, @@ -6510,13 +6510,13 @@ export class ApprovalService implements IApprovalService { // action is in the comment. The notifications below forward no actor for // the same reason, and the auto-decision runs under SYSTEM_CTX, which // vouches for nobody, so it records nobody either. - await this.appendAction(raw, { + await this.engine.insert('sys_approval_action', await this.actionRow(raw, { id: uid('aact'), request_id: raw.id, organization_id: raw.organization_id ?? null, step_name: raw.flow_node_id ?? raw.current_step ?? null, step_index: 0, action: 'escalate', actor_id: null, comment: `${action}${escalateTo ? ` → ${escalateTo}` : ''}`, created_at: now, - }); + }), { context: SYSTEM_CTX }); if (action === 'reassign' && escalatees.length) { await this.engine.update('sys_approval_request', { @@ -6699,9 +6699,9 @@ export class ApprovalService implements IApprovalService { } /** - * [#23889] Append one `sys_approval_action` row for `request` — the ONE - * writer of the decision log. Beside the caller's columns it copies, from the - * request the action belongs to, what an action list shows without a join: + * [#23889] One `sys_approval_action` row for `request`: the caller's columns, + * plus what an action list shows without a join, copied from the request the + * action belongs to. Every writer of the decision log builds its row here: * * - `step_label` — the request's `current_step_label`, the node label the * engine snapshotted at open (a row opened before that column carries the @@ -6715,18 +6715,18 @@ export class ApprovalService implements IApprovalService { * `bindRequestChildReadGates`), or a name the approvals door already serves * them for it. */ - private async appendAction(request: any, action: Record): Promise { + private async actionRow(request: any, action: Record): Promise> { const actedAs = typeof action.acted_as === 'string' && action.acted_as ? action.acted_as : null; const stepLabel = request?.current_step_label ?? parseJson(request?.node_config_json, undefined)?.__nodeLabel ?? null; - await this.engine.insert('sys_approval_action', { + return { ...action, step_label: stepLabel, object_name: request?.object_name ?? null, record_id: request?.record_id ?? null, acted_as_label: actedAs ? (await this.slotLabels([actedAs]))[0] : null, - }, { context: SYSTEM_CTX }); + }; } /** diff --git a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts index 9424c3b93b0..a6c25ca4086 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts @@ -45,7 +45,7 @@ export const SysApprovalAction = ObjectSchema.create({ // The display columns (`step_label`, `acted_as_label`) and the record pointer // (`object_name` + `record_id`) are written by `ApprovalService` with the row, - // copied from the request it belongs to (`appendAction`). The row is + // copied from the request it belongs to (`actionRow`). The row is // append-only, so each is the engine's fact at the moment of the action; a // list shows them as stored and never re-derives them. listViews: { From 0bb31612f28e61217bd98f949c33609c8ce67132 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 16:34:05 +0000 Subject: [PATCH 06/17] feat(plugin-approvals): boot-time backfill of the approval-list display columns (WIP) Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../src/action-slot-backfill.ts | 15 +- .../plugin-approvals/src/approval-service.ts | 134 +++++++---- .../plugin-approvals/src/approvals-plugin.ts | 30 ++- .../src/display-label-backfill.ts | 209 ++++++++++++++++++ .../plugin-approvals/src/display-labels.ts | 73 ++++++ .../src/sys-approval-action.object.ts | 17 +- .../src/sys-approval-request.object.ts | 14 +- 7 files changed, 435 insertions(+), 57 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/display-label-backfill.ts create mode 100644 packages/plugins/plugin-approvals/src/display-labels.ts diff --git a/packages/plugins/plugin-approvals/src/action-slot-backfill.ts b/packages/plugins/plugin-approvals/src/action-slot-backfill.ts index 8d14cbb104a..88452fc15ab 100644 --- a/packages/plugins/plugin-approvals/src/action-slot-backfill.ts +++ b/packages/plugins/plugin-approvals/src/action-slot-backfill.ts @@ -69,6 +69,7 @@ */ import { keysetWalk } from '@objectstack/types'; +import type { SlotLabelSource } from './display-label-backfill.js'; /** * The engine surface the repair needs — a structural subset of @@ -123,9 +124,17 @@ export function isSlotLiteral(actorId: string): boolean { */ export async function backfillActionSlots( engine: ActionSlotBackfillEngine, - options: { pageSize?: number } = {}, + options: { pageSize?: number; labels?: Pick } = {}, ): Promise { const pageSize = options.pageSize ?? PAGE; + // [#23889] Every `acted_as` this repair writes gets its display label in the + // same write, so `acted_as_label` is null only where `acted_as` is (or where + // the name could not be read — the display-label backfill retries that). + const labelOf = async (slot: string): Promise> => { + if (!options.labels) return {}; + const [label] = await options.labels.slotLabels([slot]); + return label ? { acted_as_label: label } : {}; + }; let literalsMoved = 0; let sentinelsCleared = 0; let votesStamped = 0; @@ -155,7 +164,7 @@ export async function backfillActionSlots( // Either way the column ends up empty. A slot also moves to `acted_as`; // a sentinel does not — the sweep took no slot. const patch: Record = { id, actor_id: null }; - if (!sentinel && !nonEmpty(row.acted_as)) patch.acted_as = actorId; + if (!sentinel && !nonEmpty(row.acted_as)) Object.assign(patch, { acted_as: actorId }, await labelOf(actorId)); await engine.update('sys_approval_action', patch, { context: SYSTEM_CONTEXT }); if (sentinel) sentinelsCleared++; else literalsMoved++; @@ -190,7 +199,7 @@ export async function backfillActionSlots( // Pass 1 has already emptied every sentinel; a machine took no slot, // so one is never stamped as a vote even if a write raced the scan. if (isMachineSentinel(actorId)) continue; - await engine.update('sys_approval_action', { id, acted_as: actorId }, { context: SYSTEM_CONTEXT }); + await engine.update('sys_approval_action', { id, acted_as: actorId, ...(await labelOf(actorId)) }, { context: SYSTEM_CONTEXT }); votesStamped++; } } diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 55ada4ce937..6e6cd7996c1 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -90,6 +90,8 @@ import { slateNamesAPerson, type ActingCaller, } from './approver-address.js'; +import { labelOrMachineName, requestDisplayLabels } from './display-labels.js'; +import type { SlotLabelSource } from './display-label-backfill.js'; import { redactSnapshot, resolveReadableSnapshotFields, @@ -1147,29 +1149,12 @@ function isBlankDecisionOutput(value: unknown): boolean { return false; } -/** - * Humanize a machine name for display fallback: strips a `flow:` prefix and - * title-cases underscore/dash segments (`flow:manager_review` → "Manager - * Review"). Used only when no authored label was snapshotted on the row. - */ -function prettifyMachineName(raw: string | null | undefined): string | undefined { - if (!raw) return undefined; - const base = String(raw).replace(/^flow:/, '').trim(); - if (!base) return undefined; - return base - .split(/[_\-\s]+/) - .filter(Boolean) - .map(w => w.charAt(0).toUpperCase() + w.slice(1)) - .join(' '); -} - function rowFromRequest(row: any): ApprovalRequestRow { - // Authored display labels are the row's engine-written columns - // (`process_label` / `current_step_label`, #23889). A row opened before those - // columns carries the same labels on the node-config snapshot (`__flowLabel` - // / `__nodeLabel`); one written before labels were captured at all falls - // back to a prettified machine name. + // Display labels by the ONE chain (`display-labels.ts`, #23889) the open + // write and the boot backfill use: the row's column, else the node-config + // snapshot (`__flowLabel` / `__nodeLabel`), else the humanized machine name. const cfg = parseJson(row.node_config_json, undefined); + const labels = requestDisplayLabels(row); return { id: String(row.id), organization_id: row.organization_id ?? undefined, @@ -1194,8 +1179,8 @@ function rowFromRequest(row: any): ApprovalRequestRow { updated_at: row.updated_at ?? undefined, // The row is created at submission time; expose the stable inbox-facing name. submitted_at: row.created_at ?? undefined, - process_label: row.process_label ?? cfg?.__flowLabel ?? prettifyMachineName(row.process_name), - step_label: row.current_step_label ?? cfg?.__nodeLabel ?? prettifyMachineName(row.current_step), + process_label: labels.process_label, + step_label: labels.current_step_label, sla_due_at: slaDueAt(row.created_at, cfg), // ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1). round: typeof cfg?.__round === 'number' ? cfg.__round : undefined, @@ -1443,6 +1428,24 @@ async function findChildRequestId( */ const REQUEST_VISIBILITY = new WeakMap(); +/** + * [#23889] Each service's own slot labeller — the one its live writers label + * `pending_approver_labels` / `acted_as_label` with — for the boot backfills + * (`display-label-backfill.ts`, `action-slot-backfill.ts`). In-package only, by + * the same rule as {@link REQUEST_VISIBILITY}: the published class type stays + * what it was. + */ +const SLOT_LABELS = new WeakMap(); + +/** [#23889] The slot labeller the boot backfills use for `service`. */ +export function slotLabelSourceOf(service: ApprovalService): SlotLabelSource { + const source = SLOT_LABELS.get(service); + if (!source) { + throw new Error('[approvals] no slot label source is registered for this ApprovalService instance'); + } + return source; +} + /** * [#22559] The visibility source the read gate consumes for `service`: the * approvals door's own `visibleRequestIds`, asked for a read that arrives @@ -1524,6 +1527,10 @@ export class ApprovalService implements IApprovalService { // its `request_id` is in it. There `target.row` names a child row loaded // by id: the request it belongs to is the anchor, as `authorizeFileRead` // reads it — `findChildRequestId`, shared with that method. + SLOT_LABELS.set(this, { + slotLabels: (slots) => this.slotLabels(slots), + slateLabels: (slate) => this.slateLabels(slate), + }); REQUEST_VISIBILITY.set(this, { visibleRequestIdsFor: async (context, target) => { // `organizationId` is not on the envelope — see isOverrideActor(). @@ -3555,9 +3562,11 @@ export class ApprovalService implements IApprovalService { // the node's authored labels as the flow engine handed them over, and // whom the request waits on. Engine-written here and never re-derived by // a reader (`sys-approval-request.object.ts` documents each column). - process_label: input.flowLabel ?? null, + // An empty authored label counts as none: the chain falls through to + // the humanized machine name, the label the approvals door served. + process_label: labelOrMachineName(input.flowLabel, processName) ?? null, current_step: input.nodeId, - current_step_label: input.nodeLabel ?? null, + current_step_label: labelOrMachineName(input.nodeLabel, input.nodeId) ?? null, current_step_index: 0, pending_approvers: approvers.join(','), pending_approver_labels: await this.slateLabels(approvers), @@ -6638,11 +6647,30 @@ export class ApprovalService implements IApprovalService { /** * Batch-resolve `sys_user` display names for identifiers that may be user * ids or emails. Best-effort — failures leave entries unresolved. + * + * [#23889] ⚠️ STORED COPIES: this answer is also written into rows, not only + * served per read — `sys_approval_request.pending_approver_labels` and + * `sys_approval_action.acted_as_label` (via {@link slotLabels}). A later + * narrowing of these names by the reader's `sys_user` field-level security, + * here or in `enrichRows` / `listActions`, does not reach those stored values: + * they must be narrowed (or rewritten) too. */ private async resolveUserNames(identifiers: Array): Promise> { + return (await this.readUserNames(identifiers)).names; + } + + /** + * {@link resolveUserNames}, saying whether a `sys_user` read FAILED — which a + * per-read enrichment can ignore, and a STORED label cannot: an unresolved + * name after a failed read is not "this address names no account". + */ + private async readUserNames( + identifiers: Array, + ): Promise<{ names: Map; failed: boolean }> { const names = new Map(); + let failed = false; const targets = Array.from(new Set(identifiers.filter(Boolean))) as string[]; - if (!targets.length) return names; + if (!targets.length) return { names, failed }; try { const users = await this.engine.find('sys_user', { where: { id: { $in: targets } }, fields: ['id', 'name', 'email'], @@ -6651,7 +6679,7 @@ export class ApprovalService implements IApprovalService { for (const u of (users ?? []) as any[]) { if (u?.id && (u.name || u.email)) names.set(String(u.id), String(u.name ?? u.email)); } - } catch { /* best-effort */ } + } catch { failed = true; } const unresolvedEmails = targets.filter(t => !names.has(t) && t.includes('@')); if (unresolvedEmails.length) { try { @@ -6662,16 +6690,19 @@ export class ApprovalService implements IApprovalService { for (const u of (users ?? []) as any[]) { if (u?.email && u.name) names.set(String(u.email), String(u.name)); } - } catch { /* best-effort */ } + } catch { failed = true; } } - return names; + return { names, failed }; } /** * [#23889] The display label of each slot address, in order: the account * name {@link resolveUserNames} gives a slot that names a person (a user id, * or an email an account carries), else the address as stored — a - * `position:` / `team:` literal, or an address no account answers to. + * `position:` / `team:` literal, or an address no account answers to. When + * the `sys_user` read FAILED, a person slot's label is null (said once at + * `warn`), never its address: an address is the label of a slot that names + * no account, and a failed read has not shown that. * * The same resolver, under the same system read, that the approvals door * names `pending_approver_names` and the timeline's hand-off parties with, @@ -6681,21 +6712,39 @@ export class ApprovalService implements IApprovalService { * the generic data door serves the row to the same readers * (`request-read-gate.ts`). */ - private async slotLabels(slots: readonly string[]): Promise { - const names = await this.resolveUserNames(slots.filter((slot) => !!slot && !slot.includes(':'))); - return slots.map((slot) => names.get(slot) ?? slot); + private async slotLabels(slots: readonly string[]): Promise> { + const persons = slots.filter((slot) => !!slot && !slot.includes(':')); + const { names, failed } = await this.readUserNames(persons); + if (failed) { + // A failed read is not an answer: a person slot gets no label (null) rather + // than its raw address frozen into a stored row, and the boot backfill + // (`display-label-backfill.ts`) fills it on the next boot. + this.logger?.warn?.( + '[approvals] the sys_user names for an approval display label could not be read — the label is left ' + + 'empty and is filled by the display-label backfill on the next boot', + { slots: persons.length }, + ); + } + return slots.map((slot) => { + const name = names.get(slot); + if (name) return name; + const isPerson = !!slot && !slot.includes(':'); + return failed && isPerson ? null : slot; + }); } /** * [#23889] `sys_approval_request.pending_approver_labels` for a slate: each * slot's {@link slotLabels} label in slate order, joined with ", "; null for - * an empty slate, as the slate itself is cleared. Written beside + * an empty slate, as the slate itself is cleared, and null when any slot's + * name could not be read (the boot backfill retries). Written beside * `pending_approvers` on every write that sets it, so the two describe the * same slate. */ private async slateLabels(slate: readonly string[]): Promise { if (!slate.length) return null; - return (await this.slotLabels(slate)).join(', '); + const labels = await this.slotLabels(slate); + return labels.some((label) => label === null) ? null : labels.join(', '); } /** @@ -6703,12 +6752,12 @@ export class ApprovalService implements IApprovalService { * plus what an action list shows without a join, copied from the request the * action belongs to. Every writer of the decision log builds its row here: * - * - `step_label` — the request's `current_step_label`, the node label the - * engine snapshotted at open (a row opened before that column carries the - * same label in its config snapshot, which is read instead); + * - `step_label` — the request's step label by `requestDisplayLabels`: its + * `current_step_label`, else (a row opened before that column) the same + * chain the approvals door serves; * - `object_name` + `record_id` — the request's pointer pair; - * - `acted_as_label` — `acted_as` labelled by {@link slotLabels}; null - * exactly when `acted_as` is. + * - `acted_as_label` — `acted_as` labelled by {@link slotLabels}; null when + * `acted_as` is, or when the slot's name could not be read. * * Every value is a column of the request the row's readers already read * (the decision log is served only where its request is, @@ -6717,14 +6766,13 @@ export class ApprovalService implements IApprovalService { */ private async actionRow(request: any, action: Record): Promise> { const actedAs = typeof action.acted_as === 'string' && action.acted_as ? action.acted_as : null; - const stepLabel = request?.current_step_label - ?? parseJson(request?.node_config_json, undefined)?.__nodeLabel - ?? null; + const stepLabel = request ? requestDisplayLabels(request).current_step_label ?? null : null; return { ...action, step_label: stepLabel, object_name: request?.object_name ?? null, record_id: request?.record_id ?? null, + // Null when the slot's name could not be read: the boot backfill retries. acted_as_label: actedAs ? (await this.slotLabels([actedAs]))[0] : null, }; } diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts index ffdaebd5269..8d994b321b1 100644 --- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts +++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts @@ -17,6 +17,7 @@ import { renderConfirmPage, renderResultPage } from './action-link-pages.js'; import { ApprovalService, requestVisibilitySourceOf, + slotLabelSourceOf, ESCALATION_JOB_NAME, ESCALATION_SCAN_INTERVAL_MS, type ApprovalEngine, @@ -35,6 +36,7 @@ import { bindRequestChildReadGates, bindRequestReadGate } from './request-read-g import type { FieldVisibilitySource } from './payload-redaction.js'; import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js'; import { backfillActionSlots } from './action-slot-backfill.js'; +import { backfillApprovalDisplayLabels } from './display-label-backfill.js'; export interface ApprovalsPluginOptions { /** Disable runtime registration (schemas still register). */ @@ -501,7 +503,8 @@ export class ApprovalsServicePlugin implements Plugin { const slotEngine = engine as ApprovalEngine; const backfillSlots = async () => { try { - const out = await backfillActionSlots(slotEngine); + const svc = this.service; + const out = await backfillActionSlots(slotEngine, svc ? { labels: slotLabelSourceOf(svc) } : {}); if (out.literalsMoved > 0 || out.sentinelsCleared > 0 || out.votesStamped > 0) { ctx.logger.info('ApprovalsServicePlugin: action slots backfilled', out); } @@ -518,16 +521,41 @@ export class ApprovalsServicePlugin implements Plugin { } }; + // Display-label backfill (#23889): fill the approval lists' display columns + // on rows written before they existed, from stored data and by the live + // writers' own rules, null-only and idempotent; see the module. After the + // action-slot backfill, whose `acted_as` writes it labels if that pass could + // not. `warn`: a failed run leaves list cells blank — visibly — and changes + // no fact any decision, filter or index reads; the next boot retries. + const backfillDisplayLabels = async () => { + try { + const svc = this.service; + if (!svc) return; + const out = await backfillApprovalDisplayLabels(slotEngine, slotLabelSourceOf(svc)); + if (out.requestsFilled > 0 || out.actionsFilled > 0) { + ctx.logger.info('ApprovalsServicePlugin: approval display labels backfilled', out); + } + } catch (err: any) { + ctx.logger.warn?.( + '[approvals] display-label backfill failed — approval list cells for rows written before the label ' + + 'columns stay blank until a later boot fills them', + { error: err?.message ?? String(err) }, + ); + } + }; + if (typeof (ctx as any).hook === 'function') { (ctx as any).hook('kernel:ready', wireEscalationClock); (ctx as any).hook('kernel:ready', mountActionPages); (ctx as any).hook('kernel:ready', backfillApproverIndex); (ctx as any).hook('kernel:ready', backfillSlots); + (ctx as any).hook('kernel:ready', backfillDisplayLabels); } else { await wireEscalationClock(); await mountActionPages(); await backfillApproverIndex(); await backfillSlots(); + await backfillDisplayLabels(); } // ADR-0019: contribute the `approval` node to the flow engine when one is diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.ts new file mode 100644 index 00000000000..c86f3e4df07 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.ts @@ -0,0 +1,209 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * display-label-backfill — the boot-time fill of the approval lists' display + * columns on rows written before those columns existed (#23889). + * + * ## Why rows need filling + * + * The approval lists are generic list views: they show stored columns only. + * Their columns are engine-written display snapshots — on + * `sys_approval_request` the flow's and step's labels (`process_label`, + * `current_step_label`) and whom a pending request waits on + * (`pending_approver_labels`); on `sys_approval_action` the step label, the + * record pointer (`object_name` + `record_id`) and the label of the slot taken + * (`acted_as_label`). The live writers set them on every new write, but a row + * written before them holds null, and most such rows are never written again: + * a terminal request, and every action row (append-only). Without this pass an + * upgraded deployment's lists show blank cells where they showed the machine + * names before, permanently, while the approvals door serves labels for the + * same rows from the node-config snapshot. + * + * ## What it fills, and from what + * + * Every value comes from stored data, by the same rule the live writer uses: + * + * - request `process_label` / `current_step_label` — `requestDisplayLabels` + * (`display-labels.ts`), the chain the approvals door serves, so the two + * doors agree and the pass leaves no request blank; + * - request `pending_approver_labels`, on a `pending` row with a slate — + * `slateLabels`, the live writers' own labeller; + * - action `step_label` / `object_name` / `record_id` — from its request + * (filled first), exactly what `actionRow` copies on a new action; an + * action whose request no longer exists keeps a null pointer, and its step + * label is its own humanized `step_name`; + * - action `acted_as_label` — `slotLabels(acted_as)`. + * + * ## Idempotent, null-only, and run at every boot + * + * Each write names only columns that were null on the row it read, so the + * pass can never overwrite what a live writer wrote, and a second run over an + * unchanged database writes nothing (`display-label-backfill.integration.test.ts` + * asserts both). A label the labeller could not produce — a failed `sys_user` + * read, which `slotLabels` reports as null rather than freezing a raw address + * — stays null and is retried on the next boot. Both walks are seek walks over + * the predicate "a display column is null", so a row this pass fills leaves the + * predicate behind it, and a row it cannot fill is passed once per boot. + * Wired on `kernel:ready` after the action-slot backfill, whose `acted_as` + * writes this pass then labels if that pass could not. + */ + +import { keysetWalk } from '@objectstack/types'; +import { prettifyMachineName, requestDisplayLabels } from './display-labels.js'; + +/** + * The engine surface the fill needs — a structural subset of + * `ApprovalEngine`, so the module can be driven by the real ObjectQL engine in + * its pin without pulling the service in. + */ +export interface DisplayLabelBackfillEngine { + find(object: string, options?: unknown): Promise; + update(object: string, data: unknown, options?: unknown): Promise; +} + +/** + * The approval service's own slot labeller, as the live writers call it + * (`slotLabelSourceOf` in `approval-service.ts`): `slotLabels` answers one + * label per slot, null for a person slot whose name could not be read; + * `slateLabels` answers the joined column, null when any slot's name could not + * be read or the slate is empty. + */ +export interface SlotLabelSource { + slotLabels(slots: readonly string[]): Promise>; + slateLabels(slate: readonly string[]): Promise; +} + +/** What one run wrote. */ +export interface DisplayLabelBackfillResult { + /** Request rows that got at least one display column. */ + requestsFilled: number; + /** Action rows that got at least one display column. */ + actionsFilled: number; +} + +const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; +const PAGE = 500; + +function nonEmpty(value: unknown): string | null { + return typeof value === 'string' && value.length > 0 ? value : null; +} + +function slateOf(raw: unknown): string[] { + return typeof raw === 'string' ? raw.split(',').map((s) => s.trim()).filter(Boolean) : []; +} + +/** + * Run both passes. Throws on a failed read or write — the caller logs it, and + * the next boot retries: every write is keyed by id and fills only columns + * that were null, so a partial run completes on the next one. + */ +export async function backfillApprovalDisplayLabels( + engine: DisplayLabelBackfillEngine, + labels: SlotLabelSource, + options: { pageSize?: number } = {}, +): Promise { + const pageSize = options.pageSize ?? PAGE; + let requestsFilled = 0; + let actionsFilled = 0; + + // Pass 1 — requests with a null display column. + const requests = keysetWalk>( + (q) => engine.find('sys_approval_request', { + ...q, + fields: [ + 'id', 'status', 'process_name', 'current_step', 'node_config_json', 'pending_approvers', + 'process_label', 'current_step_label', 'pending_approver_labels', + ], + context: SYSTEM_CONTEXT, + }) as Promise[]>, + { + where: { + $or: [ + { process_label: { $null: true } }, + { current_step_label: { $null: true } }, + { status: 'pending', pending_approver_labels: { $null: true } }, + ], + }, + pageSize, + }, + ); + for await (const rows of requests.pages()) { + for (const row of rows) { + const id = nonEmpty(row.id); + if (!id) continue; + const patch: Record = {}; + const chain = requestDisplayLabels(row); + if (!nonEmpty(row.process_label) && chain.process_label) patch.process_label = chain.process_label; + if (!nonEmpty(row.current_step_label) && chain.current_step_label) { + patch.current_step_label = chain.current_step_label; + } + if (row.status === 'pending' && !nonEmpty(row.pending_approver_labels)) { + const slate = slateOf(row.pending_approvers); + const waitingOn = slate.length ? await labels.slateLabels(slate) : null; + if (waitingOn) patch.pending_approver_labels = waitingOn; + } + if (!Object.keys(patch).length) continue; + await engine.update('sys_approval_request', { id, ...patch }, { context: SYSTEM_CONTEXT }); + requestsFilled++; + } + } + + // Pass 2 — actions with a null display column, copied from their request + // (which pass 1 has just filled). + const actions = keysetWalk>( + (q) => engine.find('sys_approval_action', { + ...q, + fields: ['id', 'request_id', 'step_name', 'acted_as', 'step_label', 'object_name', 'record_id', 'acted_as_label'], + context: SYSTEM_CONTEXT, + }) as Promise[]>, + { + where: { + $or: [ + { step_label: { $null: true } }, + { object_name: { $null: true } }, + { record_id: { $null: true } }, + { acted_as: { $null: false }, acted_as_label: { $null: true } }, + ], + }, + pageSize, + }, + ); + for await (const rows of actions.pages()) { + const requestIds = [...new Set(rows.map((r) => nonEmpty(r.request_id)).filter((v): v is string => v !== null))]; + const parents = new Map>(); + if (requestIds.length) { + const found = (await engine.find('sys_approval_request', { + where: { id: { $in: requestIds } }, + fields: ['id', 'object_name', 'record_id', 'current_step', 'current_step_label', 'node_config_json', 'process_name'], + limit: requestIds.length, + context: SYSTEM_CONTEXT, + })) as Record[]; + for (const p of found ?? []) { + const pid = nonEmpty(p.id); + if (pid) parents.set(pid, p); + } + } + for (const row of rows) { + const id = nonEmpty(row.id); + if (!id) continue; + const parent = parents.get(nonEmpty(row.request_id) ?? ''); + const patch: Record = {}; + if (!nonEmpty(row.step_label)) { + const stepLabel = parent ? requestDisplayLabels(parent).current_step_label : prettifyMachineName(row.step_name); + if (stepLabel) patch.step_label = stepLabel; + } + if (parent && !nonEmpty(row.object_name) && nonEmpty(parent.object_name)) patch.object_name = parent.object_name; + if (parent && !nonEmpty(row.record_id) && nonEmpty(parent.record_id)) patch.record_id = parent.record_id; + const actedAs = nonEmpty(row.acted_as); + if (actedAs && !nonEmpty(row.acted_as_label)) { + const [actedAsLabel] = await labels.slotLabels([actedAs]); + if (actedAsLabel) patch.acted_as_label = actedAsLabel; + } + if (!Object.keys(patch).length) continue; + await engine.update('sys_approval_action', { id, ...patch }, { context: SYSTEM_CONTEXT }); + actionsFilled++; + } + } + + return { requestsFilled, actionsFilled }; +} diff --git a/packages/plugins/plugin-approvals/src/display-labels.ts b/packages/plugins/plugin-approvals/src/display-labels.ts new file mode 100644 index 00000000000..302afdf75d3 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/display-labels.ts @@ -0,0 +1,73 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23889] The ONE chain that turns a request's stored facts into its display + * labels — the flow's and the step's. + * + * Three readers take it from here, so the approval lists and the approvals door + * can never show different labels for the same row: + * + * - `ApprovalService.openNodeRequest` writes `process_label` / + * `current_step_label` with it at open; + * - `rowFromRequest` serves `process_label` / `step_label` with it on the + * approvals door; + * - the boot backfill (`display-label-backfill.ts`) fills the two columns + * with it on a row written before they existed, and the action's + * `step_label` from the filled request. + * + * The chain: the row's own column when it holds a label; else the authored + * label the node-config snapshot carries (`__flowLabel` / `__nodeLabel`, the + * place labels lived before the columns); else the machine name, humanized + * (`manager_review` → "Manager Review"). An authored label that is the empty + * string counts as no label — the spec allows one (`FlowSchema.label` and + * `FlowNodeSchema.label` are `z.string()` with no minimum) — so the chain falls + * through to the machine name instead of showing a blank cell. + */ + +/** + * Humanize a machine name: strips a `flow:` prefix and title-cases + * underscore/dash segments (`flow:manager_review` → "Manager Review"). + */ +export function prettifyMachineName(raw: unknown): string | undefined { + if (typeof raw !== 'string' || !raw) return undefined; + const base = raw.replace(/^flow:/, '').trim(); + if (!base) return undefined; + return base + .split(/[_\-\s]+/) + .filter(Boolean) + .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) + .join(' '); +} + +/** A label that names something: a non-blank string, else `undefined`. */ +function label(value: unknown): string | undefined { + return typeof value === 'string' && value.trim() ? value : undefined; +} + +/** An authored label, else the humanized machine name. */ +export function labelOrMachineName(authored: unknown, machineName: unknown): string | undefined { + return label(authored) ?? prettifyMachineName(machineName); +} + +function configSnapshot(raw: unknown): Record | undefined { + if (raw && typeof raw === 'object') return raw as Record; + if (typeof raw !== 'string' || !raw) return undefined; + try { + const parsed = JSON.parse(raw); + return parsed && typeof parsed === 'object' ? parsed : undefined; + } catch { + return undefined; + } +} + +/** A `sys_approval_request` row's flow and step labels, by the chain above. */ +export function requestDisplayLabels(row: Record): { + process_label: string | undefined; + current_step_label: string | undefined; +} { + const cfg = configSnapshot(row.node_config_json); + return { + process_label: label(row.process_label) ?? labelOrMachineName(cfg?.__flowLabel, row.process_name), + current_step_label: label(row.current_step_label) ?? labelOrMachineName(cfg?.__nodeLabel, row.current_step), + }; +} diff --git a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts index a6c25ca4086..4c8e8dc66e2 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts @@ -117,7 +117,8 @@ export const SysApprovalAction = ObjectSchema.create({ // The step's authored label at the time of the action, copied from the // request's `current_step_label` (the node label the engine snapshotted - // when the request opened). Null on rows written before this column. + // when the request opened). Filled on rows written before this column by + // the boot backfill (`display-label-backfill.ts`). step_label: Field.text({ label: 'Approval Step', required: false, @@ -129,8 +130,10 @@ export const SysApprovalAction = ObjectSchema.create({ // The record the request is about — the request's own pointer pair // (ADR-0052 §5), copied onto the action when it is written so an action - // list can name the record without a join. Null on rows written before - // these columns: their record is still reachable through `request_id`. + // list can name the record without a join. Rows written before these + // columns are filled from their request by the boot backfill + // (`display-label-backfill.ts`); an action whose request no longer exists + // keeps them null. object_name: Field.text({ label: 'Object', required: false, @@ -227,8 +230,12 @@ export const SysApprovalAction = ObjectSchema.create({ // id, or an email an account carries), resolved when the action is written // by `resolveUserNames` — the resolver `listActions` names the hand-off // parties with — or the stored address when the slot names no person (a - // `position:` literal). Null exactly when `acted_as` is null. A snapshot: - // a later rename does not rewrite an append-only row. + // `position:` literal). Null when `acted_as` is null, and — until the boot + // backfill (`display-label-backfill.ts`) fills it — when the name could not + // be read at write time, never a raw address in place of a name. Rows + // written before this column are filled by that backfill too, and every + // `acted_as` the action-slot backfill writes gets its label in the same + // write. A snapshot: a later rename does not rewrite an append-only row. acted_as_label: Field.text({ label: 'Acting As', required: false, diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 3fae6e024f3..cf200f46916 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -186,8 +186,10 @@ export const SysApprovalRequest = ObjectSchema.create({ // engine writes once, in `openNodeRequest`, from the label the flow engine // seeds as `$flowLabel` (`FlowSchema.label`, required). A rename of the flow // later does not rewrite it: the row records what the approver was asked - // under. Null on rows opened before this column, and on a request opened - // by a direct caller that named no flow label. + // under. An empty or absent authored label falls through to the humanized + // machine name (`display-labels.ts`, the chain the approvals door serves). + // Rows opened before this column are filled by the boot backfill + // (`display-label-backfill.ts`) by the same chain. process_label: Field.text({ label: 'Flow', required: false, @@ -309,7 +311,7 @@ export const SysApprovalRequest = ObjectSchema.create({ // The Approval node's authored label (`FlowNodeSchema.label`, required), // written with `current_step` when the request opens. The engine's fact at // that moment: the console shows it as stored and never reads the flow to - // re-derive it. Null on rows opened before this column. + // re-derive it. Same chain and same boot backfill as `process_label`. current_step_label: Field.text({ label: 'Approval Step', required: false, @@ -346,8 +348,10 @@ export const SysApprovalRequest = ObjectSchema.create({ // A snapshot, not a live read: a user renamed while the request is pending // keeps the old name here until the slate is next written. Display only — // no decision, filter or index reads it; `pending_approvers` stays the - // authority. Null on rows written before this column until their next - // slate write. + // authority. Null on a non-pending row, and while a person slot's name + // could not be read (never a raw address in its place); the boot backfill + // (`display-label-backfill.ts`) fills pending rows written before this + // column, and retries a failed read. pending_approver_labels: Field.textarea({ label: 'Waiting On', required: false, From e1a23c71f75073da49f147319a393444ce4535ae Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 16:40:51 +0000 Subject: [PATCH 07/17] test(plugin-approvals): pin the display-label backfill, the failed-read rider and the empty-label rider Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- ...display-label-backfill.integration.test.ts | 266 ++++++++++++++++++ .../translations/es-ES.objects.generated.ts | 4 +- 2 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts new file mode 100644 index 00000000000..d8ded70235e --- /dev/null +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -0,0 +1,266 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23889] The boot-time fill of the approval lists' display columns on rows + * written before those columns existed (`display-label-backfill.ts`), and the + * two riders that keep a stored label honest. + * + * ## What this pins + * + * Driven on a real `ObjectQL` over `SqlDriver` (better-sqlite3, in-memory, real + * DDL). The rows are seeded as an 18.0.0-next.2 writer left them: the labels + * only in `node_config_json`, no display column, actions without a pointer. + * + * (a) ONE BOOT — `ApprovalsServicePlugin.start()` then `kernel:ready`, the + * production wiring — fills every display column: a terminal request and + * a pending one, the flow and step labels by the chain the approvals door + * serves (snapshot label, else the humanized machine name, so the two + * doors agree), the waiting-on column of the pending one, and on every + * action the step label, the record pointer and the acting-as label; + * (b) a SECOND boot writes nothing; + * (c) null-only: a column a live writer already filled is never overwritten, + * and an action whose request no longer exists keeps a null pointer and + * takes its own humanized step name; + * (d) a failed `sys_user` read stores no label — never the raw address — says + * so once at `warn`, and the next run fills it; a non-person slot keeps its + * address by design; + * (e) the action-slot backfill labels every `acted_as` it writes; + * (f) an empty authored label is stored as the humanized machine name, never + * `''`, at open. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { EngineQueryOptions } from '@objectstack/spec/data'; +import { ApprovalService, slotLabelSourceOf } from './approval-service.js'; +import { ApprovalsServicePlugin } from './approvals-plugin.js'; +import { backfillApprovalDisplayLabels } from './display-label-backfill.js'; +import { backfillActionSlots } from './action-slot-backfill.js'; +import { SysApprovalRequest } from './sys-approval-request.object.js'; +import { SysApprovalAction } from './sys-approval-action.object.js'; +import { SysApprovalApprover } from './sys-approval-approver.object.js'; +import { SysApprovalDelegation } from './sys-approval-delegation.object.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] } as any; +const STAMP = '2026-09-01T09:00:00.000Z'; + +const sysUser = { + name: 'sys_user', + label: 'User', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + email: { name: 'email', label: 'Email', type: 'text' as const }, + }, +}; +const deal = { + name: 'crm_deal', + label: 'Deal', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + }, +}; + +const quiet = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; + +/** A request row exactly as the previous release wrote it, keyed `areq_`. */ +function legacyRequest(id: string, extra: Record) { + return { + id: `areq_${id}`, process_name: 'flow:deal_review', object_name: 'crm_deal', record_id: `D_${id}`, + submitter_id: 'u_sub', current_step: 'manager_review', current_step_index: 0, + flow_run_id: `run_${id}`, flow_node_id: 'manager_review', created_at: STAMP, updated_at: STAMP, + ...extra, + }; +} + +function legacyAction(id: string, requestId: string, extra: Record) { + return { + id, request_id: requestId, step_name: 'manager_review', step_index: 0, created_at: STAMP, ...extra, + }; +} + +/** One plugin boot on `engine`: `start()`, then `kernel:ready`, as the kernel runs it. */ +async function boot(engine: ObjectQL): Promise<{ warn: string[] }> { + const hooks: Record Promise | void>> = {}; + const warn: string[] = []; + const ctx: any = { + hook: (name: string, fn: () => Promise | void) => { (hooks[name] ??= []).push(fn); }, + getService: (name: string) => { + if (name === 'objectql') return engine; + throw new Error(`no service '${name}'`); + }, + registerService: () => {}, + logger: { ...quiet, warn: (msg: string) => { warn.push(String(msg)); } }, + }; + await new ApprovalsServicePlugin({ disableAutoHooks: true }).start(ctx); + for (const fn of hooks['kernel:ready'] ?? []) await fn(); + return { warn }; +} + +describe('#23889 — the boot-time display-label backfill', () => { + let engine: ObjectQL; + + const row = async (object: string, id: string) => + ((await engine.find(object, { where: { id }, limit: 1, context: SYSTEM } satisfies EngineQueryOptions)) as any[])[0]; + + beforeEach(async () => { + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + }), true); + await engine.init(); + for (const def of [sysUser, deal, SysApprovalRequest, SysApprovalAction, SysApprovalApprover, SysApprovalDelegation]) { + engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); + } + await engine.syncSchemas(); + for (const u of [ + { id: 'u_sub', name: 'Sam Submitter', email: 'sam@example.com' }, + { id: 'u_alice', name: 'Alice', email: 'alice@example.com' }, + { id: 'u_mail', name: 'Mia Mail', email: 'mail@example.com' }, + ]) await engine.insert('sys_user', u, { context: SYSTEM } as any); + + // A terminal request whose snapshot carries the authored labels. + await engine.insert('sys_approval_request', legacyRequest('done', { + status: 'approved', pending_approvers: null, completed_at: STAMP, + node_config_json: JSON.stringify({ approvers: [], __flowLabel: 'Deal Review', __nodeLabel: 'Legal sign-off' }), + }), { context: SYSTEM } as any); + // A pending request written before labels were captured at all. + await engine.insert('sys_approval_request', legacyRequest('open', { + status: 'pending', pending_approvers: 'u_alice,position:legal', + node_config_json: JSON.stringify({ approvers: [] }), + }), { context: SYSTEM } as any); + await engine.insert('sys_approval_action', legacyAction('a_submit', 'areq_done', { action: 'submit', actor_id: 'u_sub' }), { context: SYSTEM } as any); + await engine.insert('sys_approval_action', legacyAction('a_vote', 'areq_done', { + action: 'approve', actor_id: 'u_alice', acted_as: 'u_alice', + }), { context: SYSTEM } as any); + await engine.insert('sys_approval_action', legacyAction('a_thread', 'areq_open', { + action: 'comment', actor_id: null, acted_as: 'position:legal', + }), { context: SYSTEM } as any); + // A request id that no longer resolves. + await engine.insert('sys_approval_action', legacyAction('a_orphan', 'areq_gone', { + action: 'comment', step_name: 'final_check', actor_id: null, + }), { context: SYSTEM } as any); + }); + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('(a) one boot fills every display column on terminal and pending requests and on their actions', async () => { + await boot(engine); + + expect(await row('sys_approval_request', 'areq_done')).toMatchObject({ + process_label: 'Deal Review', current_step_label: 'Legal sign-off', pending_approver_labels: null, + }); + expect(await row('sys_approval_request', 'areq_open')).toMatchObject({ + // No snapshot label: the humanized machine name, as the approvals door serves it. + process_label: 'Deal Review', current_step_label: 'Manager Review', + pending_approver_labels: 'Alice, position:legal', + }); + expect(await row('sys_approval_action', 'a_submit')).toMatchObject({ + step_label: 'Legal sign-off', object_name: 'crm_deal', record_id: 'D_done', acted_as_label: null, + }); + expect(await row('sys_approval_action', 'a_vote')).toMatchObject({ + step_label: 'Legal sign-off', object_name: 'crm_deal', record_id: 'D_done', acted_as_label: 'Alice', + }); + expect(await row('sys_approval_action', 'a_thread')).toMatchObject({ + step_label: 'Manager Review', object_name: 'crm_deal', record_id: 'D_open', acted_as_label: 'position:legal', + }); + + // The two doors agree on the same row. + const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); + const served: any = await svc.getRequest('areq_open', SYSTEM); + const stored = await row('sys_approval_request', 'areq_open'); + expect(served.process_label).toBe(stored.process_label); + expect(served.step_label).toBe(stored.current_step_label); + }); + + it('(b) a second boot writes nothing', async () => { + await boot(engine); + const update = vi.spyOn(engine, 'update'); + await boot(engine); + const displayWrites = update.mock.calls.filter(([object]) => + object === 'sys_approval_request' || object === 'sys_approval_action'); + expect(displayWrites).toEqual([]); + update.mockRestore(); + }); + + it('(c) never overwrites a column a live writer filled; an orphaned action keeps a null pointer and its own step name', async () => { + await engine.update('sys_approval_request', { id: 'areq_open', process_label: 'Live label' }, { context: SYSTEM } as any); + const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); + const out = await backfillApprovalDisplayLabels(engine as any, slotLabelSourceOf(svc)); + expect(out.requestsFilled).toBe(2); + + expect((await row('sys_approval_request', 'areq_open')).process_label).toBe('Live label'); + expect(await row('sys_approval_action', 'a_orphan')).toMatchObject({ + step_label: 'Final Check', object_name: null, record_id: null, acted_as_label: null, + }); + expect(await backfillApprovalDisplayLabels(engine as any, slotLabelSourceOf(svc))) + .toEqual({ requestsFilled: 0, actionsFilled: 0 }); + }); + + it('(d) a failed name read stores no label and says so once; the next run fills it', async () => { + const warn: string[] = []; + // The same engine, except that every `sys_user` read fails. + const failing: any = Object.create(engine); + failing.find = (object: string, options: unknown) => + object === 'sys_user' ? Promise.reject(new Error('sys_user unavailable')) : engine.find(object, options as any); + const broken = new ApprovalService({ engine: failing, logger: { ...quiet, warn: (m: string) => { warn.push(m); } } as any }); + + const labels = slotLabelSourceOf(broken); + // A person slot gets null; a non-person slot keeps its address by design. + expect(await labels.slotLabels(['u_alice', 'position:legal'])).toEqual([null, 'position:legal']); + expect(await labels.slateLabels(['u_alice', 'position:legal'])).toBeNull(); + expect(warn.filter((m) => m.includes('could not be read'))).toHaveLength(2); + + await backfillApprovalDisplayLabels(engine as any, labels); + expect((await row('sys_approval_request', 'areq_open')).pending_approver_labels).toBeNull(); + expect((await row('sys_approval_action', 'a_vote')).acted_as_label).toBeNull(); + expect((await row('sys_approval_action', 'a_thread')).acted_as_label).toBe('position:legal'); + + const healthy = new ApprovalService({ engine: engine as any, logger: quiet as any }); + await backfillApprovalDisplayLabels(engine as any, slotLabelSourceOf(healthy)); + expect((await row('sys_approval_request', 'areq_open')).pending_approver_labels).toBe('Alice, position:legal'); + expect((await row('sys_approval_action', 'a_vote')).acted_as_label).toBe('Alice'); + }); + + it('(e) the action-slot backfill labels every acted_as it writes', async () => { + await engine.insert('sys_approval_action', legacyAction('a_slot_email', 'areq_open', { + action: 'comment', actor_id: 'mail@example.com', + }), { context: SYSTEM } as any); + await engine.insert('sys_approval_action', legacyAction('a_slot_vote', 'areq_open', { + action: 'approve', actor_id: 'u_alice', + }), { context: SYSTEM } as any); + const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); + const out = await backfillActionSlots(engine as any, { labels: slotLabelSourceOf(svc) }); + expect(out).toMatchObject({ literalsMoved: 1, votesStamped: 1 }); + + expect(await row('sys_approval_action', 'a_slot_email')).toMatchObject({ + actor_id: null, acted_as: 'mail@example.com', acted_as_label: 'Mia Mail', + }); + expect(await row('sys_approval_action', 'a_slot_vote')).toMatchObject({ + acted_as: 'u_alice', acted_as_label: 'Alice', + }); + }); + + it('(f) an empty authored label is stored as the humanized machine name, never the empty string', async () => { + await engine.insert('crm_deal', { id: 'D_f', title: 'f' }, { context: SYSTEM } as any); + const svc = new ApprovalService({ + engine: engine as any, logger: quiet as any, + automation: { getRun: async (id: string) => ({ id, status: 'paused' }) } as any, + }); + const opened: any = await svc.openNodeRequest({ + object: 'crm_deal', recordId: 'D_f', runId: 'run_f', nodeId: 'legal_review', + flowName: 'deal_review', flowLabel: '', nodeLabel: '', + config: { approvers: [{ type: 'user', value: 'u_alice' }], behavior: 'first_response' } as any, + submitterId: 'u_sub', record: { id: 'D_f', title: 'f' }, + }, { userId: 'u_sub', positions: [], permissions: [] } as any); + expect(await row('sys_approval_request', opened.id)).toMatchObject({ + process_label: 'Deal Review', current_step_label: 'Legal Review', + }); + expect(opened.process_label).toBe('Deal Review'); + expect(opened.step_label).toBe('Legal Review'); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index 1a5aa879cad..f01371aa782 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -87,7 +87,7 @@ export const esESObjects: NonNullable = { }, pending_approver_labels: { label: "En espera de", - help: "A quién espera la solicitud: el nombre de cada aprobador pendiente, o la dirección del puesto cuando no designa a ninguna persona" + help: "A quién espera la solicitud: el nombre de cada aprobador pendiente, o la dirección del turno cuando no designa a ninguna persona" }, payload_json: { label: "Instantánea", @@ -299,7 +299,7 @@ export const esESObjects: NonNullable = { }, acted_as_label: { label: "En nombre de", - help: "El puesto con el que se realizó esta acción, para mostrar: el nombre de la persona que designa, o su dirección cuando no designa a ninguna persona" + help: "El turno en calidad del cual se realizó esta acción, para mostrar: el nombre de la persona que designa, o su dirección cuando no designa a ninguna persona" }, comment: { label: "Comentario" From eebddf3371f3b8ccd24da6ff9049469248db0037 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 16:46:46 +0000 Subject: [PATCH 08/17] =?UTF-8?q?chore(changeset):=20plugin-approvals=20di?= =?UTF-8?q?splay=20labels=20=E2=80=94=20the=20boot-time=20fill=20of=20earl?= =?UTF-8?q?ier=20rows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .changeset/23889-approval-list-display-labels.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.changeset/23889-approval-list-display-labels.md b/.changeset/23889-approval-list-display-labels.md index b5dfb53614f..7dcf297cf2b 100644 --- a/.changeset/23889-approval-list-display-labels.md +++ b/.changeset/23889-approval-list-display-labels.md @@ -25,4 +25,14 @@ The inbox door's `process_label` / `step_label` now read the new columns first. **Read security.** Every new value is either a column of the request the row's readers can already read, or a name the approvals door already serves those readers for that request: `pending_approver_names`, and the timeline's names. The names come from the same resolver, under the same system read. The generic data door serves both objects only to the callers the approvals door would serve. -**Rows written before this release** carry null in the new columns. A pending request gets its waiting-on column at its next slate write. Its actions copy the step label from the request's config snapshot. Older action rows have no record pointer, but their record can still be reached through `request_id`. No backfill runs. +**Rows written before this release are filled at boot.** A new boot-time pass runs on `kernel:ready`, after the action-slot repair. It fills the new columns on rows that hold null, writing only into null columns, so it never overwrites a value a live writer stored. A second boot writes nothing. + +- On every request, the flow and step labels come from the chain the approvals door serves: the authored labels the config snapshot carries, else the humanized machine name. The list and the inbox therefore show the same label for the same row, and no request is left blank. +- On a pending request, whom it waits on is filled. +- On every action, the step label and the record pointer are copied from its request, and its acting-as label is filled. An action whose request no longer exists keeps a null pointer. +- The action-slot repair now labels every `acted_as` it writes. + +**Two details.** + +- An empty authored label (the spec allows `label: ''`) is stored as the humanized machine name, never `''`. +- A name that cannot be read because the `sys_user` read failed is stored as null, never as the raw slot address. The failure is logged once at `warn`, and the boot pass fills it on the next boot. A slot that names no person still shows its address. From 4d9ca5a54c949cbaaf6cc7aa1107999a003a70d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 16:49:16 +0000 Subject: [PATCH 09/17] test(plugin-approvals): classify the display backfill's acted_as presence test as a slot-vs-slot site Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../plugin-approvals/src/approver-address-readers.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts index 6a17ea2913e..4fa4dabecaf 100644 --- a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts @@ -142,6 +142,11 @@ const SLOT_SITES: Readonly> = { role: 'slot-vs-slot', why: 'is the token\'s bound slot still on the slate (ADR-0043 invalidation)', }, + 'display-label-backfill.ts · backfillApprovalDisplayLabels · acted_as: { $null: false }': { + role: 'slot-vs-slot', + why: 'a presence test, not a comparison: selects the action rows that hold a slot to label (#23889 boot ' + + 'backfill); the slot is compared with nothing, and no caller identity is involved', + }, "sys-approval-request.object.ts · (module) · field: 'pending_approvers'": { role: 'declarative', why: 'the `my_pending` list view\'s substring filter on {current_user_id}; served only by the generic data ' From 22b217924be090545d4d871fa85c0b149f6ab9ff Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 16:53:46 +0000 Subject: [PATCH 10/17] test(plugin-approvals): pin the display backfill's null-only writes on an action whose labels differ from its request's Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../src/display-label-backfill.integration.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts index d8ded70235e..9a254973d19 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -189,11 +189,20 @@ describe('#23889 — the boot-time display-label backfill', () => { it('(c) never overwrites a column a live writer filled; an orphaned action keeps a null pointer and its own step name', async () => { await engine.update('sys_approval_request', { id: 'areq_open', process_label: 'Live label' }, { context: SYSTEM } as any); + // An action whose step label and acting-as label a writer already stored, + // different from what its request would give: the fill copies only its + // null pointer columns and leaves both labels as stored. + await engine.update('sys_approval_action', { + id: 'a_vote', step_label: 'Recorded step', acted_as_label: 'Recorded name', + }, { context: SYSTEM } as any); const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); const out = await backfillApprovalDisplayLabels(engine as any, slotLabelSourceOf(svc)); expect(out.requestsFilled).toBe(2); expect((await row('sys_approval_request', 'areq_open')).process_label).toBe('Live label'); + expect(await row('sys_approval_action', 'a_vote')).toMatchObject({ + step_label: 'Recorded step', acted_as_label: 'Recorded name', object_name: 'crm_deal', record_id: 'D_done', + }); expect(await row('sys_approval_action', 'a_orphan')).toMatchObject({ step_label: 'Final Check', object_name: null, record_id: null, acted_as_label: null, }); From 2f4ee5ebe8cccba712a653e4ac70267d8272a0b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 17:08:37 +0000 Subject: [PATCH 11/17] docs(census): tenant-audit census counts the display backfill's two SYSTEM_CTX write sites (227 -> 229) Regenerated with scripts/tenant-audit-census.mjs --write; the hand-written figures the gate holds to the census follow it. Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../docs/permissions/tenant-audit-census.mdx | 32 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 18 ++++++----- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 188515925b6..1777c88cb49 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **53 of the 227 sites are spelled that way**. A +forwarding shim cannot, and **53 of the 229 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **227** | +| 175 write call sites | quoted in the merged changeset | **229** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **156 of 227** decidable, **71** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 123 decidably elevated, 0 decidably not, 96 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **158 of 229** decidable, **71** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 125 decidably elevated, 0 decidably not, 96 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -207,11 +207,11 @@ would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -123 of 227 (54%) as decidably elevated, with 96 more whose elevation is a +125 of 229 (55%) as decidably elevated, with 96 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 227`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 229`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -223,28 +223,28 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **227** | -| …whose object name is statically decidable | 156 | +| write call sites on the application surface | **229** | +| …whose object name is statically decidable | 158 | | …whose object name is chosen at run time | 71 | -| …against an object with tenancy ENABLED | 155 | +| …against an object with tenancy ENABLED | 157 | | …against an object that declares tenancy off | 1 | -| threading a tenant context | 166 | +| threading a tenant context | 168 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 53 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 123 | +| threading a decidably ELEVATED (`isSystem`) context | 125 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 96 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 189 | +| receiver carried a readable engine type | 191 | | receiver erased, placed by the object NAME | 28 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | -| object name spelled inline | 104 | +| object name spelled inline | 106 | | object name spelled through a `const` | 52 | | object name is an `object: string` parameter | 7 | | object name is some other run-time expression | 64 | @@ -297,12 +297,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-16 at `caebc6a7b`. +Measured on 2026-10-16 at `22b217924`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 655 | -| engine-shaped types recognised | 78 | +| tracked non-test sources scanned | 657 | +| engine-shaped types recognised | 79 | | declared objects in the registry | 116 | | same-named calls subtracted as non-engine | 165 | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 986c02c7c76..15309bd214e 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 227 | -| Object name statically decidable | 156 | +| Write call sites | 229 | +| Object name statically decidable | 158 | | Object name chosen at run time | 71 | -| Against a tenancy-enabled object | 155 | +| Against a tenancy-enabled object | 157 | | Against an object declaring tenancy off | 1 | -| Threading a tenant context | 166 | +| Threading a tenant context | 168 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 53 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 123 | +| Threading a decidably elevated context | 125 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 96 | @@ -90,12 +90,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-16 at `caebc6a7b`. +Measured on 2026-10-16 at `22b217924`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 655 | -| engine-shaped types recognised | 78 | +| tracked non-test sources scanned | 657 | +| engine-shaped types recognised | 79 | | declared objects in the registry | 116 | | same-named calls subtracted as non-engine | 165 | @@ -115,6 +115,8 @@ Measured on 2026-10-16 at `caebc6a7b`. | `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_token` | enabled | elevated | 1 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_token` | enabled | elevated | 1 | | `packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` | `update` | `objectPlan.object` | undecidable | context, elevation undecidable | 1 | +| `packages/plugins/plugin-approvals/src/display-label-backfill.ts` | `update` | `sys_approval_action` | enabled | elevated | 1 | +| `packages/plugins/plugin-approvals/src/display-label-backfill.ts` | `update` | `sys_approval_request` | enabled | elevated | 1 | | `packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts` | `update` | `sys_activity` | enabled | elevated | 1 | | `packages/plugins/plugin-audit/src/auth-event-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 | | `packages/plugins/plugin-audit/src/comment-parent-pair-backfill.ts` | `update` | `sys_comment` | enabled | elevated | 1 | From b74901ee356d28612c75dacce9a45d3b6e0e276e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 17:33:52 +0000 Subject: [PATCH 12/17] test(plugin-approvals): type the failing-engine double's find options instead of erasing them Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../src/display-label-backfill.integration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts index 9a254973d19..f19c37869df 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -214,8 +214,8 @@ describe('#23889 — the boot-time display-label backfill', () => { const warn: string[] = []; // The same engine, except that every `sys_user` read fails. const failing: any = Object.create(engine); - failing.find = (object: string, options: unknown) => - object === 'sys_user' ? Promise.reject(new Error('sys_user unavailable')) : engine.find(object, options as any); + failing.find = (object: string, options: EngineQueryOptions) => + object === 'sys_user' ? Promise.reject(new Error('sys_user unavailable')) : engine.find(object, options); const broken = new ApprovalService({ engine: failing, logger: { ...quiet, warn: (m: string) => { warn.push(m); } } as any }); const labels = slotLabelSourceOf(broken); From 28c5d1e26a45a36533f5620fddc7d709a707c134 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 18:17:50 +0000 Subject: [PATCH 13/17] fix(plugin-approvals): the boot backfills keep each row's updated_at; name reads batched per page Both walks of the display-label backfill and both write sites of the action-slot repair read updated_at with the page and write under preserveAudit passing it back (the comment-parent-pair backfill's shape), so a filled request keeps its place in the lists that sort on it. The labeller is called once per page. Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../23889-approval-list-display-labels.md | 4 +- .../src/action-slot-backfill.ts | 52 +++++++++++++--- .../plugin-approvals/src/approval-service.ts | 6 +- ...display-label-backfill.integration.test.ts | 46 +++++++++++++- .../src/display-label-backfill.ts | 62 +++++++++++++++---- .../plugin-approvals/src/display-labels.ts | 11 ++++ 6 files changed, 151 insertions(+), 30 deletions(-) diff --git a/.changeset/23889-approval-list-display-labels.md b/.changeset/23889-approval-list-display-labels.md index 7dcf297cf2b..e3934fd3313 100644 --- a/.changeset/23889-approval-list-display-labels.md +++ b/.changeset/23889-approval-list-display-labels.md @@ -18,14 +18,14 @@ The Requests and Action History lists are generic list views. They show stored c - `step_label`: the request's step label. - `object_name` + `record_id`: the request's record pointer (`record_id` declares `referenceVia: 'object_name'`), so a list can name the record. -- `acted_as_label`: `acted_as` labelled the same way as the waiting-on column. It is null exactly when `acted_as` is null. +- `acted_as_label`: `acted_as` labelled the same way as the waiting-on column. It is null when `acted_as` is null, and also while a person slot's name could not be read (see below). - The list views: `step_label` replaces `step_name` in `recent`, `all_actions` and `by_actor`. `recent` and `all_actions` add the record pointer and show `acted_as_label` in place of `acted_as`. The inbox door's `process_label` / `step_label` now read the new columns first. They fall back to the config snapshot for rows opened earlier, so what they serve is unchanged. **Read security.** Every new value is either a column of the request the row's readers can already read, or a name the approvals door already serves those readers for that request: `pending_approver_names`, and the timeline's names. The names come from the same resolver, under the same system read. The generic data door serves both objects only to the callers the approvals door would serve. -**Rows written before this release are filled at boot.** A new boot-time pass runs on `kernel:ready`, after the action-slot repair. It fills the new columns on rows that hold null, writing only into null columns, so it never overwrites a value a live writer stored. A second boot writes nothing. +**Rows written before this release are filled at boot.** A new boot-time pass runs on `kernel:ready`, after the action-slot repair. It fills the new columns on rows that hold null, writing only into null columns, so it never overwrites a value a live writer stored. Every row it fills keeps its own `updated_at`: the write passes the stored value back under `preserveAudit`, so the request lists, which sort on `updated_at`, keep their order. The action-slot repair's writes keep it the same way. Where `plugin-audit` is composed, the first boot after the upgrade writes one system audit entry per row it fills. A second boot writes nothing. - On every request, the flow and step labels come from the chain the approvals door serves: the authored labels the config snapshot carries, else the humanized machine name. The list and the inbox therefore show the same label for the same row, and no request is left blank. - On a pending request, whom it waits on is filled. diff --git a/packages/plugins/plugin-approvals/src/action-slot-backfill.ts b/packages/plugins/plugin-approvals/src/action-slot-backfill.ts index 88452fc15ab..b5adcbcdd7a 100644 --- a/packages/plugins/plugin-approvals/src/action-slot-backfill.ts +++ b/packages/plugins/plugin-approvals/src/action-slot-backfill.ts @@ -100,8 +100,20 @@ export interface ActionSlotBackfillResult { } const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; +/** + * [#23889] The repair's writes keep each row's own `updated_at`: moving a slot + * between columns, or labelling it, is not an edit of the decision. + * `preserveAudit` makes the engine's update stamp and the SQL driver keep the + * value the write passes back (the read row's); a row with none passes none. + */ +const PRESERVING_CONTEXT = { ...SYSTEM_CONTEXT, preserveAudit: true }; const PAGE = 500; +/** The read row's own `updated_at`, to hand back on the write. */ +function keptStamp(row: Record): Record { + return row.updated_at !== undefined && row.updated_at !== null ? { updated_at: row.updated_at } : {}; +} + function nonEmpty(value: unknown): string | null { return typeof value === 'string' && value.length > 0 ? value : null; } @@ -129,10 +141,18 @@ export async function backfillActionSlots( const pageSize = options.pageSize ?? PAGE; // [#23889] Every `acted_as` this repair writes gets its display label in the // same write, so `acted_as_label` is null only where `acted_as` is (or where - // the name could not be read — the display-label backfill retries that). - const labelOf = async (slot: string): Promise> => { - if (!options.labels) return {}; - const [label] = await options.labels.slotLabels([slot]); + // the name could not be read — the display-label backfill retries that). The + // labels of a page are read in ONE labeller call, never one per row. + const pageLabels = async (slots: string[]): Promise> => { + const out = new Map(); + const distinct = [...new Set(slots)]; + if (!options.labels || !distinct.length) return out; + const answered = await options.labels.slotLabels(distinct); + distinct.forEach((slot, i) => out.set(slot, answered[i] ?? null)); + return out; + }; + const labelOf = (named: Map, slot: string): Record => { + const label = named.get(slot); return label ? { acted_as_label: label } : {}; }; let literalsMoved = 0; @@ -146,7 +166,7 @@ export async function backfillActionSlots( const nonIds = keysetWalk>( (q) => engine.find('sys_approval_action', { ...q, - fields: ['id', 'actor_id', 'acted_as'], + fields: ['id', 'actor_id', 'acted_as', 'updated_at'], context: SYSTEM_CONTEXT, }) as Promise[]>, { @@ -155,6 +175,10 @@ export async function backfillActionSlots( }, ); for await (const rows of nonIds.pages()) { + const named = await pageLabels(rows + .filter((row) => !nonEmpty(row.acted_as)) + .map((row) => nonEmpty(row.actor_id)) + .filter((v): v is string => v !== null && isSlotLiteral(v))); for (const row of rows) { const id = nonEmpty(row.id); const actorId = nonEmpty(row.actor_id); @@ -163,9 +187,9 @@ export async function backfillActionSlots( if (!sentinel && !isSlotLiteral(actorId)) continue; // Either way the column ends up empty. A slot also moves to `acted_as`; // a sentinel does not — the sweep took no slot. - const patch: Record = { id, actor_id: null }; - if (!sentinel && !nonEmpty(row.acted_as)) Object.assign(patch, { acted_as: actorId }, await labelOf(actorId)); - await engine.update('sys_approval_action', patch, { context: SYSTEM_CONTEXT }); + const patch: Record = { id, actor_id: null, ...keptStamp(row) }; + if (!sentinel && !nonEmpty(row.acted_as)) Object.assign(patch, { acted_as: actorId }, labelOf(named, actorId)); + await engine.update('sys_approval_action', patch, { context: PRESERVING_CONTEXT }); if (sentinel) sentinelsCleared++; else literalsMoved++; } @@ -186,12 +210,16 @@ export async function backfillActionSlots( const votes = keysetWalk>( (q) => engine.find('sys_approval_action', { ...q, - fields: ['id', 'actor_id', 'acted_as'], + fields: ['id', 'actor_id', 'acted_as', 'updated_at'], context: SYSTEM_CONTEXT, }) as Promise[]>, { where: { request_id: { $in: ids }, action: 'approve', step_index: 0 }, pageSize }, ); for await (const rows of votes.pages()) { + const named = await pageLabels(rows + .filter((row) => !nonEmpty(row.acted_as)) + .map((row) => nonEmpty(row.actor_id)) + .filter((v): v is string => v !== null && !isMachineSentinel(v))); for (const row of rows) { const id = nonEmpty(row.id); const actorId = nonEmpty(row.actor_id); @@ -199,7 +227,11 @@ export async function backfillActionSlots( // Pass 1 has already emptied every sentinel; a machine took no slot, // so one is never stamped as a vote even if a write raced the scan. if (isMachineSentinel(actorId)) continue; - await engine.update('sys_approval_action', { id, acted_as: actorId, ...(await labelOf(actorId)) }, { context: SYSTEM_CONTEXT }); + await engine.update( + 'sys_approval_action', + { id, acted_as: actorId, ...labelOf(named, actorId), ...keptStamp(row) }, + { context: PRESERVING_CONTEXT }, + ); votesStamped++; } } diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 6e6cd7996c1..f08a7e74dbf 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -90,7 +90,7 @@ import { slateNamesAPerson, type ActingCaller, } from './approver-address.js'; -import { labelOrMachineName, requestDisplayLabels } from './display-labels.js'; +import { labelOrMachineName, requestDisplayLabels, slateLabelOf } from './display-labels.js'; import type { SlotLabelSource } from './display-label-backfill.js'; import { redactSnapshot, @@ -1529,7 +1529,6 @@ export class ApprovalService implements IApprovalService { // reads it — `findChildRequestId`, shared with that method. SLOT_LABELS.set(this, { slotLabels: (slots) => this.slotLabels(slots), - slateLabels: (slate) => this.slateLabels(slate), }); REQUEST_VISIBILITY.set(this, { visibleRequestIdsFor: async (context, target) => { @@ -6743,8 +6742,7 @@ export class ApprovalService implements IApprovalService { */ private async slateLabels(slate: readonly string[]): Promise { if (!slate.length) return null; - const labels = await this.slotLabels(slate); - return labels.some((label) => label === null) ? null : labels.join(', '); + return slateLabelOf(await this.slotLabels(slate)); } /** diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts index f19c37869df..9983c33b3e3 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -26,7 +26,9 @@ * address by design; * (e) the action-slot backfill labels every `acted_as` it writes; * (f) an empty authored label is stored as the humanized machine name, never - * `''`, at open. + * `''`, at open; + * (g) every row a boot fills keeps its own `updated_at` exactly — the request + * lists sort on it, and a derived display column is not an edit. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; @@ -37,6 +39,7 @@ import { ApprovalService, slotLabelSourceOf } from './approval-service.js'; import { ApprovalsServicePlugin } from './approvals-plugin.js'; import { backfillApprovalDisplayLabels } from './display-label-backfill.js'; import { backfillActionSlots } from './action-slot-backfill.js'; +import { slateLabelOf } from './display-labels.js'; import { SysApprovalRequest } from './sys-approval-request.object.js'; import { SysApprovalAction } from './sys-approval-action.object.js'; import { SysApprovalApprover } from './sys-approval-approver.object.js'; @@ -65,6 +68,9 @@ const deal = { const quiet = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; +/** A fixed past instant the rows are back-dated to after their insert (an insert stamps now). */ +const BACKDATED = '2026-03-04T05:06:07.000Z'; + /** A request row exactly as the previous release wrote it, keyed `areq_`. */ function legacyRequest(id: string, extra: Record) { return { @@ -102,6 +108,10 @@ async function boot(engine: ObjectQL): Promise<{ warn: string[] }> { describe('#23889 — the boot-time display-label backfill', () => { let engine: ObjectQL; + /** Set a row's `updated_at` to {@link BACKDATED} — a preserving write, as an import would. */ + const backdate = async (object: string, id: string) => + engine.update(object, { id, updated_at: BACKDATED }, { context: { ...SYSTEM, preserveAudit: true } } as any); + const row = async (object: string, id: string) => ((await engine.find(object, { where: { id }, limit: 1, context: SYSTEM } satisfies EngineQueryOptions)) as any[])[0]; @@ -221,7 +231,7 @@ describe('#23889 — the boot-time display-label backfill', () => { const labels = slotLabelSourceOf(broken); // A person slot gets null; a non-person slot keeps its address by design. expect(await labels.slotLabels(['u_alice', 'position:legal'])).toEqual([null, 'position:legal']); - expect(await labels.slateLabels(['u_alice', 'position:legal'])).toBeNull(); + expect(slateLabelOf(await labels.slotLabels(['u_alice', 'position:legal']))).toBeNull(); expect(warn.filter((m) => m.includes('could not be read'))).toHaveLength(2); await backfillApprovalDisplayLabels(engine as any, labels); @@ -254,6 +264,21 @@ describe('#23889 — the boot-time display-label backfill', () => { }); }); + it('(e2) the action-slot backfill keeps each row\'s updated_at', async () => { + await engine.insert('sys_approval_action', legacyAction('a_slot_kept', 'areq_open', { + action: 'comment', actor_id: 'mail@example.com', + }), { context: SYSTEM } as any); + await engine.insert('sys_approval_action', legacyAction('a_vote_kept', 'areq_open', { + action: 'approve', actor_id: 'u_alice', + }), { context: SYSTEM } as any); + await backdate('sys_approval_action', 'a_slot_kept'); + await backdate('sys_approval_action', 'a_vote_kept'); + const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); + await backfillActionSlots(engine as any, { labels: slotLabelSourceOf(svc) }); + expect(await row('sys_approval_action', 'a_slot_kept')).toMatchObject({ acted_as: 'mail@example.com', updated_at: BACKDATED }); + expect(await row('sys_approval_action', 'a_vote_kept')).toMatchObject({ acted_as: 'u_alice', updated_at: BACKDATED }); + }); + it('(f) an empty authored label is stored as the humanized machine name, never the empty string', async () => { await engine.insert('crm_deal', { id: 'D_f', title: 'f' }, { context: SYSTEM } as any); const svc = new ApprovalService({ @@ -272,4 +297,21 @@ describe('#23889 — the boot-time display-label backfill', () => { expect(opened.process_label).toBe('Deal Review'); expect(opened.step_label).toBe('Legal Review'); }); + + it('(g) one boot keeps the updated_at of every row it fills', async () => { + for (const id of ['areq_done', 'areq_open']) await backdate('sys_approval_request', id); + for (const id of ['a_submit', 'a_vote', 'a_thread']) await backdate('sys_approval_action', id); + expect((await row('sys_approval_request', 'areq_open')).updated_at).toBe(BACKDATED); + + await boot(engine); + + // Filled — and still dated where they were. + expect(await row('sys_approval_request', 'areq_done')).toMatchObject({ process_label: 'Deal Review', updated_at: BACKDATED }); + expect(await row('sys_approval_request', 'areq_open')).toMatchObject({ + pending_approver_labels: 'Alice, position:legal', updated_at: BACKDATED, + }); + for (const id of ['a_submit', 'a_vote', 'a_thread']) { + expect(await row('sys_approval_action', id), id).toMatchObject({ object_name: 'crm_deal', updated_at: BACKDATED }); + } + }); }); diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.ts index c86f3e4df07..2b83925bee5 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.ts @@ -34,6 +34,13 @@ * label is its own humanized `step_name`; * - action `acted_as_label` — `slotLabels(acted_as)`. * + * ## Each row keeps its `updated_at` + * + * Every write passes the read row's `updated_at` back under `preserveAudit` + * ({@link PRESERVING_CONTEXT}), so a filled request keeps its place in the + * lists that sort on it. Where `plugin-audit` is composed, each filled row still + * gets one system audit entry for the write. + * * ## Idempotent, null-only, and run at every boot * * Each write names only columns that were null on the row it read, so the @@ -49,7 +56,7 @@ */ import { keysetWalk } from '@objectstack/types'; -import { prettifyMachineName, requestDisplayLabels } from './display-labels.js'; +import { prettifyMachineName, requestDisplayLabels, slateLabelOf } from './display-labels.js'; /** * The engine surface the fill needs — a structural subset of @@ -63,14 +70,13 @@ export interface DisplayLabelBackfillEngine { /** * The approval service's own slot labeller, as the live writers call it - * (`slotLabelSourceOf` in `approval-service.ts`): `slotLabels` answers one - * label per slot, null for a person slot whose name could not be read; - * `slateLabels` answers the joined column, null when any slot's name could not - * be read or the slate is empty. + * (`slotLabelSourceOf` in `approval-service.ts`): one label per slot, null for + * a person slot whose name could not be read. It takes the whole page's slots + * in one call, so a page costs one `sys_user` read (two when an email slot + * does not resolve by id), never one per row. */ export interface SlotLabelSource { slotLabels(slots: readonly string[]): Promise>; - slateLabels(slate: readonly string[]): Promise; } /** What one run wrote. */ @@ -82,8 +88,32 @@ export interface DisplayLabelBackfillResult { } const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; +/** + * Writes keep each row's own `updated_at`: filling a derived display column is + * not an edit of the request or the action, and the request lists sort on + * `updated_at`. `preserveAudit` makes the engine's update stamp and the SQL + * driver keep the value the write passes back (the read row's), the same way + * `plugin-audit`'s comment-parent-pair backfill does. A row with no stored + * `updated_at` passes none, and the write stamps as usual. + */ +const PRESERVING_CONTEXT = { ...SYSTEM_CONTEXT, preserveAudit: true }; const PAGE = 500; +/** The read row's own `updated_at`, to hand back on the write. */ +function keptStamp(row: Record): Record { + return row.updated_at !== undefined && row.updated_at !== null ? { updated_at: row.updated_at } : {}; +} + +/** Label every distinct slot on the page in one labeller call. */ +async function pageLabels(labels: SlotLabelSource, slots: Iterable): Promise> { + const distinct = [...new Set(slots)]; + const out = new Map(); + if (!distinct.length) return out; + const answered = await labels.slotLabels(distinct); + distinct.forEach((slot, i) => out.set(slot, answered[i] ?? null)); + return out; +} + function nonEmpty(value: unknown): string | null { return typeof value === 'string' && value.length > 0 ? value : null; } @@ -112,7 +142,7 @@ export async function backfillApprovalDisplayLabels( ...q, fields: [ 'id', 'status', 'process_name', 'current_step', 'node_config_json', 'pending_approvers', - 'process_label', 'current_step_label', 'pending_approver_labels', + 'process_label', 'current_step_label', 'pending_approver_labels', 'updated_at', ], context: SYSTEM_CONTEXT, }) as Promise[]>, @@ -128,6 +158,10 @@ export async function backfillApprovalDisplayLabels( }, ); for await (const rows of requests.pages()) { + const waitingOnSlots = rows + .filter((row) => row.status === 'pending' && !nonEmpty(row.pending_approver_labels)) + .flatMap((row) => slateOf(row.pending_approvers)); + const named = await pageLabels(labels, waitingOnSlots); for (const row of rows) { const id = nonEmpty(row.id); if (!id) continue; @@ -139,11 +173,11 @@ export async function backfillApprovalDisplayLabels( } if (row.status === 'pending' && !nonEmpty(row.pending_approver_labels)) { const slate = slateOf(row.pending_approvers); - const waitingOn = slate.length ? await labels.slateLabels(slate) : null; + const waitingOn = slate.length ? slateLabelOf(slate.map((slot) => named.get(slot) ?? null)) : null; if (waitingOn) patch.pending_approver_labels = waitingOn; } if (!Object.keys(patch).length) continue; - await engine.update('sys_approval_request', { id, ...patch }, { context: SYSTEM_CONTEXT }); + await engine.update('sys_approval_request', { id, ...patch, ...keptStamp(row) }, { context: PRESERVING_CONTEXT }); requestsFilled++; } } @@ -153,7 +187,7 @@ export async function backfillApprovalDisplayLabels( const actions = keysetWalk>( (q) => engine.find('sys_approval_action', { ...q, - fields: ['id', 'request_id', 'step_name', 'acted_as', 'step_label', 'object_name', 'record_id', 'acted_as_label'], + fields: ['id', 'request_id', 'step_name', 'acted_as', 'step_label', 'object_name', 'record_id', 'acted_as_label', 'updated_at'], context: SYSTEM_CONTEXT, }) as Promise[]>, { @@ -183,6 +217,10 @@ export async function backfillApprovalDisplayLabels( if (pid) parents.set(pid, p); } } + const named = await pageLabels( + labels, + rows.filter((row) => !nonEmpty(row.acted_as_label)).map((row) => nonEmpty(row.acted_as)).filter((v): v is string => v !== null), + ); for (const row of rows) { const id = nonEmpty(row.id); if (!id) continue; @@ -196,11 +234,11 @@ export async function backfillApprovalDisplayLabels( if (parent && !nonEmpty(row.record_id) && nonEmpty(parent.record_id)) patch.record_id = parent.record_id; const actedAs = nonEmpty(row.acted_as); if (actedAs && !nonEmpty(row.acted_as_label)) { - const [actedAsLabel] = await labels.slotLabels([actedAs]); + const actedAsLabel = named.get(actedAs) ?? null; if (actedAsLabel) patch.acted_as_label = actedAsLabel; } if (!Object.keys(patch).length) continue; - await engine.update('sys_approval_action', { id, ...patch }, { context: SYSTEM_CONTEXT }); + await engine.update('sys_approval_action', { id, ...patch, ...keptStamp(row) }, { context: PRESERVING_CONTEXT }); actionsFilled++; } } diff --git a/packages/plugins/plugin-approvals/src/display-labels.ts b/packages/plugins/plugin-approvals/src/display-labels.ts index 302afdf75d3..9691d4e5885 100644 --- a/packages/plugins/plugin-approvals/src/display-labels.ts +++ b/packages/plugins/plugin-approvals/src/display-labels.ts @@ -71,3 +71,14 @@ export function requestDisplayLabels(row: Record): { current_step_label: label(row.current_step_label) ?? labelOrMachineName(cfg?.__nodeLabel, row.current_step), }; } + +/** + * `sys_approval_request.pending_approver_labels` from the slate's slot labels, + * in slate order: joined with ", ", or null when the slate is empty or any + * slot's label is null (its name could not be read). The live writers and the + * boot backfill both join through here. + */ +export function slateLabelOf(labels: ReadonlyArray): string | null { + if (!labels.length || labels.some((label) => label === null)) return null; + return labels.join(', '); +} From 109541cfb913cd0dfb08e6875f54ddfa3a02121c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 18:18:38 +0000 Subject: [PATCH 14/17] test(plugin-approvals): pin one labeller call per backfill page Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../src/display-label-backfill.integration.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts index 9983c33b3e3..c408ff0730b 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -206,7 +206,13 @@ describe('#23889 — the boot-time display-label backfill', () => { id: 'a_vote', step_label: 'Recorded step', acted_as_label: 'Recorded name', }, { context: SYSTEM } as any); const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); - const out = await backfillApprovalDisplayLabels(engine as any, slotLabelSourceOf(svc)); + // The labeller is asked once per page, never once per row: here one request + // page (the pending slate) and one action page (two acted_as slots). + const source = slotLabelSourceOf(svc); + const asked: string[][] = []; + const counting = { slotLabels: (slots: readonly string[]) => { asked.push([...slots]); return source.slotLabels(slots); } }; + const out = await backfillApprovalDisplayLabels(engine as any, counting); + expect(asked).toEqual([['u_alice', 'position:legal'], ['position:legal']]); expect(out.requestsFilled).toBe(2); expect((await row('sys_approval_request', 'areq_open')).process_label).toBe('Live label'); From f8efc941a1882bac4355954f6c9f5eab57e6d4ae Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 18:27:18 +0000 Subject: [PATCH 15/17] test(plugin-approvals): re-classify the action-slot repair's actor_id reads (the per-page label batch reads it once per pass) Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../plugin-approvals/src/approver-address-readers.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts index 4fa4dabecaf..cee7434e772 100644 --- a/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts +++ b/packages/plugins/plugin-approvals/src/approver-address-readers.test.ts @@ -198,9 +198,10 @@ const PERSON_SITES: Readonly Date: Fri, 16 Oct 2026 19:27:27 +0000 Subject: [PATCH 16/17] fix(plugin-approvals): the display backfill re-reads each request before writing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A live writer (the boot SLA / dead-run sweep this plugin starts without awaiting, or another node) can write a pending request between the fill's page read and its row write. The request pass now re-reads the row by id, patches only columns still null there, writes the waiting-on only while the row is still pending on the slate the names were read for, and passes back the re-read updated_at — the one-engine-call window the comment-parent-pair backfill accepts. No conditional where. Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../23889-approval-list-display-labels.md | 2 +- ...display-label-backfill.integration.test.ts | 72 +++++++++++++++++-- .../src/display-label-backfill.ts | 72 +++++++++++++------ 3 files changed, 120 insertions(+), 26 deletions(-) diff --git a/.changeset/23889-approval-list-display-labels.md b/.changeset/23889-approval-list-display-labels.md index e3934fd3313..6238439fcb3 100644 --- a/.changeset/23889-approval-list-display-labels.md +++ b/.changeset/23889-approval-list-display-labels.md @@ -25,7 +25,7 @@ The inbox door's `process_label` / `step_label` now read the new columns first. **Read security.** Every new value is either a column of the request the row's readers can already read, or a name the approvals door already serves those readers for that request: `pending_approver_names`, and the timeline's names. The names come from the same resolver, under the same system read. The generic data door serves both objects only to the callers the approvals door would serve. -**Rows written before this release are filled at boot.** A new boot-time pass runs on `kernel:ready`, after the action-slot repair. It fills the new columns on rows that hold null, writing only into null columns, so it never overwrites a value a live writer stored. Every row it fills keeps its own `updated_at`: the write passes the stored value back under `preserveAudit`, so the request lists, which sort on `updated_at`, keep their order. The action-slot repair's writes keep it the same way. Where `plugin-audit` is composed, the first boot after the upgrade writes one system audit entry per row it fills. A second boot writes nothing. +**Rows written before this release are filled at boot.** A new boot-time pass runs on `kernel:ready`, after the action-slot repair. It fills the new columns on rows that hold null. Each request is re-read just before it is written, and the write names only the columns that are still null on that re-read row. Whom a request waits on is written only while the request is still pending on the slate its names were read for. A value a live writer stored, such as the boot sweep or another node, is therefore kept. The only window left is the single engine call between the re-read and the write. Every row it fills keeps its own `updated_at`: the write passes the stored value back under `preserveAudit`, so the request lists, which sort on `updated_at`, keep their order. The action-slot repair's writes keep it the same way. Where `plugin-audit` is composed, the first boot after the upgrade writes one system entry in the audit log and one in the record's activity timeline for each row it fills. A second boot writes nothing. - On every request, the flow and step labels come from the chain the approvals door serves: the authored labels the config snapshot carries, else the humanized machine name. The list and the inbox therefore show the same label for the same row, and no request is left blank. - On a pending request, whom it waits on is filled. diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts index c408ff0730b..7c54b06ffe8 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -28,7 +28,10 @@ * (f) an empty authored label is stored as the humanized machine name, never * `''`, at open; * (g) every row a boot fills keeps its own `updated_at` exactly — the request - * lists sort on it, and a derived display column is not an edit. + * lists sort on it, and a derived display column is not an edit; + * (h) a live write landing between a page's read and its row writes — a + * recall, an SLA-style slate change — is kept exactly as it left the row, + * and the rest of the page is still filled with its stamps kept. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; @@ -199,6 +202,11 @@ describe('#23889 — the boot-time display-label backfill', () => { it('(c) never overwrites a column a live writer filled; an orphaned action keeps a null pointer and its own step name', async () => { await engine.update('sys_approval_request', { id: 'areq_open', process_label: 'Live label' }, { context: SYSTEM } as any); + // A second pending request on the same page, so one labeller call per page + // and one per row give different calls. + await engine.insert('sys_approval_request', legacyRequest('open2', { + status: 'pending', pending_approvers: 'u_mail', node_config_json: JSON.stringify({ approvers: [] }), + }), { context: SYSTEM } as any); // An action whose step label and acting-as label a writer already stored, // different from what its request would give: the fill copies only its // null pointer columns and leaves both labels as stored. @@ -206,14 +214,15 @@ describe('#23889 — the boot-time display-label backfill', () => { id: 'a_vote', step_label: 'Recorded step', acted_as_label: 'Recorded name', }, { context: SYSTEM } as any); const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); - // The labeller is asked once per page, never once per row: here one request - // page (the pending slate) and one action page (two acted_as slots). + // The labeller is asked once per page, never once per row: one request page + // (two pending slates) and one action page. const source = slotLabelSourceOf(svc); const asked: string[][] = []; const counting = { slotLabels: (slots: readonly string[]) => { asked.push([...slots]); return source.slotLabels(slots); } }; const out = await backfillApprovalDisplayLabels(engine as any, counting); - expect(asked).toEqual([['u_alice', 'position:legal'], ['position:legal']]); - expect(out.requestsFilled).toBe(2); + expect(asked).toEqual([['u_alice', 'position:legal', 'u_mail'], ['position:legal']]); + expect(out.requestsFilled).toBe(3); + expect((await row('sys_approval_request', 'areq_open2')).pending_approver_labels).toBe('Mia Mail'); expect((await row('sys_approval_request', 'areq_open')).process_label).toBe('Live label'); expect(await row('sys_approval_action', 'a_vote')).toMatchObject({ @@ -320,4 +329,57 @@ describe('#23889 — the boot-time display-label backfill', () => { expect(await row('sys_approval_action', id), id).toMatchObject({ object_name: 'crm_deal', updated_at: BACKDATED }); } }); + + it('(h) a live write between the page read and the row writes is kept as it left the row', async () => { + // Two more pending requests on the page: one a sweep recalls, one it moves to another slate. + await engine.insert('sys_approval_request', legacyRequest('recalled', { + status: 'pending', pending_approvers: 'u_alice', node_config_json: JSON.stringify({ approvers: [] }), + }), { context: SYSTEM } as any); + await engine.insert('sys_approval_request', legacyRequest('moved', { + status: 'pending', pending_approvers: 'u_alice', node_config_json: JSON.stringify({ approvers: [] }), + }), { context: SYSTEM } as any); + for (const id of ['areq_done', 'areq_open', 'areq_recalled', 'areq_moved']) await backdate('sys_approval_request', id); + + const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); + const source = slotLabelSourceOf(svc); + let left: Record | undefined; + // The labeller is asked once the request page has been read and before any + // of its rows is written — the window. The live writes land there, as the + // sweep's do: plain system writes that stamp now. + const racing = { + slotLabels: async (slots: readonly string[]) => { + if (!left) { + await engine.update('sys_approval_request', { + id: 'areq_recalled', status: 'recalled', pending_approvers: null, pending_approver_labels: null, + completed_at: '2026-10-01T00:00:00.000Z', + }, { context: SYSTEM } as any); + await engine.update('sys_approval_request', { + id: 'areq_moved', pending_approvers: 'u_mail', pending_approver_labels: 'Mia Mail', + }, { context: SYSTEM } as any); + left = { + recalled: await row('sys_approval_request', 'areq_recalled'), + moved: await row('sys_approval_request', 'areq_moved'), + }; + } + return source.slotLabels(slots); + }, + }; + await backfillApprovalDisplayLabels(engine as any, racing); + + const pick = (r: any) => ({ + status: r.status, pending_approvers: r.pending_approvers, + pending_approver_labels: r.pending_approver_labels, updated_at: r.updated_at, + }); + expect(left!.recalled.updated_at).not.toBe(BACKDATED); + expect(pick(await row('sys_approval_request', 'areq_recalled'))).toEqual(pick(left!.recalled)); + expect(pick(await row('sys_approval_request', 'areq_moved'))).toEqual(pick(left!.moved)); + // Their null label columns are still filled, under the stamp the live write left. + expect((await row('sys_approval_request', 'areq_recalled')).process_label).toBe('Deal Review'); + + // The untouched rows on the page are filled, their stamps kept. + expect(await row('sys_approval_request', 'areq_open')).toMatchObject({ + pending_approver_labels: 'Alice, position:legal', updated_at: BACKDATED, + }); + expect(await row('sys_approval_request', 'areq_done')).toMatchObject({ process_label: 'Deal Review', updated_at: BACKDATED }); + }); }); diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.ts index 2b83925bee5..5a937a73ecb 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.ts @@ -36,17 +36,25 @@ * * ## Each row keeps its `updated_at` * - * Every write passes the read row's `updated_at` back under `preserveAudit` - * ({@link PRESERVING_CONTEXT}), so a filled request keeps its place in the - * lists that sort on it. Where `plugin-audit` is composed, each filled row still - * gets one system audit entry for the write. + * Every write passes the row's `updated_at` back under `preserveAudit` + * ({@link PRESERVING_CONTEXT}) — on a request, the value the row holds when it + * is re-read just before the write — so a filled request keeps its place in + * the lists that sort on it. A row with no stored `updated_at` takes the write's + * instant: no update can keep it null. Where `plugin-audit` is composed, the + * write still records one system entry in the audit log and one in the record's + * activity timeline for each row it fills. * * ## Idempotent, null-only, and run at every boot * - * Each write names only columns that were null on the row it read, so the - * pass can never overwrite what a live writer wrote, and a second run over an - * unchanged database writes nothing (`display-label-backfill.integration.test.ts` - * asserts both). A label the labeller could not produce — a failed `sys_user` + * Each write names only columns that are null on the row as it stands just + * before the write: a request is re-read by id first, and its waiting-on is + * written only while it is still pending on the slate the names were read for. + * So a value a live writer stored before that re-read is never overwritten; + * what remains is the one engine call between the re-read and the write, the + * window `plugin-audit`'s comment-parent-pair backfill accepts. No live writer + * updates an action row, so the action pass writes from its page read. A + * second run over an unchanged database writes nothing + * (`display-label-backfill.integration.test.ts` asserts both). A label the labeller could not produce — a failed `sys_user` * read, which `slotLabels` reports as null rather than freezing a raw address * — stays null and is retried on the next boot. Both walks are seek walks over * the predicate "a display column is null", so a row this pass fills leaves the @@ -99,6 +107,12 @@ const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; const PRESERVING_CONTEXT = { ...SYSTEM_CONTEXT, preserveAudit: true }; const PAGE = 500; +/** What the request pass reads — on the page, and again on the row just before its write. */ +const REQUEST_FIELDS = [ + 'id', 'status', 'process_name', 'current_step', 'node_config_json', 'pending_approvers', + 'process_label', 'current_step_label', 'pending_approver_labels', 'updated_at', +]; + /** The read row's own `updated_at`, to hand back on the write. */ function keptStamp(row: Record): Record { return row.updated_at !== undefined && row.updated_at !== null ? { updated_at: row.updated_at } : {}; @@ -140,10 +154,7 @@ export async function backfillApprovalDisplayLabels( const requests = keysetWalk>( (q) => engine.find('sys_approval_request', { ...q, - fields: [ - 'id', 'status', 'process_name', 'current_step', 'node_config_json', 'pending_approvers', - 'process_label', 'current_step_label', 'pending_approver_labels', 'updated_at', - ], + fields: REQUEST_FIELDS, context: SYSTEM_CONTEXT, }) as Promise[]>, { @@ -162,22 +173,43 @@ export async function backfillApprovalDisplayLabels( .filter((row) => row.status === 'pending' && !nonEmpty(row.pending_approver_labels)) .flatMap((row) => slateOf(row.pending_approvers)); const named = await pageLabels(labels, waitingOnSlots); - for (const row of rows) { - const id = nonEmpty(row.id); + for (const paged of rows) { + const id = nonEmpty(paged.id); if (!id) continue; + // The page is a snapshot; live writers keep writing pending requests while + // this pass runs — the boot SLA / dead-run sweep this plugin starts itself + // (not awaited), and every other node. So the row is RE-READ by id + // immediately before its write, and the patch is computed from that row: + // only columns still null there, the stamp it holds now, and the + // waiting-on only for the slate the names were read for. That narrows the + // read-then-write race to the one engine call, as `plugin-audit`'s + // comment-parent-pair backfill does. ⛔ No conditional `where` on the + // update: that would put the write on the predicate path. + const reread = ((await engine.find('sys_approval_request', { + where: { id }, fields: REQUEST_FIELDS, limit: 1, context: SYSTEM_CONTEXT, + })) as Record[])[0]; + if (!reread) continue; const patch: Record = {}; - const chain = requestDisplayLabels(row); - if (!nonEmpty(row.process_label) && chain.process_label) patch.process_label = chain.process_label; - if (!nonEmpty(row.current_step_label) && chain.current_step_label) { + const chain = requestDisplayLabels(reread); + if (!nonEmpty(reread.process_label) && chain.process_label) patch.process_label = chain.process_label; + if (!nonEmpty(reread.current_step_label) && chain.current_step_label) { patch.current_step_label = chain.current_step_label; } - if (row.status === 'pending' && !nonEmpty(row.pending_approver_labels)) { - const slate = slateOf(row.pending_approvers); + // Whom it waits on: only while it is still pending on the very slate the + // page's names were read for. A slate that moved was moved by a live + // writer, which wrote that slate's own labels (or cleared them). + if ( + reread.status === 'pending' + && !nonEmpty(reread.pending_approver_labels) + && paged.status === 'pending' + && reread.pending_approvers === paged.pending_approvers + ) { + const slate = slateOf(reread.pending_approvers); const waitingOn = slate.length ? slateLabelOf(slate.map((slot) => named.get(slot) ?? null)) : null; if (waitingOn) patch.pending_approver_labels = waitingOn; } if (!Object.keys(patch).length) continue; - await engine.update('sys_approval_request', { id, ...patch, ...keptStamp(row) }, { context: PRESERVING_CONTEXT }); + await engine.update('sys_approval_request', { id, ...patch, ...keptStamp(reread) }, { context: PRESERVING_CONTEXT }); requestsFilled++; } } From f215e09f7876e489f6949f3d777f295edb5ffad4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 19:28:18 +0000 Subject: [PATCH 17/17] test(plugin-approvals): pin that page labels never land on a slate that replaced it Claude-Session: https://claude.ai/code/session_01LZZjeuwVdnwn6dFDPLwTBu Co-authored-by: Claude --- .../src/display-label-backfill.integration.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts index 7c54b06ffe8..3ebd47128db 100644 --- a/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/display-label-backfill.integration.test.ts @@ -338,7 +338,13 @@ describe('#23889 — the boot-time display-label backfill', () => { await engine.insert('sys_approval_request', legacyRequest('moved', { status: 'pending', pending_approvers: 'u_alice', node_config_json: JSON.stringify({ approvers: [] }), }), { context: SYSTEM } as any); - for (const id of ['areq_done', 'areq_open', 'areq_recalled', 'areq_moved']) await backdate('sys_approval_request', id); + // And one the sweep moves while its name read fails, so the live writer leaves no label. + await engine.insert('sys_approval_request', legacyRequest('moved_unnamed', { + status: 'pending', pending_approvers: 'u_alice', node_config_json: JSON.stringify({ approvers: [] }), + }), { context: SYSTEM } as any); + for (const id of ['areq_done', 'areq_open', 'areq_recalled', 'areq_moved', 'areq_moved_unnamed']) { + await backdate('sys_approval_request', id); + } const svc = new ApprovalService({ engine: engine as any, logger: quiet as any }); const source = slotLabelSourceOf(svc); @@ -356,9 +362,13 @@ describe('#23889 — the boot-time display-label backfill', () => { await engine.update('sys_approval_request', { id: 'areq_moved', pending_approvers: 'u_mail', pending_approver_labels: 'Mia Mail', }, { context: SYSTEM } as any); + await engine.update('sys_approval_request', { + id: 'areq_moved_unnamed', pending_approvers: 'u_mail', pending_approver_labels: null, + }, { context: SYSTEM } as any); left = { recalled: await row('sys_approval_request', 'areq_recalled'), moved: await row('sys_approval_request', 'areq_moved'), + movedUnnamed: await row('sys_approval_request', 'areq_moved_unnamed'), }; } return source.slotLabels(slots); @@ -373,6 +383,8 @@ describe('#23889 — the boot-time display-label backfill', () => { expect(left!.recalled.updated_at).not.toBe(BACKDATED); expect(pick(await row('sys_approval_request', 'areq_recalled'))).toEqual(pick(left!.recalled)); expect(pick(await row('sys_approval_request', 'areq_moved'))).toEqual(pick(left!.moved)); + // Labels made for the page's slate are never written onto the slate that replaced it. + expect(pick(await row('sys_approval_request', 'areq_moved_unnamed'))).toEqual(pick(left!.movedUnnamed)); // Their null label columns are still filled, under the stamp the live write left. expect((await row('sys_approval_request', 'areq_recalled')).process_label).toBe('Deal Review');