From 42c7aa4cea267562a19814a012d5ef8307a01c88 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 03:29:10 +0000 Subject: [PATCH 1/6] feat(spec,plugin-approvals,rest): an unstaffed arm on the approvals inbox read returns an admin_rescue request to a caller who may override it (#22725) IApprovalService.listRequests' filter declares `unstaffed?: boolean`, ORed with `approverId`: pending requests whose current slate names no person, returned only where isOverrideActor admits the caller. listRequests and countRequests read it through one helper; the REST list door declares `?unstaffed=true|false` and refuses any other value with 400 invalid_boolean. The open path and the arm share one shape test, slateNamesAPerson. Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK Co-authored-by: Claude --- .changeset/22725-approvals-unstaffed-arm.md | 16 ++ content/docs/automation/approvals.mdx | 21 ++- .../src/approval-service.test.ts | 173 ++++++++++++++++++ .../plugin-approvals/src/approval-service.ts | 117 ++++++++++-- .../plugin-approvals/src/approver-address.ts | 19 ++ ...st-server-approvals-unknown-filter.test.ts | 58 +++++- packages/rest/src/rest-server.ts | 59 +++++- .../spec/src/contracts/approval-service.ts | 33 +++- 8 files changed, 475 insertions(+), 21 deletions(-) create mode 100644 .changeset/22725-approvals-unstaffed-arm.md diff --git a/.changeset/22725-approvals-unstaffed-arm.md b/.changeset/22725-approvals-unstaffed-arm.md new file mode 100644 index 00000000000..5a675186073 --- /dev/null +++ b/.changeset/22725-approvals-unstaffed-arm.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-approvals": minor +"@objectstack/rest": minor +--- + +feat(spec,plugin-approvals,rest): the approvals inbox read gets an `unstaffed` arm, so an `admin_rescue` request is in the queue of a caller who may override it + +Clause-②: yes (widening) + +- **What is new.** `IApprovalService.listRequests`' filter declares an optional `unstaffed?: boolean`. `countRequests` takes the same filter, so it answers it too. `GET /api/v1/approvals/requests` accepts it as the declared query parameter `?unstaffed=true|false`. +- **What it selects.** The PENDING requests whose current slate names no person: no user id and no email, only `type:value` literals such as `position:

`, or nothing at all. This is what `onEmptyApprovers: 'admin_rescue'` (the default) opens. The test is the one the open path makes before it warns "decidable only by a privileged admin", applied to the slate as it stands. A slate whose last named approver acted and left only literals matches too. A slate that still names a person never matches, even if that person has left. +- **Who it returns them to.** Only a caller who may override them, judged by the privileged-override rule that admits the override decision itself. Every row it adds reads `viewer.can_override: true`, and a tenant admin's arm stops at their organization. For anyone else the member adds nothing and refuses nothing. +- **How it composes.** It is ORed with `approverId`, never ANDed. `?status=pending&approverId=&unstaffed=true` answers "my pending approvals, rescues included" in one call, paged server-side, and `total` counts both arms. Without `approverId` it selects the rescue requests alone. `unstaffed=false` is the same as leaving it out. +- **What is refused.** Any value but `true` or `false` (`1`, `TRUE`, an empty `?unstaffed=`) answers `400 VALIDATION_FAILED` with `fields[0]` `{ field: 'unstaffed', code: 'invalid_boolean' }`, and a repeated `?unstaffed=` answers the route's multiplicity `400`. Nothing changes for a caller who does not send it: `approverId` matches exactly what it matched before. +- **Before this.** The request was in no queue. `approverId` matches a slot against the addresses a caller names, and an override actor's addresses never equal an unstaffed `position:

` literal. So no "My Pending" listed it, no bell counted it, and a `lockRecord: true` record stayed locked until an admin browsed the unfiltered list. diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 97a7b7a5cb0..be635cbf6b1 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -453,7 +453,26 @@ position under as their own identity. The deprecated pre-rename prefix is no alias of it (ADR-0090 D3): it matches only a slot stored under that exact spelling, and no holder acts under it. Other filters: `status`, `object`, `recordId`, `submitterId`, `q`, `limit`, -`offset`. +`offset`, and `unstaffed`. + +`unstaffed=true` adds the requests nobody on the slate can decide: pending +requests whose slate names no person, only unstaffed literals such as +`position:`, or nothing at all. This is what `admin_rescue` opens. They +are added only for a caller who may override them: an admin with override +authority (`admin_full_access`, or `organization_admin` within their +organization). Every such row reads `viewer.can_override: true`. For anyone else the parameter +adds nothing. It is ORed with `approverId`, never ANDed, so one call answers +"my pending approvals, rescues included", and its `total` counts both: + +```bash +curl -b cookies.txt \ + "https://your-app.example.com/api/v1/approvals/requests?status=pending&approverId=usr_123&unstaffed=true" +``` + +It reads `true` or `false` only (`false` is the same as leaving it out); any +other value is refused with `400`. A slate that still names a person never +matches, even if that person has since left. Reassign such a request from the +**All** view. **`approverId` is a filter, not authorization.** What you may see is decided diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 87528cf3113..50acf81acf9 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -2066,6 +2066,179 @@ describe('ApprovalService — admin override (#3424)', () => { }); }); +// ── #22725: the `unstaffed` arm of the inbox read ──────────────────────────── +// +// An `onEmptyApprovers: 'admin_rescue'` request opens on a slate that names no +// person, so it was in nobody's queue: `approverId` matches a slot against the +// addresses a caller names, and an override actor's addresses never equal an +// unstaffed `position:

` literal. Only the unfiltered list showed it. The +// `unstaffed` member is the arm that returns it to the callers who may decide +// it. It is ORed with `approverId`, and both `listRequests` and +// `countRequests` answer it. +// +// Every case reads BOTH faces (the list and its count), because the console's +// paged inbox shows the one and the bell shows the other. +describe('ApprovalService — the unstaffed arm (#22725)', () => { + let engine: ReturnType; + let svc: ApprovalService; + let n = 0; + const baseTime = new Date('2026-01-15T10:00:00Z').getTime(); + + const PLATFORM_ADMIN = { userId: 'root', tenantId: 't1', positions: [], permissions: ['admin_full_access'] } as any; + const TENANT_ADMIN = { userId: 'owner', tenantId: 't1', positions: [], permissions: ['organization_admin'] } as any; + const OTHER_TENANT_ADMIN = { userId: 'owner2', tenantId: 't2', positions: [], permissions: ['organization_admin'] } as any; + const MEMBER = { userId: 'nobody', tenantId: 't1', positions: [], permissions: [] } as any; + const APPROVER = { userId: 'u9', tenantId: 't1', positions: [], permissions: [] } as any; + + /** A node routed to a position nobody holds: the `position:sales_manager` literal. */ + const stuck = (recordId: string) => ({ + object: 'opportunity', recordId, runId: `run_${recordId}`, nodeId: 'approve_step', + flowName: 'deal_approval', + config: { approvers: [{ type: 'position' as const, value: 'sales_manager' }], behavior: 'first_response' as const, lockRecord: true }, + record: { id: recordId, amount: 100 }, + }); + /** An expression that resolves to nobody: `admin_rescue` opens on an EMPTY slate. */ + const empty = (recordId: string) => ({ + ...stuck(recordId), + config: { approvers: [{ type: 'expression' as const, value: 'vars.picked' }], behavior: 'first_response' as const, lockRecord: true }, + variables: { picked: [] }, + }); + /** A staffed node: a concrete user on the slate. */ + const staffed = (recordId: string, user = 'u9') => ({ + ...openInput([user]), recordId, runId: `run_${recordId}`, record: { id: recordId, amount: 100 }, + }); + + const ids = (rows: Array<{ id: string }>) => rows.map(r => r.id).sort(); + /** Both faces of one filter, as one reader: the list's ids and the count. */ + const read = async (filter: Record, ctx: any) => { + const rows = await svc.listRequests(filter as any, ctx); + const total = await svc.countRequests(filter as any, ctx); + return { ids: ids(rows), total, rows }; + }; + + beforeEach(() => { + engine = makeFakeEngine(); + n = 0; + svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } }); + }); + + it('an admin_rescue request appears for an override actor, on both faces', async () => { + const onLiteral = await svc.openNodeRequest(stuck('opp_literal'), CTX) as any; + const onNothing = await svc.openNodeRequest(empty('opp_empty'), CTX) as any; + expect(onLiteral.pending_approvers).toEqual(['position:sales_manager']); + expect(onNothing.pending_approvers).toEqual([]); + const control = await svc.openNodeRequest(staffed('opp_staffed'), CTX) as any; + + for (const admin of [TENANT_ADMIN, PLATFORM_ADMIN]) { + const got = await read({ status: 'pending', unstaffed: true }, admin); + expect(got.ids).toEqual([onLiteral.id, onNothing.id].sort()); + expect(got.total).toBe(2); + expect(got.ids).not.toContain(control.id); + // Every row the arm adds is one the caller may override. + for (const row of got.rows) { + expect(row.viewer).toMatchObject({ can_act: false, can_override: true }); + } + } + }); + + it('it does not appear for a member: the flag adds nothing and refuses nothing', async () => { + await svc.openNodeRequest(stuck('opp_literal'), CTX); + await svc.openNodeRequest(empty('opp_empty'), CTX); + const mine = await svc.openNodeRequest(staffed('opp_mine', 'nobody'), CTX) as any; + + expect(await read({ status: 'pending', unstaffed: true }, MEMBER)).toMatchObject({ ids: [], total: 0 }); + // Beside the member's own identities, the arm leaves their queue exactly as + // `approverId` alone answers it. + const withFlag = await read({ status: 'pending', approverId: ['nobody'], unstaffed: true }, MEMBER); + const without = await read({ status: 'pending', approverId: ['nobody'] }, MEMBER); + expect(withFlag.ids).toEqual([mine.id]); + expect(withFlag.total).toBe(1); + expect(without.ids).toEqual(withFlag.ids); + expect(without.total).toBe(withFlag.total); + }); + + it('CONTROL: a staffed request\'s queue is unchanged', async () => { + await svc.openNodeRequest(stuck('opp_literal'), CTX); + const control = await svc.openNodeRequest(staffed('opp_staffed'), CTX) as any; + + // Its approver's queue reads the same with and without the arm. + for (const filter of [ + { status: 'pending', approverId: ['u9'] }, + { status: 'pending', approverId: ['u9'], unstaffed: true }, + ]) { + expect(await read(filter, APPROVER)).toMatchObject({ ids: [control.id], total: 1 }); + } + // And an override actor's rescue arm never claims it. + const rescue = await read({ status: 'pending', unstaffed: true }, TENANT_ADMIN); + expect(rescue.ids).not.toContain(control.id); + // Without the arm, the override actor's "My Pending" is what it was. + expect(await read({ status: 'pending', approverId: ['owner'] }, TENANT_ADMIN)).toMatchObject({ ids: [], total: 0 }); + }); + + it('with approverId the arms are ORed: one read answers "my pending, rescues included"', async () => { + const stuckReq = await svc.openNodeRequest(stuck('opp_literal'), CTX) as any; + const adminOwn = await svc.openNodeRequest(staffed('opp_admin_own', 'owner'), CTX) as any; + await svc.openNodeRequest(staffed('opp_other'), CTX); + + const both = await read({ status: 'pending', approverId: ['owner'], unstaffed: true }, TENANT_ADMIN); + expect(both.ids).toEqual([stuckReq.id, adminOwn.id].sort()); + expect(both.total).toBe(2); + // The paged window covers the union too: two pages of one. + const page1 = await svc.listRequests({ status: 'pending', approverId: ['owner'], unstaffed: true, limit: 1, offset: 0 }, TENANT_ADMIN); + const page2 = await svc.listRequests({ status: 'pending', approverId: ['owner'], unstaffed: true, limit: 1, offset: 1 }, TENANT_ADMIN); + expect(ids([...page1, ...page2])).toEqual(both.ids); + }); + + it('`unstaffed: false` is the same as absent', async () => { + await svc.openNodeRequest(stuck('opp_literal'), CTX); + await svc.openNodeRequest(staffed('opp_staffed'), CTX); + const absent = await read({ status: 'pending' }, TENANT_ADMIN); + const off = await read({ status: 'pending', unstaffed: false }, TENANT_ADMIN); + expect(off.ids).toEqual(absent.ids); + expect(off.total).toBe(absent.total); + expect(absent.total).toBe(2); + }); + + it('a tenant admin\'s rescue queue stops at their organization; a platform admin\'s crosses it', async () => { + const t1Stuck = await svc.openNodeRequest(stuck('opp_literal'), CTX) as any; // organization t1 + + expect(await read({ status: 'pending', unstaffed: true }, OTHER_TENANT_ADMIN)).toMatchObject({ ids: [], total: 0 }); + const crossing = { ...PLATFORM_ADMIN, tenantId: 't2' }; + expect(await read({ status: 'pending', unstaffed: true }, crossing)).toMatchObject({ ids: [], total: 0 }); + // The platform admin's own read is walled by its context organization the + // way every list read is (`buildRequestWhere`); with no organization on + // the context, the arm reaches every organization's rescue queue. + const unscoped = { userId: 'root', positions: [], permissions: ['admin_full_access'] } as any; + expect(await read({ status: 'pending', unstaffed: true }, unscoped)).toMatchObject({ ids: [t1Stuck.id], total: 1 }); + }); + + it('reads the slate as it stands: a slate left holding only literals joins the arm', async () => { + const mixed = await svc.openNodeRequest({ + ...stuck('opp_mixed'), + config: { + approvers: [{ type: 'user' as const, value: 'u9' }, { type: 'position' as const, value: 'sales_manager' }], + behavior: 'unanimous' as const, + lockRecord: true, + }, + }, CTX) as any; + expect(mixed.pending_approvers).toEqual(['u9', 'position:sales_manager']); + expect((await read({ status: 'pending', unstaffed: true }, TENANT_ADMIN)).ids).toEqual([]); + + // u9 approves; the unanimous tally keeps the request open on the literal alone. + await svc.decideNode(mixed.id, { decision: 'approve', actorId: 'u9' }, APPROVER); + const after = await svc.getRequest(mixed.id, TENANT_ADMIN); + expect(after?.status).toBe('pending'); + expect(after?.pending_approvers).toEqual(['position:sales_manager']); + expect(await read({ status: 'pending', unstaffed: true }, TENANT_ADMIN)).toMatchObject({ ids: [mixed.id], total: 1 }); + }); + + it('a finalized rescue leaves the arm', async () => { + const req = await svc.openNodeRequest(stuck('opp_literal'), CTX) as any; + await svc.decide(req.id, { decision: 'approve', actorId: 'owner' }, TENANT_ADMIN); + expect(await read({ unstaffed: true }, TENANT_ADMIN)).toMatchObject({ ids: [], total: 0 }); + }); +}); + // ── approval_comment: the declared reply against the route it posts to ────── // // `sys_approval_request` declares the thread reply as the action diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index f4370556fe1..99175cc9f12 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -79,6 +79,7 @@ import { equivalentApproverAddresses, heldSlot, positionAddresses, + slateNamesAPerson, type ActingCaller, } from './approver-address.js'; import { @@ -3110,8 +3111,9 @@ export class ApprovalService implements IApprovalService { // Empty-slate policy (#3447 P2). "Empty" = no CONCRETE person — an // unstaffed position / empty expression result leaves only `type:value` - // literal slots, decidable by nobody. - if (!approvers.some(a => a && !a.includes(':'))) { + // literal slots, decidable by nobody. `slateNamesAPerson` is the one + // spelling of that test: the inbox's `unstaffed` arm reads it too (#22725). + if (!slateNamesAPerson(approvers)) { const emptyPolicy = (input.config as any).onEmptyApprovers ?? 'admin_rescue'; if (emptyPolicy === 'fail') { throw new Error( @@ -3160,7 +3162,7 @@ export class ApprovalService implements IApprovalService { for (const [k, v] of Object.entries(fallbackResolvedFrom)) { resolvedFrom[`fallback:${k}`] = v; } - if (fallbackApprovers.some(a => a && !a.includes(':'))) { + if (slateNamesAPerson(fallbackApprovers)) { this.logger?.warn?.( `[approvals] approval node '${input.nodeId}' on ${input.object}/${input.recordId} resolved to no ` + `concrete approver — opening on the declared fallbackApprovers per onEmptyApprovers: 'fallback'.`, @@ -3203,7 +3205,7 @@ export class ApprovalService implements IApprovalService { // Re-tested rather than assumed: a `fallback` that landed people above // has already turned this into a normal request, and warning "decidable // only by a privileged admin" about it would be false. - if (!approvers.some(a => a && !a.includes(':'))) { + if (!slateNamesAPerson(approvers)) { this.logger?.warn?.( `[approvals] approval node '${input.nodeId}' on ${input.object}/${input.recordId} resolved to no concrete approver` + ' — the request is decidable only by a privileged admin. Check that the approver target(s) are staffed.', @@ -6819,6 +6821,97 @@ export class ApprovalService implements IApprovalService { return [...new Set(list.map(r => String(r.request_id)))]; } + /** + * The "who acts next" constraint of a list or count read: the request ids + * its two arms match together, or `null` when the filter asked neither + * (callers then skip the id constraint, exactly as before #22725). + * + * - `approverId`: requests where one of those addresses holds a pending + * slot ({@link ApprovalService.approverRequestIds}); + * - `unstaffed: true`: pending requests nobody on the slate can decide, + * which this caller may override + * ({@link ApprovalService.unstaffedOverridableRequestIds}). + * + * The arms are ORed, never ANDed. The contract states this on the + * `unstaffed` member: one request answers "my pending approvals, rescues + * included", and the paged total counts both. `listRequests` and + * `countRequests` both call this, so the list and the total it pages + * cannot disagree about either arm. + */ + private async awaitingRequestIds( + filter: { approverId?: string | string[]; unstaffed?: boolean } | undefined, + context: ExecutionContext, + tenantOrg: string | null, + ): Promise { + const approverTargets = (Array.isArray(filter?.approverId) ? filter!.approverId : filter?.approverId ? [filter.approverId] : []) + .map(t => String(t).trim()) + .filter(Boolean); + const byApprover = await this.approverRequestIds(approverTargets, tenantOrg); + if (filter?.unstaffed !== true) return byApprover; + const unstaffed = await this.unstaffedOverridableRequestIds(context, tenantOrg); + return [...new Set([...(byApprover ?? []), ...unstaffed])]; + } + + /** + * [#22725] The `unstaffed` arm. These are the PENDING requests whose slate + * names no person and which this caller may override. This is the queue an + * `onEmptyApprovers: 'admin_rescue'` request waits in. Before this arm it + * waited in none: `approverRequestIds` matches a slot against the addresses + * a caller names, and an override actor's own addresses never equal an + * unstaffed `position:

` literal. So no "My Pending" listed the request + * and no bell counted it. A `lockRecord: true` record stayed locked until + * someone browsed the unfiltered list. + * + * "Names no person" is {@link slateNamesAPerson}, the test the open path + * applies before it warns "decidable only by a privileged admin". It is read + * off the stored slate as it stands now, never off a flag written at open + * time. So a slate that loses its last named approver to a decision and + * keeps only literals is caught as well. A slate that still names a person + * is not, even if that person has left: proving they left needs a directory + * read per slot, which neither this arm nor the open path makes. + * + * "May override" is {@link ApprovalService.isOverrideActor} and nothing else. + * There is no second notion of "admin" here. It is asked twice: + * - first as the visibility read asks it (`visibleRequestIds`' first line, + * the caller's organization). A caller who may override nothing pays no + * read, and gets nothing; + * - then per row, as `attachViewers` asks it for `viewer.can_override` + * (the row's organization). So every row this arm adds reads + * `can_override: true`, and a tenant admin's rescue queue never reaches + * past their organization. + * + * Read with `SYSTEM_CTX`, like every probe in this file. The scan is the + * live pending queue inside the caller's organization wall, bounded by the + * same window as the approver index. A failed read throws; it is not read as + * an empty queue. + */ + private async unstaffedOverridableRequestIds( + context: ExecutionContext, + tenantOrg: string | null, + ): Promise { + if (!this.isOverrideActor(context, tenantOrg)) return []; + const where: any = { status: 'pending' }; + if (tenantOrg) where.organization_id = tenantOrg; + const rows = await this.engine.find('sys_approval_request', { + where, fields: ['id', 'pending_approvers', 'organization_id'], + limit: ApprovalService.APPROVER_INDEX_CAP, context: SYSTEM_CTX, + }); + const list: any[] = Array.isArray(rows) ? rows : []; + if (list.length >= ApprovalService.APPROVER_INDEX_CAP) { + this.logger?.warn?.('[approvals] unstaffed-request probe hit its window — results may be truncated', { + cap: ApprovalService.APPROVER_INDEX_CAP, + }); + } + const ids: string[] = []; + for (const row of list) { + if (row?.id == null) continue; + if (slateNamesAPerson(csvSplit(row.pending_approvers))) continue; + if (!this.isOverrideActor(context, row.organization_id ?? null)) continue; + ids.push(String(row.id)); + } + return ids; + } + /** * The request ids this caller is a PARTICIPANT of, or `null` for a caller * who may see everything in scope (#3590). @@ -7110,6 +7203,7 @@ export class ApprovalService implements IApprovalService { recordId?: string; status?: ApprovalStatus | ApprovalStatus[]; approverId?: string | string[]; + unstaffed?: boolean; submitterId?: string; q?: string; limit?: number; @@ -7118,14 +7212,13 @@ export class ApprovalService implements IApprovalService { context: ExecutionContext, ): Promise { const { where, tenantOrg } = await this.buildRequestWhere(filter, context); - const approverTargets = (Array.isArray(filter?.approverId) ? filter!.approverId : filter?.approverId ? [filter.approverId] : []) - .map(t => String(t).trim()) - .filter(Boolean); // Every filter now pushes into the engine (issue #1745): approver via // the normalized index, status arrays via $in — so the page window is - // always engine-side and correct at any table size. - const ids = await this.approverRequestIds(approverTargets, tenantOrg); + // always engine-side and correct at any table size. [#22725] The + // `unstaffed` arm joins the approver arm as an id set too, so a rescue + // request pages exactly like any other row. + const ids = await this.awaitingRequestIds(filter, context, tenantOrg); if (ids) { if (ids.length === 0) return []; where.id = ids.length === 1 ? ids[0] : { $in: ids }; @@ -7168,11 +7261,9 @@ export class ApprovalService implements IApprovalService { context: ExecutionContext, ): Promise { const { where, tenantOrg } = await this.buildRequestWhere(filter, context); - const approverTargets = (Array.isArray(filter?.approverId) ? filter!.approverId : filter?.approverId ? [filter.approverId] : []) - .map(t => String(t).trim()) - .filter(Boolean); - const ids = await this.approverRequestIds(approverTargets, tenantOrg); + // [#22725] The same two arms `listRequests` reads, from the same helper. + const ids = await this.awaitingRequestIds(filter, context, tenantOrg); if (ids) { if (ids.length === 0) return 0; where.id = ids.length === 1 ? ids[0] : { $in: ids }; diff --git a/packages/plugins/plugin-approvals/src/approver-address.ts b/packages/plugins/plugin-approvals/src/approver-address.ts index c5691d3dd64..68c1030803e 100644 --- a/packages/plugins/plugin-approvals/src/approver-address.ts +++ b/packages/plugins/plugin-approvals/src/approver-address.ts @@ -134,6 +134,25 @@ export function actorAddresses(actorId: string, caller: ActingCaller | null): st return [...new Set([actorId, ...equivalentApproverAddresses(actorId)])]; } +/** + * Does `slate` name a PERSON? A concrete approver address is a user id or an + * email: a slot one person takes by being who they are. Anything else on a + * slate is a `type:value` literal, left by an approver target that resolved to + * no holder (`position:

`, `team:`, a stored `role:

`). + * + * `false` is the slate the open path calls "decidable only by a privileged + * admin" (the `onEmptyApprovers` policies decide it there). The + * `unstaffed` arm of the inbox list reads the same test on the slate as it + * stands (#22725). One test, so the slate the open path warns about and the + * slate that arm returns cannot drift apart. + * + * This is a test of the slate's SHAPE. It compares no slot with a caller, so it + * is not a reader of the equivalence above. + */ +export function slateNamesAPerson(slate: readonly string[]): boolean { + return slate.some((address) => !!address && !address.includes(':')); +} + /** * THE slot test: the pending slot `actorId` takes on `pending`, or `undefined` * when it holds none — the first of {@link actorAddresses} the slate holds, so diff --git a/packages/rest/src/rest-server-approvals-unknown-filter.test.ts b/packages/rest/src/rest-server-approvals-unknown-filter.test.ts index af18fbbc45c..071dd7a2a6d 100644 --- a/packages/rest/src/rest-server-approvals-unknown-filter.test.ts +++ b/packages/rest/src/rest-server-approvals-unknown-filter.test.ts @@ -253,14 +253,70 @@ describe('#7527 §2 — every parameter the route really reads still works', () it('every name in the closed set is accepted — none of them is refused', async () => { // Asserts against the exported array rather than a hand-copied list, so // the pin cannot drift away from what the route actually declares. + // `unstaffed` (#22725) reads `true` / `false` only, so it is sent one of + // its own values; `'1'` is its refusal case in §4. + const SAMPLE: Readonly> = { unstaffed: 'true' }; for (const name of APPROVAL_REQUEST_LIST_PARAMS) { const { drive } = boot(); - const answer = await drive({ [name]: '1' }); + const answer = await drive({ [name]: SAMPLE[name] ?? '1' }); expect(answer.status, `"${name}" is declared supported but was refused`).toBe(200); } }); }); +// ───────────────────────────────────────────────────────────────────────────── +// 4. #22725 — `unstaffed`, the rescue arm, is a DECLARED parameter +// ───────────────────────────────────────────────────────────────────────────── + +describe('#22725 §4 — `unstaffed` is declared, read as a boolean, and refused otherwise', () => { + it.each([ + ['true', true], + ['false', false], + ])('?unstaffed=%s reaches listRequests AND countRequests as %s', async (raw, value) => { + const { drive, approvals } = boot(); + const answer = await drive({ status: 'pending', approverId: 'u_42', unstaffed: raw, limit: '10' }); + expect(answer.status).toBe(200); + expect(answer.body?.total).toBe(2); + // The arm rides beside `approverId` on the SAME filter: the service ORs + // them, so the door must hand over both, unchanged. + const expected = expect.objectContaining({ approverId: ['u_42'], unstaffed: value, status: 'pending' }); + expect(approvals.listRequests).toHaveBeenCalledWith(expected, expect.anything()); + expect(approvals.countRequests).toHaveBeenCalledWith(expected, expect.anything()); + }); + + it('absent, the service is handed no arm at all', async () => { + const { drive, approvals } = boot(); + const answer = await drive({ approverId: 'u_42' }); + expect(answer.status).toBe(200); + expect(approvals.listRequests.mock.calls[0][0].unstaffed).toBeUndefined(); + }); + + it.each(['1', 'TRUE', 'yes', ''])('?unstaffed=%j is refused 400 invalid_boolean, and the service never runs', async (raw) => { + const { drive, approvals } = boot(); + const answer = await drive({ approverId: 'u_42', unstaffed: raw }); + expect( + answer.status, + `expected a 400 refusal for unstaffed=${JSON.stringify(raw)}, got ${answer.status} ` + + `with body ${JSON.stringify(answer.body)}`, + ).toBe(400); + expect(answer.body?.code).toBe('VALIDATION_FAILED'); + expect(answer.body?.fields?.[0]).toMatchObject({ field: 'unstaffed', code: 'invalid_boolean' }); + // A guessed `false` would have answered 200 without the arm the caller + // asked for: the defect this refusal exists to stop. + expect(approvals.listRequests).not.toHaveBeenCalled(); + expect(approvals.countRequests).not.toHaveBeenCalled(); + }); + + it('a repeated ?unstaffed= is refused by the multiplicity gate, before any read', async () => { + const { drive, approvals } = boot(); + const answer = await drive({ unstaffed: ['true', 'false'] }); + expect(answer.status).toBe(400); + expect(answer.body?.error?.code).toBe('VALIDATION_ERROR'); + expect(String(answer.body?.error?.message)).toContain('unstaffed'); + expect(approvals.listRequests).not.toHaveBeenCalled(); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // 3. The helper itself // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 750f77b63de..f057e5e820d 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -583,9 +583,10 @@ export function apiAccessDenialFromEnable( * * Measured from the handler's own reads, not from the card or the docs: the * five filters (`object`, `recordId`, `status`, `approverId`, `submitterId`), - * the free-text `q`, the paging pair (`limit`, `offset`), and the snake_case - * alias spellings the handler honours for the three camelCase filters. A name - * outside this set is refused with a located `400` instead of being dropped. + * the `unstaffed` arm (#22725), the free-text `q`, the paging pair (`limit`, + * `offset`), and the snake_case alias spellings the handler honours for the + * three camelCase filters. A name outside this set is refused with a located + * `400` instead of being dropped. * * Exported so the pin tests assert against THIS array rather than a * hand-copied second list that can drift away from what the route accepts. @@ -595,6 +596,7 @@ export const APPROVAL_REQUEST_LIST_PARAMS: readonly string[] = [ 'recordId', 'record_id', 'status', 'approverId', 'approver_id', + 'unstaffed', 'submitterId', 'submitter_id', 'q', 'limit', 'offset', @@ -807,6 +809,45 @@ function readDeclaredQueryNumber( ); } +/** + * [#22725] Read ONE boolean query parameter on a door whose request has no + * declared schema: `true` or `false`, and nothing else. + * + * The bare form `q.x === 'true'` is the defect this refuses to write. It does + * not fail; it reads every other spelling as `false`. `?unstaffed=1` or + * `?unstaffed=TRUE` would then answer `200` without the arm the caller asked + * for, and that answer is shaped exactly like "there is nothing to rescue". + * So a third spelling is refused, never guessed. This is the rule + * `@objectstack/runtime`'s `parseBooleanParam` applies on the notifications + * inbox's `?read=`; this package cannot import it (runtime depends on rest). + * The accepted set and the field code are the same. + * + * Absent is `undefined`. A present-but-empty `?x=` is refused: no answer was + * ever given for it, so there is none to preserve (contrast the per-door + * judgement in {@link readDeclaredQueryNumber}). A real boolean, which an + * in-process delegation can hand over, is itself. + * + * THROWN as `validationFailure`, never written here, so the door's + * `handleRouteError` answers `400 VALIDATION_FAILED` with `fields[0]` + * `{ field: , code: 'invalid_boolean' }`. That is the envelope its + * `limit` / `offset` refusals answer. Call it AFTER + * `refuseRepeatedQueryParams` has run for `param`. + */ +function readDeclaredQueryBoolean( + queryParams: Record | undefined, + param: string, +): boolean | undefined { + const raw = queryParams?.[param]; + if (raw === undefined || raw === null) return undefined; + if (raw === true || raw === 'true') return true; + if (raw === false || raw === 'false') return false; + const received = typeof raw === 'string' + ? JSON.stringify(raw.length > 40 ? `${raw.slice(0, 40)}…` : raw) + : 'a structured value'; + const message = `Invalid \`${param}\` query parameter: expected \`true\` or \`false\`, received ${received}`; + throw validationFailure(message, [{ field: param, code: 'invalid_boolean', message }]); +} + /** * [#16674] Which `services.*` slot each `routes.*` key is the address OF. * @@ -12416,7 +12457,7 @@ export class RestServer { * are no process-authoring or submit routes anymore. * * Routes (all under {basePath}/approvals): - * GET /requests — list (filters: status, object, recordId, approverId, submitterId) + * GET /requests — list (filters: status, object, recordId, approverId, unstaffed, submitterId) * GET /requests/:id — get request * POST /requests/:id/approve — record an approve decision (resumes the flow) * POST /requests/:id/reject — record a reject decision (resumes the flow) @@ -12548,7 +12589,7 @@ export class RestServer { // PURPOSE. Everything else here narrows the list to one // value and is declared single-valued. if (refuseRepeatedQueryParams(req, res, [ - 'object', 'recordId', 'record_id', 'status', + 'object', 'recordId', 'record_id', 'status', 'unstaffed', 'submitterId', 'submitter_id', 'q', 'limit', 'offset', ])) return; const q = req.query ?? {}; @@ -12577,11 +12618,18 @@ export class RestServer { // door cannot read is the caller's `400`. let limit: number | undefined; let offset: number | undefined; + // [#22725] The `unstaffed` arm: the pending requests no + // one on the slate can decide, for a caller who may + // override them, ORed with `approverId` by the service + // (the contract's member docblock). Read `true` / `false` + // only: a guessed `false` would be a 200 without the arm. + let unstaffed: boolean | undefined; try { limit = readDeclaredQueryNumber(q, 'limit', UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); offset = readDeclaredQueryNumber(q, 'offset', UNDECLARED_WHOLE_NUMBER_PARAM, { emptyIsAbsent: false }); + unstaffed = readDeclaredQueryBoolean(q, 'unstaffed'); } catch (refusal: any) { handleRouteError(res, refusal); return; @@ -12591,6 +12639,7 @@ export class RestServer { recordId: q.recordId ?? q.record_id, status: q.status, approverId: approverIds.length ? approverIds : undefined, + unstaffed, submitterId: q.submitterId ?? q.submitter_id, q: typeof q.q === 'string' ? q.q : undefined, limit, diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index c46255afcc3..e41e4ac44e0 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -845,7 +845,9 @@ export interface ApprovalDecisionResult { export interface IApprovalService { /** * "My approvals" inbox. Supports filtering by status, target object, - * record id, or by the user expected to act next. + * record id, or by who is expected to act next: the caller's approver + * identities (`approverId`) and, for a caller who may override them, the + * pending requests nobody on the slate can decide (`unstaffed`). */ listRequests( filter: { @@ -860,6 +862,35 @@ export interface IApprovalService { * instead of one request per identity. */ approverId?: string | string[]; + /** + * Also match the PENDING requests that nobody on the slate can decide, + * returned only to a caller who may override them. + * + * Unstaffed means the request's current `pending_approvers` names no + * person: no user id and no email, only `type:value` literals left by an + * approver target with no holder (`position:

`, `team:`), or + * nothing at all. It is the same test the open path makes before it + * warns that a request is "decidable only by a privileged admin", the + * `onEmptyApprovers: 'admin_rescue'` default. It is applied to the slate + * as it stands, so a slate whose last named approver has already acted + * and left only literals matches too. A slate that still names a person + * never matches, even when that person has since left the organization. + * + * "May override" is the privileged-override rule that admits the + * override decision itself and sets `viewer.can_override`. A request + * this member adds is never one the caller is refused on: each returned + * row reads `viewer.can_override: true`. A caller who may override + * nothing gets nothing from it. No row is added and nothing is refused. + * + * It is an ARM of the approver match, not a narrowing of it. With + * `approverId`, a request matches when EITHER holds (OR): one of those + * identities holds a pending slot, or the request is unstaffed and the + * caller may override it. So one request answers "my pending approvals, + * rescues included", and its `countRequests` total counts both arms. + * Without `approverId` it selects the unstaffed requests alone. `false` + * is the same as absent. Every other member still narrows the result. + */ + unstaffed?: boolean; submitterId?: string; /** * Free-text search, pushed into the engine query: matches the source From 67b9a7b3fc01971e3c8b9b802757908d992ddbcd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 03:35:30 +0000 Subject: [PATCH 2/6] test(plugin-approvals): the unstaffed arm's per-row override test keeps an org-less tenant admin to org-less rescues Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK Co-authored-by: Claude --- .../src/approval-service.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 50acf81acf9..7d4cf35e734 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -2212,6 +2212,22 @@ describe('ApprovalService — the unstaffed arm (#22725)', () => { expect(await read({ status: 'pending', unstaffed: true }, unscoped)).toMatchObject({ ids: [t1Stuck.id], total: 1 }); }); + it('per row, the arm returns only what the caller may override: an org-less tenant admin reaches org-less rescues only', async () => { + const t1Stuck = await svc.openNodeRequest(stuck('opp_literal'), CTX) as any; // organization t1 + const globalStuck = await svc.openNodeRequest(stuck('opp_global'), SYS) as any; // no organization + expect(engine._tables['sys_approval_request'].find(r => r.id === globalStuck.id)?.organization_id ?? null).toBeNull(); + + // No organization on the context: the list read is unwalled, and the + // visibility read lets this caller see every row. Only the per-row + // override test keeps the t1 request, which this caller may NOT + // override, out of the arm. + const orgLessTenantAdmin = { userId: 'owner', positions: [], permissions: ['organization_admin'] } as any; + const got = await read({ status: 'pending', unstaffed: true }, orgLessTenantAdmin); + expect(got).toMatchObject({ ids: [globalStuck.id], total: 1 }); + expect(got.ids).not.toContain(t1Stuck.id); + expect(got.rows[0].viewer).toMatchObject({ can_override: true }); + }); + it('reads the slate as it stands: a slate left holding only literals joins the arm', async () => { const mixed = await svc.openNodeRequest({ ...stuck('opp_mixed'), From e034bdd48a14f497f6f8a62f3a81837dbd09c968 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 03:36:19 +0000 Subject: [PATCH 3/6] test(plugin-approvals): a caller who may override nothing pays no unstaffed-slate scan Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK Co-authored-by: Claude --- .../plugins/plugin-approvals/src/approval-service.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 7d4cf35e734..1a025e6f9e5 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -2146,7 +2146,12 @@ describe('ApprovalService — the unstaffed arm (#22725)', () => { await svc.openNodeRequest(empty('opp_empty'), CTX); const mine = await svc.openNodeRequest(staffed('opp_mine', 'nobody'), CTX) as any; + const slateScans = () => engine._finds.filter(f => f.object === 'sys_approval_request' + && Array.isArray(f.options?.fields) && f.options.fields.includes('pending_approvers')).length; + const before = slateScans(); expect(await read({ status: 'pending', unstaffed: true }, MEMBER)).toMatchObject({ ids: [], total: 0 }); + // A caller who may override nothing is answered without the arm's scan. + expect(slateScans()).toBe(before); // Beside the member's own identities, the arm leaves their queue exactly as // `approverId` alone answers it. const withFlag = await read({ status: 'pending', approverId: ['nobody'], unstaffed: true }, MEMBER); From 5be6e501bb969a2f3769b2d3334681f2c875b507 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 03:38:21 +0000 Subject: [PATCH 4/6] test(dogfood): the approvals inbox's unstaffed arm through the REST door on a real boot Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK Co-authored-by: Claude --- .../approval-unstaffed-arm.dogfood.test.ts | 151 ++++++++++++++++++ .../test/fixtures/unstaffed-arm-fixture.ts | 127 +++++++++++++++ 2 files changed, 278 insertions(+) create mode 100644 packages/qa/dogfood/test/approval-unstaffed-arm.dogfood.test.ts create mode 100644 packages/qa/dogfood/test/fixtures/unstaffed-arm-fixture.ts diff --git a/packages/qa/dogfood/test/approval-unstaffed-arm.dogfood.test.ts b/packages/qa/dogfood/test/approval-unstaffed-arm.dogfood.test.ts new file mode 100644 index 00000000000..e3c65c90bba --- /dev/null +++ b/packages/qa/dogfood/test/approval-unstaffed-arm.dogfood.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #22725 — an `admin_rescue` request was in no one's pending queue. +// +// A node whose approvers resolve to no holder opens, under the default +// `onEmptyApprovers: 'admin_rescue'`, on a slate that names no person (here +// the literal `position:

`). A privileged admin may decide it, but the +// inbox's "My Pending" door asked by the caller's own approver identities, and +// an admin's identities never equal an unstaffed `position:

`. So no My +// Pending listed it, the bell counted 0, and the `lockRecord: true` record +// stayed locked until someone browsed the unfiltered list (measured on HotCRM). +// +// The fix is the `unstaffed` arm on `GET /approvals/requests`, ORed with +// `approverId`. This pins the card's three cases on a real boot, through the +// door the console calls, on both faces it reads (the paged list and its +// `total`, which is the bell's number): +// +// ⭐ an admin_rescue request appears for an override actor, with +// `viewer.can_override: true`, beside the admin's own identities; +// ⭐ it does not appear for a member, who sent the same parameter; +// ⭐ CONTROL: a staffed request's queue is unchanged. Its approver reads the +// same rows with the arm and without it, and the arm never claims it. +// +// And the door's half: a value other than `true` / `false` is refused, so a +// malformed ask never reads as an empty rescue queue. + +import { describe, it, expect } from 'vitest'; +import { bootStack } from '@objectstack/verify'; +import { ApprovalsServicePlugin } from '@objectstack/plugin-approvals'; +import { RecordChangeTriggerPlugin } from '@objectstack/trigger-record-change'; +import { + unstaffedArmStack, + unstaffedArmSecurity, + UNSTAFFED_POSITION, + STAFFED_APPROVER_EMAIL, +} from './fixtures/unstaffed-arm-fixture.js'; + +const SYS = { context: { isSystem: true } } as const; +const SUBMITTER = 'unstaffed-arm-submitter@example.com'; +/** `bootStack`'s seeded dev admin: a platform admin, so an override actor. */ +const ADMIN_EMAIL = 'admin@objectos.ai'; + +interface Row { + id: string; + object_name?: string; + pending_approvers?: string[]; + viewer?: { can_act: boolean; is_submitter: boolean; can_override: boolean }; +} +interface ListBody { + data: Row[]; + total?: number; +} + +describe('the approvals inbox\'s unstaffed arm, through the REST door (#22725)', () => { + it( + 'returns an admin_rescue request to an override actor, not to a member, and leaves a staffed queue unchanged', + async () => { + const stack = await bootStack(unstaffedArmStack as unknown as Parameters[0], { + automation: true, + security: unstaffedArmSecurity(), + extraPlugins: [new RecordChangeTriggerPlugin(), new ApprovalsServicePlugin()], + }); + try { + const ql: any = await stack.kernel.getServiceAsync('objectql'); + const idOf = async (email: string): Promise => { + const u = await ql.findOne('sys_user', { where: { email }, context: SYS.context }); + return String(u?.id ?? ''); + }; + // Seed the dev admin FIRST, so the sign-ups below are plain members. + const adminToken = await stack.signIn(); + const submitterToken = await stack.signUp(SUBMITTER); + const approverToken = await stack.signUp(STAFFED_APPROVER_EMAIL); + const submitterId = await idOf(SUBMITTER); + const approverId = await idOf(STAFFED_APPROVER_EMAIL); + const adminId = await idOf(ADMIN_EMAIL); + expect(submitterId).not.toBe(''); + expect(approverId).not.toBe(''); + expect(adminId).not.toBe(''); + + // A plain member submits both records: one opens on the unstaffed + // literal (admin_rescue), one on the named approver (the control). + const stuckRes = await stack.apiAs(submitterToken, 'POST', '/data/unstaffed_arm_stuck', { name: 'stuck' }); + expect(stuckRes.status).toBe(201); + const staffedRes = await stack.apiAs(submitterToken, 'POST', '/data/unstaffed_arm_staffed', { name: 'staffed' }); + expect(staffedRes.status).toBe(201); + + const list = async (token: string, query: string): Promise => { + const res = await stack.apiAs(token, 'GET', `/approvals/requests?${query}`); + expect(res.status, `GET /approvals/requests?${query}`).toBe(200); + return (await res.json()) as ListBody; + }; + + // The scene, before anything is read off it: both requests are open, + // and each slate is what its half needs. A zero-row answer below must + // never be readable as "nothing was opened". + const all = await list(adminToken, 'status=pending'); + expect(all.data).toHaveLength(2); + const stuck = all.data.find((r) => r.object_name === 'unstaffed_arm_stuck'); + const staffed = all.data.find((r) => r.object_name === 'unstaffed_arm_staffed'); + expect(stuck?.pending_approvers).toEqual([`position:${UNSTAFFED_POSITION}`]); + expect(staffed?.pending_approvers).toEqual([STAFFED_APPROVER_EMAIL]); + + // ⭐ 1 — an override actor finds it. Beside their own identities (the + // console's "My Pending" shape) and alone, on both faces. + const adminIdentities = `&approverId=${encodeURIComponent(adminId)},${encodeURIComponent(ADMIN_EMAIL)}`; + const before = await list(adminToken, `status=pending${adminIdentities}&limit=50`); + expect(before.data.map((r) => r.id)).not.toContain(stuck!.id); + for (const query of [ + `status=pending${adminIdentities}&unstaffed=true&limit=50`, + 'status=pending&unstaffed=true&limit=50', + ]) { + const got = await list(adminToken, query); + expect(got.data.map((r) => r.id), query).toEqual([stuck!.id]); + expect(got.total, query).toBe(1); + expect(got.data[0].viewer).toEqual({ can_act: false, is_submitter: false, can_override: true }); + } + + // ⭐ 2 — a member who sends the same parameter gets nothing extra. + const memberQuery = `status=pending&approverId=${encodeURIComponent(submitterId)},${encodeURIComponent(SUBMITTER)}`; + for (const query of [`${memberQuery}&unstaffed=true&limit=50`, 'status=pending&unstaffed=true&limit=50']) { + const got = await list(submitterToken, query); + expect(got.data, query).toEqual([]); + expect(got.total, query).toBe(0); + } + + // ⭐ 3 — CONTROL: the staffed request's approver reads the same queue + // with the arm and without it, and the arm never claims that request. + const approverQuery = `status=pending&approverId=${encodeURIComponent(approverId)},${encodeURIComponent(STAFFED_APPROVER_EMAIL)}&limit=50`; + const without = await list(approverToken, approverQuery); + const withArm = await list(approverToken, `${approverQuery}&unstaffed=true`); + expect(without.data.map((r) => r.id)).toEqual([staffed!.id]); + expect(without.total).toBe(1); + expect(withArm.data.map((r) => r.id)).toEqual(without.data.map((r) => r.id)); + expect(withArm.total).toBe(without.total); + const rescue = await list(adminToken, 'status=pending&unstaffed=true'); + expect(rescue.data.map((r) => r.id)).not.toContain(staffed!.id); + + // The door's half: a value the door cannot read is refused, never + // guessed into "no arm". + const refused = await stack.apiAs(adminToken, 'GET', '/approvals/requests?status=pending&unstaffed=1'); + expect(refused.status).toBe(400); + const refusal: any = await refused.json(); + expect(refusal?.code).toBe('VALIDATION_FAILED'); + expect(refusal?.fields?.[0]).toMatchObject({ field: 'unstaffed', code: 'invalid_boolean' }); + } finally { + await stack.stop(); + } + }, + 120_000, + ); +}); diff --git a/packages/qa/dogfood/test/fixtures/unstaffed-arm-fixture.ts b/packages/qa/dogfood/test/fixtures/unstaffed-arm-fixture.ts new file mode 100644 index 00000000000..fe5b57f1be9 --- /dev/null +++ b/packages/qa/dogfood/test/fixtures/unstaffed-arm-fixture.ts @@ -0,0 +1,127 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Minimal booted-app fixture for the #22725 pin: the approvals inbox's +// `unstaffed` arm, read through the REST door on a real boot. +// +// Two objects, each with one autolaunched `approval` flow that fires on insert: +// +// - `unstaffed_arm_stuck` routes to a POSITION nothing in this fixture ever +// staffs. Its node declares no `onEmptyApprovers`, so the default, +// `admin_rescue`, opens the request on the literal `position:

` slot: +// decidable only by a privileged admin, and before #22725 in nobody's +// queue. +// - `unstaffed_arm_staffed` routes to one named user, by email. Its slate +// names a person, so it is the CONTROL: the arm must never claim it, and +// its approver's queue must read the same with the arm and without it. +// +// Two objects rather than two flows on one: one pending request per record, +// and a record that started both flows would refuse the second. +// +// Purpose-built for the reason `override-composite-fixture.ts` gives: +// showcase's `onEnable` staffs its own approval positions on every boot, and +// "nobody holds the position" has to be a precondition this pin owns. + +import { defineStack, defineFlow } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +/** A position nothing in this fixture ever staffs. */ +export const UNSTAFFED_POSITION = 'unstaffed_arm_nobody'; + +/** The one person the staffed flow routes to (by email, the slot it stores). */ +export const STAFFED_APPROVER_EMAIL = 'unstaffed-arm-approver@example.com'; + +export const UnstaffedArmStuck = ObjectSchema.create({ + name: 'unstaffed_arm_stuck', + label: 'Unstaffed Arm Stuck', + pluralLabel: 'Unstaffed Arm Stuck', + sharingModel: 'public_read_write', + fields: { + name: Field.text({ label: 'Name', required: true }), + }, +}); + +export const UnstaffedArmStaffed = ObjectSchema.create({ + name: 'unstaffed_arm_staffed', + label: 'Unstaffed Arm Staffed', + pluralLabel: 'Unstaffed Arm Staffed', + sharingModel: 'public_read_write', + fields: { + name: Field.text({ label: 'Name', required: true }), + }, +}); + +function approvalFlow(name: string, objectName: string, approver: { type: 'position' | 'user'; value: string }) { + return defineFlow({ + name, + label: name, + description: `Fires on insert of ${objectName} and opens one approval step.`, + type: 'autolaunched', + status: 'active', + nodes: [ + { + id: 'start', + type: 'start', + label: 'On Create', + config: { objectName, triggerType: 'record-after-create' }, + }, + { + id: 'gate', + type: 'approval', + label: 'Gate', + config: { approvers: [approver], behavior: 'first_response', lockRecord: true }, + }, + { id: 'approved', type: 'end', label: 'Approved' }, + { id: 'rejected', type: 'end', label: 'Rejected' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'gate' }, + { id: 'e2', source: 'gate', target: 'approved', label: 'approve' }, + { id: 'e3', source: 'gate', target: 'rejected', label: 'reject' }, + ], + }); +} + +export const unstaffedArmStack = defineStack({ + manifest: { + id: 'com.dogfood.unstaffed-arm', + namespace: 'unstaffed_arm', + version: '0.0.0', + type: 'app', + name: 'Unstaffed Arm Fixture', + description: 'One request routed to a position nobody holds, one routed to a named user.', + }, + // ADR-0097: the flows' `record-after-create` start nodes need the trigger + // capability, and `defineStack` refuses `triggers` without `automation`. + requires: ['automation', 'triggers'], + objects: [UnstaffedArmStuck, UnstaffedArmStaffed], + flows: [ + approvalFlow('unstaffed_arm_stuck_flow', 'unstaffed_arm_stuck', { type: 'position', value: UNSTAFFED_POSITION }), + approvalFlow('unstaffed_arm_staffed_flow', 'unstaffed_arm_staffed', { type: 'user', value: STAFFED_APPROVER_EMAIL }), + ], +}); + +const FIXTURE_SUBMITTER_SET = 'unstaffed_arm_submitter'; + +/** + * The fallback set a fresh member resolves to: create + read on both objects, + * and nothing that confers override standing. So the submitter is a plain + * member, which is what the "it does not appear for a member" half needs. + */ +export const unstaffedArmSubmitterSet: PermissionSet = PermissionSetSchema.parse({ + name: FIXTURE_SUBMITTER_SET, + label: 'Unstaffed Arm Submitter — create + read on the two fixture objects only', + objects: { + unstaffed_arm_stuck: { allowRead: true, allowCreate: true, allowEdit: false, allowDelete: false }, + unstaffed_arm_staffed: { allowRead: true, allowCreate: true, allowEdit: false, allowDelete: false }, + }, +}); + +/** SecurityPlugin whose fresh-member fallback is the submitter-only set above. */ +export function unstaffedArmSecurity(): SecurityPlugin { + return new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, unstaffedArmSubmitterSet], + fallbackPermissionSet: unstaffedArmSubmitterSet.name, + }); +} From adf1654932afa5f954ef23219d24b411bce7314a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 03:55:54 +0000 Subject: [PATCH 5/6] =?UTF-8?q?docs(spec,rest):=20the=20unstaffed=20member?= =?UTF-8?q?=20reads=20the=20slate,=20not=20the=20directory=20=E2=80=94=20a?= =?UTF-8?q?=20later-staffed=20position=20slot=20still=20matches?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK Co-authored-by: Claude --- content/docs/automation/approvals.mdx | 14 +++++----- packages/rest/src/rest-server.ts | 6 ++--- .../spec/src/contracts/approval-service.ts | 26 +++++++++++-------- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index be635cbf6b1..c63dd2e3309 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -455,9 +455,9 @@ spelling, and no holder acts under it. Other filters: `status`, `object`, `recordId`, `submitterId`, `q`, `limit`, `offset`, and `unstaffed`. -`unstaffed=true` adds the requests nobody on the slate can decide: pending -requests whose slate names no person, only unstaffed literals such as -`position:`, or nothing at all. This is what `admin_rescue` opens. They +`unstaffed=true` adds the pending requests whose slate names no person: only +unstaffed literals such as `position:`, or nothing at all. This is what +`admin_rescue` opens. They are added only for a caller who may override them: an admin with override authority (`admin_full_access`, or `organization_admin` within their organization). Every such row reads `viewer.can_override: true`. For anyone else the parameter @@ -470,9 +470,11 @@ curl -b cookies.txt \ ``` It reads `true` or `false` only (`false` is the same as leaving it out); any -other value is refused with `400`. A slate that still names a person never -matches, even if that person has since left. Reassign such a request from the -**All** view. +other value is refused with `400`. The test reads the slate, not the directory: +a `position:` slot whose position gained a holder after the request opened +still matches (that holder also finds it under `approverId`), and a slate that +still names a person never matches, even if that person has since left. +Reassign such a request from the **All** view. **`approverId` is a filter, not authorization.** What you may see is decided diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index f057e5e820d..d9a2b0461fc 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -12618,9 +12618,9 @@ export class RestServer { // door cannot read is the caller's `400`. let limit: number | undefined; let offset: number | undefined; - // [#22725] The `unstaffed` arm: the pending requests no - // one on the slate can decide, for a caller who may - // override them, ORed with `approverId` by the service + // [#22725] The `unstaffed` arm: the pending requests whose + // slate names no person, for a caller who may override + // them, ORed with `approverId` by the service // (the contract's member docblock). Read `true` / `false` // only: a guessed `false` would be a 200 without the arm. let unstaffed: boolean | undefined; diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index e41e4ac44e0..1bd462d54cb 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -863,18 +863,22 @@ export interface IApprovalService { */ approverId?: string | string[]; /** - * Also match the PENDING requests that nobody on the slate can decide, - * returned only to a caller who may override them. + * Also match the PENDING requests whose slate names no person, returned + * only to a caller who may override them. These are the requests the + * `onEmptyApprovers: 'admin_rescue'` default leaves to a privileged + * override. * - * Unstaffed means the request's current `pending_approvers` names no - * person: no user id and no email, only `type:value` literals left by an - * approver target with no holder (`position:

`, `team:`), or - * nothing at all. It is the same test the open path makes before it - * warns that a request is "decidable only by a privileged admin", the - * `onEmptyApprovers: 'admin_rescue'` default. It is applied to the slate - * as it stands, so a slate whose last named approver has already acted - * and left only literals matches too. A slate that still names a person - * never matches, even when that person has since left the organization. + * Unstaffed means the request's current `pending_approvers` holds no + * user id and no email: only `type:value` literals left by an approver + * target with no holder (`position:

`, `team:`), or nothing at all. + * It is the same test the open path makes before it warns that a + * request is "decidable only by a privileged admin". It reads the slate + * as it stands, not the directory: a slate whose last named approver + * has already acted and left only literals matches too, and so does a + * `position:

` slot whose position gained a holder after the request + * opened (that holder also finds it under `approverId`). A slate that + * still names a person never matches, even when that person has since + * left the organization. * * "May override" is the privileged-override rule that admits the * override decision itself and sets `viewer.can_override`. A request From 640aed576dcfc7cd43e18b43c424296299124c51 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 06:01:47 +0000 Subject: [PATCH 6/6] =?UTF-8?q?docs(spec):=20listRequests'=20limit=20docbl?= =?UTF-8?q?ock=20states=20the=20engine-side=20window=20the=20head=20serves?= =?UTF-8?q?=20=E2=80=94=20the=20who-acts-next=20arms=20resolve=20to=20a=20?= =?UTF-8?q?bounded=20id=20set=20first?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK Co-authored-by: Claude --- packages/spec/src/contracts/approval-service.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 1bd462d54cb..d9f4cb6dbf0 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -903,10 +903,14 @@ export interface IApprovalService { */ q?: string; /** - * Page window. Honoured as an engine-level window when the filter is - * fully pushable; an `approverId` / status-array filter still - * post-filters in memory (bounded personal queues), where the window - * is applied after filtering. + * Page window, applied by the engine for every filter; a status array + * pushes down as `$in`. The two "who acts next" arms (`approverId`, + * `unstaffed`) first resolve to a bounded set of request ids: the + * approver index for `approverId`, the pending-request scan for + * `unstaffed`, each within the implementation's id window + * (`APPROVER_INDEX_CAP`, 10,000 in `@objectstack/plugin-approvals`, + * which warns when a probe reaches it). The window then applies + * engine-side to the rows in that set. */ limit?: number; offset?: number;