Skip to content

Commit 8efa395

Browse files
os-zhuangclaude
andauthored
feat(approvals): gate declared decision actions on a server-computed viewer capability (#3344)
`getRequest` / `listRequests` now attach a per-viewer block to each row from the caller's context — `viewer: { can_act, is_submitter }` (ApprovalRequestRow): - can_act — the caller is a current pending approver (their user id is in the resolved pending_approvers while status is pending). This is the exact check the decision methods authorize with, so it already reflects position/team/manager resolution. - is_submitter — the caller submitted the request. The declared decision actions on sys_approval_request now gate their `visible` CEL on it: approver actions (approve/reject/reassign/send-back/request-info) use `record.viewer.can_act`; submitter levers (remind/recall/resubmit) use `record.viewer.is_submitter`. Before this, approver actions only trimmed the non-pending case, so a submitter viewing their own pending request saw buttons they couldn't use (the backend 403'd), and a position-addressed approver could be wrongly hidden by a client-side identity heuristic. Where `viewer` is absent (a row surfaced outside a service read with a user context), the predicate fails closed. Tests: getRequest returns can_act true for a pending approver, false for the submitter and strangers, and false once finalized; listRequests attaches viewer to every row; the object contract test pins the new predicates. Claude-Session: https://claude.ai/code/session_016ypkQikZ55oWUHUnMebwXA Co-authored-by: Claude <noreply@anthropic.com>
1 parent bfa3c3f commit 8efa395

6 files changed

Lines changed: 123 additions & 21 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-approvals": minor
4+
---
5+
6+
feat(approvals): server-computed `viewer` capability for precise decision-action gating
7+
8+
`getRequest` / `listRequests` now attach a per-viewer block —
9+
`viewer: { can_act, is_submitter }` — computed from the caller's context
10+
(`ApprovalRequestRow.viewer`):
11+
12+
- `can_act` — the caller is a *current pending approver* (their user id is in the
13+
request's resolved `pending_approvers` while it is still `pending`). This is
14+
the same check the decision methods authorize with, so it already reflects
15+
position/team/manager resolution — strictly more accurate than a client-side
16+
identity guess.
17+
- `is_submitter` — the caller submitted the request.
18+
19+
The declared decision actions on `sys_approval_request` now gate on it: approver
20+
actions (approve/reject/reassign/send-back/request-info) use
21+
`record.viewer.can_act`; submitter levers (remind/recall/resubmit) use
22+
`record.viewer.is_submitter`. Previously approver actions only trimmed the
23+
non-pending case, so a submitter viewing their own pending request saw buttons
24+
they couldn't use (the backend 403'd); a position-addressed approver could be
25+
wrongly hidden by the old client heuristic. Where `viewer` is absent (a row
26+
surfaced outside a service read with a user context), the predicate fails closed.

packages/plugins/plugin-approvals/src/approval-service.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,33 @@ describe('ApprovalService (node era)', () => {
414414
expect(await svc.getRequest('nope', SYS)).toBeNull();
415415
});
416416

417+
// ── viewer capability (#3310) ───────────────────────────────────
418+
it('getRequest: viewer.can_act is true for a pending approver, false for the submitter', async () => {
419+
const req = await svc.openNodeRequest(openInput(['u9']), CTX); // submitter u1, approver u9
420+
const asApprover = await svc.getRequest(req.id, { userId: 'u9', tenantId: 't1' } as any);
421+
expect(asApprover!.viewer).toEqual({ can_act: true, is_submitter: false });
422+
const asSubmitter = await svc.getRequest(req.id, { userId: 'u1', tenantId: 't1' } as any);
423+
expect(asSubmitter!.viewer).toEqual({ can_act: false, is_submitter: true });
424+
const asOther = await svc.getRequest(req.id, { userId: 'u_stranger', tenantId: 't1' } as any);
425+
expect(asOther!.viewer).toEqual({ can_act: false, is_submitter: false });
426+
});
427+
428+
it('getRequest: viewer.can_act drops to false once the request is finalized', async () => {
429+
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
430+
await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS); // → approved
431+
const after = await svc.getRequest(req.id, { userId: 'u9', tenantId: 't1' } as any);
432+
expect(after!.status).toBe('approved');
433+
expect(after!.viewer!.can_act).toBe(false);
434+
});
435+
436+
it('listRequests: attaches viewer to every row from the caller context', async () => {
437+
await svc.openNodeRequest(openInput(['u9']), CTX);
438+
const rows = await svc.listRequests({ status: 'pending' }, { userId: 'u9', tenantId: 't1' } as any);
439+
expect(rows.length).toBeGreaterThan(0);
440+
expect(rows.every(r => r.viewer != null)).toBe(true);
441+
expect(rows[0].viewer!.can_act).toBe(true);
442+
});
443+
417444
// ── recall ──────────────────────────────────────────────────────
418445

419446
it('recall: submitter withdraws a pending request', async () => {

packages/plugins/plugin-approvals/src/approval-service.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2174,6 +2174,7 @@ export class ApprovalService implements IApprovalService {
21742174
const rows = await this.engine.find('sys_approval_request', findOpts);
21752175
const list = Array.isArray(rows) ? rows.map(rowFromRequest) : [];
21762176
await this.enrichRows(list);
2177+
this.attachViewers(list, context);
21772178
return list;
21782179
}
21792180

@@ -2221,6 +2222,7 @@ export class ApprovalService implements IApprovalService {
22212222
await this.enrichRows([row]);
22222223
await this.attachFlowSteps(row);
22232224
await this.attachDecisionProgress(row, rows[0]);
2225+
this.attachViewers([row], context);
22242226
return row;
22252227
}
22262228

@@ -2270,6 +2272,26 @@ export class ApprovalService implements IApprovalService {
22702272
} catch { /* display-only enrichment */ }
22712273
}
22722274

2275+
/**
2276+
* Attach the per-viewer capability block (#3310) from the caller's context.
2277+
* `can_act` mirrors the exact authorization the decision methods enforce — the
2278+
* caller's user id is in the resolved `pending_approvers` while the request is
2279+
* still `pending` (position/team/manager approvers are already resolved to
2280+
* concrete user ids at open time, so a plain membership test is faithful).
2281+
* `is_submitter` is a straight owner check. System/tokenless contexts get a
2282+
* both-false block. Cheap + synchronous — safe on list reads.
2283+
*/
2284+
private attachViewers(rows: ApprovalRequestRow[], context: SharingExecutionContext): void {
2285+
const uid = (context as any)?.userId != null ? String((context as any).userId) : null;
2286+
for (const row of rows) {
2287+
const pending = row.pending_approvers ?? [];
2288+
(row as any).viewer = {
2289+
can_act: row.status === 'pending' && !!uid && pending.includes(uid),
2290+
is_submitter: !!uid && row.submitter_id != null && String(row.submitter_id) === uid,
2291+
};
2292+
}
2293+
}
2294+
22732295
/**
22742296
* Derive approval-step progress from the owning flow's graph (single-read
22752297
* enrichment only — list reads skip it). Walks from the start node

packages/plugins/plugin-approvals/src/sys-approval-request.object.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,16 @@ describe('sys_approval_request declared actions', () => {
6161
}
6262
});
6363

64-
it('gates submitter-only levers on the current user; approver actions only on pending', () => {
64+
it('gates on the server-computed viewer block (#3310): approver actions on can_act, submitter levers on is_submitter', () => {
6565
for (const name of ['approval_remind', 'approval_recall', 'approval_resubmit']) {
66-
expect(vis(name)).toContain('record.submitter_id == ctx.user.id');
66+
expect(vis(name)).toContain('record.viewer.is_submitter');
67+
expect(vis(name)).not.toContain('can_act');
6768
}
68-
// Approver-side actions defer who-can-act to the service; they only trim the
69-
// non-pending case in the UI.
7069
for (const name of ['approval_approve', 'approval_reject', 'approval_send_back', 'approval_request_info', 'approval_reassign']) {
71-
expect(vis(name)).toContain('record.status == "pending"');
70+
expect(vis(name)).toContain('record.viewer.can_act');
71+
// who-can-act is server-derived, never a client identity guess
7272
expect(vis(name)).not.toContain('ctx.user.id');
73+
expect(vis(name)).not.toContain('is_submitter');
7374
}
7475
});
7576

packages/plugins/plugin-approvals/src/sys-approval-request.object.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,12 @@ export const SysApprovalRequest = ObjectSchema.create({
236236
// (and their params) ship as metadata, not as hand-written buttons. Each
237237
// targets the existing approvals REST route; `{id}` resolves from the row
238238
// and `actorId` defaults to the caller server-side. The service remains the
239-
// authority on who may act (pending-approver check) — `visible` only trims
240-
// the obvious non-pending case.
239+
// authority on who may act; `visible` gates on the server-computed
240+
// per-viewer block (#3310): approver actions on `record.viewer.can_act`
241+
// (the caller is a current pending approver — same check the service
242+
// authorizes a decision with, so position/team approvers resolve correctly),
243+
// submitter actions on `record.viewer.is_submitter`. `viewer` is attached by
244+
// getRequest/listRequests; where it is absent the predicate fails closed.
241245
actions: [
242246
{
243247
name: 'approval_approve',
@@ -253,7 +257,7 @@ export const SysApprovalRequest = ObjectSchema.create({
253257
// string[]`; the decision route persists them on `sys_approval_action`.
254258
{ name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false },
255259
],
256-
visible: 'record.status == "pending"',
260+
visible: 'record.viewer.can_act',
257261
locations: ['record_section', 'list_item'],
258262
successMessage: 'Approved.',
259263
refreshAfter: true,
@@ -269,7 +273,7 @@ export const SysApprovalRequest = ObjectSchema.create({
269273
{ name: 'comment', label: 'Comment', type: 'textarea', required: false },
270274
{ name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false },
271275
],
272-
visible: 'record.status == "pending"',
276+
visible: 'record.viewer.can_act',
273277
confirmText: 'Reject this request? A rejection is final for every approver.',
274278
locations: ['record_section', 'list_item'],
275279
successMessage: 'Rejected.',
@@ -291,16 +295,16 @@ export const SysApprovalRequest = ObjectSchema.create({
291295
{ field: 'submitter_id', name: 'to', label: 'New approver', required: true, helpText: 'User to hand this step to' },
292296
{ name: 'comment', label: 'Comment', type: 'textarea', required: false },
293297
],
294-
visible: 'record.status == "pending"',
298+
visible: 'record.viewer.can_act',
295299
locations: ['record_section'],
296300
successMessage: 'Reassigned.',
297301
refreshAfter: true,
298302
},
299303

300304
// ── Approver secondary decisions ────────────────────────────────
301305
// Send back for revision / request more info (ADR-0044). Both are approver
302-
// actions on a pending request; the service is the authority on who may act,
303-
// so `visible` only trims the non-pending case (matching approve/reject).
306+
// actions, so `visible` gates on `record.viewer.can_act` (a current pending
307+
// approver) — same as approve/reject. The service stays the authority.
304308
{
305309
name: 'approval_send_back',
306310
label: 'Send back',
@@ -311,7 +315,7 @@ export const SysApprovalRequest = ObjectSchema.create({
311315
params: [
312316
{ name: 'comment', label: 'Reason', type: 'textarea', required: false },
313317
],
314-
visible: 'record.status == "pending"',
318+
visible: 'record.viewer.can_act',
315319
locations: ['record_section'],
316320
successMessage: 'Sent back for revision.',
317321
refreshAfter: true,
@@ -326,18 +330,18 @@ export const SysApprovalRequest = ObjectSchema.create({
326330
params: [
327331
{ name: 'comment', label: 'What do you need?', type: 'textarea', required: true },
328332
],
329-
visible: 'record.status == "pending"',
333+
visible: 'record.viewer.can_act',
330334
locations: ['record_section'],
331335
successMessage: 'Information requested.',
332336
refreshAfter: true,
333337
},
334338

335339
// ── Submitter continuity actions ────────────────────────────────
336340
// Remind / recall (pending) and resubmit / recall (returned). These are the
337-
// submitter's own levers, so `visible` gates on `submitter_id == ctx.user.id`
338-
// the current user is exposed via the console's predicate scope. The
339-
// service re-checks ownership; the predicate keeps a non-submitter from ever
340-
// seeing a button they cannot use.
341+
// submitter's own levers, so `visible` gates on `record.viewer.is_submitter`
342+
// (server-computed on the current viewer). The service re-checks ownership;
343+
// the predicate keeps a non-submitter from ever seeing a button they cannot
344+
// use.
341345
{
342346
name: 'approval_remind',
343347
label: 'Send reminder',
@@ -348,7 +352,7 @@ export const SysApprovalRequest = ObjectSchema.create({
348352
params: [
349353
{ name: 'comment', label: 'Note', type: 'textarea', required: false },
350354
],
351-
visible: 'record.status == "pending" && record.submitter_id == ctx.user.id',
355+
visible: 'record.status == "pending" && record.viewer.is_submitter',
352356
locations: ['record_section'],
353357
successMessage: 'Reminder sent.',
354358
refreshAfter: true,
@@ -365,7 +369,7 @@ export const SysApprovalRequest = ObjectSchema.create({
365369
],
366370
// Recall applies while the request is live for the submitter — pending
367371
// (withdraw) or returned (abandon the revision instead of resubmitting).
368-
visible: '(record.status == "pending" || record.status == "returned") && record.submitter_id == ctx.user.id',
372+
visible: '(record.status == "pending" || record.status == "returned") && record.viewer.is_submitter',
369373
confirmText: 'Recall this request? Approvers can no longer act on it and the record is unlocked.',
370374
locations: ['record_section'],
371375
successMessage: 'Recalled.',
@@ -381,7 +385,7 @@ export const SysApprovalRequest = ObjectSchema.create({
381385
params: [
382386
{ name: 'comment', label: 'What changed?', type: 'textarea', required: false },
383387
],
384-
visible: 'record.status == "returned" && record.submitter_id == ctx.user.id',
388+
visible: 'record.status == "returned" && record.viewer.is_submitter',
385389
locations: ['record_section'],
386390
successMessage: 'Resubmitted.',
387391
refreshAfter: true,

packages/spec/src/contracts/approval-service.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,28 @@ export interface ApprovalRequestRow {
112112
need: number;
113113
groups?: Array<{ group: string; got: number; need: number; satisfied: boolean }>;
114114
};
115+
116+
/**
117+
* Server-computed capability for THE CURRENT VIEWER (#3310), attached by
118+
* `getRequest` / `listRequests` from the caller's context. Lets a client gate
119+
* decision actions precisely without re-deriving identity resolution:
120+
* declared approver actions use `record.viewer.can_act`, submitter actions use
121+
* `record.viewer.is_submitter`.
122+
*
123+
* - `can_act` — the caller is a *current pending approver* (their user id is in
124+
* the request's resolved `pending_approvers` while it is still `pending`).
125+
* This mirrors the exact check the service uses to authorize a decision, so
126+
* it is strictly more accurate than a client-side identity guess (it already
127+
* reflects position/team/manager resolution baked into `pending_approvers`).
128+
* - `is_submitter` — the caller submitted the request.
129+
*
130+
* Absent when the row is surfaced outside a service read with a user context
131+
* (e.g. a raw data-API grid); a `record.viewer.*` predicate then fails closed.
132+
*/
133+
viewer?: {
134+
can_act: boolean;
135+
is_submitter: boolean;
136+
};
115137
}
116138

117139
/** Kinds of entries on a request's audit trail. */

0 commit comments

Comments
 (0)