From e53e059b7c35002088f59ffc18fab82e005985df Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 12:50:42 +0000 Subject: [PATCH 1/2] feat(approvals): server-computed decision progress + notification deep links (#3266, objectui#2678 P1.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UX follow-ups on quorum/per_group, both SDUI-shaped (server computes, any client renders): - getRequest now attaches `decision_progress` for PENDING multi-approver requests: unanimous/quorum report approvals got/need (quorum against the clamped threshold); per_group reports satisfied-groups got/need plus a per-group {group, got, need, satisfied} breakdown computed from the open-time approver-group snapshot. Single-read enrichment only, display-only (decideNode's tally stays authoritative), best-effort. Typed on the ApprovalRequestRow contract. - Approval notifications now deep-link the inbox: `notify()` centrally rewrites the bare `/system/approvals` actionUrl to `/system/approvals?request=` from the notification's source, so all twelve call sites — and any future one — land the recipient on the exact request (the console inbox consumes the param and auto-opens the drawer). Tests: 4 new (per-group progress updates per approval, quorum clamped threshold, first_response carries no progress, deep-link rewrite). 136 green in plugin-approvals. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA --- .../src/approval-service.test.ts | 56 +++++++++++++++++ .../plugin-approvals/src/approval-service.ts | 61 ++++++++++++++++++- .../spec/src/contracts/approval-service.ts | 15 +++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index a7770c6982..280c6ff122 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -1390,3 +1390,59 @@ describe('ApprovalService — quorum & per_group (#3266)', () => { expect(act.attachments).toEqual(['file_1', 'file_2']); }); }); + +// ── Decision progress + notification deep links (#2678 P1.5) ────────── +describe('ApprovalService — decision_progress & deep links (#2678 P1.5)', () => { + let engine: ReturnType; + let svc: ApprovalService; + + beforeEach(() => { + engine = makeFakeEngine(); + let n = 0; + const base = new Date('2026-09-01T09:00:00Z').getTime(); + svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(base + (n++) * 1000) } }); + }); + + const cfg = (approvers: any[], behavior: string, extra: Record = {}) => ({ + ...openInput([]), + config: { approvers, behavior, lockRecord: true, ...extra }, + }); + const U = (v: string, group?: string) => (group ? { type: 'user', value: v, group } : { type: 'user', value: v }); + + it('per_group: getRequest exposes per-group progress that updates per approval', async () => { + const req = await svc.openNodeRequest(cfg([U('l1', 'legal'), U('f1', 'finance')], 'per_group'), CTX); + let row: any = await svc.getRequest(req.id, SYS); + expect(row.decision_progress).toMatchObject({ behavior: 'per_group', got: 0, need: 2 }); + expect(row.decision_progress.groups).toEqual([ + { group: 'finance', got: 0, need: 1, satisfied: false }, + { group: 'legal', got: 0, need: 1, satisfied: false }, + ]); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'l1' }, SYS); + row = await svc.getRequest(req.id, SYS); + expect(row.decision_progress.got).toBe(1); + expect(row.decision_progress.groups.find((g: any) => g.group === 'legal')).toMatchObject({ got: 1, satisfied: true }); + expect(row.decision_progress.groups.find((g: any) => g.group === 'finance')).toMatchObject({ got: 0, satisfied: false }); + }); + + it('quorum: progress reports approvals against the clamped threshold', async () => { + const req = await svc.openNodeRequest(cfg([U('u1'), U('u2'), U('u3')], 'quorum', { minApprovals: 2 }), CTX); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u1' }, SYS); + const row: any = await svc.getRequest(req.id, SYS); + expect(row.decision_progress).toMatchObject({ behavior: 'quorum', got: 1, need: 2 }); + }); + + it('first_response: no decision_progress', async () => { + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + const row: any = await svc.getRequest(req.id, SYS); + expect(row.decision_progress).toBeUndefined(); + }); + + it('notify: inbox actionUrl is rewritten to a request deep link', async () => { + const emitted: any[] = []; + svc.attachMessaging({ async emit(input) { emitted.push(input); } }); + const req = await svc.openNodeRequest(openInput(['u1', 'u2']), CTX); + await svc.reassign(req.id, { actorId: 'u1', to: 'u7' }, SYS); + const note = emitted.find(e => e.topic === 'approval.reassigned'); + expect(note.payload.actionUrl).toBe(`/system/approvals?request=${encodeURIComponent(req.id)}`); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 4cb2800eaa..defbd358d9 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -268,8 +268,20 @@ export class ApprovalService implements IApprovalService { }): Promise { const audience = input.audience.filter(a => a && !a.includes(':')); if (!this.messaging || !audience.length) return 0; + // Deep-link the inbox (#2678 P1.5): a notification about one request should + // land on that request, not the bare inbox. Rewritten centrally so every + // call site — and any future one — inherits it; the query param is read by + // the console inbox to auto-open the drawer. + let payload = input.payload; + if ( + payload?.actionUrl === '/system/approvals' + && input.source?.object === 'sys_approval_request' + && input.source.id + ) { + payload = { ...payload, actionUrl: `/system/approvals?request=${encodeURIComponent(input.source.id)}` }; + } try { - await this.messaging.emit({ severity: 'info', ...input, audience }); + await this.messaging.emit({ severity: 'info', ...input, payload, audience }); return audience.length; } catch (err: any) { this.logger?.warn?.('[approvals] notification failed', { @@ -2204,9 +2216,56 @@ export class ApprovalService implements IApprovalService { const row = rowFromRequest(rows[0]); await this.enrichRows([row]); await this.attachFlowSteps(row); + await this.attachDecisionProgress(row, rows[0]); return row; } + /** + * Server-computed decision aggregation progress (#3266 / objectui#2678 P1.5). + * Single-read enrichment only (like {@link ApprovalService.attachFlowSteps}): + * for a PENDING request whose behavior aggregates multiple approvals + * (`unanimous` / `quorum` / `per_group`), expose + * `decision_progress: { behavior, got, need, groups? }` so any client renders + * "2 of 3" or per-group ticks without re-deriving the engine's tally rules. + * `first_response` requests carry no progress (one approval finalizes). + * Display-only and best-effort — errors leave the row untouched. + */ + private async attachDecisionProgress(row: ApprovalRequestRow, raw: any): Promise { + try { + if (row.status !== 'pending') return; + const cfg = parseJson(raw.node_config_json, undefined); + const behavior = cfg?.behavior ?? 'first_response'; + if (behavior !== 'unanimous' && behavior !== 'quorum' && behavior !== 'per_group') return; + + const acts = await this.engine.find('sys_approval_action', { + where: { request_id: row.id, step_index: 0, action: 'approve' }, limit: 1000, context: SYSTEM_CTX, + }); + const approved = new Set((acts ?? []).map((a: any) => String(a.actor_id ?? '')).filter(Boolean)); + + const snapshot = cfg?.__approverGroups as Record | undefined; + const slate = snapshot ? Object.keys(snapshot) : [...approved, ...(row.pending_approvers ?? [])]; + const total = slate.length || 1; + + const progress: any = { behavior, got: approved.size, need: total }; + if (behavior === 'quorum') { + progress.need = Math.min(Math.max(1, cfg?.minApprovals ?? total), total); + } else if (behavior === 'per_group' && snapshot) { + const perGroupNeed = Math.max(1, cfg?.minApprovals ?? 1); + const size: Record = {}; + for (const gs of Object.values(snapshot)) for (const g of gs) size[g] = (size[g] ?? 0) + 1; + const got: Record = {}; + for (const a of approved) for (const g of (snapshot[a] ?? [])) got[g] = (got[g] ?? 0) + 1; + progress.groups = Object.keys(size).sort().map(g => { + const need = Math.min(perGroupNeed, size[g]); + return { group: g, got: Math.min(got[g] ?? 0, need), need, satisfied: (got[g] ?? 0) >= need }; + }); + progress.got = progress.groups.filter((g: any) => g.satisfied).length; + progress.need = progress.groups.length; + } + (row as any).decision_progress = progress; + } catch { /* display-only enrichment */ } + } + /** * Derive approval-step progress from the owning flow's graph (single-read * enrichment only — list reads skip it). Walks from the start node diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 142d4162d0..e48fefdcf6 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -97,6 +97,21 @@ export interface ApprovalRequestRow { * `node_config_json` snapshot (`__round`), so no schema migration. */ round?: number; + /** + * Server-computed decision aggregation progress (#3266, single-request reads + * of PENDING requests only). Present when the node's behavior aggregates + * multiple approvals: `unanimous` (got/need = approvals of total), + * `quorum` (got/need = approvals of the M threshold), `per_group` + * (got/need = satisfied groups of total groups, plus per-group detail). + * Absent for `first_response`. Display-only — the engine's finalization + * tally in decideNode stays authoritative. + */ + decision_progress?: { + behavior: 'unanimous' | 'quorum' | 'per_group'; + got: number; + need: number; + groups?: Array<{ group: string; got: number; need: number; satisfied: boolean }>; + }; } /** Kinds of entries on a request's audit trail. */ From 04a0a916a38643b442b14a7d73512d4d6b923cbf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 13:03:32 +0000 Subject: [PATCH 2/2] fix(approvals): surface decision attachments through the listActions contract mapping (#3266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser verification caught a #3268 gap: the `attachments` column landed on sys_approval_action and decide() persisted it, but rowFromAction never mapped it — every listActions consumer (REST /actions, the console timeline) saw none while the raw engine row carried the fileIds. Map it (and type it on the ApprovalActionRow contract) so the audit trail actually exposes what was attached. Regression test added: 137 green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA --- .../plugin-approvals/src/approval-service.test.ts | 15 +++++++++++++++ .../plugin-approvals/src/approval-service.ts | 4 ++++ packages/spec/src/contracts/approval-service.ts | 2 ++ 3 files changed, 21 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 280c6ff122..3e1e1f333a 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -1446,3 +1446,18 @@ describe('ApprovalService — decision_progress & deep links (#2678 P1.5)', () = expect(note.payload.actionUrl).toBe(`/system/approvals?request=${encodeURIComponent(req.id)}`); }); }); + +// listActions must surface decision attachments through the contract mapping +// (#3266 — the column existed but rowFromAction dropped it; caught in browser). +describe('ApprovalService — listActions attachments mapping (#3266)', () => { + it('returns the attachments recorded on a decision', async () => { + const engine = makeFakeEngine(); + let n = 0; + const svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(1757000000000 + (n++) * 1000) } }); + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9', attachments: ['file_a'] }, SYS); + const acts = await svc.listActions(req.id, SYS); + const approve = acts.find(a => a.action === 'approve'); + expect(approve?.attachments).toEqual(['file_a']); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index defbd358d9..eb107ca6ff 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -205,6 +205,10 @@ function rowFromAction(row: any): ApprovalActionRow { action: row.action, actor_id: row.actor_id ?? undefined, comment: row.comment ?? undefined, + // Decision attachments (#3266). The column shipped in #3268 but this + // contract mapping didn't — the raw engine row carried the fileIds while + // every consumer of listActions saw none (caught by browser verification). + attachments: Array.isArray(row.attachments) && row.attachments.length ? row.attachments.map(String) : undefined, created_at: row.created_at ?? undefined, }; } diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index e48fefdcf6..dc23dcbf7b 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -145,6 +145,8 @@ export interface ApprovalActionRow { action: ApprovalActionKind; actor_id?: string; comment?: string; + /** File references attached to this action (decision attachments, #3266). */ + attachments?: string[]; created_at?: string; /** Display name of the actor (`sys_user.name`), when resolvable. */ actor_name?: string;